Re: [for-5.2 v4 09/10] host trust limitation: Alter virtio default properties for protected guests

[Halil Pasic]

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.

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.

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?

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

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?

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?

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.

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