|
| From: | Ramachandra Pai |
| Subject: | RE: [PATCH] spapr: Add capability for Secure (PEF) VMs |
| Date: | Mon, 4 May 2020 17:46:33 +0000 |
----- Original message -----
From: Greg Kurz <address@hidden>
To: David Gibson <address@hidden>
Cc: address@hidden, address@hidden, address@hidden, address@hidden, address@hidden, address@hidden
Subject: [EXTERNAL] Re: [PATCH] spapr: Add capability for Secure (PEF) VMs
Date: Mon, May 4, 2020 12:51 AM
On Fri, 1 May 2020 16:02:49 +1000
David Gibson <address@hidden> wrote:
> Recent POWER9 machines have a system called PEF (Protected Execution
> Framework) which uses a small ultravisor to allow guests to run in a
In most docs I've seen so far, the 'F' stands for 'Facility'.
https://www.kernel.org/doc/html/latest/powerpc/ultravisor.html
> way that they can't be eavesdropped by the hypervisor. The effect is
> roughly similar to AMD SEV, although the mechanisms are quite
> different.
>
> Most of the work of this is done between the guest, KVM and the
> ultravisor, with little need for involvement by qemu. However qemu
> does need to tell KVM to allow secure VMs.
>
> Because the availability of secure mode is a guest visible difference
> which depends on havint the right hardware and firmware, we don't
> enable this by default. In order to run a secure guest you need to
> set the new 'cap-allow-secure-guest' flag on. Note that this just
> *allows* secure guests, the architecture of PEF is such that the guest
> still needs to talk to the ultravisor to enter secure mode, so we
> don't know if the guest actually is secure at machine creation time.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
> hw/ppc/spapr_caps.c | 32 ++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 4 +++-
> target/ppc/kvm.c | 11 +++++++++++
> target/ppc/kvm_ppc.h | 25 +++++++++++++++++++++++++
> 4 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index eb54f94227..d37992b389 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -525,6 +525,29 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
> }
> }
>
> +static void cap_allow_secure_guest_apply(SpaprMachineState *spapr,
> + uint8_t val, Error **errp)
> +{
> + if (!val) {
> + /* capability disabled by default */
> + return;
> + }
> +
> + if (tcg_enabled()) {
> + error_setg(errp,
> + "No Secure VM support in tcg,"
> + " try appending -machine cap-allow-secure-guest=off");
I suggest you use the error_append_hint() API. Appart from being clearer, it
also has the effect of not printing the hint in the case of QMP, which is the
expected behavior (hints are intended for a human user only).
This being said, I see that only the recently added fwnmi cap does that,
so maybe worth fixing this globally as a follow-up...
> + } else if (kvm_enabled()) {
> + if (!kvmppc_has_cap_secure_guest()) {
> + error_setg(errp,
> +"KVM implementation does not support Secure VMs (is an ultravisor running?)");
> + } else if (kvmppc_set_cap_secure_guest(val) < 0) {
> + error_setg(errp,
> +"Error enabling cap-allow-secure-guest with KVM, try cap-allow-secure-guest=off");
Ditto.
> + }
> + }
> +}
> +
> SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> [SPAPR_CAP_HTM] = {
> .name = "htm",
> @@ -633,6 +656,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
> .type = "bool",
> .apply = cap_fwnmi_apply,
> },
> + [SPAPR_CAP_ALLOW_SECURE_GUEST] = {
> + .name = "allow-secure-guest",
> + .description = "Allows guest to enter Ultravisor secure mode",
> + .index = SPAPR_CAP_ALLOW_SECURE_GUEST,
> + .get = spapr_cap_get_bool,
> + .set = spapr_cap_set_bool,
> + .type = "bool",
> + .apply = cap_allow_secure_guest_apply,
> + },
> };
>
> static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index e579eaf28c..eef773c0cc 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -81,8 +81,10 @@ typedef enum {
> #define SPAPR_CAP_CCF_ASSIST 0x09
> /* Implements PAPR FWNMI option */
> #define SPAPR_CAP_FWNMI 0x0A
> +/* Allows guest to enter secure mode with ultravisor */
> +#define SPAPR_CAP_ALLOW_SECURE_GUEST 0x0B
> /* Num Caps */
> -#define SPAPR_CAP_NUM (SPAPR_CAP_FWNMI + 1)
> +#define SPAPR_CAP_NUM (SPAPR_CAP_ALLOW_SECURE_GUEST + 1)
>
> /*
> * Capability Values
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 2692f76130..1e68a11745 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2539,6 +2539,17 @@ int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
> return 0;
> }
>
> +bool kvmppc_has_cap_secure_guest(void)
> +{
> + return !!kvm_check_extension(kvm_state, KVM_CAP_PPC_SECURE_GUEST);
> +}
> +
> +int kvmppc_set_cap_secure_guest(int enable)
> +{
> + return kvm_vm_enable_cap(kvm_state, KVM_CAP_PPC_SECURE_GUEST, 0, enable);
> +}
> +
> +
> PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
> {
> uint32_t host_pvr = mfpvr();
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index fcaf745516..4bd2f7e255 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -72,6 +72,8 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
> int kvmppc_set_cap_nested_kvm_hv(int enable);
> int kvmppc_get_cap_large_decr(void);
> int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> +bool kvmppc_has_cap_secure_vm(void);
> +int kvmppc_set_cap_secure_vm(int enable);
Hmm... these two aren't used anywhere. It looks like a leftover.
> int kvmppc_enable_hwrng(void);
> int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> @@ -87,6 +89,9 @@ void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
>
> int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
>
> +bool kvmppc_has_cap_secure_guest(void);
> +int kvmppc_set_cap_secure_guest(int enable);
> +
> #else
>
> static inline uint32_t kvmppc_get_tbfreq(void)
> @@ -231,6 +236,16 @@ static inline void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
> {
> }
>
> +static inline bool kvmppc_has_cap_secure_guest(void)
> +{
> + return false;
> +}
> +
> +static inline int kvmppc_set_cap_secure_guest(int enable)
> +{
> + g_assert_not_reached();
> +}
> +
> #ifndef CONFIG_USER_ONLY
> static inline bool kvmppc_spapr_use_multitce(void)
> {
> @@ -386,6 +401,16 @@ static inline int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
> return -1;
> }
>
> +static inline bool kvmppc_has_cap_secure_vm(void)
> +{
> + return false;
> +}
> +
> +static inline int kvmppc_set_cap_secure_vm(int enable)
> +{
> + return -1;
> +}
> +
> static inline int kvmppc_enable_hwrng(void)
> {
> return -1;
| [Prev in Thread] | Current Thread | [Next in Thread] |