[v4,2/8] iommu/dma: Introduce generic helper to retrieve RMR info

Message ID 20210513134550.2117-3-shameerali.kolothum.thodi@huawei.com
State New
Headers show
Series
  • ACPI/IORT: Support for IORT RMR node
Related show

Commit Message

Shameerali Kolothum Thodi May 13, 2021, 1:45 p.m.
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 | 33 +++++++++++++++++++++++++++++++++
 include/linux/dma-iommu.h | 10 ++++++++++
 include/linux/iommu.h     | 19 +++++++++++++++++++
 3 files changed, 62 insertions(+)

-- 
2.17.1

Comments

Joerg Roedel May 18, 2021, 8:49 a.m. | #1
On Thu, May 13, 2021 at 02:45:44PM +0100, Shameer Kolothum wrote:
> +/**

> + * struct iommu_rmr - Reserved Memory Region details per IOMMU

> + * @list: Linked list pointers to hold RMR region info

> + * @base_address: base address of Reserved Memory Region

> + * @length: length of memory region

> + * @sid: associated stream id

> + * @flags: flags that apply to the RMR node

> + */

> +struct iommu_rmr {

> +	struct list_head	list;

> +	phys_addr_t		base_address;

> +	u64			length;

> +	u32			sid;

> +	u32			flags;

> +};

> +

> +/* RMR Remap permitted */

> +#define IOMMU_RMR_REMAP_PERMITTED	(1 << 0)

> +


This struct has lots of overlap with 'struct iommu_resv_region'. Any
reason the existing struct can't be used here?

Regards,

	Joerg
Shameerali Kolothum Thodi May 19, 2021, 9:30 a.m. | #2
> -----Original Message-----

> From: Joerg Roedel [mailto:joro@8bytes.org]

> Sent: 18 May 2021 09:50

> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>

> Cc: linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;

> iommu@lists.linux-foundation.org; Linuxarm <linuxarm@huawei.com>;

> lorenzo.pieralisi@arm.com; robin.murphy@arm.com; 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 v4 2/8] iommu/dma: Introduce generic helper to retrieve

> RMR info

> 

> On Thu, May 13, 2021 at 02:45:44PM +0100, Shameer Kolothum wrote:

> > +/**

> > + * struct iommu_rmr - Reserved Memory Region details per IOMMU

> > + * @list: Linked list pointers to hold RMR region info

> > + * @base_address: base address of Reserved Memory Region

> > + * @length: length of memory region

> > + * @sid: associated stream id

> > + * @flags: flags that apply to the RMR node

> > + */

> > +struct iommu_rmr {

> > +	struct list_head	list;

> > +	phys_addr_t		base_address;

> > +	u64			length;

> > +	u32			sid;

> > +	u32			flags;

> > +};

> > +

> > +/* RMR Remap permitted */

> > +#define IOMMU_RMR_REMAP_PERMITTED	(1 << 0)

> > +

> 

> This struct has lots of overlap with 'struct iommu_resv_region'. Any

> reason the existing struct can't be used here?

> 


Hmm..main reason is "sid". RMRs are associated with stream ids and
that is used to install bypass STEs/SMRs in SMMU drivers and also to check
whether a dev has any RMR regions associated with it.

I think we could add sid/dev_id to 'struct iommu_resv_region', and modify
iommu_alloc_resv_region() accordingly. That can get rid of the above struct
and iommu_dma_alloc_rmr() fn. Not sure this will complicate things as 
the dev_id is only valid for RMR reservation region cases. 

Please let me know your thoughts.

Thanks,
Shameer
Robin Murphy May 19, 2021, 11:48 a.m. | #3
On 2021-05-19 10:30, Shameerali Kolothum Thodi wrote:
> 

> 

>> -----Original Message-----

>> From: Joerg Roedel [mailto:joro@8bytes.org]

>> Sent: 18 May 2021 09:50

>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>

>> Cc: linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;

>> iommu@lists.linux-foundation.org; Linuxarm <linuxarm@huawei.com>;

>> lorenzo.pieralisi@arm.com; robin.murphy@arm.com; 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 v4 2/8] iommu/dma: Introduce generic helper to retrieve

>> RMR info

>>

