On Thu, May 24, 2018 at 08:27:04AM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote:
quoted
I re-read that discussion and I'm still unclear on the
original question, since I got several apparently
conflicting answers.
I asked:
Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
hypervisor side sufficient?
I thought I had replied to this...
There are a couple of reasons:
- First qemu doesn't know that the guest will switch to "secure mode"
in advance. There is no difference between a normal and a secure
partition until the partition does the magic UV call to "enter secure
mode" and qemu doesn't see any of it. So who can set the flag here ?
Not sure I understand. Just set the flag e.g. on qemu command line.
I might be wrong, but these secure mode things usually
a. require hypervisor side tricks anyway
- Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
vhost) go through the emulated MMIO for every access to the guest,
which adds additional overhead.
Cheers,
Ben.
Well it's not supposed to be much slower for the static case.
vhost has a cache so should be fine.
A while ago Paolo implemented a translation cache which should be
perfect for this case - most of the code got merged but
never enabled because of stability issues.
If all else fails, we could teach QEMU to handle the no-iommu case
as if VIRTIO_F_IOMMU_PLATFORM was off.
quoted
quoted
arch/powerpc/include/asm/dma-mapping.h | 6 ++++++
arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
drivers/virtio/virtio_ring.c | 10 ++++++++++
3 files changed, 27 insertions(+)
diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index 8fa3945..056e578 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
#define ARCH_HAS_DMA_MMAP_COHERENT
#endif /* __KERNEL__ */
+
+#define platform_forces_virtio_dma platform_forces_virtio_dma
+
+struct virtio_device;
+
+extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
#endif /* _ASM_DMA_MAPPING_H */
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 06f0296..a2ec15a 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -38,6 +38,7 @@
#include <linux/of.h>
#include <linux/iommu.h>
#include <linux/rculist.h>
+#include <linux/virtio.h>
#include <asm/io.h>
#include <asm/prom.h>
#include <asm/rtas.h>
@@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
__setup("multitce=", disable_multitce);
machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
+
+bool platform_forces_virtio_dma(struct virtio_device *vdev)
+{
+ /*
+ * On protected guest platforms, force virtio core to use DMA
+ * MAP API for all virtio devices. But there can also be some
+ * exceptions for individual devices like virtio balloon.
+ */
+ return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
+}
Isn't this kind of slow? vring_use_dma_api is on
data path and supposed to be very fast.
quoted
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 21d464a..47ea6c3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -141,8 +141,18 @@ struct vring_virtqueue {
* unconditionally on data path.
*/
+#ifndef platform_forces_virtio_dma
+static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
+{
+ return false;
+}
+#endif
+
static bool vring_use_dma_api(struct virtio_device *vdev)
{
+ if (platform_forces_virtio_dma(vdev))
+ return true;
+
if (!virtio_has_iommu_quirk(vdev))
return true;
--
2.9.3