[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

This adds a flag to enable/disable bypassing the IOMMU by
virtio devices.

This is on top of patch
http://article.gmane.org/gmane.comp.emulators.qemu/403467
virtio: convert to use DMA api

Tested with patchset
http://article.gmane.org/gmane.linux.kernel.virtualization/27545
virtio-pci: iommu support (note: bit number has been kept at 34
intentionally to match posted guest code. a non-RFC version will
renumber bits to be contigious).

changes from v1:
    drop PASSTHROUGH flag

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.

On some systems, including Xen and any system with a physical device
that speaks virtio behind a physical IOMMU, we must use the DMA API
for virtio DMA to work at all.

Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.

If not there, we preserve historic behavior and bypass the DMA
API unless within Xen guest. This is actually required for
systems, including SPARC and PPC64, where 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.

Re: non-virtio devices.

It turns out that on old QEMU hosts, only emulated devices which were
part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
devices *only*, it would be rather easy to detect them by looking at
subsystem vendor and device ID. Thus, no new interfaces are required
except for virtio which always uses the same subsystem vendor and device ID.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-access.h              | 3 ++-
 include/hw/virtio/virtio.h                     | 4 +++-
 include/standard-headers/linux/virtio_config.h | 2 ++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 967cc75..bb6f34e 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev)
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
-    if (k->get_dma_as) {
+    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
+        k->get_dma_as) {
         return k->get_dma_as(qbus->parent);
     }
     return &address_space_memory;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b12faa9..44f3788 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
     DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
                       VIRTIO_F_NOTIFY_ON_EMPTY, true), \
     DEFINE_PROP_BIT64("any_layout", _state, _field, \
-                      VIRTIO_F_ANY_LAYOUT, true)
+                      VIRTIO_F_ANY_LAYOUT, true), \
+    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
+                      VIRTIO_F_IOMMU_PLATFORM, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
index bcc445b..3fcfbb1 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -61,4 +61,6 @@
 /* v1.0 compliant. */
 #define VIRTIO_F_VERSION_1		32
 
+/* Do not bypass the IOMMU (if configured) */
+#define VIRTIO_F_IOMMU_PLATFORM		34
 #endif /* _LINUX_VIRTIO_CONFIG_H */
-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Wei Liu]

Add Stefano and Anthony

On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin wrote:
> This adds a flag to enable/disable bypassing the IOMMU by
> virtio devices.
> 
> This is on top of patch
> http://article.gmane.org/gmane.comp.emulators.qemu/403467
> virtio: convert to use DMA api
> 
> Tested with patchset
> http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> virtio-pci: iommu support (note: bit number has been kept at 34
> intentionally to match posted guest code. a non-RFC version will
> renumber bits to be contigious).
> 
> changes from v1:
>     drop PASSTHROUGH flag
> 
> 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.
> 
> On some systems, including Xen and any system with a physical device
> that speaks virtio behind a physical IOMMU, we must use the DMA API
> for virtio DMA to work at all.
> 
> Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> 
> If not there, we preserve historic behavior and bypass the DMA
> API unless within Xen guest. This is actually required for
> systems, including SPARC and PPC64, where 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.
> 
> Re: non-virtio devices.
> 
> It turns out that on old QEMU hosts, only emulated devices which were
> part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> devices *only*, it would be rather easy to detect them by looking at
> subsystem vendor and device ID. Thus, no new interfaces are required
> except for virtio which always uses the same subsystem vendor and device ID.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/virtio/virtio-access.h              | 3 ++-
>  include/hw/virtio/virtio.h                     | 4 +++-
>  include/standard-headers/linux/virtio_config.h | 2 ++
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index 967cc75..bb6f34e 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev)
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  
> -    if (k->get_dma_as) {
> +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> +        k->get_dma_as) {
>          return k->get_dma_as(qbus->parent);
>      }
>      return &address_space_memory;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b12faa9..44f3788 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
>                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
>      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> -                      VIRTIO_F_ANY_LAYOUT, true)
> +                      VIRTIO_F_ANY_LAYOUT, true), \
> +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> +                      VIRTIO_F_IOMMU_PLATFORM, false)
>  
>  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
>  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
> index bcc445b..3fcfbb1 100644
> --- a/include/standard-headers/linux/virtio_config.h
> +++ b/include/standard-headers/linux/virtio_config.h
> @@ -61,4 +61,6 @@
>  /* v1.0 compliant. */
>  #define VIRTIO_F_VERSION_1		32
>  
> +/* Do not bypass the IOMMU (if configured) */
> +#define VIRTIO_F_IOMMU_PLATFORM		34
>  #endif /* _LINUX_VIRTIO_CONFIG_H */
> -- 
> MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]


> > On some systems, including Xen and any system with a physical device
> > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > for virtio DMA to work at all.
> > 
> > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> > 
> > If not there, we preserve historic behavior and bypass the DMA
> > API unless within Xen guest. This is actually required for
> > systems, including SPARC and PPC64, where 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.
> > 
> > Re: non-virtio devices.
> > 
> > It turns out that on old QEMU hosts, only emulated devices which were
> > part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> > devices *only*, it would be rather easy to detect them by looking at
> > subsystem vendor and device ID. Thus, no new interfaces are required
> > except for virtio which always uses the same subsystem vendor and device ID.

Apologies for dropping this thread; I've been travelling.

But seriously, NO!

I understand why you want to see this as a virtio-specific issue, but
it isn't. And we don't *want* it to be.

In the guest, drivers SHALL use the DMA API. And the DMA API SHALL do
the right thing for each device according to its needs.

So any information passed from qemu to the guest should be directed at
the platform IOMMU code (or handled by qemu-detection quirks in the
guest, if we must).

It is *not* acceptable for the virtio drivers in the guest to just
eschew the DMA API completely, triggered by some device-specific flag.

The qemu implementation is, of course, monolithic. In qemu the fact
that virtio doesn't get translated by the emulated IOMMU *is* actually
down to code in the virtio implementation. I get that.

But then again, it's not just virtio. *Any* device which we emulate for
the guest could have that same issue, and appear as untranslated. (And
assigned PCI devices currently do).

Let's think about the parallel with a system-on-chip. Let's say we have
a peripheral which got included, but which was wired up such that it
bypasses the IOMMU and gets to do direct physical DMA. Is that a
feature of that specific peripheral? Do we hack its drivers to make the
distinction between this incarnation, and a normal discrete version of
the same device? No! It's a feature of the *system* and needs to be
conveyed to the OS IOMMU code to do the right thing. Not to the driver.

