| Message ID | 20240412081516.31168-6-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommufd support pasid attach/replace | expand |
On 4/12/24 4:15 PM, Yi Liu wrote: > Today, the iommu layer gets the max_pasids by pci_max_pasids() or reading > the "pasid-num-bits" property. This requires the non-PCI devices to have a > "pasid-num-bits" property. Like the mock device used in iommufd selftest, > otherwise the max_pasids check would fail in iommu layer. > > While there is an alternative, the iommu layer can allow the iommu driver > to set the max_pasids in its probe_device() callback and populate it. This > is simpler and has no impact on the existing cases. > > Suggested-by: Jason Gunthorpe<jgg@nvidia.com> > Signed-off-by: Yi Liu<yi.l.liu@intel.com> > --- > drivers/iommu/iommu.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) The code does not appear to match the commit message here. The code in change is a refactoring by folding the max_pasid assignment into its helper. However, the commit message suggests a desire to expose some kind of kAPI for device drivers. Best regards, baolu
> From: Baolu Lu <baolu.lu@linux.intel.com> > Sent: Monday, April 15, 2024 1:42 PM > > On 4/12/24 4:15 PM, Yi Liu wrote: > > Today, the iommu layer gets the max_pasids by pci_max_pasids() or > reading > > the "pasid-num-bits" property. This requires the non-PCI devices to have a > > "pasid-num-bits" property. Like the mock device used in iommufd selftest, > > otherwise the max_pasids check would fail in iommu layer. > > > > While there is an alternative, the iommu layer can allow the iommu driver > > to set the max_pasids in its probe_device() callback and populate it. This > > is simpler and has no impact on the existing cases. > > > > Suggested-by: Jason Gunthorpe<jgg@nvidia.com> > > Signed-off-by: Yi Liu<yi.l.liu@intel.com> > > --- > > drivers/iommu/iommu.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > The code does not appear to match the commit message here. > > The code in change is a refactoring by folding the max_pasid assignment > into its helper. However, the commit message suggests a desire to expose > some kind of kAPI for device drivers. > it's not about exposing a new kAPI. Instead it allows the driver to manually set dev->iommu->max_pasids before calling iommu_init_device(). kind of another contract to convey the max pasid. But as how you are confused, I prefer to defining a "pasid-num-bits" property in the mock driver. It's easier to understand.
On 2024/4/17 16:49, Tian, Kevin wrote: >> From: Baolu Lu <baolu.lu@linux.intel.com> >> Sent: Monday, April 15, 2024 1:42 PM >> >> On 4/12/24 4:15 PM, Yi Liu wrote: >>> Today, the iommu layer gets the max_pasids by pci_max_pasids() or >> reading >>> the "pasid-num-bits" property. This requires the non-PCI devices to have a >>> "pasid-num-bits" property. Like the mock device used in iommufd selftest, >>> otherwise the max_pasids check would fail in iommu layer. >>> >>> While there is an alternative, the iommu layer can allow the iommu driver >>> to set the max_pasids in its probe_device() callback and populate it. This >>> is simpler and has no impact on the existing cases. >>> >>> Suggested-by: Jason Gunthorpe<jgg@nvidia.com> >>> Signed-off-by: Yi Liu<yi.l.liu@intel.com> >>> --- >>> drivers/iommu/iommu.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> The code does not appear to match the commit message here. >> >> The code in change is a refactoring by folding the max_pasid assignment >> into its helper. However, the commit message suggests a desire to expose >> some kind of kAPI for device drivers. >> > > it's not about exposing a new kAPI. Instead it allows the driver to > manually set dev->iommu->max_pasids before calling > iommu_init_device(). kind of another contract to convey the > max pasid. > > But as how you are confused, I prefer to defining a "pasid-num-bits" > property in the mock driver. It's easier to understand. @Jason, how about your thought? :) Adding a "pasid-num-bits" may be more straightforward.
On Sat, Apr 20, 2024 at 01:45:57PM +0800, Yi Liu wrote: > On 2024/4/17 16:49, Tian, Kevin wrote: > > > From: Baolu Lu <baolu.lu@linux.intel.com> > > > Sent: Monday, April 15, 2024 1:42 PM > > > > > > On 4/12/24 4:15 PM, Yi Liu wrote: > > > > Today, the iommu layer gets the max_pasids by pci_max_pasids() or > > > reading > > > > the "pasid-num-bits" property. This requires the non-PCI devices to have a > > > > "pasid-num-bits" property. Like the mock device used in iommufd selftest, > > > > otherwise the max_pasids check would fail in iommu layer. > > > > > > > > While there is an alternative, the iommu layer can allow the iommu driver > > > > to set the max_pasids in its probe_device() callback and populate it. This > > > > is simpler and has no impact on the existing cases. > > > > > > > > Suggested-by: Jason Gunthorpe<jgg@nvidia.com> > > > > Signed-off-by: Yi Liu<yi.l.liu@intel.com> > > > > --- > > > > drivers/iommu/iommu.c | 9 +++++---- > > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > The code does not appear to match the commit message here. > > > > > > The code in change is a refactoring by folding the max_pasid assignment > > > into its helper. However, the commit message suggests a desire to expose > > > some kind of kAPI for device drivers. > > > > > > > it's not about exposing a new kAPI. Instead it allows the driver to > > manually set dev->iommu->max_pasids before calling > > iommu_init_device(). kind of another contract to convey the > > max pasid. > > > > But as how you are confused, I prefer to defining a "pasid-num-bits" > > property in the mock driver. It's easier to understand. > > @Jason, how about your thought? :) Adding a "pasid-num-bits" may be > more straightforward. Sure, just make sure it doesn't leak memory... Jason
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 343683e646e0..dc85c251237f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -368,9 +368,9 @@ static bool dev_has_iommu(struct device *dev) return dev->iommu && dev->iommu->iommu_dev; } -static u32 dev_iommu_get_max_pasids(struct device *dev) +static void dev_iommu_set_max_pasids(struct device *dev) { - u32 max_pasids = 0, bits = 0; + u32 max_pasids = dev->iommu->max_pasids, bits = 0; int ret; if (dev_is_pci(dev)) { @@ -383,7 +383,8 @@ static u32 dev_iommu_get_max_pasids(struct device *dev) max_pasids = 1UL << bits; } - return min_t(u32, max_pasids, dev->iommu->iommu_dev->max_pasids); + dev->iommu->max_pasids = min_t(u32, max_pasids, + dev->iommu->iommu_dev->max_pasids); } void dev_iommu_priv_set(struct device *dev, void *priv) @@ -433,7 +434,7 @@ static int iommu_init_device(struct device *dev, const struct iommu_ops *ops) } dev->iommu_group = group; - dev->iommu->max_pasids = dev_iommu_get_max_pasids(dev); + dev_iommu_set_max_pasids(dev); if (ops->is_attach_deferred) dev->iommu->attach_deferred = ops->is_attach_deferred(dev); return 0;
Today, the iommu layer gets the max_pasids by pci_max_pasids() or reading the "pasid-num-bits" property. This requires the non-PCI devices to have a "pasid-num-bits" property. Like the mock device used in iommufd selftest, otherwise the max_pasids check would fail in iommu layer. While there is an alternative, the iommu layer can allow the iommu driver to set the max_pasids in its probe_device() callback and populate it. This is simpler and has no impact on the existing cases. Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/iommu/iommu.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)