diff mbox series

[v7,3/9] iommu/dma: Introduce generic helper to retrieve RMR info

Message ID 20210805080724.480-4-shameerali.kolothum.thodi@huawei.com (mailing list archive)
State Not Applicable, archived
Headers show
Series ACPI/IORT: Support for IORT RMR node | expand

Commit Message

Shameerali Kolothum Thodi Aug. 5, 2021, 8:07 a.m. UTC
Reserved Memory Regions(RMR) associated with an IOMMU can be
described through ACPI IORT tables in systems with devices
that require a unity mapping or bypass for those
regions.

Introduce a generic interface so that IOMMU drivers can retrieve
and set up necessary mappings.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/dma-iommu.c | 29 +++++++++++++++++++++++++++++
 include/linux/dma-iommu.h | 13 +++++++++++++
 2 files changed, 42 insertions(+)

Comments

Robin Murphy Oct. 8, 2021, 1:03 p.m. UTC | #1
On 2021-08-05 09:07, Shameer Kolothum wrote:
> Reserved Memory Regions(RMR) associated with an IOMMU can be
> described through ACPI IORT tables in systems with devices
> that require a unity mapping or bypass for those
> regions.
> 
> Introduce a generic interface so that IOMMU drivers can retrieve
> and set up necessary mappings.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>   drivers/iommu/dma-iommu.c | 29 +++++++++++++++++++++++++++++
>   include/linux/dma-iommu.h | 13 +++++++++++++
>   2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 98ba927aee1a..2fa2445e9070 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -174,6 +174,35 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
>   }
>   EXPORT_SYMBOL(iommu_put_dma_cookie);
>   
> +/**
> + *
> + * iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
> + *                      with a given IOMMU
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @list: RMR list to be populated
> + *
> + */
> +int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode,
> +		       struct list_head *list)
> +{
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(iommu_dma_get_rmrs);
> +
> +/**
> + *
> + * iommu_dma_put_rmrs - Release Reserved Memory Regions(RMRs) associated
> + *                      with a given IOMMU
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @list: RMR list
> + *
> + */
> +void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
> +			struct list_head *list)
> +{
> +}
> +EXPORT_SYMBOL(iommu_dma_put_rmrs);

Unless there's something special you expect to need to do here, can we 
just uphold the prevailing expectation that resv_regions are kmalloc()ed 
and can be freed directly by the generic function?

> +
>   /**
>    * iommu_dma_get_resv_regions - Reserved region driver helper
>    * @dev: Device from iommu_get_resv_regions()
> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 758ca4694257..3b7b2d096c6e 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -42,12 +42,16 @@ void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
>   
>   extern bool iommu_dma_forcedac;
>   
> +int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list);
> +void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list);
> +
>   #else /* CONFIG_IOMMU_DMA */
>   
>   struct iommu_domain;
>   struct msi_desc;
>   struct msi_msg;
>   struct device;
> +struct fwnode_handle;
>   
>   static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
>   				       u64 dma_limit)
> @@ -83,5 +87,14 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
>   {
>   }
>   
> +static int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list)
> +{
> +	return -ENODEV;

Hmm, if this needs to be stubbed at all then returning an error seems 
like probably the wrong thing to do. I guess it's for 32-bit builds of 
arm-smmu? It is not an error if the firmware describes no RMRs because 
there are no RMRs, so it hardly deserves to be an error if there are no 
RMRs simply because the firmware isn't ACPI.

Robin.

> +}
> +
> +static void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list)
> +{
> +}
> +
>   #endif	/* CONFIG_IOMMU_DMA */
>   #endif	/* __DMA_IOMMU_H */
>
Shameerali Kolothum Thodi Oct. 11, 2021, 5:51 a.m. UTC | #2
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 08 October 2021 14:04
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; eric.auger@redhat.com; yangyicong
> <yangyicong@huawei.com>
> Subject: Re: [PATCH v7 3/9] iommu/dma: Introduce generic helper to retrieve
> RMR info
> 
> On 2021-08-05 09:07, Shameer Kolothum wrote:
> > Reserved Memory Regions(RMR) associated with an IOMMU can be
> > described through ACPI IORT tables in systems with devices
> > that require a unity mapping or bypass for those
> > regions.
> >
> > Introduce a generic interface so that IOMMU drivers can retrieve
> > and set up necessary mappings.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >   drivers/iommu/dma-iommu.c | 29 +++++++++++++++++++++++++++++
> >   include/linux/dma-iommu.h | 13 +++++++++++++
> >   2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 98ba927aee1a..2fa2445e9070 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -174,6 +174,35 @@ void iommu_put_dma_cookie(struct
> iommu_domain *domain)
> >   }
> >   EXPORT_SYMBOL(iommu_put_dma_cookie);
> >
> > +/**
> > + *
> > + * iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs)
> associated
> > + *                      with a given IOMMU
> > + * @iommu_fwnode: fwnode associated with IOMMU
> > + * @list: RMR list to be populated
> > + *
> > + */
> > +int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode,
> > +		       struct list_head *list)
> > +{
> > +	return -EINVAL;
> > +}
> > +EXPORT_SYMBOL(iommu_dma_get_rmrs);
> > +
> > +/**
> > + *
> > + * iommu_dma_put_rmrs - Release Reserved Memory Regions(RMRs)
> associated
> > + *                      with a given IOMMU
> > + * @iommu_fwnode: fwnode associated with IOMMU
> > + * @list: RMR list
> > + *
> > + */
> > +void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
> > +			struct list_head *list)
> > +{
> > +}
> > +EXPORT_SYMBOL(iommu_dma_put_rmrs);
> 
> Unless there's something special you expect to need to do here, can we
> just uphold the prevailing expectation that resv_regions are kmalloc()ed
> and can be freed directly by the generic function?

