Re: [PATCH] spapr: Add capability for Secure (PEF) VMs

[Dr. David Alan Gilbert]

* David Gibson (address@hidden) wrote:
> On Fri, May 01, 2020 at 04:02:49PM +1000, David Gibson wrote:
> > Recent POWER9 machines have a system called PEF (Protected Execution
> > Framework) which uses a small ultravisor to allow guests to run in a
> > 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>
> 
> Hm, so.  I'm reconsidering this.  I'm thinking I should probably try
> to make this configuration more like what AMD SEV does, since this is
> a very similar functionality.

Other than setting the 'we support PEF' flag, is there anything else
you're going to have to do - for example with SEV there's stuff to pass
a block of data and to do attestations and .... it's not just setting a
flag; but my understanding of PEF it's more driven from the guest.

Dave

> > ---
> >  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");
> > +    } 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");
> > +        }
> > +    }
> > +}
> > +
> >  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);
> >  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;
> 
> -- 
> David Gibson                  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au        | minimalist, thank you.  NOT _the_ 
> _other_
>                               | _way_ _around_!
> http://www.ozlabs.org/~dgibson


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK