On Mon, Aug 19, 2024 at 02:33:32PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 15, 2024 at 05:21:57PM -0700, Nicolin Chen wrote: > > > > Why not? The idev becomes linked to the viommu when the dev id is set > > > > > Unless we are also going to enforce the idev is always attached to a > > > nested then I don't think we need to check it here. > > > > > > Things will definately not entirely work as expected if the vdev is > > > directly attached to the s2 or a blocking, but it won't harm anything. > > > > My view is that, the moment there is a VIOMMU object, that must > > be a nested IOMMU case, so there must be a nested hwpt. Blocking > > domain would be a hwpt_nested too (vSTE=Abort) as we previously > > concluded. > > I'm not sure other vendors can do that vSTE=Abort/Bypass thing though > yet.. > > > Then, in a nested case, it feels odd that an idev is attached to > > an S2 hwpt.. > > > > That being said, I think we can still do that with validations: > > If idev->hwpt is nested, compare input viommu v.s idev->hwpt->viommu. > > If idev->hwpt is paging, compare input viommu->hwpt v.s idev->hwpt. > > But again, if you don't contiguously validate those invariants in all > the other attach paths it is sort of pointless to check them since the > userspace can still violate things. Hmm, would that be unsafe? I start to wonder if we should allow an attach to viommu and put validations on that? > > This complicates things overall especially with the VIRQ that has > > involved interrupt context polling vdev_id, where semaphore/mutex > > won't fit very well. Perhaps it would need a driver-level bottom > > half routine to call those helpers with locks. I am glad that you > > noticed the problem early. > > I think you have to show the xarray to the driver and the driver can > use the spinlock to access it safely. Keeping it hidden in the core > code is causing all these locking problems. Yea, I just figured that out... You have been right. I was able to get rid of the locking problem with invalidation API. But then irq became a headache as drivers would only know the dev pointer, so everything that the dev could convert to would be unsafe as it can not grab the idev/viommu locks until it converts. Thanks Nicolin
