Hi guys, On Wed, Nov 06, 2024 at 10:35:06PM -0400, Jason Gunthorpe wrote: > On Wed, Nov 06, 2024 at 09:05:26PM +0000, Robin Murphy wrote: > > You should take "We discussed this already" > > as more of a clue to yourself than to me - if 4 different people have all > > said the exact same thing in so many words, perhaps there's something in > > it... > > And all seemed to agree it was not a big deal after the discussion. > > I think Mostafa was driving in a direction that we break up the IDR > into explicit fields and thus be explicit about what information the > VMM is able to access. This would effectively document and enforce > what the baseline is. As one of the four people mentioned above, I figured I'd chime in with my rationale for queuing this in case it's of any help or interest. Initially, I was reasonably sure that we should be sanitising the ID registers and being selective about what we advertise to userspace. However, after Jason's reply to my comments, mulling it over in my head and having lively conversations with Mostafa at lunchtime, I've come full circle. Is it a great interface? Not at all. It's 8 register values copied from the hardware to userspace. But is it good enough? I think it is. It's also extremely simple (i.e. easy to explain what it does and trivial to implement), which I think is a huge benefit given that the IOMMUFD work around it is still evolving. I'm firmly of the opinion that the VMM is going to need a tonne of help from other sources to expose a virtual IOMMU successfully. For example, anything relating to policy or configuration choices should be driven from userspace rather than the kernel. If we start to expose policy in the id registers for the cases where it happens to fit nicely, I fear that this will backfire and the VMM will end up second-guessing the kernel in cases where it decides it knows best. I'm not sold on the analogy with CPU ID registers as (a) we don't have big/little idiocy to deal with in the SMMU and (b) I don't think SMMU features always need support code in the host, which is quite unlike many CPU features that cannot be exposed safely without hypervisor context-switching support. On top of all that, this interface can be extended if we change our minds. If we decide we need to mask out fields, I think we could add that after the fact. Hell, if we all decide that it's a disaster in a few releases time, we can try something else. Ultimately, Jason is the one maintaining IOMMUFD and he gets to deal with the problems if his UAPI doesn't work out :) So, given where we are in the cycle, I think the pragmatic thing to do is to land this change now and enable the ongoing IOMMUFD work to continue. We still have over two months to resolve any major problems with the interface (and even unplug it entirely from the driver if we really get stuck) but for now I think it's "fine". Will
