Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Jon Nettleton <jon@solid-run.com>]

On Wed, May 26, 2021 at 7:11 PM Laurentiu Tudor <laurentiu.tudor@nxp.com> wrote:
>
>
>
> On 5/26/2021 7:36 PM, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Laurentiu Tudor [mailto:laurentiu.tudor@nxp.com]
> >> Sent: 26 May 2021 08:53
> >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> >> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> >> iommu@lists.linux-foundation.org
> >> Cc: jon@solid-run.com; Linuxarm <linuxarm@huawei.com>;
> >> steven.price@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> >> yangyicong <yangyicong@huawei.com>; Sami.Mujawar@arm.com;
> >> robin.murphy@arm.com; wanghuiqiang <wanghuiqiang@huawei.com>
> >> 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@huawei.com>
> >>> ---
> >>>  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.

If so maybe we can touch base off list and align on the implementation.

-Jon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel