From: Marc Zyngier <maz@misterjones.org>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Ard Biesheuvel <ardb@kernel.org>,
Russell King <linux@armlinux.org.uk>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Linus Walleij <linus.walleij@linaro.org>,
linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>,
Sudeep Holla <sudeep.holla@arm.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 5/5] KVM: arm64: implement the TRNG hypervisor call
Date: Thu, 05 Nov 2020 14:13:10 +0000 [thread overview]
Message-ID: <56c0505297b4588b413ae57ee39b894f@misterjones.org> (raw)
In-Reply-To: <20201105125656.25259-6-andre.przywara@arm.com>
On 2020-11-05 12:56, Andre Przywara wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Provide a hypervisor implementation of the ARM architected TRNG
> firmware
> interface described in ARM spec DEN0098. All function IDs are
> implemented,
> including both 32-bit and 64-bit versions of the TRNG_RND service,
> which
> is the centerpiece of the API.
>
> The API is backed by arch_get_unsigned_seed_long(), which is
> implemented
> in terms of RNDRRS currently, and will be alternatively backed by a SMC
> call to the secure firmware using same interface after a future patch.
> If neither are available, the kernel's entropy pool is used instead.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 2 +
> arch/arm64/kvm/Makefile | 2 +-
> arch/arm64/kvm/hypercalls.c | 6 ++
> arch/arm64/kvm/trng.c | 91 +++++++++++++++++++++++++++++++
> 4 files changed, 100 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kvm/trng.c
>
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 781d029b8aa8..615932bacf76 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -652,4 +652,6 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu
> *vcpu);
> #define kvm_arm_vcpu_sve_finalized(vcpu) \
> ((vcpu)->arch.flags & KVM_ARM64_VCPU_SVE_FINALIZED)
>
> +int kvm_trng_call(struct kvm_vcpu *vcpu);
> +
> #endif /* __ARM64_KVM_HOST_H__ */
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 1504c81fbf5d..a510037e3270 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -16,7 +16,7 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o
> $(KVM)/eventfd.o \
> inject_fault.o regmap.o va_layout.o handle_exit.o \
> guest.o debug.o reset.o sys_regs.o \
> vgic-sys-reg-v3.o fpsimd.o pmu.o \
> - aarch32.o arch_timer.o \
> + aarch32.o arch_timer.o trng.o \
> vgic/vgic.o vgic/vgic-init.o \
> vgic/vgic-irqfd.o vgic/vgic-v2.o \
> vgic/vgic-v3.o vgic/vgic-v4.o \
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 25ea4ecb6449..ead21b98b620 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -71,6 +71,12 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> if (gpa != GPA_INVALID)
> val = gpa;
> break;
> + case ARM_SMCCC_TRNG_VERSION:
> + case ARM_SMCCC_TRNG_FEATURES:
> + case ARM_SMCCC_TRNG_GET_UUID:
> + case ARM_SMCCC_TRNG_RND32:
> + case ARM_SMCCC_TRNG_RND64:
> + return kvm_trng_call(vcpu);
> default:
> return kvm_psci_call(vcpu);
> }
> diff --git a/arch/arm64/kvm/trng.c b/arch/arm64/kvm/trng.c
> new file mode 100644
> index 000000000000..5a27b2d99977
> --- /dev/null
> +++ b/arch/arm64/kvm/trng.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2020 Arm Ltd.
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/kvm_host.h>
> +
> +#include <asm/kvm_emulate.h>
> +
> +#include <kvm/arm_hypercalls.h>
> +
> +#define ARM_SMCCC_TRNG_VERSION_1_0 0x10000UL
> +
> +#define TRNG_SUCCESS 0UL
SMCCC_RET_SUCCESS
> +#define TRNG_NOT_SUPPORTED ((unsigned long)-1)
SMCCC_RET_NOT_SUPPORTED
> +#define TRNG_INVALID_PARAMETER ((unsigned long)-2)
*crap*. Why isn't that the same value as SMCCC_RET_INVALID_PARAMETER?
Is it too late to fix the spec?
> +#define TRNG_NO_ENTROPY ((unsigned long)-3)
> +
> +#define MAX_BITS32 96
> +#define MAX_BITS64 192
Nothing seems to be using these definitions.
> +
> +static const uuid_t arm_smc_trng_uuid __aligned(4) = UUID_INIT(
> + 0x023534a2, 0xe0bc, 0x45ec, 0x95, 0xdd, 0x33, 0x34, 0xc1, 0xcc, 0x31,
> 0x89);
I object to the lack of Easter egg in this UUID. Or at least one I can
understand. ;-)
> +
> +static int kvm_trng_do_rnd(struct kvm_vcpu *vcpu, int size)
> +{
> + u32 num_bits = smccc_get_arg1(vcpu);
> + u64 bits[3];
> + int i;
> +
> + if (num_bits > 3 * size) {
> + smccc_set_retval(vcpu, TRNG_INVALID_PARAMETER, 0, 0, 0);
> + return 1;
> + }
> +
> + /* get as many bits as we need to fulfil the request */
> + for (i = 0; i < DIV_ROUND_UP(num_bits, 64); i++)
> + /* use the arm64 specific backend directly if one exists */
> + if (!arch_get_random_seed_long((unsigned long *)&bits[i]))
> + bits[i] = get_random_long();
> +
> + if (num_bits % 64)
> + bits[i - 1] &= U64_MAX >> (64 - (num_bits % 64));
> +
> + while (i < ARRAY_SIZE(bits))
> + bits[i++] = 0;
I just wasted 3 minutes trying to understand what this was doing, only
to
realise this is clearing the [MAX_BITS:num_bits] range.
How about using a bitmap instead? It would result in the exact same data
structure, only much more readable (and no u64->unsigned long casts).
> +
> + if (size == 32)
> + smccc_set_retval(vcpu, TRNG_SUCCESS, lower_32_bits(bits[1]),
> + upper_32_bits(bits[0]), lower_32_bits(bits[0]));
> + else
> + smccc_set_retval(vcpu, TRNG_SUCCESS, bits[2], bits[1], bits[0]);
> +
> + memzero_explicit(bits, sizeof(bits));
> + return 1;
> +}
> +
> +int kvm_trng_call(struct kvm_vcpu *vcpu)
> +{
> + const __le32 *u = (__le32 *)arm_smc_trng_uuid.b;
> + u32 func_id = smccc_get_function(vcpu);
> + unsigned long val = TRNG_NOT_SUPPORTED;
> + int size = 64;
> +
> + switch (func_id) {
> + case ARM_SMCCC_TRNG_VERSION:
> + val = ARM_SMCCC_TRNG_VERSION_1_0;
> + break;
> + case ARM_SMCCC_TRNG_FEATURES:
> + switch (smccc_get_arg1(vcpu)) {
> + case ARM_SMCCC_TRNG_VERSION:
> + case ARM_SMCCC_TRNG_FEATURES:
> + case ARM_SMCCC_TRNG_GET_UUID:
> + case ARM_SMCCC_TRNG_RND32:
> + case ARM_SMCCC_TRNG_RND64:
> + val = TRNG_SUCCESS;
> + }
> + break;
> + case ARM_SMCCC_TRNG_GET_UUID:
> + smccc_set_retval(vcpu, le32_to_cpu(u[0]), le32_to_cpu(u[1]),
> + le32_to_cpu(u[2]), le32_to_cpu(u[3]));
> + return 1;
> + case ARM_SMCCC_TRNG_RND32:
> + size = 32;
> + fallthrough;
> + case ARM_SMCCC_TRNG_RND64:
> + return kvm_trng_do_rnd(vcpu, size);
> + }
> +
> + smccc_set_retval(vcpu, val, 0, 0, 0);
> + return 1;
> +}
Thanks,
M.
--
Who you jivin' with that Cosmik Debris?
next prev parent reply other threads:[~2020-11-05 14:55 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-05 12:56 [PATCH v2 0/5] ARM: arm64: Add SMCCC TRNG entropy service Andre Przywara
2020-11-05 12:56 ` [PATCH v2 1/5] firmware: smccc: Add SMCCC TRNG function call IDs Andre Przywara
2020-11-05 12:56 ` [PATCH v2 2/5] firmware: smccc: Introduce SMCCC TRNG framework Andre Przywara
2020-11-05 12:56 ` [PATCH v2 3/5] ARM: implement support for SMCCC TRNG entropy source Andre Przywara
2020-11-05 17:15 ` kernel test robot
2020-11-05 17:56 ` André Przywara
2020-11-05 17:15 ` kernel test robot
2020-11-06 15:29 ` Marc Zyngier
2020-11-06 15:30 ` Ard Biesheuvel
2020-11-06 15:35 ` Marc Zyngier
2020-11-05 12:56 ` [PATCH v2 4/5] arm64: Add " Andre Przywara
2020-11-05 13:41 ` Mark Brown
2020-11-05 14:03 ` Mark Rutland
2020-11-05 14:04 ` Ard Biesheuvel
2020-11-05 14:30 ` Mark Rutland
2020-11-05 14:34 ` Ard Biesheuvel
2020-11-05 14:45 ` Mark Rutland
2020-11-05 14:48 ` Marc Zyngier
2020-11-05 14:29 ` Mark Brown
2020-11-05 14:38 ` Mark Rutland
2020-11-12 16:03 ` André Przywara
2020-11-05 14:30 ` André Przywara
2020-11-05 12:56 ` [PATCH v2 5/5] KVM: arm64: implement the TRNG hypervisor call Andre Przywara
2020-11-05 14:13 ` Marc Zyngier [this message]
2020-11-05 16:19 ` 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=56c0505297b4588b413ae57ee39b894f@misterjones.org \
--to=maz@misterjones.org \
--cc=andre.przywara@arm.com \
--cc=ardb@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=lorenzo.pieralisi@arm.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).