| Message ID | 20220422162907.1276-2-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
On 2022/4/23 00:28, Shameer Kolothum via iommu wrote: > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index f2c45b85b9fc..ffcfa684e80c 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -2597,16 +2597,22 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list) > * @list: reserved region list for device > * > * IOMMU drivers can use this to implement their .put_resv_regions() callback > - * for simple reservations. Memory allocated for each reserved region will be > - * freed. If an IOMMU driver allocates additional resources per region, it is > - * going to have to implement a custom callback. > + * for simple reservations. If a per region callback is provided that will be > + * used to free all memory allocations associated with the reserved region or > + * else just free up the memory for the regions. If an IOMMU driver allocates > + * additional resources per region, it is going to have to implement a custom > + * callback. > */ > void generic_iommu_put_resv_regions(struct device *dev, struct list_head *list) > { > struct iommu_resv_region *entry, *next; > > - list_for_each_entry_safe(entry, next, list, list) > - kfree(entry); > + list_for_each_entry_safe(entry, next, list, list) { > + if (entry->free) > + entry->free(dev, entry); > + else > + kfree(entry); > + } > } > EXPORT_SYMBOL(generic_iommu_put_resv_regions); The generic_iommu_put_resv_regions() itself is a callback. Why bothering adding another callback from the same iommu driver in it? Or, you are going to remove the put_resv_regions from the iommu ops? Best regards, baolu
On Sat, Apr 23, 2022 at 10:04:39AM +0800, Lu Baolu wrote: > The generic_iommu_put_resv_regions() itself is a callback. Why bothering > adding another callback from the same iommu driver in it? Or, you are > going to remove the put_resv_regions from the iommu ops? It is a driver method, but these reserved entries are not actually allocated by the driver. And I do have a patch pending removing this driver method that should never have been a driver method, check the iomm list archives for iommu: remove the put_resv_regions method
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 2022/4/23 13:14, Christoph Hellwig wrote: > On Sat, Apr 23, 2022 at 10:04:39AM +0800, Lu Baolu wrote: >> The generic_iommu_put_resv_regions() itself is a callback. Why bothering >> adding another callback from the same iommu driver in it? Or, you are >> going to remove the put_resv_regions from the iommu ops? > > It is a driver method, but these reserved entries are not actually > allocated by the driver. And I do have a patch pending removing this > driver method that should never have been a driver method, check > the iomm list archives for > > iommu: remove the put_resv_regions method > Yeah! That's a good idea. Best regards, baolu
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f2c45b85b9fc..ffcfa684e80c 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2597,16 +2597,22 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list) * @list: reserved region list for device * * IOMMU drivers can use this to implement their .put_resv_regions() callback - * for simple reservations. Memory allocated for each reserved region will be - * freed. If an IOMMU driver allocates additional resources per region, it is - * going to have to implement a custom callback. + * for simple reservations. If a per region callback is provided that will be + * used to free all memory allocations associated with the reserved region or + * else just free up the memory for the regions. If an IOMMU driver allocates + * additional resources per region, it is going to have to implement a custom + * callback. */ void generic_iommu_put_resv_regions(struct device *dev, struct list_head *list) { struct iommu_resv_region *entry, *next; - list_for_each_entry_safe(entry, next, list, list) - kfree(entry); + list_for_each_entry_safe(entry, next, list, list) { + if (entry->free) + entry->free(dev, entry); + else + kfree(entry); + } } EXPORT_SYMBOL(generic_iommu_put_resv_regions); diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 9208eca4b0d1..68bcfb3a06d7 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -134,6 +134,7 @@ enum iommu_resv_type { * @length: Length of the region in bytes * @prot: IOMMU Protection flags (READ/WRITE/...) * @type: Type of the reserved region + * @free: Callback to free associated memory allocations */ struct iommu_resv_region { struct list_head list; @@ -141,6 +142,7 @@ struct iommu_resv_region { size_t length; int prot; enum iommu_resv_type type; + void (*free)(struct device *dev, struct iommu_resv_region *region); }; /**
A callback is introduced to struct iommu_resv_region to free memory allocations associated with the reserved region. This will be useful when we introduce support for IORT RMR based reserved regions. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- drivers/iommu/iommu.c | 16 +++++++++++----- include/linux/iommu.h | 2 ++ 2 files changed, 13 insertions(+), 5 deletions(-)