In my opinion, adding the VIRTIO_F_IOMMU_PLATFORM feature bit is
absolutely the wrong thing to do.

What we *should* do is a patchset in the guest which both fixes virtio
drivers to *always* use the DMA API, and fixes the DMA API to DTRT at
the same time — by detecting qemu and installing no-op DMA ops for the
appropriate devices, perhaps.

Then we can look at giving qemu a way to properly indicate which
devices it actually does DMA mapping for, so we can remove those
heuristic assumptions.

But that flag does *not* live in the virtio host←→guest ABI.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 01:18:21PM +0100, David Woodhouse wrote:
> 
> > > On some systems, including Xen and any system with a physical device
> > > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > > for virtio DMA to work at all.
> > > 
> > > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> > > 
> > > If not there, we preserve historic behavior and bypass the DMA
> > > API unless within Xen guest. This is actually required for
> > > systems, including SPARC and PPC64, where 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.
> > > 
> > > Re: non-virtio devices.
> > > 
> > > It turns out that on old QEMU hosts, only emulated devices which were
> > > part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> > > devices *only*, it would be rather easy to detect them by looking at
> > > subsystem vendor and device ID. Thus, no new interfaces are required
> > > except for virtio which always uses the same subsystem vendor and device ID.
> 
> Apologies for dropping this thread; I've been travelling.
> 
> But seriously, NO!
> 
> I understand why you want to see this as a virtio-specific issue, but
> it isn't. And we don't *want* it to be.
> 
> In the guest, drivers SHALL use the DMA API. And the DMA API SHALL do
> the right thing for each device according to its needs.
> 
> So any information passed from qemu to the guest should be directed at
> the platform IOMMU code (or handled by qemu-detection quirks in the
> guest, if we must).
> 
> It is *not* acceptable for the virtio drivers in the guest to just
> eschew the DMA API completely, triggered by some device-specific flag.
> 
> The qemu implementation is, of course, monolithic. In qemu the fact
> that virtio doesn't get translated by the emulated IOMMU *is* actually
> down to code in the virtio implementation. I get that.
> 
> But then again, it's not just virtio. *Any* device which we emulate for
> the guest could have that same issue, and appear as untranslated. (And
> assigned PCI devices currently do).
> 
> Let's think about the parallel with a system-on-chip. Let's say we have
> a peripheral which got included, but which was wired up such that it
> bypasses the IOMMU and gets to do direct physical DMA. Is that a
> feature of that specific peripheral? Do we hack its drivers to make the
> distinction between this incarnation, and a normal discrete version of
> the same device? No! It's a feature of the *system*

One correction: it's a feature of the device in the system.
There could be a mix of devices bypassing and not
bypassing the IOMMU.

> and needs to be
> conveyed to the OS IOMMU code to do the right thing. Not to the driver.
> 
> In my opinion, adding the VIRTIO_F_IOMMU_PLATFORM feature bit is
> absolutely the wrong thing to do.
> 
> What we *should* do is a patchset in the guest which both fixes virtio
> drivers to *always* use the DMA API, and fixes the DMA API to DTRT at
> the same time — by detecting qemu and installing no-op DMA ops for the
> appropriate devices, perhaps.

Sounds good. And a way to detect appropriate devices could
be by looking at the feature flag, perhaps?


> Then we can look at giving qemu a way to properly indicate which
> devices it actually does DMA mapping for, so we can remove those
> heuristic assumptions.
> 
> But that flag does *not* live in the virtio host←→guest ABI.
> 
> -- 
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
> 

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Joerg Roedel]

On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
> One correction: it's a feature of the device in the system.
> There could be a mix of devices bypassing and not
> bypassing the IOMMU.

No, it really is not. A device can't chose to bypass the IOMMU. But the
IOMMU can chose to let the device bypass. So any fix here belongs
into the platform/iommu code too and not into some driver.

> Sounds good. And a way to detect appropriate devices could
> be by looking at the feature flag, perhaps?

Again, no! The way to detect that is to look into the iommu description
structures provided by the firmware. They provide everything necessary
to tell the iommu code which devices are not translated.



	Joerg

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Andy Lutomirski]

On Wed, Apr 27, 2016 at 7:23 AM, Joerg Roedel <joro@8bytes.org> wrote:
> On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
>> One correction: it's a feature of the device in the system.
>> There could be a mix of devices bypassing and not
>> bypassing the IOMMU.
>
> No, it really is not. A device can't chose to bypass the IOMMU. But the
> IOMMU can chose to let the device bypass. So any fix here belongs
> into the platform/iommu code too and not into some driver.
>
>> Sounds good. And a way to detect appropriate devices could
>> be by looking at the feature flag, perhaps?
>
> Again, no! The way to detect that is to look into the iommu description
> structures provided by the firmware. They provide everything necessary
> to tell the iommu code which devices are not translated.
>

Except on PPC and SPARC.  As far as I know, those are the only
problematic platforms.

Is it too late to *disable* QEMU's q35-iommu thingy until it can be
fixed to report correct data in the DMAR tables?

--Andy

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 07:31:43AM -0700, Andy Lutomirski wrote:
> On Wed, Apr 27, 2016 at 7:23 AM, Joerg Roedel <joro@8bytes.org> wrote:
> > On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
> >> One correction: it's a feature of the device in the system.
> >> There could be a mix of devices bypassing and not
> >> bypassing the IOMMU.
> >
> > No, it really is not. A device can't chose to bypass the IOMMU. But the
> > IOMMU can chose to let the device bypass. So any fix here belongs
> > into the platform/iommu code too and not into some driver.
> >
> >> Sounds good. And a way to detect appropriate devices could
> >> be by looking at the feature flag, perhaps?
> >
> > Again, no! The way to detect that is to look into the iommu description
> > structures provided by the firmware. They provide everything necessary
> > to tell the iommu code which devices are not translated.
> >
> 
> Except on PPC and SPARC.  As far as I know, those are the only
> problematic platforms.
> 
> Is it too late to *disable* QEMU's q35-iommu thingy until it can be
> fixed to report correct data in the DMAR tables?
> 
> --Andy

Meaning virtio or assigned devices?
For virtio - it's way too late since these are working configurations.
For assigned devices - they don't work on x86 so it doesn't have
to be disabled, it's safe to ignore.

