Re: [PATCH v7 1/9] iommu: Introduce a union to struct iommu_resv_region

Jon Nettleton
2021-10-09T06:58:31Z

On Fri, Oct 8, 2021 at 2:14 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-08-05 09:07, Shameer Kolothum wrote:
> > A union is introduced to struct iommu_resv_region to hold
> > any firmware specific data. This is in preparation to add
> > support for IORT RMR reserve regions and the union now holds
> > the RMR specific information.
> >
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> > include/linux/iommu.h | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 32d448050bf7..bd0e4641c569 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -114,6 +114,13 @@ enum iommu_resv_type {
> > IOMMU_RESV_SW_MSI,
> > };
> >
> > +struct iommu_iort_rmr_data {
> > +#define IOMMU_RMR_REMAP_PERMITTED (1 << 0)
> > + u32 flags;
> > + u32 sid; /* Stream Id associated with RMR entry */
> > + void *smmu; /* Associated IORT SMMU node pointer */
> > +};
>
> Do we really need to duplicate all this data? AFAICS we could just save
> the acpi_iort_rmr pointer in the iommu_resv_region (with a forward
> declaration here if necessary) and defer parsing its actual mappings
> until the point where we can directly consume the results.

From earlier discussions on this patchset, the original goal was also for
device-tree mechanisms to be able to hook into this code to support
similar RMR's and SMMU initialization, just not through the ACPI / IORT
path.

>
> Robin.
>
> > +
> > /**
> > * struct iommu_resv_region - descriptor for a reserved memory region
> > * @list: Linked list pointers
> > @@ -121,6 +128,7 @@ enum iommu_resv_type {
> > * @length: Length of the region in bytes
> > * @prot: IOMMU Protection flags (READ/WRITE/...)
> > * @type: Type of the reserved region
> > + * @rmr: ACPI IORT RMR specific data
> > */
> > struct iommu_resv_region {
> > struct list_head list;
> > @@ -128,6 +136,9 @@ struct iommu_resv_region {
> > size_t length;
> > int prot;
> > enum iommu_resv_type type;
> > + union {
> > + struct iommu_iort_rmr_data rmr;
> > + } fw_data;
> > };
> >
> > /**
> >