All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: "André Przywara" <andre.przywara@arm.com>
Cc: James Morse <james.morse@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2] arm64: Add support for SMCCC TRNG firmware interface
Date: Mon, 12 Oct 2020 08:35:26 +0200	[thread overview]
Message-ID: <CAMj1kXGrHxZprjY9ZJPKMLeFHVhcsQNT2uCcqCv2LJuuQ2fAUg@mail.gmail.com> (raw)
In-Reply-To: <d5ca622f-5eec-4898-e3b5-af3e26de7cf0@arm.com>

On Wed, 7 Oct 2020 at 16:44, André Przywara <andre.przywara@arm.com> wrote:
>
> On 07/10/2020 15:16, James Morse wrote:
>
> Hi,
>
> > On 06/10/2020 21:18, Andre Przywara wrote:
> >> The ARM architected TRNG firmware interface, described in ARM spec
> >> DEN0098[1], defines an ARM SMCCC based interface to a true random number
> >> generator, provided by firmware.
> >> This can be discovered via the SMCCC >=v1.1 interface, and provides
> >> up to 192 bits of entropy per call.
> >>
> >> Hook this SMC call into arm64's arch_get_random_*() implementation,
> >> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
> >> system registers.
> >>
> >> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
> >> us the conduit to use: hvc or smc), then try to call the
> >> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
> >> not implemented.
> >
> >>  arch/arm64/include/asm/archrandom.h | 83 +++++++++++++++++++++++++----
> >>  1 file changed, 73 insertions(+), 10 deletions(-)
> >
> >> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> >> index ffb1a40d5475..b6c291c42a48 100644
> >> --- a/arch/arm64/include/asm/archrandom.h
> >> +++ b/arch/arm64/include/asm/archrandom.h
> >> @@ -7,6 +7,13 @@
> >>  #include <linux/bug.h>
> >>  #include <linux/kernel.h>
> >>  #include <asm/cpufeature.h>
> >> +#include <linux/arm-smccc.h>
> >> +
> >> +static enum smc_trng_status {
> >> +    SMC_TRNG_UNKNOWN,
> >> +    SMC_TRNG_NOT_SUPPORTED,
> >> +    SMC_TRNG_SUPPORTED
> >> +} smc_trng_status = SMC_TRNG_UNKNOWN;
> >
> > Doesn't this static variable in a header file mean each file that includes this has its
> > own copy? Is that intentional?
>
> Right, and it's not intentional. It doesn't really break, but since
> random.h includes archrandom.h, we get an instance everywhere :-(
>
> I wasn't too happy with this detection method to begin with (and also
> not with stuffing everything into a header file), but wanted to
> accommodate the early case, where PSCI hasn't been initialised yet, and
> so we don't know the SMCCC conduit. A static key sounds better, but gets
> a bit hairy with this scenario, I think.
>
> Any ideas here?

I think the early case isn't worth obsessing about. PSCI is
initialized in setup_arch(), which gets called way before
rand_initialize(), which is where this functionality will get used the
first time typically. And kaslr_early_init() is called extremely
early, i.e., straight from head.S, and we should avoid adding any more
code there that sets global state (if kaslr_early_init() exits
successfully, the kernel will be unmapped and remapped again in a
different place, and BSS cleared again etc etc)

> I could copy Ard's solution and introduce random.c, if that makes more
> sense.
>
> Cheers,
> Andre

WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org>
To: "André Przywara" <andre.przywara@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/2] arm64: Add support for SMCCC TRNG firmware interface
Date: Mon, 12 Oct 2020 08:35:26 +0200	[thread overview]
Message-ID: <CAMj1kXGrHxZprjY9ZJPKMLeFHVhcsQNT2uCcqCv2LJuuQ2fAUg@mail.gmail.com> (raw)
In-Reply-To: <d5ca622f-5eec-4898-e3b5-af3e26de7cf0@arm.com>