-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Andy Lutomirski]

On Wed, Apr 27, 2016 at 7:38 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Apr 27, 2016 at 07:31:43AM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 27, 2016 at 7:23 AM, Joerg Roedel <joro@8bytes.org> wrote:
>> > On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
>> >> One correction: it's a feature of the device in the system.
>> >> There could be a mix of devices bypassing and not
>> >> bypassing the IOMMU.
>> >
>> > No, it really is not. A device can't chose to bypass the IOMMU. But the
>> > IOMMU can chose to let the device bypass. So any fix here belongs
>> > into the platform/iommu code too and not into some driver.
>> >
>> >> Sounds good. And a way to detect appropriate devices could
>> >> be by looking at the feature flag, perhaps?
>> >
>> > Again, no! The way to detect that is to look into the iommu description
>> > structures provided by the firmware. They provide everything necessary
>> > to tell the iommu code which devices are not translated.
>> >
>>
>> Except on PPC and SPARC.  As far as I know, those are the only
>> problematic platforms.
>>
>> Is it too late to *disable* QEMU's q35-iommu thingy until it can be
>> fixed to report correct data in the DMAR tables?
>>
>> --Andy
>
> Meaning virtio or assigned devices?
> For virtio - it's way too late since these are working configurations.
> For assigned devices - they don't work on x86 so it doesn't have
> to be disabled, it's safe to ignore.

I mean actually prevent QEMU from running in q35-iommu mode with any
virtio devices attached or maybe even turn off q35-iommu mode entirely
[1].  Doesn't it require that the user literally pass the word
"experimental" into QEMU right now?  It did at some point IIRC.

