RE: [PATCH v7 9/9] iommu/dma: Reserve any RMR regions associated with a dev

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 11 October 2021 16:01
> To: Jon Nettleton <jon@solid-run.com>
> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel <linux-arm-kernel@lists.infradead.org>; ACPI Devel Maling
> List <linux-acpi@vger.kernel.org>; Linux IOMMU
> <iommu@lists.linux-foundation.org>; Linuxarm <linuxarm@huawei.com>;
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Joerg Roedel
> <joro@8bytes.org>; Will Deacon <will@kernel.org>; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; Steven Price <steven.price@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>; Eric Auger <eric.auger@redhat.com>;
> yangyicong <yangyicong@huawei.com>
> 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@arm.com>
> 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