Re: [PATCH] iommu/virtio: Detach domain on endpoint release

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, May 18, 2023 at 05:49:37PM +0100, Jean-Philippe Brucker wrote:
> On Thu, May 18, 2023 at 10:59:12AM -0300, Jason Gunthorpe wrote:
> > On Thu, May 18, 2023 at 02:56:38PM +0100, Jean-Philippe Brucker wrote:
> > > > Can you wrapper this into a BLOCKED domain like we are moving drivers
> > > > toward, and then attach the blocked domain instead of introducing this
> > > > special case?
> > > 
> > > Yes, I think the way the virtio-iommu driver should implement BLOCKED
> > > domains is initially clearing the global-bypass bit, and then issuing
> > > DETACH requests when the core asks to attach a BLOCKED domain. This has
> > > the same effect as issuing an ATTACH request with an empty domain, but
> > > requires fewer resources in the VMM.
> > 
> > Does that exclude identity though?
> 
> No, identity attaches a domain with the ATTACH_F_BYPASS flag (or an
> identity-mapped domain if the feature is missing), it doesn't rely on
> global-bypass.

Oh, so you can clear the global-bypass bit and that turns detach into
blocked then you can model this detach operation with blocked and
still get identity because that is attach with a flag :) Wow, a bit
indirect but OK

> > this, the desired translation mode should always be made
> > explicit.
> 
> I probably misunderstood your plan for BLOCKED. This particular patch is
> about removing devices from the machine, for example PCIe hot-unplug. So I
> thought you were suggesting the core will at some point attach a BLOCKED
> domain to a device being removed, in order to block translation while the
> device is being removed and while a new one is being plugged in with the
> same RID.

Yes, that would be appropriate

> For that case I think DETACH, rather than ATTACH an empty domain,
> makes more sense. Otherwise with the same reasoning we'd need to
> attach all 4 billion endpoint IDs to an empty domain at boot which
> isn't feasible.

Well, this is what the default is for. You can consider the default as
being implicitly attached to BLOCKED or IDENTIY. If you want to change
the translation then you ask to change the translation to some other
defined thing.

Detach is just a confusing idea because it doesn't really say what the
translation changes into - and there is always an active translation.

On the VMM side we always have a translation assigned in VFIO so it
doesn't really matter how the command is carried over.

This command coding is weird, but it seems like it is fine, and the
driver will be clearer when everything is labeled :)

Jason