On Fri, Oct 04, 2024 at 05:17:46PM -0300, Jason Gunthorpe wrote: > On Fri, Oct 04, 2024 at 12:25:19PM -0700, Nicolin Chen wrote: > > > With that, I wonder what is better for the initial version of > > this structure, a generic virtual ID or a driver-named ID like > > "Stream ID"? The latter might be more understandable/flexible, > > so we won't need to justify a generic virtual ID along the way > > if something changes in the nature? > > I think the name could be a bit more specific "viommu_device_id" > maybe? And elaborate in the kdoc that this is about the identifier > that the iommu HW itself uses. A "vIOMMU Device ID" might sound a bit confusing with an ID of a vIOMMU itself :-/ At this moment, I named it "virt_id" in uAPI with a description: " * @virt_id: Virtual device ID per vIOMMU" I could add to that (or just in the documentation): "e.g. ARM's Stream ID, AMD's DeviceID, Intel's Context Table ID" to clarify it further. > > That sounds wider than what I defined it for in my patch: > > * struct iommu_vdevice_alloc - ioctl(IOMMU_VDEVICE_ALLOC) > > * ... > > * Allocate a virtual device instance (for a physical device) against a vIOMMU. > > * This instance holds the device's information in a VM, related to its vIOMMU. > > > > Would you please help rephrase it? It'd be also helpful for me > > to update the doc. > > I think that is still OK for the moment. > > > Though I feel slightly odd if we define it wider than "vIOMMU" > > since this is an iommufd header... > > The notion I have is that vIOMMU would expand to encompass not just > the physical hypervisor controled vIOMMU but also the vIOMMU > controlled by the trusted "lowervisor" in a pkvm/cc/whatever world. > > Alexey is working on vIOMMU support in CC which has the trusted world > do some of the trusted vIOMMU components. I'm hoping the other people > in this area will look at his design and make it fit nicely to > everyone. Oh, I didn't connect the dots that lowervisor must rely on the vIOMMU too -- I'd need to check the CC stuff in detail. In that case, having it in vIOMMU uAPI totally makes sense. Thanks Nicolin
