Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1390262 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-28 16:40 +0200
    Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api David Woodhouse <dwmw2@infradead.org> - 2016-04-28 17:20 +0200
      Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api "Michael S. Tsirkin" <mst@redhat.com> - 2016-04-28 17:40 +0200
        Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api David Woodhouse <dwmw2@infradead.org> - 2016-04-28 17:50 +0200
          Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api "Michael S. Tsirkin" <mst@redhat.com> - 2016-05-01 12:40 +0200
        Re: [PATCH V2 RFC] fixup! virtio: convert to use DMA api Paolo Bonzini <pbonzini@redhat.com> - 2016-05-09 13:10 +0200

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

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
> 

[toc] | [next] | [standalone]

#1390301

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1390324

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
> 

[toc] | [prev] | [next] | [standalone]

#1390336

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1391791

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
> 

[toc] | [prev] | [next] | [standalone]

#1396943

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web