mbox series

[for-5.2,v4,00/10] Generalize memory encryption models

Message ID 20200724025744.69644-1-david@gibson.dropbear.id.au
Headers show
Series Generalize memory encryption models | expand

Message

David Gibson July 24, 2020, 2:57 a.m. UTC
A number of hardware platforms are implementing mechanisms whereby the
hypervisor does not have unfettered access to guest memory, in order
to mitigate the security impact of a compromised hypervisor.

AMD's SEV implements this with in-cpu memory encryption, and Intel has
its own memory encryption mechanism.  POWER has an upcoming mechanism
to accomplish this in a different way, using a new memory protection
level plus a small trusted ultravisor.  s390 also has a protected
execution environment.

The current code (committed or draft) for these features has each
platform's version configured entirely differently.  That doesn't seem
ideal for users, or particularly for management layers.

AMD SEV introduces a notionally generic machine option
"machine-encryption", but it doesn't actually cover any cases other
than SEV.

This series is a proposal to at least partially unify configuration
for these mechanisms, by renaming and generalizing AMD's
"memory-encryption" property.  It is replaced by a
"host-trust-limitation" property pointing to a platform specific
object which configures and manages the specific details.

Please apply.

Changes since v3:
 * Rebased
 * Added first cut at handling of s390 protected virtualization
