All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Daniel Henrique Barboza <danielhb413@gmail.com>,
	Jason Wang <jasowang@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	qemu-stable@nongnu.org, Halil Pasic <pasic@linux.ibm.com>,
	Jakob Naucke <Jakob.Naucke@ibm.com>
Subject: [PULL v2 11/47] virtio: fix the condition for iommu_platform not supported
Date: Mon, 7 Mar 2022 05:01:54 -0500	[thread overview]
Message-ID: <20220307100058.449628-12-mst@redhat.com> (raw)
In-Reply-To: <20220307100058.449628-1-mst@redhat.com>

From: Halil Pasic <pasic@linux.ibm.com>

The commit 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but
unsupported") claims to fail the device hotplug when iommu_platform
is requested, but not supported by the (vhost) device. On the first
glance the condition for detecting that situation looks perfect, but
because a certain peculiarity of virtio_platform it ain't.

In fact the aforementioned commit introduces a regression. It breaks
virtio-fs support for Secure Execution, and most likely also for AMD SEV
or any other confidential guest scenario that relies encrypted guest
memory.  The same also applies to any other vhost device that does not
support _F_ACCESS_PLATFORM.

The peculiarity is that iommu_platform and _F_ACCESS_PLATFORM collates
"device can not access all of the guest RAM" and "iova != gpa, thus
device needs to translate iova".

Confidential guest technologies currently rely on the device/hypervisor
offering _F_ACCESS_PLATFORM, so that, after the feature has been
negotiated, the guest  grants access to the portions of memory the
device needs to see. So in for confidential guests, generally,
_F_ACCESS_PLATFORM is about the restricted access to memory, but not
about the addresses used being something else than guest physical
addresses.

This is the very reason for which commit f7ef7e6e3b ("vhost: correctly
turn on VIRTIO_F_IOMMU_PLATFORM") fences _F_ACCESS_PLATFORM from the
vhost device that does not need it, because on the vhost interface it
only means "I/O address translation is needed".

This patch takes inspiration from f7ef7e6e3b ("vhost: correctly turn on
VIRTIO_F_IOMMU_PLATFORM"), and uses the same condition for detecting the
situation when _F_ACCESS_PLATFORM is requested, but no I/O translation
by the device, and thus no device capability is needed. In this
situation claiming that the device does not support iommu_plattform=on
is counter-productive. So let us stop doing that!

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Reported-by: Jakob Naucke <Jakob.Naucke@ibm.com>
Fixes: 04ceb61a40 ("virtio: Fail if iommu_platform is requested, but
unsupported")
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org

Message-Id: <20220207112857.607829-1-pasic@linux.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-bus.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index d23db98c56..0f69d1c742 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -48,6 +48,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
     bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+    bool vdev_has_iommu;
     Error *local_err = NULL;
 
     DPRINTF("%s: plug device.\n", qbus->name);
@@ -69,11 +70,6 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
         return;
     }
 
-    if (has_iommu && !virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
-        error_setg(errp, "iommu_platform=true is not supported by the device");
-        return;
-    }
-
     if (klass->device_plugged != NULL) {
         klass->device_plugged(qbus->parent, &local_err);
     }
@@ -82,9 +78,15 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
         return;
     }
 
+    vdev_has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
     if (klass->get_dma_as != NULL && has_iommu) {
         virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
         vdev->dma_as = klass->get_dma_as(qbus->parent);
+        if (!vdev_has_iommu && vdev->dma_as != &address_space_memory) {
+            error_setg(errp,
+                       "iommu_platform=true is not supported by the device");
+            return;
+        }
     } else {
         vdev->dma_as = &address_space_memory;
     }
-- 
MST



  parent reply	other threads:[~2022-03-07 10:16 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-07 10:01 [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 01/47] qom: assert integer does not overflow Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 02/47] ACPI ERST: specification for ERST support Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 03/47] MAINTAINERS: no need to add my name explicitly as a reviewer for VIOT tables Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 04/47] docs/acpi/erst: add device id for ACPI ERST device in pci-ids.txt Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 05/47] hw/acpi/erst: clean up unused IS_UEFI_CPER_RECORD macro Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 06/47] hw/smbios: code cleanup - use macro definitions for table header handles Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 07/47] hw/smbios: fix overlapping table handle numbers with large memory vms Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 08/47] hw/smbios: add assertion to ensure handles of tables 19 and 32 do not collide Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 09/47] vhost-user: remove VirtQ notifier restore Michael S. Tsirkin
