On Fri, Jul 27, 2018 at 10:58:05AM +0100, Will Deacon wrote:> > I just wanted to say that this patch series provides a means for us to > force the coherent DMA ops for legacy virtio devices on arm64, which in turn > means that we can enable the SMMU with legacy devices in our fastmodel > emulation platform (which is slowly being upgraded to virtio 1.0) without > hanging during boot. Patch below.Yikes, this is a nightmare. That is exactly where I do not want things to end up. We really need to distinguish between legacy virtual crappy virtio (and that includes v1) that totally ignores the bus it pretends to be on, and sane virtio (to be defined) that sit on a real (or properly emulated including iommu and details for dma mapping) bus. Having a mumble jumble of arch specific undocumented magic as in the powerpc patch replied to or this arm patch is a complete no-go. Nacked-by: Christoph Hellwig <hch at lst.de> for both.
On Mon, Jul 30, 2018 at 02:34:14AM -0700, Christoph Hellwig wrote:> We really need to distinguish between legacy virtual crappy > virtio (and that includes v1) that totally ignores the bus it pretends > to be on, and sane virtio (to be defined) that sit on a real (or > properly emulated including iommu and details for dma mapping) bus.Let me reply to the "crappy" part first: So virtio devices can run on another CPU or on a PCI bus. Configuration can happen over mupltiple transports. There is a discovery protocol to figure out where it is. It has some warts but any real system has warts. So IMHO virtio running on another CPU isn't "legacy virtual crappy virtio". virtio devices that actually sit on a PCI bus aren't "sane" simply because the DMA is more convoluted on some architectures. Performance impact of the optimizations possible when you know your "device" is in fact just another CPU has been measured, it is real, so we aren't interested in adding all that overhead back just so we can use DMA API. The "correct then fast" mantra doesn't apply to something that is as widely deployed as virtio. And I can accept an argument that maybe the DMA API isn't designed to support such virtual DMA. Whether it should I don't know. With this out of my system: I agree these approaches are hacky. I think it is generally better to have virtio feature negotiation tell you whether device runs on a CPU or not rather than rely on platform specific ways for this. To this end there was a recent proposal to rename VIRTIO_F_IO_BARRIER to VIRTIO_F_REAL_DEVICE. It got stuck since "real" sounds vague to people, e.g. what if it's a VF - is that real or not? But I can see something like e.g. VIRTIO_F_PLATFORM_DMA gaining support. We would then rename virtio_has_iommu_quirk to virtio_has_dma_quirk and test VIRTIO_F_PLATFORM_DMA in addition to the IOMMU thing. -- MST
On Mon, Jul 30, 2018 at 01:28:03PM +0300, Michael S. Tsirkin wrote:> Let me reply to the "crappy" part first: > So virtio devices can run on another CPU or on a PCI bus. Configuration > can happen over mupltiple transports. There is a discovery protocol to > figure out where it is. It has some warts but any real system has warts. > > So IMHO virtio running on another CPU isn't "legacy virtual crappy > virtio". virtio devices that actually sit on a PCI bus aren't "sane" > simply because the DMA is more convoluted on some architectures.All of what you said would be true if virtio didn't claim to be a PCI device. Once it claims to be a PCI device and we also see real hardware written to the interface I stand to all what I said above.> With this out of my system: > I agree these approaches are hacky. I think it is generally better to > have virtio feature negotiation tell you whether device runs on a CPU or > not rather than rely on platform specific ways for this. To this end > there was a recent proposal to rename VIRTIO_F_IO_BARRIER to > VIRTIO_F_REAL_DEVICE. It got stuck since "real" sounds vague to people, > e.g. what if it's a VF - is that real or not? But I can see something > like e.g. VIRTIO_F_PLATFORM_DMA gaining support. > > We would then rename virtio_has_iommu_quirk to virtio_has_dma_quirk > and test VIRTIO_F_PLATFORM_DMA in addition to the IOMMU thing.I don't really care about the exact naming, and indeed a device that sets the flag doesn't have to be a 'real' device - it just has to act like one. I explained all the issues that this means (at least relating to DMA) in one of the previous threads. The important bit is that we can specify exact behavior for both devices that sets the "I'm real!" flag and that ones that don't exactly in the spec. And that very much excludes arch-specific (or Xen-specific) overrides.