Changes since RFCv2:
 * Rebased
 * Removed preliminary SEV cleanups (they've been merged)
 * Changed name to "host trust limitation"
 * Added migration blocker to the PEF code (based on SEV's version)
Changes since RFCv1:
 * Rebased
 * Fixed some errors pointed out by Dave Gilbert

David Gibson (10):
  host trust limitation: Introduce new host trust limitation interface
  host trust limitation: Handle memory encryption via interface
  host trust limitation: Move side effect out of
    machine_set_memory_encryption()
  host trust limitation: Rework the "memory-encryption" property
  host trust limitation: Decouple kvm_memcrypt_*() helpers from KVM
  host trust limitation: Add Error ** to HostTrustLimitation::kvm_init
  spapr: Add PEF based host trust limitation
  spapr: PEF: block migration
  host trust limitation: Alter virtio default properties for protected
    guests
  s390: Recognize host-trust-limitation option

 accel/kvm/kvm-all.c                  |  40 ++------
 accel/kvm/sev-stub.c                 |   7 +-
 accel/stubs/kvm-stub.c               |  10 --
 backends/Makefile.objs               |   2 +
 backends/host-trust-limitation.c     |  29 ++++++
 hw/core/machine.c                    |  61 +++++++++--
 hw/i386/pc_sysfw.c                   |   6 +-
 hw/s390x/pv.c                        |  61 +++++++++++
 include/exec/host-trust-limitation.h |  72 +++++++++++++
 include/hw/boards.h                  |   2 +-
 include/qemu/typedefs.h              |   1 +
 include/sysemu/kvm.h                 |  17 ---
 include/sysemu/sev.h                 |   4 +-
 target/i386/sev.c                    | 148 ++++++++++++---------------
 target/ppc/Makefile.objs             |   2 +-
 target/ppc/pef.c                     |  89 ++++++++++++++++
 16 files changed, 387 insertions(+), 164 deletions(-)
 create mode 100644 backends/host-trust-limitation.c
 create mode 100644 include/exec/host-trust-limitation.h
 create mode 100644 target/ppc/pef.c

-- 
2.26.2

Comments

David Gibson Sept. 11, 2020, 2:04 a.m. UTC | #1
On Mon, Sep 07, 2020 at 05:10:46PM +0200, Halil Pasic wrote:
> On Fri, 24 Jul 2020 12:57:43 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The default behaviour for virtio devices is not to use the platforms normal
> > DMA paths, but instead to use the fact that it's running in a hypervisor
> > to directly access guest memory.  That doesn't work if the guest's memory
> > is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> > 
> > So, if a host trust limitation mechanism is enabled, then apply the
> > iommu_platform=on option so it will go through normal DMA mechanisms.
> > Those will presumably have some way of marking memory as shared with the
> > hypervisor or hardware so that DMA will work.
> 
> Sorry for being this late. I had to do some high priority debugging,
> which made me drop everything else, and after that I had some vacation.
> 
> I have some questions about the bigger picture. The promised benefit of
> this patch for users that invoke QEMU manually is relatively clear: it
> alters the default value of some virtio properties, so that using the
> defaults does not result in a bugous configuration.

Right.

> This comes at a price. I used to think of device property default values
> like this. If I don't specify it and I use the default machine, I will
> effectively get the the default value of of the property (as reported by
> qemu -device dev-name,?). If I use a compat machine, then I will get the
> compatibility default value: i.e. the what is reported as the default
> value, if I invoke the binary whose default machine is my compat machine.

Hm, ok.  My mental model has always been that defaults were
essentially per-machine-type.  Which, I grant you isn't really
consistent with the existence of the -device dev,? probing.  On the
other hand, it's possible for a machine/platforms to impose
restrictions on almost any property of almost any device, and it would
suck for the user to have to know all of them just in order to start
things up with default options.

Given that model, extending that to per-machine-variant, including
machine options like htl seemed natural.

> With this patch, that reasoning is not valid any more. Did we do
> something like this before, or is this the first time we introduce this
> complication?

I don't know off hand if we have per-machine differences for certain
options in practice, or only in theory.

> In any case, I suppose, this change needs a documentation update, which I
> could not find in the series.

Uh.. fair enough.. I just need to figure out where.

> How are things supposed to pan out when QEMU is used with management
> software?
> 
> I was told that libvirt's policy is to be explicit and not let QEMU use
> defaults. But this policy does not seem to apply to iommu_platform -- at
> least not on s390x. Why is this? Is this likely to change in the future?

Ugh.. so.  That policy of libvirt's is very double edged.  It's there
because it allows libvirt to create consistent machines that can be
migrated properly and so forth.  However, it basically locks libvirt
into having to know about every option of qemu, ever.  Unsurprisingly
there are some gaps, hence things like this.

Unfortunately that can't be fixed without substantially redesigning
libvirt in a way that can't maintain compatibility.

> Furthermore, the libvirt documentation is IMHO not that great when it
> comes to iommu_platform. All I've found is 
> 
> """
> Virtio-related options
> 
> 
> QEMU's virtio devices have some attributes related to the virtio transport under the driver element: The iommu attribute enables the use of emulated IOMMU by the device. 
> """
> 
> which:
> * Is not explicit about the default, but suggests that default is off
>   (because it needs to be enabled), which would reflect the current state
>   of affairs (without this patch).
> * Makes me wonder, to what extent does the libvirt concept correspond
>   to the virtio semantics of _F_ACCESS_PLATFORM. I.e. we don't really
>   do any IOMMU emulation with virtio-ccw.
> 
> I guess host trust limitation is something that is to be expressed in
> libvirt, or? Do we have a design for that?

Yeah, I guess we'd need to.  See "having to know about every option"
above :/.  No, I haven't thought about a design for that.

> I was also reflecting on how does this patch compare to on/off/auto, but
> this email is already too long, so decided keep my thoughts for myself
> -- for now.

on/off/auto works for your case on s390, but I don't think it works
for POWER, though I forget the details, so maybe I'm wrong about that.


> 
> Regards,
> Halil
> 
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/core/machine.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index b599b0ba65..2a723bf07b 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -28,6 +28,8 @@
> >  #include "hw/mem/nvdimm.h"
> >  #include "migration/vmstate.h"
> >  #include "exec/host-trust-limitation.h"
> > +#include "hw/virtio/virtio.h"
> > +#include "hw/virtio/virtio-pci.h"
> >  
> >  GlobalProperty hw_compat_5_0[] = {
> >      { "virtio-balloon-device", "page-poison", "false" },
> > @@ -1161,6 +1163,15 @@ void machine_run_board_init(MachineState *machine)
> >           * areas.
> >           */
> >          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
> > +
> > +        /*
> > +         * Virtio devices can't count on directly accessing guest
> > +         * memory, so they need iommu_platform=on to use normal DMA
> > +         * mechanisms.  That requires disabling legacy virtio support
> > +         * for virtio pci devices
> > +         */
> > +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy", "on");
> > +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform", "on");
> >      }
> >  
> >      machine_class->init(machine);
> 
> 
>