Jean-Philippe Brucker
2023-May-18 16:49 UTC
[PATCH] iommu/virtio: Detach domain on endpoint release
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.> > It seems like the protocol should not have an implicit operation like > 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. 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. In addition I don't think we'll want to force the VMMs to keep endpoint ID state internally after destroying devices, though that does need to be specified one way or another. If BLOCKED is only for transient states, for example while a struct device is not bound to a driver, then attaching an empty domain works and is simpler to implement. Probably the best is to implement BLOCKED this way and still call DETACH in the release_device() op. In any case, it shouldn't make a difference to the core. I'll see which one is better for the VMMs. Thanks, Jean> Having a boot time default makes sense, but there is little > reason for an OS to go back to the boot time default... > > Jason