Re: [PATCH] KVM: arm64: implement the TRNG hypervisor call
On Sat, 3 Oct 2020 at 12:30, Andrew Jones wrote:
>
> Hi Ard,
>
Hi Drew,
Thanks for taking a look.
> On Sat, Oct 03, 2020 at 10:56:04AM +0200, Ard Biesheuvel wrote:
> > 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_random_seed_long(), which is implemented
> > in terms of RNDR 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.
> >
> > Cc: Marc Zyngier
> > Cc: James Morse
> > Cc: Julien Thierry
> > Cc: Suzuki K Poulose
> > Cc: Catalin Marinas
> > Cc: Will Deacon
> > Cc: Mark Rutland
> > Cc: Lorenzo Pieralisi
> > Cc: Sudeep Holla
> > Cc: Sami Mujawar
> > Cc: Andre Przywara
> > Cc: Alexandru Elisei
> > Cc: Laszlo Ersek
> > Cc: Leif Lindholm
> > Signed-off-by: Ard Biesheuvel
> > ---
> > 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
> > include/linux/arm-smccc.h | 31 +++
> > 5 files changed, 131 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h
> > b/arch/arm64/include/asm/kvm_host.h
> > index 65568b23868a..f76164d390ea 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -688,4 +688,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 99977c1972cc..e117d7086500 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 hyp.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 550dfa3e53cd..70c5e815907d 100644
> > --- a/arch/arm64/kvm/hypercalls.c
> > +++ b/arch/arm64/kvm/hypercalls.c
> > @@ -62,6 +62,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_RND:
> > + 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 ..71f704075e4a
> > --- /dev/null
> > +++ b/arch/arm64/kvm/trng.c
> > @@ -0,0 +1,91 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2020 Arm Ltd.
> > +
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +#include
> > +
> > +#define ARM_SMCCC_TRNG_VERSION_1_0 0x1UL
> > +
> > +#define TRNG_SUCCESS 0UL
> > +#define TRNG_NOT_SUPPORTED ((unsigned long)-1)
> > +#define TRNG_INVALID_PARAMETER ((unsigned long)-2)
> > +#define TRNG_NO_ENTROPY ((unsigned long)-3)
> > +
> > +#define MAX_BITS32 96
> > +#define MAX_BITS64 192
> > +
> > +static const uuid_t arm_smc_trng_uuid __aligned(4) = UUID_INIT(
> > + 0x023534a2, 0xe0bc, 0x45ec, 0x95, 0xdd, 0x33, 0x34, 0xc1, 0xcc, 0x31,
> > 0x89);
>
> Where does this UUID come from?
>
uuidgen
The only requirement for the UUID is that we can distinguish
implementations, in order to be able to disregard them if they are
known to have flaws.
You could make a case for passing the firmware's UUID if the calls are
relayed directly, but that would remove the caller's ability to avoid
the KVM on specifically.
> > +
> > +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_NOT_SUPPORTED, 0, 0, 0);
>
> The spec says we should return "INVALID_PARAMETERS" when N > MAX_BITS.
> Of course "INVALID_PARAMETERS", with the 'S', doesn't exist in "Return
> codes",
Re: [PATCH] KVM: arm64: implement the TRNG hypervisor call
Hi Ard,
On Sat, Oct 03, 2020 at 10:56:04AM +0200, Ard Biesheuvel wrote:
> 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_random_seed_long(), which is implemented
> in terms of RNDR 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.
>
> Cc: Marc Zyngier
> Cc: James Morse
> Cc: Julien Thierry
> Cc: Suzuki K Poulose
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Mark Rutland
> Cc: Lorenzo Pieralisi
> Cc: Sudeep Holla
> Cc: Sami Mujawar
> Cc: Andre Przywara
> Cc: Alexandru Elisei
> Cc: Laszlo Ersek
> Cc: Leif Lindholm
> Signed-off-by: Ard Biesheuvel
> ---
> 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
> include/linux/arm-smccc.h | 31 +++
> 5 files changed, 131 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 65568b23868a..f76164d390ea 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -688,4 +688,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 99977c1972cc..e117d7086500 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 hyp.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 550dfa3e53cd..70c5e815907d 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -62,6 +62,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_RND:
> + 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 ..71f704075e4a
> --- /dev/null
> +++ b/arch/arm64/kvm/trng.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2020 Arm Ltd.
> +
> +#include
> +#include
> +
> +#include
> +
> +#include
> +
> +#define ARM_SMCCC_TRNG_VERSION_1_0 0x1UL
> +
> +#define TRNG_SUCCESS 0UL
> +#define TRNG_NOT_SUPPORTED ((unsigned long)-1)
> +#define TRNG_INVALID_PARAMETER ((unsigned long)-2)
> +#define TRNG_NO_ENTROPY ((unsigned long)-3)
> +
> +#define MAX_BITS32 96
> +#define MAX_BITS64 192
> +
> +static const uuid_t arm_smc_trng_uuid __aligned(4) = UUID_INIT(
> + 0x023534a2, 0xe0bc, 0x45ec, 0x95, 0xdd, 0x33, 0x34, 0xc1, 0xcc, 0x31,
> 0x89);
Where does this UUID come from?
> +
> +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_NOT_SUPPORTED, 0, 0, 0);
The spec says we should return "INVALID_PARAMETERS" when N > MAX_BITS.
Of course "INVALID_PARAMETERS", with the 'S', doesn't exist in "Return
codes", but I guess they meant "INVALID_PARAMETER", not NOT_SUPPORTED.
> + 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 *)[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;
> +
> + if (size == 32)
> +
[PATCH] KVM: arm64: implement the TRNG hypervisor call
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_random_seed_long(), which is implemented
in terms of RNDR 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.
Cc: Marc Zyngier
Cc: James Morse
Cc: Julien Thierry
Cc: Suzuki K Poulose
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Mark Rutland
Cc: Lorenzo Pieralisi
Cc: Sudeep Holla
Cc: Sami Mujawar
Cc: Andre Przywara
Cc: Alexandru Elisei
Cc: Laszlo Ersek
Cc: Leif Lindholm
Signed-off-by: Ard Biesheuvel
---
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
include/linux/arm-smccc.h | 31 +++
5 files changed, 131 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
index 65568b23868a..f76164d390ea 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -688,4 +688,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 99977c1972cc..e117d7086500 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 hyp.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 550dfa3e53cd..70c5e815907d 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -62,6 +62,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_RND:
+ 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 ..71f704075e4a
--- /dev/null
+++ b/arch/arm64/kvm/trng.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2020 Arm Ltd.
+
+#include
+#include
+
+#include
+
+#include
+
+#define ARM_SMCCC_TRNG_VERSION_1_0 0x1UL
+
+#define TRNG_SUCCESS 0UL
+#define TRNG_NOT_SUPPORTED ((unsigned long)-1)
+#define TRNG_INVALID_PARAMETER ((unsigned long)-2)
+#define TRNG_NO_ENTROPY((unsigned long)-3)
+
+#define MAX_BITS32 96
+#define MAX_BITS64 192
+
+static const uuid_t arm_smc_trng_uuid __aligned(4) = UUID_INIT(
+ 0x023534a2, 0xe0bc, 0x45ec, 0x95, 0xdd, 0x33, 0x34, 0xc1, 0xcc, 0x31,
0x89);
+
+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_NOT_SUPPORTED, 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 *)[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;
+
+ 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 __be32 *u = (__be32 *)arm_smc_trng_uuid.b;
+ u32 func_id = smccc_get_function(vcpu);
+ unsigned long val = TRNG_NOT_SUPPORTED;
+