2022-03-07 10:01 ` [PULL v2 10/47] vhost-user: fix VirtQ notifier cleanup Michael S. Tsirkin
2022-03-07 10:01 ` Michael S. Tsirkin [this message]
2022-03-07 10:01 ` [PULL v2 12/47] hw/vhost-user-i2c: Add support for VIRTIO_I2C_F_ZERO_LENGTH_REQUEST Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 13/47] hw/virtio: vdpa: Fix leak of host-notifier memory-region Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 14/47] vhost-vdpa: make notifiers _init()/_uninit() symmetric Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 15/47] intel_iommu: support snoop control Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 16/47] hw/i386: Improve bounds checking in OVMF table parsing Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 17/47] hw/i386: Replace magic number with field length calculation Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 18/47] virtio-iommu: Default to bypass during boot Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 19/47] virtio-iommu: Support bypass domain Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 20/47] tests/qtest/virtio-iommu-test: Check bypass config Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 21/47] hw/i386/pc_piix: Mark the machine types from version 1.4 to 1.7 as deprecated Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 22/47] hw/pci-bridge/pxb: Fix missing swizzle Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 23/47] virtio-net: Unlimit tx queue size if peer is vdpa Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 24/47] pcie: Add support for Single Root I/O Virtualization (SR/IOV) Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 25/47] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 26/47] pcie: Add a helper to the SR/IOV API Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 27/47] pcie: Add 1.2 version token for the Power Management Capability Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 28/47] pci-bridge/xio3130_upstream: Fix error handling Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 29/47] pci-bridge/xio3130_downstream: " Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 30/47] headers: Add pvpanic.h Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 31/47] hw/misc/pvpanic: Use standard headers instead Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 32/47] pci: show id info when pci BDF conflict Michael S. Tsirkin
2022-03-07 10:02 ` [PULL v2 33/47] pci: expose TYPE_XIO3130_DOWNSTREAM name Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 34/47] acpi: pcihp: pcie: set power on cap on parent slot Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 35/47] pc: add option to disable PS/2 mouse/keyboard Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 36/47] vhost-vsock: detach the virqueue element in case of error Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 37/47] x86: cleanup unused compat_apic_id_mode Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 38/47] hw/smbios: Add table 4 parameter, "processor-id" Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 39/47] pci: drop COMPAT_PROP_PCP for 2.0 machine types Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 40/47] event_notifier: add event_notifier_get_wfd() Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 41/47] vhost: use wfd on functions setting vring call fd Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 42/47] configure, meson: allow enabling vhost-user on all POSIX systems Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 43/47] docs: vhost-user: add subsection for non-Linux platforms Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 44/47] tests/acpi: i386: allow FACP acpi table changes Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 45/47] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 46/47] tests/acpi: i386: update FACP table differences Michael S. Tsirkin
2022-03-07 10:03 ` [PULL v2 47/47] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present Michael S. Tsirkin
2022-03-07 17:06 ` [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes Peter Maydell
2022-03-07 17:13   ` Peter Maydell
2022-03-07 22:52     ` Michael S. Tsirkin
2022-03-08  9:05       ` Peter Maydell
2022-03-08 11:01         ` Michael S. Tsirkin
2022-03-08 11:18           ` Peter Maydell
2022-03-08 11:23             ` Michael S. Tsirkin
2022-03-08 13:19             ` Philippe Mathieu-Daudé
2022-03-15 18:41               ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220307100058.449628-12-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=Jakob.Naucke@ibm.com \
    --cc=cohuck@redhat.com \
    --cc=danielhb413@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.