On Wed, Sep 04, 2024 at 01:26:21PM -0300, Jason Gunthorpe wrote:
On Sun, Sep 01, 2024 at 10:27:09PM -0700, Nicolin Chen wrote:
On Sun, Sep 01, 2024 at 10:39:17AM +0800, Baolu Lu wrote:
On 2024/8/28 0:59, Nicolin Chen wrote:
+int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) +{
struct iommu_viommu_alloc *cmd = ucmd->cmd;struct iommufd_hwpt_paging *hwpt_paging;struct iommufd_viommu *viommu;struct iommufd_device *idev;int rc;if (cmd->flags)return -EOPNOTSUPP;idev = iommufd_get_device(ucmd, cmd->dev_id);Why does a device reference count is needed here? When is this reference count released after the VIOMMU is allocated?
Hmm, it was used to get dev->iommu->iommu_dev to pin the VIOMMU to a physical IOMMU instance (in v1). Jason suggested to remove that, yet I didn't realize that this idev is now completely useless.
With that being said, a parent HWPT could be shared across VIOMUs allocated for the same VM. So, I think we do need a dev pointer to know which physical instance the VIOMMU allocates for, especially for a driver-managed VIOMMU.
Eventually you need a way to pin the physical iommu, without pinning any idevs. Not sure how best to do that
Just trying to clarify "without pinning any idevs", does it mean we shouldn't pass in an idev_id to get dev->iommu->iommu_dev?
Otherwise, iommu_probe_device_lock and iommu_device_lock in the iommu.c are good enough to lock dev->iommu and iommu->list. And I think we just need an iommu helper refcounting the dev_iommu (or iommu_device) as we previously discussed.
Thanks Nicolin