Hi Jon, On 6/3/2021 3:27 PM, Jon Nettleton wrote: > On Wed, May 26, 2021 at 7:11 PM Laurentiu Tudor <laurentiu.tudor@xxxxxxx> wrote: >> >> >> >> 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 > > Laurentiu, > > Is this regarding the mc-bin memory block or another IP? I am currently > running this patchset with IOMMU_NOEXEC under ACPI without any problems. It's the MC firmware needing EXEC rights on its reserved memory. On my side, with IOMMU_NOEXEC, as soon as the direct mappings are created I get SMMU faults triggered by the FW. > If so maybe we can touch base off list and align on the implementation. Sure, just let me know when you have the time. --- Best Regards, Laurentiu
