|
 |
bf143f |
From 56e2aef97e750ffdc572dcecbfc31314728d37a9 Mon Sep 17 00:00:00 2001
|
|
|
bf143f |
From: Halil Pasic <pasic@linux.ibm.com>
|
|
|
bf143f |
Date: Mon, 7 Mar 2022 12:29:39 +0100
|
|
|
bf143f |
Subject: [PATCH 2/2] virtio: fix feature negotiation for ACCESS_PLATFORM
|
|
|
bf143f |
MIME-Version: 1.0
|
|
|
bf143f |
Content-Type: text/plain; charset=UTF-8
|
|
|
bf143f |
Content-Transfer-Encoding: 8bit
|
|
|
bf143f |
|
|
|
bf143f |
RH-Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
bf143f |
RH-MergeRequest: 224: virtiofs on s390 secure execution
|
|
|
bf143f |
RH-Bugzilla: 2116302
|
|
|
bf143f |
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
bf143f |
RH-Acked-by: Cornelia Huck <cohuck@redhat.com>
|
|
|
bf143f |
RH-Acked-by: Cédric Le Goater <None>
|
|
|
bf143f |
RH-Commit: [2/2] 264d3bdbbde985f16ed6f5a1786547c25fb8cc04
|
|
|
bf143f |
|
|
|
bf143f |
Unlike most virtio features ACCESS_PLATFORM is considered mandatory by
|
|
|
bf143f |
QEMU, i.e. the driver must accept it if offered by the device. The
|
|
|
bf143f |
virtio specification says that the driver SHOULD accept the
|
|
|
bf143f |
ACCESS_PLATFORM feature if offered, and that the device MAY fail to
|
|
|
bf143f |
operate if ACCESS_PLATFORM was offered but not negotiated.
|
|
|
bf143f |
|
|
|
bf143f |
While a SHOULD ain't exactly a MUST, we are certainly allowed to fail
|
|
|
bf143f |
the device when the driver fences ACCESS_PLATFORM. With commit
|
|
|
bf143f |
2943b53f68 ("virtio: force VIRTIO_F_IOMMU_PLATFORM") we already made the
|
|
|
bf143f |
decision to do so whenever the get_dma_as() callback is implemented (by
|
|
|
bf143f |
the bus), which in practice means for the entirety of virtio-pci.
|
|
|
bf143f |
|
|
|
bf143f |
That means, if the device needs to translate I/O addresses, then
|
|
|
bf143f |
ACCESS_PLATFORM is mandatory. The aforementioned commit tells us in the
|
|
|
bf143f |
commit message that this is for security reasons. More precisely if we
|
|
|
bf143f |
were to allow a less then trusted driver (e.g. an user-space driver, or
|
|
|
bf143f |
a nested guest) to make the device bypass the IOMMU by not negotiating
|
|
|
bf143f |
ACCESS_PLATFORM, then the guest kernel would have no ability to
|
|
|
bf143f |
control/police (by programming the IOMMU) what pieces of guest memory
|
|
|
bf143f |
the driver may manipulate using the device. Which would break security
|
|
|
bf143f |
assumptions within the guest.
|
|
|
bf143f |
|
|
|
bf143f |
If ACCESS_PLATFORM is offered not because we want the device to utilize
|
|
|
bf143f |
an IOMMU and do address translation, but because the device does not
|
|
|
bf143f |
have access to the entire guest RAM, and needs the driver to grant
|
|
|
bf143f |
access to the bits it needs access to (e.g. confidential guest support),
|
|
|
bf143f |
we still require the guest to have the corresponding logic and to accept
|
|
|
bf143f |
ACCESS_PLATFORM. If the driver does not accept ACCESS_PLATFORM, then
|
|
|
bf143f |
things are bound to go wrong, and we may see failures much less graceful
|
|
|
bf143f |
than failing the device because the driver didn't negotiate
|
|
|
bf143f |
ACCESS_PLATFORM.
|
|
|
bf143f |
|
|
|
bf143f |
So let us make ACCESS_PLATFORM mandatory for the driver regardless
|
|
|
bf143f |
of whether the get_dma_as() callback is implemented or not.
|
|
|
bf143f |
|
|
|
bf143f |
Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
|
|
|
bf143f |
Fixes: 2943b53f68 ("virtio: force VIRTIO_F_IOMMU_PLATFORM")
|
|
|
bf143f |
|
|
|
bf143f |
Message-Id: <20220307112939.2780117-1-pasic@linux.ibm.com>
|
|
|
bf143f |
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
|
|
|
bf143f |
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
|
|
|
bf143f |
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
|
|
|
bf143f |
(cherry picked from commit 06134e2bc35dc21543d4cbcf31f858c03d383442)
|
|
|
bf143f |
---
|
|
|
bf143f |
hw/virtio/virtio-bus.c | 22 ++++++++++++++--------
|
|
|
bf143f |
1 file changed, 14 insertions(+), 8 deletions(-)
|
|
|
bf143f |
|
|
|
bf143f |
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
|
|
|
bf143f |
index 0f69d1c742..d7ec023adf 100644
|
|
|
bf143f |
--- a/hw/virtio/virtio-bus.c
|
|
|
bf143f |
+++ b/hw/virtio/virtio-bus.c
|
|
|
bf143f |
@@ -78,17 +78,23 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)
|
|
|
bf143f |
return;
|
|
|
bf143f |
}
|
|
|
bf143f |
|
|
|
bf143f |
- vdev_has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
|
|
|
bf143f |
- if (klass->get_dma_as != NULL && has_iommu) {
|
|
|
bf143f |
+ vdev->dma_as = &address_space_memory;
|
|
|
bf143f |
+ if (has_iommu) {
|
|
|
bf143f |
+ vdev_has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
|
|
|
bf143f |
+ /*
|
|
|
bf143f |
+ * Present IOMMU_PLATFORM to the driver iff iommu_plattform=on and
|
|
|
bf143f |
+ * device operational. If the driver does not accept IOMMU_PLATFORM
|
|
|
bf143f |
+ * we fail the device.
|
|
|
bf143f |
+ */
|
|
|
bf143f |
virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
|
|
|
bf143f |
- vdev->dma_as = klass->get_dma_as(qbus->parent);
|
|
|
bf143f |
- if (!vdev_has_iommu && vdev->dma_as != &address_space_memory) {
|
|
|
bf143f |
- error_setg(errp,
|
|
|
bf143f |
+ if (klass->get_dma_as) {
|
|
|
bf143f |
+ vdev->dma_as = klass->get_dma_as(qbus->parent);
|
|
|
bf143f |
+ if (!vdev_has_iommu && vdev->dma_as != &address_space_memory) {
|
|
|
bf143f |
+ error_setg(errp,
|
|
|
bf143f |
"iommu_platform=true is not supported by the device");
|
|
|
bf143f |
- return;
|
|
|
bf143f |
+ return;
|
|
|
bf143f |
+ }
|
|
|
bf143f |
}
|
|
|
bf143f |
- } else {
|
|
|
bf143f |
- vdev->dma_as = &address_space_memory;
|
|
|
bf143f |
}
|
|
|
bf143f |
}
|
|
|
bf143f |
|
|
|
bf143f |
--
|
|
|
bf143f |
2.37.3
|
|
|
bf143f |
|