The reason I'm asking is that, other than q35-iommu, QEMU's virtio
devices *don't* bypass the IOMMU except on PPC and SPARC, simply
because there is no other configuration AFAICT that has virtio and and
IOMMU.  So maybe the right solution is to fix q35-iommu to use DMAR
correctly (thus breaking q35-iommu users with older guest kernels,
which hopefully don't actually exist) and to come up with a PPC- and
SPARC-specific solution, or maybe OpenFirmware-specific solution, to
handle PPC and SPARC down the road.

[1] I'm pretty sure I emailed the QEMU list before q35-iommu ever
showed up in a release asking the QEMU team to please not do that
until this issue was resolved.  Sadly, that email was ignored :(

--Andy

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 07:43:07AM -0700, Andy Lutomirski wrote:
> On Wed, Apr 27, 2016 at 7:38 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Wed, Apr 27, 2016 at 07:31:43AM -0700, Andy Lutomirski wrote:
> >> On Wed, Apr 27, 2016 at 7:23 AM, Joerg Roedel <joro@8bytes.org> wrote:
> >> > On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
> >> >> One correction: it's a feature of the device in the system.
> >> >> There could be a mix of devices bypassing and not
> >> >> bypassing the IOMMU.
> >> >
> >> > No, it really is not. A device can't chose to bypass the IOMMU. But the
> >> > IOMMU can chose to let the device bypass. So any fix here belongs
> >> > into the platform/iommu code too and not into some driver.
> >> >
> >> >> Sounds good. And a way to detect appropriate devices could
> >> >> be by looking at the feature flag, perhaps?
> >> >
> >> > Again, no! The way to detect that is to look into the iommu description
> >> > structures provided by the firmware. They provide everything necessary
> >> > to tell the iommu code which devices are not translated.
> >> >
> >>
> >> Except on PPC and SPARC.  As far as I know, those are the only
> >> problematic platforms.
> >>
> >> Is it too late to *disable* QEMU's q35-iommu thingy until it can be
> >> fixed to report correct data in the DMAR tables?
> >>
> >> --Andy
> >
> > Meaning virtio or assigned devices?
> > For virtio - it's way too late since these are working configurations.
> > For assigned devices - they don't work on x86 so it doesn't have
> > to be disabled, it's safe to ignore.
> 
> I mean actually prevent QEMU from running in q35-iommu mode with any
> virtio devices attached or maybe even turn off q35-iommu mode entirely
> [1].  Doesn't it require that the user literally pass the word
> "experimental" into QEMU right now?  It did at some point IIRC.
> 
> The reason I'm asking is that, other than q35-iommu, QEMU's virtio
> devices *don't* bypass the IOMMU except on PPC and SPARC, simply
> because there is no other configuration AFAICT that has virtio and and
> IOMMU.  So maybe the right solution is to fix q35-iommu to use DMAR
> correctly (thus breaking q35-iommu users with older guest kernels,
> which hopefully don't actually exist) and to come up with a PPC- and
> SPARC-specific solution, or maybe OpenFirmware-specific solution, to
> handle PPC and SPARC down the road.
> 
> [1] I'm pretty sure I emailed the QEMU list before q35-iommu ever
> showed up in a release asking the QEMU team to please not do that
> until this issue was resolved.  Sadly, that email was ignored :(
> 
> --Andy

Sorry, I didn't make myself clear.
Point is, QEMU is not the only virtio implementation out there.
So we can't know no virtio implementations have an IOMMU as long as
linux supports this IOMMU.
virtio always used physical addresses since it was born and if it
changes that it must do this in a way that does not break existing
users.

-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Joerg Roedel]

On Wed, Apr 27, 2016 at 05:54:57PM +0300, Michael S. Tsirkin wrote:
> Point is, QEMU is not the only virtio implementation out there.
> So we can't know no virtio implementations have an IOMMU as long as
> linux supports this IOMMU.
> virtio always used physical addresses since it was born and if it
> changes that it must do this in a way that does not break existing
> users.

FWIW, virtio in qemu can continue to just use physical addresses. But
qemu needs to advertise that fact correctly to the OS in the DMAR table.
This way old kernels (where virtio does not use DMA-API) will also
continue to work on the fixed qemu.



	Joerg

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 04:58:51PM +0200, Joerg Roedel wrote:
> On Wed, Apr 27, 2016 at 05:54:57PM +0300, Michael S. Tsirkin wrote:
> > Point is, QEMU is not the only virtio implementation out there.
> > So we can't know no virtio implementations have an IOMMU as long as
> > linux supports this IOMMU.
> > virtio always used physical addresses since it was born and if it
> > changes that it must do this in a way that does not break existing
> > users.
> 
> FWIW, virtio in qemu can continue to just use physical addresses. But
> qemu needs to advertise that fact correctly to the OS in the DMAR table.
> This way old kernels (where virtio does not use DMA-API) will also
> continue to work on the fixed qemu.
> 
> 
> 
> 	Joerg

It's not clear it can do this since DMAR tables seem to assume
a given slot is either bypassing IOMMU or going through it.
QEMU allows reusing same slot for virtio and non-virtio devices.

Besides, this patch is not about it, it's a flag for QEMU to
tell guest that it can trust DMAR tables.

-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Andy Lutomirski]

On Wed, Apr 27, 2016 at 7:54 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Apr 27, 2016 at 07:43:07AM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 27, 2016 at 7:38 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Wed, Apr 27, 2016 at 07:31:43AM -0700, Andy Lutomirski wrote:
>> >> On Wed, Apr 27, 2016 at 7:23 AM, Joerg Roedel <joro@8bytes.org> wrote:
>> >> > On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
>> >> >> One correction: it's a feature of the device in the system.
>> >> >> There could be a mix of devices bypassing and not
>> >> >> bypassing the IOMMU.
>> >> >
>> >> > No, it really is not. A device can't chose to bypass the IOMMU. But the
>> >> > IOMMU can chose to let the device bypass. So any fix here belongs
>> >> > into the platform/iommu code too and not into some driver.
>> >> >
>> >> >> Sounds good. And a way to detect appropriate devices could
>> >> >> be by looking at the feature flag, perhaps?
>> >> >
>> >> > Again, no! The way to detect that is to look into the iommu description
>> >> > structures provided by the firmware. They provide everything necessary
>> >> > to tell the iommu code which devices are not translated.
>> >> >
>> >>
>> >> Except on PPC and SPARC.  As far as I know, those are the only
>> >> problematic platforms.
>> >>
>> >> Is it too late to *disable* QEMU's q35-iommu thingy until it can be
>> >> fixed to report correct data in the DMAR tables?
>> >>
>> >> --Andy
>> >
>> > Meaning virtio or assigned devices?
>> > For virtio - it's way too late since these are working configurations.
>> > For assigned devices - they don't work on x86 so it doesn't have
>> > to be disabled, it's safe to ignore.
>>
>> I mean actually prevent QEMU from running in q35-iommu mode with any
>> virtio devices attached or maybe even turn off q35-iommu mode entirely
>> [1].  Doesn't it require that the user literally pass the word
>> "experimental" into QEMU right now?  It did at some point IIRC.
>>
>> The reason I'm asking is that, other than q35-iommu, QEMU's virtio
>> devices *don't* bypass the IOMMU except on PPC and SPARC, simply
>> because there is no other configuration AFAICT that has virtio and and
>> IOMMU.  So maybe the right solution is to fix q35-iommu to use DMAR
>> correctly (thus breaking q35-iommu users with older guest kernels,
>> which hopefully don't actually exist) and to come up with a PPC- and
>> SPARC-specific solution, or maybe OpenFirmware-specific solution, to
>> handle PPC and SPARC down the road.
>>
>> [1] I'm pretty sure I emailed the QEMU list before q35-iommu ever
>> showed up in a release asking the QEMU team to please not do that
>> until this issue was resolved.  Sadly, that email was ignored :(
>>
>> --Andy
>
> Sorry, I didn't make myself clear.
> Point is, QEMU is not the only virtio implementation out there.
> So we can't know no virtio implementations have an IOMMU as long as
> linux supports this IOMMU.
> virtio always used physical addresses since it was born and if it
> changes that it must do this in a way that does not break existing
> users.

Is there any non-QEMU virtio implementation can provide an
IOMMU-bypassing virtio device on a platform that has a nontrivial
IOMMU?

--Andy

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 04:23:32PM +0200, Joerg Roedel wrote:
> On Wed, Apr 27, 2016 at 04:37:04PM +0300, Michael S. Tsirkin wrote:
> > One correction: it's a feature of the device in the system.
> > There could be a mix of devices bypassing and not
> > bypassing the IOMMU.
> 
> No, it really is not. A device can't chose to bypass the IOMMU. But the
> IOMMU can chose to let the device bypass.

QEMU can choose to bypass IOMMU for one device and not the other.
IOMMU in QEMU isn't involved when it's bypassed.

> So any fix here belongs
> into the platform/iommu code too and not into some driver.

Fine but this is beside the point. Almost all virtio devices
bypass IOMMU and what this patch does is create a way
to detect devices that don't. This code can maybe go into
platform.

> > Sounds good. And a way to detect appropriate devices could
> > be by looking at the feature flag, perhaps?
> 
> Again, no! The way to detect that is to look into the iommu description
> structures provided by the firmware. They provide everything necessary
> to tell the iommu code which devices are not translated.
> 
> 
> 
> 	Joerg

It would be easy if they did but they don't do this on all systems.
In particular the idea for firmware interface was clearly
that a given bus either is connected through IOMMU or bypassing it.
Whether virtio bypasses the iommu is unrelated to the bus it's on.

-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Joerg Roedel]

On Wed, Apr 27, 2016 at 05:34:30PM +0300, Michael S. Tsirkin wrote:
> On Wed, Apr 27, 2016 at 04:23:32PM +0200, Joerg Roedel wrote:

> QEMU can choose to bypass IOMMU for one device and not the other.
> IOMMU in QEMU isn't involved when it's bypassed.

And it is QEMU's task to tell the OS, right? And the correct way to do
this is via the firmware ACPI tables.

> Fine but this is beside the point. Almost all virtio devices
> bypass IOMMU and what this patch does is create a way
> to detect devices that don't. This code can maybe go into
> platform.

Again, the way to detect this is in platform code must not be device
specific. This is what the DMAR and IVRS tables on x86 are for.

When there is no way to do this in the firmware (or there is no firmware
at all), we have to do a quirk in the platform code for it.



	Joerg

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 04:56:32PM +0200, Joerg Roedel wrote:
> On Wed, Apr 27, 2016 at 05:34:30PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Apr 27, 2016 at 04:23:32PM +0200, Joerg Roedel wrote:
> 
> > QEMU can choose to bypass IOMMU for one device and not the other.
> > IOMMU in QEMU isn't involved when it's bypassed.
> 
> And it is QEMU's task to tell the OS, right? And the correct way to do
> this is via the firmware ACPI tables.

Going forward, this might be reasonable. Of course it didn't in the past
and no one cared because virtio devices used physical addresses. We have
to keep these setups working.

> > Fine but this is beside the point. Almost all virtio devices
> > bypass IOMMU and what this patch does is create a way
> > to detect devices that don't. This code can maybe go into
> > platform.
> 
> Again, the way to detect this is in platform code must not be device
> specific. This is what the DMAR and IVRS tables on x86 are for.
> 
> When there is no way to do this in the firmware (or there is no firmware
> at all), we have to do a quirk in the platform code for it.
> 
> 
> 
> 	Joerg

I really don't get it.

There's exactly one device that works now and needs the work-around and
so that we need to support, and that is virtio. It happens to have
exactly the same issue on all platforms.

Why would we want to work hard to build platform-specific
solutions to a problem that can be solved in 5 lines of
generic code?

-- 
MST

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

On Wed, 2016-04-27 at 18:05 +0300, Michael S. Tsirkin wrote:
> 
> I really don't get it.
> 
> There's exactly one device that works now and needs the work-around and
> so that we need to support, and that is virtio. It happens to have
> exactly the same issue on all platforms.

False. We have other devices which are currently *not* translated by
the emulated IOMMU and which aren't going to be in the short term
either.

We also have other devices (emulated hardware NICs) to which precisely
the same "we don't need protection" arguments apply, and which we
*could* expose to the guest without an IOMMU translation if we really
wanted to. It makes as much sense as exposing virtio without an IOMMU,
going forward.

> Why would we want to work hard to build platform-specific
> solutions to a problem that can be solved in 5 lines of
> generic code?

Because it's a dirty hack in the *wrong* place.

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 04:15:35PM +0100, David Woodhouse wrote:
> On Wed, 2016-04-27 at 18:05 +0300, Michael S. Tsirkin wrote:
> > 
> > I really don't get it.
> > 
> > There's exactly one device that works now and needs the work-around and
> > so that we need to support, and that is virtio. It happens to have
> > exactly the same issue on all platforms.
> 
> False. We have other devices which are currently *not* translated by
> the emulated IOMMU and which aren't going to be in the short term
> either.
> 
> We also have other devices (emulated hardware NICs) to which precisely
> the same "we don't need protection" arguments apply, and which we
> *could* expose to the guest without an IOMMU translation if we really
> wanted to. It makes as much sense as exposing virtio without an IOMMU,
> going forward.

The reasons for virtio are mostly dealing legacy.
We don't need protection is a separate issue
that I'd rather drop for now.

> > Why would we want to work hard to build platform-specific
> > solutions to a problem that can be solved in 5 lines of
> > generic code?
> 
> Because it's a dirty hack in the *wrong* place.

No one came up with a better one so far :(

> -- 
> dwmw2

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

On Wed, 2016-04-27 at 21:17 +0300, Michael S. Tsirkin wrote:
> 
> > Because it's a dirty hack in the *wrong* place.
> 
> No one came up with a better one so far :(

Seriously?

Take a look at drivers/iommu/intel-iommu.c. It has quirks for all kinds
of shitty devices that have to be put in passthrough mode or otherwise
excluded.

We don't actually *need* it for the Intel IOMMU; all we need is for
QEMU to stop lying in its DMAR tables.

We *do* want the same kind of quirks in the relevant POWER and ARM
IOMMU code in the kernel. Do that (hell, a simple quirk for all virtio
devices will suffice, but NOT in the virtio driver) at the same moment
you fix the virtio devices to use the DMA API. Job done.

Some time *later* we can work on *refining* that quirk, and a way for
QEMU to tell the guest (via something generic like fwcfg, maybe) that
some devices are and aren't translated.

Actually, I'm about to look at moving dma_ops into struct device and
cleaning up the way we detect which IOMMU is attached, at device
instantiation time. Perhaps I can shove the virtio-exception quirk in
there while I'm at it...

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Wed, Apr 27, 2016 at 08:16:57PM +0100, David Woodhouse wrote:
> On Wed, 2016-04-27 at 21:17 +0300, Michael S. Tsirkin wrote:
> > 
> > > Because it's a dirty hack in the *wrong* place.
> > 
> > No one came up with a better one so far :(
> 
> Seriously?
> 
> Take a look at drivers/iommu/intel-iommu.c. It has quirks for all kinds
> of shitty devices that have to be put in passthrough mode or otherwise
> excluded.

I see work-arounds for broken IOMMUs but not for
individual devices. Could you point me to a more specific
example?


> We don't actually *need* it for the Intel IOMMU; all we need is for
> QEMU to stop lying in its DMAR tables.

We need it for legacy QEMU anyway, and it's not easy for QEMU to stop
lying about virtio, so we'll need it for a while.
I think it's easy for QEMU to stop lying about assigned devices,
so we don't need it for non-virtio devices.

> We *do* want the same kind of quirks in the relevant POWER and ARM
> IOMMU code in the kernel. Do that (hell, a simple quirk for all virtio
> devices will suffice, but NOT in the virtio driver

Sure - that works. It does not have to be in the driver.

>) at the same moment
> you fix the virtio devices to use the DMA API. Job done.
> 
> Some time *later* we can work on *refining* that quirk, and a way for
> QEMU to tell the guest (via something generic like fwcfg, maybe) that
> some devices are and aren't translated.

I don't see why how fwcfg can work here. It's a static thing,
devices can come and go with hotplug.

> Actually, I'm about to look at moving dma_ops into struct device and
> cleaning up the way we detect which IOMMU is attached, at device
> instantiation time. Perhaps I can shove the virtio-exception quirk in
> there while I'm at it...

Sounds good.

> -- 
> dwmw2
> 

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On Thu, 2016-04-28 at 17:34 +0300, Michael S. Tsirkin wrote:
> I see work-arounds for broken IOMMUs but not for
> individual devices. Could you point me to a more specific
> example?

I think the closest example is probably quirk_ioat_snb_local_iommu().

If we see this particular device, we *know* what the topology actually
looks like. We check the hardware setup, and if we're *not* being told
the truth, then we stick it in bypass mode because we know it *isn't*
actually being translated.

Actually, that's almost *identical* to what we want, isn't it?

Except instead of checking undocumented chipset registers, it wants to
be checking "am I on a version of qemu known to lie about virtio being
translated?"

> > We don't actually *need* it for the Intel IOMMU; all we need is for
> > QEMU to stop lying in its DMAR tables.
> We need it for legacy QEMU anyway, and it's not easy for QEMU to stop
> lying about virtio, so we'll need it for a while.
> I think it's easy for QEMU to stop lying about assigned devices,
> so we don't need it for non-virtio devices.

Why is it easier for QEMU to tell the truth about assigned devices,
than it is for virtio? Assuming they both remain actually untranslated
for now, why's it easier to fix the DMAR table for one and not the
other?

(Implementing translation of assigned devices is on my list, but it's a
long way off).

> I don't see why how fwcfg can work here. It's a static thing,
> devices can come and go with hotplug.

This touches on something you said elsewhere, that it's
painful/impossible to hot-unplug a translated device and hot-plug an
untranslated device in the same slot (and vice versa).

So let's assume for now that a given slot is indeed static, and either
translated or untranslated. Like the DMAR table, the fwcfg can just
give a list of slot which are (or aren't) translated.

And then you can *only* add a translated device to a translated slot,
or an untranslated device to an untranslated slot.

All the internally-emulated devices *can* be either translated or
untranslated. That's just a matter of software. Surely, you currently
*can't* have translated assigned devices (until someone implements the
whole VT-d page table shadowing or whatever), so you'll be barred from
assigning a device to a slot which *previously* had an untranslated
device. But so what? Put it in a different slot instead.

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5760 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Thu, Apr 28, 2016 at 04:11:54PM +0100, David Woodhouse wrote:
> On Thu, 2016-04-28 at 17:34 +0300, Michael S. Tsirkin wrote:
> > I see work-arounds for broken IOMMUs but not for
> > individual devices. Could you point me to a more specific
> > example?
> 
> I think the closest example is probably quirk_ioat_snb_local_iommu().

OK, so for intel, it seems that it's enough to set
	pdev->dev.archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;
for the device.

Do I have to poke at each iommu implementation to find
a way to do this, or is there some way to do it
portably?

> If we see this particular device, we *know* what the topology actually
> looks like. We check the hardware setup, and if we're *not* being told
> the truth, then we stick it in bypass mode because we know it *isn't*
> actually being translated.
> 
> Actually, that's almost *identical* to what we want, isn't it?
> 
> Except instead of checking undocumented chipset registers, it wants to
> be checking "am I on a version of qemu known to lie about virtio being
> translated?"

Not exactly - I think that future versions of qemu might lie
about some devices but not others.

> > > We don't actually *need* it for the Intel IOMMU; all we need is for
> > > QEMU to stop lying in its DMAR tables.
> > We need it for legacy QEMU anyway, and it's not easy for QEMU to stop
> > lying about virtio, so we'll need it for a while.
> > I think it's easy for QEMU to stop lying about assigned devices,
> > so we don't need it for non-virtio devices.
> 
> Why is it easier for QEMU to tell the truth about assigned devices,
> than it is for virtio? Assuming they both remain actually untranslated
> for now, why's it easier to fix the DMAR table for one and not the
> other?
> 
> (Implementing translation of assigned devices is on my list, but it's a
> long way off).

DMAR is unfortunately not a good match for what people do with QEMU.

There is a patchset on list fixing translation of assigned
devices. So the fix for these will simply be to do translation for all
assigned devices. It's harder for virtio as it isn't always
processed in QEMU - there's vhost in kernel and an out of process
vhost-user plugin. So we can end up e.g. with modern QEMU which
does translate in-process virtio but not out of process one.

> > I don't see why how fwcfg can work here. It's a static thing,
> > devices can come and go with hotplug.
> 
> This touches on something you said elsewhere, that it's
> painful/impossible to hot-unplug a translated device and hot-plug an
> untranslated device in the same slot (and vice versa).
> 
> So let's assume for now that a given slot is indeed static, and either
> translated or untranslated. Like the DMAR table, the fwcfg can just
> give a list of slot which are (or aren't) translated.
> 
> And then you can *only* add a translated device to a translated slot,
> or an untranslated device to an untranslated slot.
> 
> All the internally-emulated devices *can* be either translated or
> untranslated. That's just a matter of software. Surely, you currently
> *can't* have translated assigned devices (until someone implements the
> whole VT-d page table shadowing or whatever), so you'll be barred from
> assigning a device to a slot which *previously* had an untranslated
> device. But so what? Put it in a different slot instead.

Unfortunately people got used to be able to put any device
in any slot, and built external tools around that ability.
It's rather painful to break this assumption.

> -- 
> dwmw2
> 

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2724 bytes --]

On Thu, 2016-04-28 at 18:37 +0300, Michael S. Tsirkin wrote:
> OK, so for intel, it seems that it's enough to set
> 	pdev->dev.archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;
> for the device.

Yes, currently. Although that's vile. In fact what we *want* to happen
is for the intel-iommu code simply to decline to provide DMA ops for
this device, and let it fall back to the swiotlb or no-op DMA ops, as
appropriate.

As it is, we have the intel-iommu DMA ops *unconditionally, and they
have a hack to manually fall back to calling swiotlb. It's all just
horrid, which is why I want to clean it up with nice per-device DMA ops
and discovery thereof :)

> Do I have to poke at each iommu implementation to find
> a way to do this, or is there some way to do it
> portably?

There *will* be.... Christoph has already done some of the cleanup in
this space, and I need to take stock of what he's already done, and
finish off the parts I want to build on top of it.

> Not exactly - I think that future versions of qemu might lie
> about some devices but not others.

Can we keep this simple?

QEMU currently lies about some devices. Let's implement a heuristic for
the guest OS to know about that, and react accordingly.

Then let's fix QEMU to tell the truth. All the time, unconditionally.
Even on POWER/ARM where there's no obvious *way* for it to tell the
truth (because you don't have the flexibility that DMAR tables do), and
we need to devise a way to put it in the device-tree or fwcfg or
something else.

And only once QEMU consistently tells the *truth*, then we can start to
do new stuff and let it actually change its behaviour.

> DMAR is unfortunately not a good match for what people do with QEMU.
> 
> There is a patchset on list fixing translation of assigned
> devices. So the fix for these will simply be to do translation for
> all assigned devices. It's harder for virtio as it isn't always
> processed in QEMU - there's vhost in kernel and an out of process
> vhost-user plugin. So we can end up e.g. with modern QEMU which
> does translate in-process virtio but not out of process one.

Right... just stop. Fix QEMU to tell the truth first, and *then* once
we can trust it, we can start to change its behaviour. :)

> Unfortunately people got used to be able to put any device
> in any slot, and built external tools around that ability.
> It's rather painful to break this assumption.

Well, if you just said you have a patch set which allows translation of
assigned devices then you are most of the way there, aren't you? We
just need to fix the out-of-process virtio case, and everything can be
either translated or untranslated?

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5760 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Thu, Apr 28, 2016 at 04:48:25PM +0100, David Woodhouse wrote:
> On Thu, 2016-04-28 at 18:37 +0300, Michael S. Tsirkin wrote:
> > OK, so for intel, it seems that it's enough to set
> > 	pdev->dev.archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;
> > for the device.
> 
> Yes, currently. Although that's vile. In fact what we *want* to happen
> is for the intel-iommu code simply to decline to provide DMA ops for
> this device, and let it fall back to the swiotlb or no-op DMA ops, as
> appropriate.
> 
> As it is, we have the intel-iommu DMA ops *unconditionally, and they
> have a hack to manually fall back to calling swiotlb. It's all just
> horrid, which is why I want to clean it up with nice per-device DMA ops
> and discovery thereof :)
> 
> > Do I have to poke at each iommu implementation to find
> > a way to do this, or is there some way to do it
> > portably?
> 
> There *will* be.... Christoph has already done some of the cleanup in
> this space, and I need to take stock of what he's already done, and
> finish off the parts I want to build on top of it.
> 
> > Not exactly - I think that future versions of qemu might lie
> > about some devices but not others.
> 
> Can we keep this simple?
> 
> QEMU currently lies about some devices. Let's implement a heuristic for
> the guest OS to know about that, and react accordingly.
> 
> Then let's fix QEMU to tell the truth. All the time, unconditionally.
> Even on POWER/ARM where there's no obvious *way* for it to tell the
> truth (because you don't have the flexibility that DMAR tables do), and
> we need to devise a way to put it in the device-tree or fwcfg or
> something else.

Right.  Unfortunately all these aren't easy to implement at all.
So I'm inclined to go the "something else" route.
It has the added benefit of giving us a heuristic for free.

> And only once QEMU consistently tells the *truth*, then we can start to
> do new stuff and let it actually change its behaviour.
> 
> > DMAR is unfortunately not a good match for what people do with QEMU.
> > 
> > There is a patchset on list fixing translation of assigned
> > devices. So the fix for these will simply be to do translation for
> > all assigned devices. It's harder for virtio as it isn't always
> > processed in QEMU - there's vhost in kernel and an out of process
> > vhost-user plugin. So we can end up e.g. with modern QEMU which
> > does translate in-process virtio but not out of process one.
> 
> Right... just stop. Fix QEMU to tell the truth first, and *then* once
> we can trust it, we can start to change its behaviour. :)
> 
> > Unfortunately people got used to be able to put any device
> > in any slot, and built external tools around that ability.
> > It's rather painful to break this assumption.
> 
> Well, if you just said you have a patch set which allows translation of
> assigned devices then you are most of the way there, aren't you? We
> just need to fix the out-of-process virtio case, and everything can be
> either translated or untranslated?

Absolutely. But that "just" will take a while.  With out of process
there's always a chance that remote doesn't implement translation. E.g.
new QEMU running on an old host kernel.

> -- 
> dwmw2
> 

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Paolo Bonzini]

On 28/04/2016 17:37, Michael S. Tsirkin wrote:
> > All the internally-emulated devices *can* be either translated or
> > untranslated. That's just a matter of software. Surely, you currently
> > *can't* have translated assigned devices (until someone implements the
> > whole VT-d page table shadowing or whatever), so you'll be barred from
> > assigning a device to a slot which *previously* had an untranslated
> > device. But so what? Put it in a different slot instead.
> 
> Unfortunately people got used to be able to put any device
> in any slot, and built external tools around that ability.
> It's rather painful to break this assumption.

Once you move to PCIe, a lot of things become more complicated.  This is
just one of them; instead of needing half a dozen PCI bridges, you'll
need half a dozen plus one.

Paolo

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3778 bytes --]

