* [PATCH v3] virtio: new feature to detect IOMMU device quirk
@ 2016-07-19 2:38 Michael S. Tsirkin
2016-07-23 21:32 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-07-19 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: virtualization
The interaction between virtio and IOMMUs is messy.
On most systems with virtio, physical addresses match bus addresses,
and it doesn't particularly matter which one we use to program
the device.
On some systems, including Xen and any system with a physical device
that speaks virtio behind a physical IOMMU, we must program the IOMMU
for virtio DMA to work at all.
On other systems, including SPARC and PPC64, virtio-pci devices are
enumerated as though they are behind an IOMMU, but the virtio host
ignores the IOMMU, so we must either pretend that the IOMMU isn't
there or somehow map everything as the identity.
Add a feature bit to detect that quirk: VIRTIO_F_IOMMU_PLATFORM.
Any device with this feature bit set to 0 needs a quirk and has to be
passed physical addresses (as opposed to bus addresses) even though
the device is behind an IOMMU.
Note: it has to be a per-device quirk because for example, there could
be a mix of passed-through and virtual virtio devices. As another
example, some devices could be implemented by an out of process
hypervisor backend (in case of qemu vhost, or vhost-user) and so support
for an IOMMU needs to be coded up separately.
It would be cleanest to handle this in IOMMU core code, but that needs
per-device DMA ops. While we are waiting for that to be implemented, use
a work-around in virtio core.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
wanted to use per-device dma ops but that does not
seem to be ready. So let's put this in virtio
code for now, and move when it becomes possible.
include/uapi/linux/virtio_config.h | 4 +++-
drivers/virtio/virtio_ring.c | 15 ++++++++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 4cb65bb..3f6195e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 33
+#define VIRTIO_TRANSPORT_F_END 34
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -63,4 +63,6 @@
/* v1.0 compliant. */
#define VIRTIO_F_VERSION_1 32
+/* Do not bypass the IOMMU (if configured) */
+#define VIRTIO_F_IOMMU_PLATFORM 33
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index ca6bfdd..2a0c8bf 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -117,7 +117,10 @@ struct vring_virtqueue {
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
/*
- * The interaction between virtio and a possible IOMMU is a mess.
+ * Modern virtio devices might set feature bits to specify whether
+ * they use the platform IOMMU. If there, just use the DMA API.
+ *
+ * If not there, the interaction between virtio and DMA API is messy.
*
* On most systems with virtio, physical addresses match bus addresses,
* and it doesn't particularly matter whether we use the DMA API.
@@ -137,6 +140,10 @@ struct vring_virtqueue {
static bool vring_use_dma_api(struct virtio_device *vdev)
{
+ if (virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM))
+ return true;
+
+ /* Otherwise, we are left to guess. */
/*
* In theory, it's possible to have a buggy QEMU-supposed
* emulated Q35 IOMMU and Xen enabled at the same time. On
@@ -1099,6 +1106,12 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_F_VERSION_1:
break;
+ case VIRTIO_F_IOMMU_PASSTHROUGH:
+ break;
+ case VIRTIO_F_IOMMU_PLATFORM:
+ /* Ignore passthrough hint for now, obey kernel config. */
+ __virtio_clear_bit(vdev, VIRTIO_F_IOMMU_PASSTHROUGH);
+ break;
default:
/* We don't understand this bit. */
__virtio_clear_bit(vdev, i);
--
MST
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] virtio: new feature to detect IOMMU device quirk
2016-07-19 2:38 [PATCH v3] virtio: new feature to detect IOMMU device quirk Michael S. Tsirkin
@ 2016-07-23 21:32 ` kbuild test robot
2016-07-24 22:44 ` kbuild test robot
2016-07-25 7:50 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2016-07-23 21:32 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kbuild-all, linux-kernel, virtualization
Hi,
[auto build test WARNING on stable/master]
[also build test WARNING on v4.7-rc7 next-20160722]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/virtio-new-feature-to-detect-IOMMU-device-quirk/20160724-030032
base: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute
drivers/virtio/virtio_ring.c:1109:22: sparse: undefined identifier 'VIRTIO_F_IOMMU_PASSTHROUGH'
drivers/virtio/virtio_ring.c:1113:50: sparse: undefined identifier 'VIRTIO_F_IOMMU_PASSTHROUGH'
>> drivers/virtio/virtio_ring.c:1109:22: sparse: incompatible types for 'case' statement
drivers/virtio/virtio_ring.c:1109:22: sparse: Expected constant expression in case statement
drivers/virtio/virtio_ring.c: In function 'vring_transport_features':
drivers/virtio/virtio_ring.c:1109:8: error: 'VIRTIO_F_IOMMU_PASSTHROUGH' undeclared (first use in this function)
case VIRTIO_F_IOMMU_PASSTHROUGH:
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/virtio/virtio_ring.c:1109:8: note: each undeclared identifier is reported only once for each function it appears in
vim +/case +1109 drivers/virtio/virtio_ring.c
1093 }
1094 EXPORT_SYMBOL_GPL(vring_del_virtqueue);
1095
1096 /* Manipulates transport-specific feature bits. */
1097 void vring_transport_features(struct virtio_device *vdev)
1098 {
1099 unsigned int i;
1100
1101 for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) {
1102 switch (i) {
1103 case VIRTIO_RING_F_INDIRECT_DESC:
1104 break;
1105 case VIRTIO_RING_F_EVENT_IDX:
1106 break;
1107 case VIRTIO_F_VERSION_1:
1108 break;
> 1109 case VIRTIO_F_IOMMU_PASSTHROUGH:
1110 break;
1111 case VIRTIO_F_IOMMU_PLATFORM:
1112 /* Ignore passthrough hint for now, obey kernel config. */
1113 __virtio_clear_bit(vdev, VIRTIO_F_IOMMU_PASSTHROUGH);
1114 break;
1115 default:
1116 /* We don't understand this bit. */
1117 __virtio_clear_bit(vdev, i);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] virtio: new feature to detect IOMMU device quirk
2016-07-19 2:38 [PATCH v3] virtio: new feature to detect IOMMU device quirk Michael S. Tsirkin
2016-07-23 21:32 ` kbuild test robot
@ 2016-07-24 22:44 ` kbuild test robot
2016-07-25 7:50 ` Christoph Hellwig
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2016-07-24 22:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kbuild-all, linux-kernel, virtualization
[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]
Hi,
[auto build test ERROR on stable/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/virtio-new-feature-to-detect-IOMMU-device-quirk/20160724-030032
base: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git master
config: x86_64-lkp (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/virtio/virtio_ring.c: In function 'vring_transport_features':
>> drivers/virtio/virtio_ring.c:1109:8: error: 'VIRTIO_F_IOMMU_PASSTHROUGH' undeclared (first use in this function)
case VIRTIO_F_IOMMU_PASSTHROUGH:
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/virtio/virtio_ring.c:1109:8: note: each undeclared identifier is reported only once for each function it appears in
vim +/VIRTIO_F_IOMMU_PASSTHROUGH +1109 drivers/virtio/virtio_ring.c
1103 case VIRTIO_RING_F_INDIRECT_DESC:
1104 break;
1105 case VIRTIO_RING_F_EVENT_IDX:
1106 break;
1107 case VIRTIO_F_VERSION_1:
1108 break;
> 1109 case VIRTIO_F_IOMMU_PASSTHROUGH:
1110 break;
1111 case VIRTIO_F_IOMMU_PLATFORM:
1112 /* Ignore passthrough hint for now, obey kernel config. */
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22900 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] virtio: new feature to detect IOMMU device quirk
2016-07-19 2:38 [PATCH v3] virtio: new feature to detect IOMMU device quirk Michael S. Tsirkin
2016-07-23 21:32 ` kbuild test robot
2016-07-24 22:44 ` kbuild test robot
@ 2016-07-25 7:50 ` Christoph Hellwig
2016-07-25 19:13 ` Michael S. Tsirkin
2016-07-26 18:50 ` Michael S. Tsirkin
2 siblings, 2 replies; 6+ messages in thread
From: Christoph Hellwig @ 2016-07-25 7:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization
On Tue, Jul 19, 2016 at 05:38:23AM +0300, Michael S. Tsirkin wrote:
>
> On other systems, including SPARC and PPC64, virtio-pci devices are
> enumerated as though they are behind an IOMMU, but the virtio host
> ignores the IOMMU, so we must either pretend that the IOMMU isn't
> there or somehow map everything as the identity.
>
> Add a feature bit to detect that quirk: VIRTIO_F_IOMMU_PLATFORM.
>
> Any device with this feature bit set to 0 needs a quirk and has to be
> passed physical addresses (as opposed to bus addresses) even though
> the device is behind an IOMMU.
This is the wrong way around.
> Note: it has to be a per-device quirk because for example, there could
> be a mix of passed-through and virtual virtio devices. As another
> example, some devices could be implemented by an out of process
> hypervisor backend (in case of qemu vhost, or vhost-user) and so support
> for an IOMMU needs to be coded up separately.
>
> It would be cleanest to handle this in IOMMU core code, but that needs
> per-device DMA ops. While we are waiting for that to be implemented, use
> a work-around in virtio core.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> wanted to use per-device dma ops but that does not
> seem to be ready. So let's put this in virtio
> code for now, and move when it becomes possible.
So work on making it ready. We're close to there, and given that
virtio needs it, finish it off. We now have everyone using the
operation vectors for DMA, so the only thing you need is a dma_ops
pointer in struct device initialized to what get_dma_ops returns.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] virtio: new feature to detect IOMMU device quirk
2016-07-25 7:50 ` Christoph Hellwig
@ 2016-07-25 19:13 ` Michael S. Tsirkin
2016-07-26 18:50 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-07-25 19:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel, virtualization
On Mon, Jul 25, 2016 at 12:50:09AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 19, 2016 at 05:38:23AM +0300, Michael S. Tsirkin wrote:
> >
> > On other systems, including SPARC and PPC64, virtio-pci devices are
> > enumerated as though they are behind an IOMMU, but the virtio host
> > ignores the IOMMU, so we must either pretend that the IOMMU isn't
> > there or somehow map everything as the identity.
> >
> > Add a feature bit to detect that quirk: VIRTIO_F_IOMMU_PLATFORM.
> >
> > Any device with this feature bit set to 0 needs a quirk and has to be
> > passed physical addresses (as opposed to bus addresses) even though
> > the device is behind an IOMMU.
>
> This is the wrong way around.
Well given there are tons of quirky devices out there
and they have this bit set to 0, I don't think we can change this.
New devices will have it set to 1.
> > Note: it has to be a per-device quirk because for example, there could
> > be a mix of passed-through and virtual virtio devices. As another
> > example, some devices could be implemented by an out of process
> > hypervisor backend (in case of qemu vhost, or vhost-user) and so support
> > for an IOMMU needs to be coded up separately.
> >
> > It would be cleanest to handle this in IOMMU core code, but that needs
> > per-device DMA ops. While we are waiting for that to be implemented, use
> > a work-around in virtio core.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > wanted to use per-device dma ops but that does not
> > seem to be ready. So let's put this in virtio
> > code for now, and move when it becomes possible.
>
> So work on making it ready. We're close to there, and given that
> virtio needs it, finish it off. We now have everyone using the
> operation vectors for DMA, so the only thing you need is a dma_ops
> pointer in struct device initialized to what get_dma_ops returns.
And then pci_dma_configure would have a quirk to
set that for legacy virtio devices?
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] virtio: new feature to detect IOMMU device quirk
2016-07-25 7:50 ` Christoph Hellwig
2016-07-25 19:13 ` Michael S. Tsirkin
@ 2016-07-26 18:50 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-07-26 18:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-kernel, virtualization
On Mon, Jul 25, 2016 at 12:50:09AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 19, 2016 at 05:38:23AM +0300, Michael S. Tsirkin wrote:
> >
> > On other systems, including SPARC and PPC64, virtio-pci devices are
> > enumerated as though they are behind an IOMMU, but the virtio host
> > ignores the IOMMU, so we must either pretend that the IOMMU isn't
> > there or somehow map everything as the identity.
> >
> > Add a feature bit to detect that quirk: VIRTIO_F_IOMMU_PLATFORM.
> >
> > Any device with this feature bit set to 0 needs a quirk and has to be
> > passed physical addresses (as opposed to bus addresses) even though
> > the device is behind an IOMMU.
>
> This is the wrong way around.
>
> > Note: it has to be a per-device quirk because for example, there could
> > be a mix of passed-through and virtual virtio devices. As another
> > example, some devices could be implemented by an out of process
> > hypervisor backend (in case of qemu vhost, or vhost-user) and so support
> > for an IOMMU needs to be coded up separately.
> >
> > It would be cleanest to handle this in IOMMU core code, but that needs
> > per-device DMA ops. While we are waiting for that to be implemented, use
> > a work-around in virtio core.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > wanted to use per-device dma ops but that does not
> > seem to be ready. So let's put this in virtio
> > code for now, and move when it becomes possible.
>
> So work on making it ready. We're close to there, and given that
> virtio needs it, finish it off. We now have everyone using the
> operation vectors for DMA, so the only thing you need is a dma_ops
> pointer in struct device initialized to what get_dma_ops returns.
Given the timing, I suspect this will mean missing 4.8 though and I'd
much rather have something working finally and clean it up later.
And from experience, people seem to find it easier to re-factor
and clean up working code.
This patch is all of 10 lines so I'm comfortable including it right away -
I'll add a big TODO in v4 so we don't forget.
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-26 18:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-19 2:38 [PATCH v3] virtio: new feature to detect IOMMU device quirk Michael S. Tsirkin
2016-07-23 21:32 ` kbuild test robot
2016-07-24 22:44 ` kbuild test robot
2016-07-25 7:50 ` Christoph Hellwig
2016-07-25 19:13 ` Michael S. Tsirkin
2016-07-26 18:50 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).