> > -----Original Message-----
> > From: Pavel Machek <[email protected]>
> > Sent: Saturday, 18 April 2020 13:44
> > To: kernel list <[email protected]>; Hadar Gat
> > <[email protected]>; [email protected]
> > Subject: Arm CryptoCell driver -- default Y, even on machines where it is
> > obviously useless
> >
> > Hi!
> >
> > I'm configuring kernel for x86, and I get offered HW_RANDOM_CCTRNG with
> > default=Y, and help text suggesting I should enable it.
> >
> > That's... two wrong suggestions, right?
> >
> > Best regards,
> > Pavel
...
> ________________________________________
> From: Hadar Gat <[email protected]>
> Sent: Saturday, April 18, 2020 11:31 PM
> To: Pavel Machek; kernel list; [email protected]
> Cc: Ofir Drang; Gilad Ben Yossef; nd
> Subject: RE: Arm CryptoCell driver -- default Y, even on machines where it is obviously useless
>
> Hi Pavel,
> I think you got it right..
> Indeed, Arm CryptoCell CCTRNG driver couldn't be used and obviously useless if the Arm CryptoCell HW does not exist in the system.
There's a delicate point here though - CryptoCell is an independent
hardware block, it is not tied to a particular CPU architecture.
There are SoCs with none-Arm architecture CPU using it.
So I would say whatever the answer is, it should be the same for any
generic embedded style HW block.
And the help text is not architecture specific anyway, is it not..?
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
On Sun, 19 Apr 2020 at 08:33, Gilad Ben-Yossef <[email protected]> wrote:
>
> > > -----Original Message-----
> > > From: Pavel Machek <[email protected]>
> > > Sent: Saturday, 18 April 2020 13:44
> > > To: kernel list <[email protected]>; Hadar Gat
> > > <[email protected]>; [email protected]
> > > Subject: Arm CryptoCell driver -- default Y, even on machines where it is
> > > obviously useless
> > >
> > > Hi!
> > >
> > > I'm configuring kernel for x86, and I get offered HW_RANDOM_CCTRNG with
> > > default=Y, and help text suggesting I should enable it.
> > >
> > > That's... two wrong suggestions, right?
> > >
> > > Best regards,
> > > Pavel
> ...
> > ________________________________________
> > From: Hadar Gat <[email protected]>
> > Sent: Saturday, April 18, 2020 11:31 PM
> > To: Pavel Machek; kernel list; [email protected]
> > Cc: Ofir Drang; Gilad Ben Yossef; nd
> > Subject: RE: Arm CryptoCell driver -- default Y, even on machines where it is obviously useless
> >
> > Hi Pavel,
> > I think you got it right..
> > Indeed, Arm CryptoCell CCTRNG driver couldn't be used and obviously useless if the Arm CryptoCell HW does not exist in the system.
>
> There's a delicate point here though - CryptoCell is an independent
> hardware block, it is not tied to a particular CPU architecture.
> There are SoCs with none-Arm architecture CPU using it.
>
> So I would say whatever the answer is, it should be the same for any
> generic embedded style HW block.
>
> And the help text is not architecture specific anyway, is it not..?
>
Both the default y and and the help text are indeed incorrect. This
should be fixed. We don't enable device drivers by default, and
definitely not as as builtins. A conditional default m could be
acceptable if the condition is sufficiently narrow.
While at it, could we add a depends on CONFIG_OF since this code is
definitely unusable on non-DT systems.
Hi Ard,
> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Sunday, 19 April 2020 11:55
>
> > > > -----Original Message-----
> > > > From: Pavel Machek <[email protected]>
> > > > Sent: Saturday, 18 April 2020 13:44
> > > >
> > > > Hi!
> > > >
> > > > I'm configuring kernel for x86, and I get offered
> HW_RANDOM_CCTRNG
> > > > with default=Y, and help text suggesting I should enable it.
> > > >
> > > > That's... two wrong suggestions, right?
> > > >
> > > > Best regards,
> > > > Pavel
> > ...
> > > ________________________________________
> > > From: Hadar Gat <[email protected]>
> > > Sent: Saturday, April 18, 2020 11:31 PM
> > >
> > > Hi Pavel,
> > > I think you got it right..
> > > Indeed, Arm CryptoCell CCTRNG driver couldn't be used and obviously
> useless if the Arm CryptoCell HW does not exist in the system.
> >
> > There's a delicate point here though - CryptoCell is an independent
> > hardware block, it is not tied to a particular CPU architecture.
> > There are SoCs with none-Arm architecture CPU using it.
> >
> > So I would say whatever the answer is, it should be the same for any
> > generic embedded style HW block.
> >
> > And the help text is not architecture specific anyway, is it not..?
> >
>
> Both the default y and and the help text are indeed incorrect. This should be
> fixed. We don't enable device drivers by default, and definitely not as as
> builtins. A conditional default m could be acceptable if the condition is
> sufficiently narrow.
On one hand I totally agree with that and think the default should be N.
On the other hand, most of the HW_RANDOM drivers set the default to HW_RANDOM
and it doesn't make sense to me to do something different than almost every other HW RANDOM device.
Do I miss something here?
>
> While at it, could we add a depends on CONFIG_OF since this code is
> definitely unusable on non-DT systems.
On Sun, 26 Apr 2020 at 14:50, Hadar Gat <[email protected]> wrote:
>
> Hi Ard,
>
> > -----Original Message-----
> > From: Ard Biesheuvel <[email protected]>
> > Sent: Sunday, 19 April 2020 11:55
> >
> > > > > -----Original Message-----
> > > > > From: Pavel Machek <[email protected]>
> > > > > Sent: Saturday, 18 April 2020 13:44
> > > > >
> > > > > Hi!
> > > > >
> > > > > I'm configuring kernel for x86, and I get offered
> > HW_RANDOM_CCTRNG
> > > > > with default=Y, and help text suggesting I should enable it.
> > > > >
> > > > > That's... two wrong suggestions, right?
> > > > >
> > > > > Best regards,
> > > > > Pavel
> > > ...
> > > > ________________________________________
> > > > From: Hadar Gat <[email protected]>
> > > > Sent: Saturday, April 18, 2020 11:31 PM
> > > >
> > > > Hi Pavel,
> > > > I think you got it right..
> > > > Indeed, Arm CryptoCell CCTRNG driver couldn't be used and obviously
> > useless if the Arm CryptoCell HW does not exist in the system.
> > >
> > > There's a delicate point here though - CryptoCell is an independent
> > > hardware block, it is not tied to a particular CPU architecture.
> > > There are SoCs with none-Arm architecture CPU using it.
> > >
> > > So I would say whatever the answer is, it should be the same for any
> > > generic embedded style HW block.
> > >
> > > And the help text is not architecture specific anyway, is it not..?
> > >
> >
> > Both the default y and and the help text are indeed incorrect. This should be
> > fixed. We don't enable device drivers by default, and definitely not as as
> > builtins. A conditional default m could be acceptable if the condition is
> > sufficiently narrow.
>
> On one hand I totally agree with that and think the default should be N.
> On the other hand, most of the HW_RANDOM drivers set the default to HW_RANDOM
> and it doesn't make sense to me to do something different than almost every other HW RANDOM device.
> Do I miss something here?
>
Yes. First of all, using 'default HW_RANDOM' everywhere makes no sense
at all, but that is not your fault.
If you look at drivers/char/hw_random/Kconfig, you will see that most
drivers have additional depends lines, which means that 'default m'
(or 'default y' in case CONFIG_HW_RANDOM=y) will only take affect if
the dependency is fulfilled.
It makes no sense to enable this driver on *every* single Linux
system, right? Especially given that many architectures do not even
support device tree, which is a prerequisite to be able to even probe.
drivers/char/hw_random/Kconfig-config HW_RANDOM_INTEL
drivers/char/hw_random/Kconfig- depends on (X86 || IA64) && PCI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_AMD
drivers/char/hw_random/Kconfig- depends on (X86 || PPC_MAPLE) && PCI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_ATMEL
drivers/char/hw_random/Kconfig- depends on ARCH_AT91 && HAVE_CLK && OF
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_BCM2835
drivers/char/hw_random/Kconfig- depends on ARCH_BCM2835 ||
ARCH_BCM_NSP || ARCH_BCM_5301X || \
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_IPROC_RNG200
drivers/char/hw_random/Kconfig- depends on ARCH_BCM_IPROC ||
ARCH_BCM2835 || ARCH_BRCMSTB
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_GEODE
drivers/char/hw_random/Kconfig- depends on X86_32 && PCI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_N2RNG
drivers/char/hw_random/Kconfig- depends on SPARC64
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_VIA
drivers/char/hw_random/Kconfig- depends on X86
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_IXP4XX
drivers/char/hw_random/Kconfig- depends on ARCH_IXP4XX
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_OMAP
drivers/char/hw_random/Kconfig- depends on ARCH_OMAP16XX ||
ARCH_OMAP2PLUS || ARCH_MVEBU
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_OMAP3_ROM
drivers/char/hw_random/Kconfig- depends on ARCH_OMAP3
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_OCTEON
drivers/char/hw_random/Kconfig- depends on CAVIUM_OCTEON_SOC
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_PASEMI
drivers/char/hw_random/Kconfig- depends on PPC_PASEMI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_VIRTIO
drivers/char/hw_random/Kconfig- depends on VIRTIO
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_TX4939
drivers/char/hw_random/Kconfig- depends on SOC_TX4939
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_MXC_RNGA
drivers/char/hw_random/Kconfig- depends on SOC_IMX31
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_IMX_RNGC
drivers/char/hw_random/Kconfig- depends on HAS_IOMEM && HAVE_CLK
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_NOMADIK
drivers/char/hw_random/Kconfig- depends on ARCH_NOMADIK
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_PSERIES
drivers/char/hw_random/Kconfig- depends on PPC64 && IBMVIO
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_POWERNV
drivers/char/hw_random/Kconfig- depends on PPC_POWERNV
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_HISI
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARCH_HISI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_HISI_V2
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARM64 && ACPI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_ST
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARCH_STI
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_XGENE
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARCH_XGENE
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_STM32
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && (ARCH_STM32 ||
COMPILE_TEST)
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_PIC32
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && MACH_PIC32
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_MESON
drivers/char/hw_random/Kconfig- depends on HW_RANDOM
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_CAVIUM
drivers/char/hw_random/Kconfig- depends on HW_RANDOM && PCI && (ARM64
|| (COMPILE_TEST && 64BIT))
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_MTK
drivers/char/hw_random/Kconfig- depends on HW_RANDOM
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_S390
drivers/char/hw_random/Kconfig- depends on S390
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_EXYNOS
drivers/char/hw_random/Kconfig- depends on ARCH_EXYNOS || COMPILE_TEST
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_OPTEE
drivers/char/hw_random/Kconfig- depends on OPTEE
--
drivers/char/hw_random/Kconfig-config HW_RANDOM_NPCM
drivers/char/hw_random/Kconfig- depends on ARCH_NPCM || COMPILE_TEST
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Sunday, 19 April 2020 11:55
> > >
> > > Both the default y and and the help text are indeed incorrect. This
> > > should be fixed. We don't enable device drivers by default, and
> > > definitely not as as builtins. A conditional default m could be
> > > acceptable if the condition is sufficiently narrow.
> >
> > On one hand I totally agree with that and think the default should be N.
> > On the other hand, most of the HW_RANDOM drivers set the default to
> > HW_RANDOM and it doesn't make sense to me to do something different
> than almost every other HW RANDOM device.
> > Do I miss something here?
> >
>
> Yes. First of all, using 'default HW_RANDOM' everywhere makes no sense at
> all, but that is not your fault.
>
> If you look at drivers/char/hw_random/Kconfig, you will see that most
> drivers have additional depends lines, which means that 'default m'
> (or 'default y' in case CONFIG_HW_RANDOM=y) will only take affect if the
> dependency is fulfilled.
>
> It makes no sense to enable this driver on *every* single Linux system, right?
> Especially given that many architectures do not even support device tree,
> which is a prerequisite to be able to even probe.
>
Thanks you Ard for clearing it up for me.
I'll fix that shortly.
Hadar
>
>
>
> drivers/char/hw_random/Kconfig-config HW_RANDOM_INTEL
> drivers/char/hw_random/Kconfig- depends on (X86 || IA64) && PCI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_AMD
> drivers/char/hw_random/Kconfig- depends on (X86 || PPC_MAPLE) && PCI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_ATMEL
> drivers/char/hw_random/Kconfig- depends on ARCH_AT91 && HAVE_CLK
> && OF
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_BCM2835
> drivers/char/hw_random/Kconfig- depends on ARCH_BCM2835 ||
> ARCH_BCM_NSP || ARCH_BCM_5301X || \
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_IPROC_RNG200
> drivers/char/hw_random/Kconfig- depends on ARCH_BCM_IPROC ||
> ARCH_BCM2835 || ARCH_BRCMSTB
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_GEODE
> drivers/char/hw_random/Kconfig- depends on X86_32 && PCI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_N2RNG
> drivers/char/hw_random/Kconfig- depends on SPARC64
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_VIA
> drivers/char/hw_random/Kconfig- depends on X86
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_IXP4XX
> drivers/char/hw_random/Kconfig- depends on ARCH_IXP4XX
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_OMAP
> drivers/char/hw_random/Kconfig- depends on ARCH_OMAP16XX ||
> ARCH_OMAP2PLUS || ARCH_MVEBU
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_OMAP3_ROM
> drivers/char/hw_random/Kconfig- depends on ARCH_OMAP3
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_OCTEON
> drivers/char/hw_random/Kconfig- depends on CAVIUM_OCTEON_SOC
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_PASEMI
> drivers/char/hw_random/Kconfig- depends on PPC_PASEMI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_VIRTIO
> drivers/char/hw_random/Kconfig- depends on VIRTIO
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_TX4939
> drivers/char/hw_random/Kconfig- depends on SOC_TX4939
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_MXC_RNGA
> drivers/char/hw_random/Kconfig- depends on SOC_IMX31
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_IMX_RNGC
> drivers/char/hw_random/Kconfig- depends on HAS_IOMEM && HAVE_CLK
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_NOMADIK
> drivers/char/hw_random/Kconfig- depends on ARCH_NOMADIK
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_PSERIES
> drivers/char/hw_random/Kconfig- depends on PPC64 && IBMVIO
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_POWERNV
> drivers/char/hw_random/Kconfig- depends on PPC_POWERNV
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_HISI
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM &&
> ARCH_HISI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_HISI_V2
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARM64
> && ACPI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_ST
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM && ARCH_STI
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_XGENE
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM &&
> ARCH_XGENE
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_STM32
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM &&
> (ARCH_STM32 ||
> COMPILE_TEST)
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_PIC32
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM &&
> MACH_PIC32
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_MESON
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_CAVIUM
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM && PCI &&
> (ARM64
> || (COMPILE_TEST && 64BIT))
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_MTK
> drivers/char/hw_random/Kconfig- depends on HW_RANDOM
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_S390
> drivers/char/hw_random/Kconfig- depends on S390
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_EXYNOS
> drivers/char/hw_random/Kconfig- depends on ARCH_EXYNOS ||
> COMPILE_TEST
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_OPTEE
> drivers/char/hw_random/Kconfig- depends on OPTEE
> --
> drivers/char/hw_random/Kconfig-config HW_RANDOM_NPCM
> drivers/char/hw_random/Kconfig- depends on ARCH_NPCM ||
> COMPILE_TEST