Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Geert Uytterhoeven]

Hi Gilad,

On Mon, Apr 23, 2018 at 9:45 AM, Gilad Ben-Yossef  wrote:
> On Wed, Apr 18, 2018 at 10:47 AM, Geert Uytterhoeven
>  wrote:
>> On Wed, Apr 18, 2018 at 6:32 AM, Gilad Ben-Yossef  
>> wrote:
>>> On Tue, Apr 17, 2018 at 9:14 PM, Geert Uytterhoeven
>>>  wrote:
 The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
 depend on ARM or ARM64, unless compile-testing.
>>>
>>> Actually it is not. Despite what the name suggest, CryptoCell is
>>> designed by Arm but is
>>> not in fact limited to Arm cores. I think the only requirement is
>>> ability to provide an AMBA bus
>>> interface. Kudos to our marketing department to make that so clear and
>>> so on... :-)
>>
>> Good to know, I couldn't find any users of the compatible value in DT 
>> sources,
>> so I had to guess... and missed ;-)
>
> Yes, the original driver that went through staging was for CC 712,
> which so new it doesn't yet
> have a commercially available silicon yet :-)
>
> I've added the older 710 and 613 support just recently and will be
> working with CC hardware implementors
> to add the relevant DT bindings for their respective SoCs
>
>> Do you have a good suggestion for a platform dependency?
>> Based on the above, I'd say "depends on ARM_AMBA || COMPILE_TEST",
>> but (currently) ARM_AMBA is selected on ARM or ARM64 only?
>
> So AMBA *as a system bus* is not strictly needed AFAIK in the sense
> that you just need
> an AMBA to whatever bus interface, so not all system implementing this
> actually define ARM_AMBA.

IC.

> It's actually safer for me to rule out certain architectures rather
> than point to which are used.
> I'd say ruling out s390, um, alpha, ia64 and m68k is a safe bet.
>
> Do you want to send a patch or shall I?

Please send a patch, thanks!

(I don't want to be the one adding more negative architecture dependencies ;-)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Gilad Ben-Yossef]

On Wed, Apr 18, 2018 at 10:47 AM, Geert Uytterhoeven
 wrote:
> Hi Gilad,
>
> On Wed, Apr 18, 2018 at 6:32 AM, Gilad Ben-Yossef  wrote:
>> On Tue, Apr 17, 2018 at 9:14 PM, Geert Uytterhoeven
>>  wrote:
>>> The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
>>> depend on ARM or ARM64, unless compile-testing.
>>
>> Actually it is not. Despite what the name suggest, CryptoCell is
>> designed by Arm but is
>> not in fact limited to Arm cores. I think the only requirement is
>> ability to provide an AMBA bus
>> interface. Kudos to our marketing department to make that so clear and
>> so on... :-)
>
> Good to know, I couldn't find any users of the compatible value in DT sources,
> so I had to guess... and missed ;-)

Yes, the original driver that went through staging was for CC 712,
which so new it doesn't yet
have a commercially available silicon yet :-)

I've added the older 710 and 613 support just recently and will be
working with CC hardware implementors
to add the relevant DT bindings for their respective SoCs

>
> Do you have a good suggestion for a platform dependency?
> Based on the above, I'd say "depends on ARM_AMBA || COMPILE_TEST",
> but (currently) ARM_AMBA is selected on ARM or ARM64 only?

So AMBA *as a system bus* is not strictly needed AFAIK in the sense
that you just need
an AMBA to whatever bus interface, so not all system implementing this
actually define ARM_AMBA.

It's actually safer for me to rule out certain architectures rather
than point to which are used.
I'd say ruling out s390, um, alpha, ia64 and m68k is a safe bet.

Do you want to send a patch or shall I?

Thanks,
Gilad


>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Geert Uytterhoeven]

Hi Gilad,

On Wed, Apr 18, 2018 at 6:32 AM, Gilad Ben-Yossef  wrote:
> On Tue, Apr 17, 2018 at 9:14 PM, Geert Uytterhoeven
>  wrote:
>> The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
>> depend on ARM or ARM64, unless compile-testing.
>
> Actually it is not. Despite what the name suggest, CryptoCell is
> designed by Arm but is
> not in fact limited to Arm cores. I think the only requirement is
> ability to provide an AMBA bus
> interface. Kudos to our marketing department to make that so clear and
> so on... :-)

