On 2021-05-19 10:30, Shameerali Kolothum Thodi wrote:


-----Original Message-----
From: Joerg Roedel [mailto:j...@8bytes.org]
Sent: 18 May 2021 09:50
To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>
Cc: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
iommu@lists.linux-foundation.org; Linuxarm <linux...@huawei.com>;
lorenzo.pieral...@arm.com; robin.mur...@arm.com; wanghuiqiang
<wanghuiqi...@huawei.com>; Guohanjun (Hanjun Guo)
<guohan...@huawei.com>; steven.pr...@arm.com; sami.muja...@arm.com;
j...@solid-run.com; eric.au...@redhat.com; yangyicong
<yangyic...@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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to