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

[Greg Kurz]

On Tue, 5 May 2020 09:17:19 +0100
"Dr. David Alan Gilbert" <address@hidden> wrote:

> * 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.
> 

Yeah, PEF is controlled by a small FW called ultravisor and driven by
the guest. Here's a high level view:

https://santoshs.github.io/images/ultra.png

QEMU doesn't interact directly with the ultravisor, but KVM HV does.
It has a KVM_CAP_PPC_SECURE_GUEST capability which can be used by
QEMU to authorize/forbid the VM to be secure. Also when the VM is
reset, QEMU needs to invoke a KVM_PPC_SVM_OFF ioctl for housekeeping
purposes.

> 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
>