On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin wrote:
> This adds a flag to enable/disable bypassing the IOMMU by
> virtio devices.
> 
> This is on top of patch
> http://article.gmane.org/gmane.comp.emulators.qemu/403467
> virtio: convert to use DMA api
> 
> Tested with patchset
> http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> virtio-pci: iommu support (note: bit number has been kept at 34
> intentionally to match posted guest code. a non-RFC version will
> renumber bits to be contigious).
> 
> changes from v1:
>     drop PASSTHROUGH flag
> 
> 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.
> 
> On some systems, including Xen and any system with a physical device
> that speaks virtio behind a physical IOMMU, we must use the DMA API
> for virtio DMA to work at all.
> 
> Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> 
> If not there, we preserve historic behavior and bypass the DMA
> API unless within Xen guest. This is actually required for
> systems, including SPARC and PPC64, where 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.
> 
> Re: non-virtio devices.
> 
> It turns out that on old QEMU hosts, only emulated devices which were
> part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> devices *only*, it would be rather easy to detect them by looking at
> subsystem vendor and device ID. Thus, no new interfaces are required
> except for virtio which always uses the same subsystem vendor and device ID.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/virtio/virtio-access.h              | 3 ++-
>  include/hw/virtio/virtio.h                     | 4 +++-
>  include/standard-headers/linux/virtio_config.h | 2 ++
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> index 967cc75..bb6f34e 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev)
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  
> -    if (k->get_dma_as) {
> +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> +        k->get_dma_as) {
>          return k->get_dma_as(qbus->parent);
>      }
>      return &address_space_memory;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b12faa9..44f3788 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
>                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
>      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> -                      VIRTIO_F_ANY_LAYOUT, true)
> +                      VIRTIO_F_ANY_LAYOUT, true), \
> +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> +                      VIRTIO_F_IOMMU_PLATFORM, false)

