Re: [for-5.2 v4 10/10] s390: Recognize host-trust-limitation option

[David Gibson]

On Thu, Sep 10, 2020 at 08:29:24PM +0200, Halil Pasic wrote:
> On Thu, 10 Sep 2020 13:36:09 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > On Mon, 7 Sep 2020 17:22:53 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> > 
> > > On Fri, 24 Jul 2020 12:57:44 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > 
> > > > At least some s390 cpu models support "Protected Virtualization" (PV),
> > > > a mechanism to protect guests from eavesdropping by a compromised
> > > > hypervisor.
> > > > 
> > > > This is similar in function to other mechanisms like AMD's SEV and
> > > > POWER's PEF, which are controlled bythe "host-trust-limitation"
> > > > machine option.  s390 is a slightly special case, because we already
> > > > supported PV, simply by using a CPU model with the required feature
> > > > (S390_FEAT_UNPACK).
> > > > 
> > > > To integrate this with the option used by other platforms, we
> > > > implement the following compromise:
> > > > 
> > > >  - When the host-trust-limitation option is set, s390 will recognize
> > > >    it, verify that the CPU can support PV (failing if not) and set
> > > >    virtio default options necessary for encrypted or protected guests,
> > > >    as on other platforms.  i.e. if host-trust-limitation is set, we
> > > >    will either create a guest capable of entering PV mode, or fail
> > > >    outright  
> > > 
> > > Shouldn't we also fail outright if the virtio features are not PV
> > > compatible (invalid configuration)?
> > > 
> > > I would like to see something like follows as a part of this series.
> > > ----------------------------8<--------------------------
> > > From: Halil Pasic <pasic@linux.ibm.com>
> > > Date: Mon, 7 Sep 2020 15:00:17 +0200
> > > Subject: [PATCH] virtio: handle host trust limitation
> > > 
> > > If host_trust_limitation_enabled() returns true, then emulated virtio
> > > devices must offer VIRTIO_F_ACCESS_PLATFORM, because the device is not
> > > capable of accessing all of the guest memory. Otherwise we are in
> > > violation of the virtio specification.
> > > 
> > > Let's fail realize if we detect that VIRTIO_F_ACCESS_PLATFORM feature is
> > > obligatory but missing.
> > > 
> > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> > > ---
> > >  hw/virtio/virtio.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index 5bd2a2f621..19b4b0a37a 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -27,6 +27,7 @@
> > >  #include "hw/virtio/virtio-access.h"
> > >  #include "sysemu/dma.h"
> > >  #include "sysemu/runstate.h"
> > > +#include "exec/host-trust-limitation.h"
> > >  
> > >  /*
> > >   * The alignment to use between consumer and producer parts of vring.
> > > @@ -3618,6 +3619,12 @@ static void virtio_device_realize(DeviceState 
> > > *dev, Error **errp)
> > >      /* Devices should either use vmsd or the load/save methods */
> > >      assert(!vdc->vmsd || !vdc->load);
> > >  
> > > +    if (host_trust_limitation_enabled(MACHINE(qdev_get_machine()))
> > > +        && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
> > > +        error_setg(&err, "devices without VIRTIO_F_ACCESS_PLATFORM are 
> > > not compatible with host trust imitation");
> > > +        error_propagate(errp, err);
> > > +        return;
> > 
> > How can we get here? I assume only if the user explicitly turned the
> > feature off while turning HTL on, as otherwise patch 9 should have
> > taken care of it?
> > 
> 
> Yes, we can get here only if iommu_platform is explicitly turned off.

Right.. my assumption was that if you really want to specify
contradictory options, you get to keep both pieces.  Or, more
seriously, there might be some weird experimental cases where this
combination could do something useful if you really know what you're
doing, and explicitly telling qemu to do this implies you know what
you're doing.

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

signature.asc
Description: PGP signature