> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > Sent: 11 October 2021 16:01 > To: Jon Nettleton <jon@xxxxxxxxxxxxx> > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; > linux-arm-kernel <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; ACPI Devel Maling > List <linux-acpi@xxxxxxxxxxxxxxx>; Linux IOMMU > <iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx>; Linuxarm <linuxarm@xxxxxxxxxx>; > Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>; Joerg Roedel > <joro@xxxxxxxxxx>; Will Deacon <will@xxxxxxxxxx>; wanghuiqiang > <wanghuiqiang@xxxxxxxxxx>; Guohanjun (Hanjun Guo) > <guohanjun@xxxxxxxxxx>; Steven Price <steven.price@xxxxxxx>; Sami > Mujawar <Sami.Mujawar@xxxxxxx>; Eric Auger <eric.auger@xxxxxxxxxx>; > yangyicong <yangyicong@xxxxxxxxxx> > Subject: Re: [PATCH v7 9/9] iommu/dma: Reserve any RMR regions associated > with a dev > > On 2021-10-09 08:07, Jon Nettleton wrote: > > On Fri, Oct 8, 2021 at 3:10 PM Robin Murphy <robin.murphy@xxxxxxx> > wrote: > >> > >> On 2021-08-05 09:07, Shameer Kolothum wrote: > >>> Get ACPI IORT RMR regions associated with a dev reserved > >>> so that there is a unity mapping for them in SMMU. > >> > >> This feels like most of it belongs in the IORT code rather than > >> iommu-dma (which should save the temporary list copy as well). > > > > See previous comment. The original intent was for device-tree to also > > be able to use these mechanisms to create RMR's and support them > > in the SMMU. > > Can you clarify how code behind an "if (!is_of_node(...))" check > alongside other IORT-specific code is expected to be useful for DT? > > Yes, iommu_dma_get_resv_regions() itself wants to end up serving as an > abstraction layer, but that still doesn't mean it has to do much more > than dispatch into firmware-specific backends as appropriate. (Resending as I accidently replied earlier from our internal ML id. Sorry) The way I thought about is as below, 1. iommu_dma_get_resv_regions() will invoke the common iommu_dma_get_rmr_resv_regions(). Yes, the if (!is_of_node(...)) is not required here. 2. iommu_dma_get_rmr_resv_regions() calls iommu_dma_get_rmrs(). iommu_dma_get_rmrs() has the (!is_of_node(...)) check to call into IORT or DT specific functions to retrieve the RMR reserve regions associated with a given iommu_fwnode. 3. The common iommu_dma_get_rmr_resv_regions() further checks for PCI host preserve_config and whether the returned RMR list actually has any dev specific region to reserve or not. So the only firmware specific backend is handled inside the iommu_dma_get_rmrs() and that is also called from the SMMU driver probe to install bypass SIDs. Anyway, if the eventual DT implementation or further IORT spec changes makes this abstraction irrelevant I am Ok to move this into the IORT code. Thanks, Shameer