Looks like the impact of this patch is that users who relied on
k->get_dma_as today may now have to explicitly add iommu_platform=on.
Are there any such users (e.g. Xen)?

Instead of breaking the command-line for these users you could invert
the flag's meaning ("iommu_bypass=on") and set it in the SPARC/PPC
machine types.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Michael S. Tsirkin]

On Thu, Apr 21, 2016 at 03:56:53PM +0100, Stefan Hajnoczi wrote:
> On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin wrote:
> > This adds a flag to enable/disable bypassing the IOMMU by
> > virtio devices.
> > 
> > This is on top of patch
> > http://article.gmane.org/gmane.comp.emulators.qemu/403467
> > virtio: convert to use DMA api
> > 
> > Tested with patchset
> > http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> > virtio-pci: iommu support (note: bit number has been kept at 34
> > intentionally to match posted guest code. a non-RFC version will
> > renumber bits to be contigious).
> > 
> > changes from v1:
> >     drop PASSTHROUGH flag
> > 
> > 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.
> > 
> > On some systems, including Xen and any system with a physical device
> > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > for virtio DMA to work at all.
> > 
> > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> > 
> > If not there, we preserve historic behavior and bypass the DMA
> > API unless within Xen guest. This is actually required for
> > systems, including SPARC and PPC64, where 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.
> > 
> > Re: non-virtio devices.
> > 
> > It turns out that on old QEMU hosts, only emulated devices which were
> > part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> > devices *only*, it would be rather easy to detect them by looking at
> > subsystem vendor and device ID. Thus, no new interfaces are required
> > except for virtio which always uses the same subsystem vendor and device ID.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/hw/virtio/virtio-access.h              | 3 ++-
> >  include/hw/virtio/virtio.h                     | 4 +++-
> >  include/standard-headers/linux/virtio_config.h | 2 ++
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> > index 967cc75..bb6f34e 100644
> > --- a/include/hw/virtio/virtio-access.h
> > +++ b/include/hw/virtio/virtio-access.h
> > @@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev)
> >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >  
> > -    if (k->get_dma_as) {
> > +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> > +        k->get_dma_as) {
> >          return k->get_dma_as(qbus->parent);
> >      }
> >      return &address_space_memory;
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b12faa9..44f3788 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> >      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
> >                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
> >      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> > -                      VIRTIO_F_ANY_LAYOUT, true)
> > +                      VIRTIO_F_ANY_LAYOUT, true), \
> > +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> > +                      VIRTIO_F_IOMMU_PLATFORM, false)
> 
> Looks like the impact of this patch is that users who relied on
> k->get_dma_as today may now have to explicitly add iommu_platform=on.
> Are there any such users (e.g. Xen)?

