> -----Original Message----- > From: Christoph Hellwig [mailto:hch@xxxxxxxxxxxxx] > Sent: 07 April 2022 15:00 > To: Robin Murphy <robin.murphy@xxxxxxx> > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@xxxxxxxxxx>; jon@xxxxxxxxxxxxx; Linuxarm > <linuxarm@xxxxxxxxxx>; steven.price@xxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; wanghuiqiang > <wanghuiqiang@xxxxxxxxxx>; Guohanjun (Hanjun Guo) > <guohanjun@xxxxxxxxxx>; yangyicong <yangyicong@xxxxxxxxxx>; > Sami.Mujawar@xxxxxxx; will@xxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR > reserved regions > > On Thu, Apr 07, 2022 at 02:53:38PM +0100, Robin Murphy wrote: > > > Why can't this just go into generic_iommu_put_resv_regions? The idea > > > that the iommu low-level drivers need to call into dma-iommu which is > > > a consumer of the IOMMU API is odd. Especially if that just calls out > > > to ACPI code and generic IOMMU code only anyway. > > > > Because assuming ACPI means IORT is not generic. Part of the aim in adding > > the union to iommu_resv_region is that stuff like AMD's unity_map_entry > and > > Intel's dmar_rmrr_unit can be folded into it as well, and their reserved > > region handling correspondingly simplified too. > > > > The iommu_dma_{get,put}_resv_region() helpers are kind of intended to be > > specific to the fwnode mechanism which deals with IORT and devicetree > (once > > the reserved region bindings are fully worked out). > > But IORT is not driver₋specific code. So we'll need a USE_IORT flag > somewhere in core IOMMU code instead of trying to stuff this into > driver operations. and dma-iommu mostly certainly implies IORT even > less than ACPI. Sorry for the delayed response(was on holidays). So if we move the iommu_dma_put_resv_region() call to generic_iommu_put_resv_regions() , will that address the concerns here? I think it will resolve the issue in 05/11 as well pointed out by Christoph where we end up not releasing reserved regions when CONFIG_IOMMU_DMA is not set. Something like below, diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f2c45b85b9fc..d08204a25ba8 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2605,6 +2605,8 @@ void generic_iommu_put_resv_regions(struct device *dev, struct list_head *list) { struct iommu_resv_region *entry, *next; + iommu_dma_put_resv_regions(dev, list); + list_for_each_entry_safe(entry, next, list, list) kfree(entry); } diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 5811233dc9fb..8cb1e419db49 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -393,8 +393,6 @@ void iommu_dma_put_resv_regions(struct device *dev, struct list_head *list) { if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) iort_iommu_put_resv_regions(dev, list); - - generic_iommu_put_resv_regions(dev, list); } Please let me know if this is not good enough. Thanks, Shameer