>> On Thu, May 13, 2021 at 02:45:44PM +0100, Shameer Kolothum wrote:

>>> +/**

>>> + * struct iommu_rmr - Reserved Memory Region details per IOMMU

>>> + * @list: Linked list pointers to hold RMR region info

>>> + * @base_address: base address of Reserved Memory Region

>>> + * @length: length of memory region

>>> + * @sid: associated stream id

>>> + * @flags: flags that apply to the RMR node

>>> + */

>>> +struct iommu_rmr {

>>> +	struct list_head	list;

>>> +	phys_addr_t		base_address;

>>> +	u64			length;

>>> +	u32			sid;

>>> +	u32			flags;

>>> +};

>>> +

>>> +/* RMR Remap permitted */

>>> +#define IOMMU_RMR_REMAP_PERMITTED	(1 << 0)

>>> +

>>

>> This struct has lots of overlap with 'struct iommu_resv_region'. Any

>> reason the existing struct can't be used here?

>>

> 

> Hmm..main reason is "sid". RMRs are associated with stream ids and

> that is used to install bypass STEs/SMRs in SMMU drivers and also to check

> whether a dev has any RMR regions associated with it.

> 

> I think we could add sid/dev_id to 'struct iommu_resv_region', and modify

> iommu_alloc_resv_region() accordingly. That can get rid of the above struct

> and iommu_dma_alloc_rmr() fn. Not sure this will complicate things as

> the dev_id is only valid for RMR reservation region cases.

> 

> Please let me know your thoughts.


Maybe add a union for FW-specific data to struct resv_region, so that it 
could eventually subsume AMD's struct unity_map_entry and Intel's struct 
dmar_rmrr_unit as well? They're essentially doing the same dance.

We might still have to create copies of the firmware-allocated entries 
to actually assign to domains (certainly where one entry covers multiple 
devices), but kmemdup() is still a lot neater than various translations 
from private formats.

Robin.

Patch

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7bcdd1205535..674bd8815159 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -193,6 +193,39 @@  void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 
+/**
+ * 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);
+
+struct iommu_rmr *iommu_dma_alloc_rmr(u64 base, u64 length, u32 sid,
+				      u32 flags)
+{
+	struct iommu_rmr *rmr;
+
+	rmr = kzalloc(sizeof(*rmr), GFP_KERNEL);
+	if (!rmr)
+		return NULL;
+
+	INIT_LIST_HEAD(&rmr->list);
+	rmr->base_address = base;
+	rmr->length = length;
+	rmr->sid = sid;
+	rmr->flags = flags;
+
+	return rmr;
+}
+EXPORT_SYMBOL(iommu_dma_alloc_rmr);
+
 static int cookie_init_hw_msi_region(struct iommu_dma_cookie *cookie,
 		phys_addr_t start, phys_addr_t end)
 {
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 6e75a2d689b4..319f332c279f 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -42,12 +42,17 @@  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);
+struct iommu_rmr *iommu_dma_alloc_rmr(u64 base, u64 length, u32 sid, u32 flags);
+
 #else /* CONFIG_IOMMU_DMA */
 
 struct iommu_domain;
 struct msi_desc;
 struct msi_msg;
 struct device;
+struct fwnode_handle;
+struct iommu_rmr;
 
 static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
 		u64 size)
@@ -83,5 +88,10 @@  static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
 {
 }
 
+int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+	return -ENODEV;
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __DMA_IOMMU_H */
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..73cd2831cb45 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -555,6 +555,25 @@  struct iommu_sva {
 	struct device			*dev;
 };
 
+/**
+ * struct iommu_rmr - Reserved Memory Region details per IOMMU
+ * @list: Linked list pointers to hold RMR region info
+ * @base_address: base address of Reserved Memory Region
+ * @length: length of memory region
+ * @sid: associated stream id
+ * @flags: flags that apply to the RMR node
+ */
+struct iommu_rmr {
+	struct list_head	list;
+	phys_addr_t		base_address;
+	u64			length;
+	u32			sid;
+	u32			flags;
+};
+
+/* RMR Remap permitted */
+#define IOMMU_RMR_REMAP_PERMITTED	(1 << 0)
+
 int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
 		      const struct iommu_ops *ops);
 void iommu_fwspec_free(struct device *dev);