Right. I think we can do that. 

> 
> > +
> >   /**
> >    * iommu_dma_get_resv_regions - Reserved region driver helper
> >    * @dev: Device from iommu_get_resv_regions()
> > diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> > index 758ca4694257..3b7b2d096c6e 100644
> > --- a/include/linux/dma-iommu.h
> > +++ b/include/linux/dma-iommu.h
> > @@ -42,12 +42,16 @@ void iommu_dma_free_cpu_cached_iovas(unsigned
> int cpu,
> >
> >   extern bool iommu_dma_forcedac;
> >
> > +int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head
> *list);
> > +void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head
> *list);
> > +
> >   #else /* CONFIG_IOMMU_DMA */
> >
> >   struct iommu_domain;
> >   struct msi_desc;
> >   struct msi_msg;
> >   struct device;
> > +struct fwnode_handle;
> >
> >   static inline void iommu_setup_dma_ops(struct device *dev, u64
> dma_base,
> >   				       u64 dma_limit)
> > @@ -83,5 +87,14 @@ static inline void
> iommu_dma_get_resv_regions(struct device *dev, struct list_he
> >   {
> >   }
> >
> > +static int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct
> list_head *list)
> > +{
> > +	return -ENODEV;
> 
> Hmm, if this needs to be stubbed at all then returning an error seems
> like probably the wrong thing to do. I guess it's for 32-bit builds of
> arm-smmu? It is not an error if the firmware describes no RMRs because
> there are no RMRs, so it hardly deserves to be an error if there are no
> RMRs simply because the firmware isn't ACPI.

Yes, definitely not an error return. I will change that.

Thanks,
Shameer

> 
> Robin.
> 
> > +}
> > +
> > +static void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct
> list_head *list)
> > +{
> > +}
> > +
> >   #endif	/* CONFIG_IOMMU_DMA */
> >   #endif	/* __DMA_IOMMU_H */
> >
diff mbox series

Patch

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..2fa2445e9070 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -174,6 +174,35 @@  void iommu_put_dma_cookie(struct iommu_domain *domain)
 }
 EXPORT_SYMBOL(iommu_put_dma_cookie);
 
+/**
+ *
+ * iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @list: RMR list to be populated
+ *
+ */
+int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode,
+		       struct list_head *list)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(iommu_dma_get_rmrs);
+
+/**
+ *
+ * iommu_dma_put_rmrs - Release Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @list: RMR list
+ *
+ */
+void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
+			struct list_head *list)
+{
+}
+EXPORT_SYMBOL(iommu_dma_put_rmrs);
+
 /**
  * iommu_dma_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 758ca4694257..3b7b2d096c6e 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -42,12 +42,16 @@  void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
 
 extern bool iommu_dma_forcedac;
 
+int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list);
+void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list);
+
 #else /* CONFIG_IOMMU_DMA */
 
 struct iommu_domain;
 struct msi_desc;
 struct msi_msg;
 struct device;
+struct fwnode_handle;
 
 static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
 				       u64 dma_limit)
@@ -83,5 +87,14 @@  static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
 {
 }
 
+static int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+	return -ENODEV;
+}
+
+static void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __DMA_IOMMU_H */