diff mbox series

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

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

Commit Message

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

Comments

Joerg Roedel May 18, 2021, 8:49 a.m. UTC | #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. UTC | #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. UTC | #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.
diff mbox series

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);