On 5/26/2021 7:36 PM, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: Laurentiu Tudor [mailto:laurentiu.tudor@xxxxxxx] >> Sent: 26 May 2021 08:53 >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx>; >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; >> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx >> Cc: jon@xxxxxxxxxxxxx; Linuxarm <linuxarm@xxxxxxxxxx>; >> steven.price@xxxxxxx; Guohanjun (Hanjun Guo) <guohanjun@xxxxxxxxxx>; >> yangyicong <yangyicong@xxxxxxxxxx>; Sami.Mujawar@xxxxxxx; >> robin.murphy@xxxxxxx; wanghuiqiang <wanghuiqiang@xxxxxxxxxx> >> Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory >> regions >> >> Hi Shameer, >> >> On 5/24/2021 2:02 PM, Shameer Kolothum wrote: >>> Add a helper function that retrieves RMR memory descriptors >>> associated with a given IOMMU. This will be used by IOMMU >>> drivers to setup necessary mappings. >>> >>> Now that we have this, invoke it from the generic helper >>> interface. >>> >>> Signed-off-by: Shameer Kolothum >> <shameerali.kolothum.thodi@xxxxxxxxxx> >>> --- >>> drivers/acpi/arm64/iort.c | 50 >> +++++++++++++++++++++++++++++++++++++++ >>> drivers/iommu/dma-iommu.c | 4 ++++ >>> include/linux/acpi_iort.h | 7 ++++++ >>> 3 files changed, 61 insertions(+) >>> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>> index fea1ffaedf3b..01917caf58de 100644 >>> --- a/drivers/acpi/arm64/iort.c >>> +++ b/drivers/acpi/arm64/iort.c >>> @@ -12,6 +12,7 @@ >>> >>> #include <linux/acpi_iort.h> >>> #include <linux/bitfield.h> >>> +#include <linux/dma-iommu.h> >>> #include <linux/iommu.h> >>> #include <linux/kernel.h> >>> #include <linux/list.h> >>> @@ -837,6 +838,53 @@ static inline int iort_add_device_replay(struct >> device *dev) >>> return err; >>> } >>> >>> +/** >>> + * iort_iommu_get_rmrs - Helper to retrieve RMR info associated with >> IOMMU >>> + * @iommu: fwnode for the IOMMU >>> + * @head: RMR list head to be populated >>> + * >>> + * Returns: 0 on success, <0 failure >>> + */ >>> +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, >>> + struct list_head *head) >>> +{ >>> + struct iort_rmr_entry *e; >>> + struct acpi_iort_node *iommu; >>> + int rmrs = 0; >>> + >>> + iommu = iort_get_iort_node(iommu_fwnode); >>> + if (!iommu || list_empty(&iort_rmr_list)) >>> + return -ENODEV; >>> + >>> + list_for_each_entry(e, &iort_rmr_list, list) { >>> + int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_NOEXEC | >> IOMMU_MMIO; >> >> We have a case with an IP block that needs EXEC rights on its reserved >> memory, so could you please drop the IOMMU_NOEXEC flag? > > Ok, I think I can drop that one if there are no other concerns. I was not quite > sure what to include here in the first place as the IORT spec is not giving any > further details about the RMR regions(May be the flags field can be extended to > describe these details). > That would be great, given that some preliminary investigations on my side revealed that our IP block seems to be quite sensitive to memory attributes. I need to spend some more time on this but at first sight looks like it needs cacheable, normal memory (not mmio mapping). --- Thanks & Best Regards, Laurentiu