No because upstream this is ignored. This is an incremental patch
on top of Jason's one.

> Instead of breaking the command-line for these users you could invert
> the flag's meaning ("iommu_bypass=on") and set it in the SPARC/PPC
> machine types.
> 
> Stefan

I hope I made it clear that there are no such users.

Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4487 bytes --]

On Thu, Apr 21, 2016 at 06:11:40PM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 21, 2016 at 03:56:53PM +0100, Stefan Hajnoczi wrote:
> > On Thu, Apr 21, 2016 at 04:43:45PM +0300, Michael S. Tsirkin wrote:
> > > This adds a flag to enable/disable bypassing the IOMMU by
> > > virtio devices.
> > > 
> > > This is on top of patch
> > > http://article.gmane.org/gmane.comp.emulators.qemu/403467
> > > virtio: convert to use DMA api
> > > 
> > > Tested with patchset
> > > http://article.gmane.org/gmane.linux.kernel.virtualization/27545
> > > virtio-pci: iommu support (note: bit number has been kept at 34
> > > intentionally to match posted guest code. a non-RFC version will
> > > renumber bits to be contigious).
> > > 
> > > changes from v1:
> > >     drop PASSTHROUGH flag
> > > 
> > > 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.
> > > 
> > > On some systems, including Xen and any system with a physical device
> > > that speaks virtio behind a physical IOMMU, we must use the DMA API
> > > for virtio DMA to work at all.
> > > 
> > > Add a feature bit to detect that: VIRTIO_F_IOMMU_PLATFORM.
> > > 
> > > If not there, we preserve historic behavior and bypass the DMA
> > > API unless within Xen guest. This is actually required for
> > > systems, including SPARC and PPC64, where 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.
> > > 
> > > Re: non-virtio devices.
> > > 
> > > It turns out that on old QEMU hosts, only emulated devices which were
> > > part of QEMU use the IOMMU.  Should we want to bypass the IOMMU for such
> > > devices *only*, it would be rather easy to detect them by looking at
> > > subsystem vendor and device ID. Thus, no new interfaces are required
> > > except for virtio which always uses the same subsystem vendor and device ID.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  include/hw/virtio/virtio-access.h              | 3 ++-
> > >  include/hw/virtio/virtio.h                     | 4 +++-
> > >  include/standard-headers/linux/virtio_config.h | 2 ++
> > >  3 files changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
> > > index 967cc75..bb6f34e 100644
> > > --- a/include/hw/virtio/virtio-access.h
> > > +++ b/include/hw/virtio/virtio-access.h
> > > @@ -23,7 +23,8 @@ static inline AddressSpace *virtio_get_dma_as(VirtIODevice *vdev)
> > >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> > >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> > >  
> > > -    if (k->get_dma_as) {
> > > +    if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM) &&
> > > +        k->get_dma_as) {
> > >          return k->get_dma_as(qbus->parent);
> > >      }
> > >      return &address_space_memory;
> > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > > index b12faa9..44f3788 100644
> > > --- a/include/hw/virtio/virtio.h
> > > +++ b/include/hw/virtio/virtio.h
> > > @@ -228,7 +228,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> > >      DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
> > >                        VIRTIO_F_NOTIFY_ON_EMPTY, true), \
> > >      DEFINE_PROP_BIT64("any_layout", _state, _field, \
> > > -                      VIRTIO_F_ANY_LAYOUT, true)
> > > +                      VIRTIO_F_ANY_LAYOUT, true), \
> > > +    DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
> > > +                      VIRTIO_F_IOMMU_PLATFORM, false)
> > 
> > Looks like the impact of this patch is that users who relied on
> > k->get_dma_as today may now have to explicitly add iommu_platform=on.
> > Are there any such users (e.g. Xen)?
> 
> No because upstream this is ignored. This is an incremental patch
> on top of Jason's one.
> 
> > Instead of breaking the command-line for these users you could invert
> > the flag's meaning ("iommu_bypass=on") and set it in the SPARC/PPC
> > machine types.
> > 
> > Stefan
> 
> I hope I made it clear that there are no such users.

Okay, thanks!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]