Message-ID: <8f85d15478a578f7d9ab63b7e5edecda94830a84.camel@infradead.org>
Date: Mon, 07 Apr 2025 13:46:16 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Christoph Hellwig <hch@...radead.org>, virtio-comment@...ts.linux.dev, 
 Claire Chang <tientzu@...omium.org>, linux-devicetree
 <devicetree@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>,
 Jörg Roedel <joro@...tes.org>, 
 iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
 graf@...zon.de
Subject: Re: [RFC PATCH 1/3] content: Add VIRTIO_F_SWIOTLB to negotiate use
 of SWIOTLB bounce buffers

On Mon, 2025-04-07 at 08:14 -0400, Michael S. Tsirkin wrote:
> On Fri, Apr 04, 2025 at 12:15:52PM +0100, David Woodhouse wrote:
> > > What I don't get, is what does the *device* want, exactly?
> > 
> > The device wants to know that a driver won't try to use it without
> > understanding the restriction. Because otherwise the driver will just
> > give it system addresses for DMA and be sad, without any coherent
> > error/failure report about why.
> > 
> > (You could ask the same question about what the *device* wants with
> > VIRTIO_F_ACCESS_PLATFORM, and the answer is much the same).
> > 
> > Or maybe not the *device* per se, but the *system integrator* wants to
> > know that only operating systems which understand the restriction
> > described above, will attempt to drive the device in question.
> > 
> > We could achieve that by presenting the device with a completely new
> > PCI device/vendor ID so that old drivers don't match, or in the DT
> > model you could make a new "compatible" string for it. I chose to use a
> > VIRTIO_F_ bit for it instead, which seemed natural and allows the
> > device model (under the influence of the system integrator) to *choose*
> > whether a failure to negotiate such bit is fatal or not.
> 
> Let's focus on the mmio part, for simplicity.
> So IIUC there's a devicetree attribute restricted dma, that
> guests currently simply ignore.

No, I think Linux guests with CONFIG_DMA_RESTRICTED_POOL enabled will
honour it just fine and do the right thing. For virtio-mmio.

> You want to fix it in the guest, but you also want to find a clean way
> to detect that it's fixed. Right?

For virtio-mmio I believe there's no bug to fix.

Sure, it would have been nice if a feature bit had been added when this
attribute was defined, so that a device which *knows* it has this
restriction could gracefully decline to operate with a driver which
doesn't understand it.

But as you pointed out, that ship has sailed. Enforcing a new feature
bit now for virtio-mmio devices would break compatibility even with
existing drivers in kernels which *do* have CONFIG_DMA_RESTRICTED_POOL
enabled and which do work today.

There's nothing to fix here for virtio-mmio.

> And if so, my question is, why this specific bug especially?
> There likely are a ton of bugs, some more catastrophic than just
> crashing the guest, like data corruption.
> Is it because we were supposed to add it to the virtio spec but
> did not?

If I could easily expose virtio-mmio on an ACPI-afflicted guest instead
of using virtio-pci, maybe that might suffice¹.

But while we might be able to hack that up for Linux guests, I don't
think that's feasible for the general case. So what I'm trying to do
here is just bring the same capability to virtio-pci, that already
exists for virtio-mmio.

And if we do so I *would* like to get it right this time and add that
feature bit.




¹ It'd want MSI support, and the CONFIG_DMA_RESTRICTED_POOL support in
the kernel is a bit OF-specific and probably needs to be made a bit
more generic in some places. But I'm prepared to consider those as
implementation details, and the latter probably needs to be done anyway
for any of the approaches we've considered for supporting this in
virtio-pci.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5069 bytes)

Powered by blists - more mailing lists