Good to know, I couldn't find any users of the compatible value in DT sources,
so I had to guess... and missed ;-)

Do you have a good suggestion for a platform dependency?
Based on the above, I'd say "depends on ARM_AMBA || COMPILE_TEST",
but (currently) ARM_AMBA is selected on ARM or ARM64 only?

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Geert Uytterhoeven]

Hi Arnd,

On Tue, Apr 17, 2018 at 9:53 PM, Arnd Bergmann  wrote:
> On Tue, Apr 17, 2018 at 8:14 PM, Geert Uytterhoeven
>  wrote:
>> The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
>> depend on ARM or ARM64, unless compile-testing.
>>
>> Drop the dependency on HAS_DMA, as DMA is always available on ARM and
>> ARM64 platforms, and doing so will increase compile coverage.
>>
>> Signed-off-by: Geert Uytterhoeven 
>> ---
>> Is ARM || ARM64 OK?
>> Or should this be limited to either ARM or ARM64? Or something else?
>
> ARM || ARM64 seems fine, but don't you need '|| (HAS_DMA && COMPILE_TEST)'?
>
> I assume the HAS_DMA dependency was added to prevent compile
> testing to run into a build error.

Probably it was. But in v4.17-rc1, dummies are present in the NO_DMA case,
so everything compile-tests fine.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Gilad Ben-Yossef]

Hi Geert,

On Tue, Apr 17, 2018 at 9:14 PM, Geert Uytterhoeven
 wrote:
> The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
> depend on ARM or ARM64, unless compile-testing.
>

Actually it is not. Despite what the name suggest, CryptoCell is
designed by Arm but is
not in fact limited to Arm cores. I think the only requirement is
ability to provide an AMBA bus
interface. Kudos to our marketing department to make that so clear and
so on... :-)

There are in fact licensees of this IP which use it with other (Linux
running) architectures, perhaps
thanks to the fact that the design originated in an outside company
(Discretix) which was bought by Arm.

Therefore, NAK on the specific patch. However, if there is some build
issue with a none Arm
architecture I of course very interested to hear about it.

Many thanks,
Gilad





-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Arnd Bergmann]

On Tue, Apr 17, 2018 at 8:14 PM, Geert Uytterhoeven
 wrote:
> The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
> depend on ARM or ARM64, unless compile-testing.
>
> Drop the dependency on HAS_DMA, as DMA is always available on ARM and
> ARM64 platforms, and doing so will increase compile coverage.
>
> Signed-off-by: Geert Uytterhoeven 
> ---
> Is ARM || ARM64 OK?
> Or should this be limited to either ARM or ARM64? Or something else?

ARM || ARM64 seems fine, but don't you need '|| (HAS_DMA && COMPILE_TEST)'?

I assume the HAS_DMA dependency was added to prevent compile
testing to run into a build error.

Arnd

[PATCH/RFC] crypto: Add platform dependencies for CRYPTO_DEV_CCREE

[Geert Uytterhoeven]

The ARM TrustZone CryptoCell is found on ARM SoCs only.  Hence make it
depend on ARM or ARM64, unless compile-testing.

Drop the dependency on HAS_DMA, as DMA is always available on ARM and
ARM64 platforms, and doing so will increase compile coverage.

Signed-off-by: Geert Uytterhoeven 
---
Is ARM || ARM64 OK?
Or should this be limited to either ARM or ARM64? Or something else?
---
 drivers/crypto/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 3dbc47528667b77c..104640b72354d93a 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -719,7 +719,8 @@ config CRYPTO_DEV_ARTPEC6
 
 config CRYPTO_DEV_CCREE
tristate "Support for ARM TrustZone CryptoCell family of security 
processors"
-   depends on CRYPTO && CRYPTO_HW && OF && HAS_DMA
+   depends on CRYPTO && CRYPTO_HW && OF
+   depends on ARM || ARM64 || COMPILE_TEST
default n
select CRYPTO_HASH
select CRYPTO_BLKCIPHER
-- 
2.7.4