On Wed, 7 Oct 2020 at 16:44, André Przywara <andre.przywara@arm.com> wrote:
>
> On 07/10/2020 15:16, James Morse wrote:
>
> Hi,
>
> > On 06/10/2020 21:18, Andre Przywara wrote:
> >> The ARM architected TRNG firmware interface, described in ARM spec
> >> DEN0098[1], defines an ARM SMCCC based interface to a true random number
> >> generator, provided by firmware.
> >> This can be discovered via the SMCCC >=v1.1 interface, and provides
> >> up to 192 bits of entropy per call.
> >>
> >> Hook this SMC call into arm64's arch_get_random_*() implementation,
> >> coming to the rescue when the CPU does not implement the ARM v8.5 RNG
> >> system registers.
> >>
> >> For the detection, we piggy back on the PSCI/SMCCC discovery (which gives
> >> us the conduit to use: hvc or smc), then try to call the
> >> ARM_SMCCC_TRNG_VERSION function, which returns -1 if this interface is
> >> not implemented.
> >
> >>  arch/arm64/include/asm/archrandom.h | 83 +++++++++++++++++++++++++----
> >>  1 file changed, 73 insertions(+), 10 deletions(-)
> >
> >> diff --git a/arch/arm64/include/asm/archrandom.h b/arch/arm64/include/asm/archrandom.h
> >> index ffb1a40d5475..b6c291c42a48 100644
> >> --- a/arch/arm64/include/asm/archrandom.h
> >> +++ b/arch/arm64/include/asm/archrandom.h
> >> @@ -7,6 +7,13 @@
> >>  #include <linux/bug.h>
> >>  #include <linux/kernel.h>
> >>  #include <asm/cpufeature.h>
> >> +#include <linux/arm-smccc.h>
> >> +
> >> +static enum smc_trng_status {
> >> +    SMC_TRNG_UNKNOWN,
> >> +    SMC_TRNG_NOT_SUPPORTED,
> >> +    SMC_TRNG_SUPPORTED
> >> +} smc_trng_status = SMC_TRNG_UNKNOWN;
> >
> > Doesn't this static variable in a header file mean each file that includes this has its
> > own copy? Is that intentional?
>
> Right, and it's not intentional. It doesn't really break, but since
> random.h includes archrandom.h, we get an instance everywhere :-(
>
> I wasn't too happy with this detection method to begin with (and also
> not with stuffing everything into a header file), but wanted to
> accommodate the early case, where PSCI hasn't been initialised yet, and
> so we don't know the SMCCC conduit. A static key sounds better, but gets
> a bit hairy with this scenario, I think.
>
> Any ideas here?

I think the early case isn't worth obsessing about. PSCI is
initialized in setup_arch(), which gets called way before
rand_initialize(), which is where this functionality will get used the
first time typically. And kaslr_early_init() is called extremely
early, i.e., straight from head.S, and we should avoid adding any more
code there that sets global state (if kaslr_early_init() exits
successfully, the kernel will be unmapped and remapped again in a
different place, and BSS cleared again etc etc)

> I could copy Ard's solution and introduce random.c, if that makes more
> sense.
>
> Cheers,
> Andre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-10-12  6:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06 20:18 [PATCH 0/2] arm64: Add support for SMCCC TRNG interface Andre Przywara
2020-10-06 20:18 ` Andre Przywara
2020-10-06 20:18 ` [PATCH 1/2] firmware: smccc: Add SMCCC TRNG function call IDs Andre Przywara
2020-10-06 20:18   ` Andre Przywara
2020-10-06 20:18 ` [PATCH 2/2] arm64: Add support for SMCCC TRNG firmware interface Andre Przywara
2020-10-06 20:18   ` Andre Przywara
2020-10-07 10:48   ` Mark Brown
2020-10-07 10:48     ` Mark Brown
2020-10-07 14:16   ` James Morse
2020-10-07 14:16     ` James Morse
2020-10-07 14:43     ` André Przywara
2020-10-07 14:43       ` André Przywara
2020-10-12  6:35       ` Ard Biesheuvel [this message]
2020-10-12  6:35         ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMj1kXGrHxZprjY9ZJPKMLeFHVhcsQNT2uCcqCv2LJuuQ2fAUg@mail.gmail.com \
    --to=ardb@kernel.org \
    --cc=andre.przywara@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=richard.henderson@linaro.org \
    --cc=sudeep.holla@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.