| Message ID | 20210524110222.2212-4-shameerali.kolothum.thodi@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
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? --- Thanks & Best Regards, Laurentiu
> -----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). Thanks, Shameer
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
On Wed, May 26, 2021 at 6:36 PM Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> 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). We probably need to bring this back up to the ACPI forums. This is probably something that should be attached to the memory range node which does have 4 extra reserved bytes. -Jon > > Thanks, > Shameer > >
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
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@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. 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
On 2021-05-26 17:36, 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). Right, it's reserved for the device to use *somehow* - that's all we know, so it's not our place to try and enforce any restrictive permissions. All it could possibly achieve is the ability to log an error to say "a thing did an unknown thing in a way we didn't expect!", and there's hardly much value in that. Robin.
On 2021-05-24 12:02, 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; > + struct iommu_resv_region *region; > + enum iommu_resv_type type; > + struct acpi_iort_rmr_desc *rmr_desc; > + > + if (e->smmu != iommu) > + continue; > + > + rmr_desc = e->rmr_desc; > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > + type = IOMMU_RESV_DIRECT_RELAXABLE; > + else > + type = IOMMU_RESV_DIRECT; Wasn't the idea that we can do all this during the initial parsing, i.e. we don't even have iort_rmr_entry, we store them as iommu_resv_regions and simply kmemdup() or whatever at this point? Robin. > + > + region = iommu_alloc_resv_region(rmr_desc->base_address, > + rmr_desc->length, > + prot, type); > + if (region) { > + region->fw_data.rmr.flags = e->flags; > + region->fw_data.rmr.sid = e->sid; > + list_add_tail(®ion->list, head); > + rmrs++; > + } > + } > + > + return (rmrs == 0) ? -ENODEV : 0; > +} > + > /** > * iort_iommu_msi_get_resv_regions - Reserved region driver helper > * @dev: Device from iommu_get_resv_regions() > @@ -1108,6 +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head) > const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > const u32 *input_id) > { return NULL; } > +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head *head) > +{ return -ENODEV; } > #endif > > static int nc_dma_get_range(struct device *dev, u64 *size) > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 229ec65d98be..f893d460cfa4 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); > int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, > struct list_head *list) > { > + if (!is_of_node(iommu_fwnode)) > + return iort_iommu_get_rmrs(iommu_fwnode, list); > + > return -EINVAL; > } > EXPORT_SYMBOL(iommu_dma_get_rmrs); > @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); > void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > struct list_head *list) > { > + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); > } > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h > index 1a12baa58e40..e8c45fa59531 100644 > --- a/include/linux/acpi_iort.h > +++ b/include/linux/acpi_iort.h > @@ -39,6 +39,8 @@ const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > const u32 *id_in); > int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head); > phys_addr_t acpi_iort_dma_get_max_cpu_address(void); > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > + struct list_head *list); > #else > static inline void acpi_iort_init(void) { } > static inline u32 iort_msi_map_id(struct device *dev, u32 id) > @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head) > > static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) > { return PHYS_ADDR_MAX; } > + > +static inline > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > + struct list_head *list) > +{ return -ENODEV; } > #endif > > #endif /* __ACPI_IORT_H__ */ >
> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@arm.com] > Sent: 14 June 2021 12:23 > 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; > wanghuiqiang <wanghuiqiang@huawei.com> > Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory > regions > > On 2021-05-24 12:02, 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; > > + struct iommu_resv_region *region; > > + enum iommu_resv_type type; > > + struct acpi_iort_rmr_desc *rmr_desc; > > + > > + if (e->smmu != iommu) > > + continue; > > + > > + rmr_desc = e->rmr_desc; > > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > > + type = IOMMU_RESV_DIRECT_RELAXABLE; > > + else > > + type = IOMMU_RESV_DIRECT; > > Wasn't the idea that we can do all this during the initial parsing, i.e. > we don't even have iort_rmr_entry, we store them as iommu_resv_regions > and simply kmemdup() or whatever at this point? Hmm... Not yet. I removed struct iommu_rmr() from v4. But yes, it looks like we can get rid of iort_rmr_entry as well. Will give it a go in next. Thanks, Shameer > Robin. > > > + > > + region = iommu_alloc_resv_region(rmr_desc->base_address, > > + rmr_desc->length, > > + prot, type); > > + if (region) { > > + region->fw_data.rmr.flags = e->flags; > > + region->fw_data.rmr.sid = e->sid; > > + list_add_tail(®ion->list, head); > > + rmrs++; > > + } > > + } > > + > > + return (rmrs == 0) ? -ENODEV : 0; > > +} > > + > > /** > > * iort_iommu_msi_get_resv_regions - Reserved region driver helper > > * @dev: Device from iommu_get_resv_regions() > > @@ -1108,6 +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct > device *dev, struct list_head *head) > > const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > > const u32 *input_id) > > { return NULL; } > > +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head > *head) > > +{ return -ENODEV; } > > #endif > > > > static int nc_dma_get_range(struct device *dev, u64 *size) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index 229ec65d98be..f893d460cfa4 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); > > int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, > > struct list_head *list) > > { > > + if (!is_of_node(iommu_fwnode)) > > + return iort_iommu_get_rmrs(iommu_fwnode, list); > > + > > return -EINVAL; > > } > > EXPORT_SYMBOL(iommu_dma_get_rmrs); > > @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); > > void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > > struct list_head *list) > > { > > + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); > > } > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h > > index 1a12baa58e40..e8c45fa59531 100644 > > --- a/include/linux/acpi_iort.h > > +++ b/include/linux/acpi_iort.h > > @@ -39,6 +39,8 @@ const struct iommu_ops > *iort_iommu_configure_id(struct device *dev, > > const u32 *id_in); > > int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head > *head); > > phys_addr_t acpi_iort_dma_get_max_cpu_address(void); > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > + struct list_head *list); > > #else > > static inline void acpi_iort_init(void) { } > > static inline u32 iort_msi_map_id(struct device *dev, u32 id) > > @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct device > *dev, struct list_head *head) > > > > static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) > > { return PHYS_ADDR_MAX; } > > + > > +static inline > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > + struct list_head *list) > > +{ return -ENODEV; } > > #endif > > > > #endif /* __ACPI_IORT_H__ */ > >
On Mon, Jun 14, 2021 at 2:49 PM Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > -----Original Message----- > > From: Robin Murphy [mailto:robin.murphy@arm.com] > > Sent: 14 June 2021 12:23 > > 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; > > wanghuiqiang <wanghuiqiang@huawei.com> > > Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory > > regions > > > > On 2021-05-24 12:02, 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; > > > + struct iommu_resv_region *region; > > > + enum iommu_resv_type type; > > > + struct acpi_iort_rmr_desc *rmr_desc; > > > + > > > + if (e->smmu != iommu) > > > + continue; > > > + > > > + rmr_desc = e->rmr_desc; > > > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > > > + type = IOMMU_RESV_DIRECT_RELAXABLE; > > > + else > > > + type = IOMMU_RESV_DIRECT; > > I have been looking at this a bit more and looking at the history of RMR_REMAP_PERMITTED. Based on the history I have found it seems to me like this would be the better options for prot. @@ -840,7 +840,7 @@ int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, return -ENODEV; list_for_each_entry(e, &iort_rmr_list, list) { - int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + int prot = IOMMU_READ | IOMMU_WRITE; struct iommu_resv_region *region; enum iommu_resv_type type; struct acpi_iort_rmr_desc *rmr_desc; @@ -849,10 +849,13 @@ int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, continue; rmr_desc = e->rmr_desc; - if (e->flags & IOMMU_RMR_REMAP_PERMITTED) + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) { type = IOMMU_RESV_DIRECT_RELAXABLE; - else + prot |= IOMMU_CACHE; + } else { type = IOMMU_RESV_DIRECT; + prot |= IOMMU_MMIO; + } region = iommu_alloc_resv_region(rmr_desc->base_address, rmr_desc->length, any input on this? My reasoning is that IOMMU_RESV_DIRECT is specifically referenced for things like MSI doorbell and in all the examples needs IOMMU_MMIO, while REMAP_PERMITTED seems to be used for allocated system memory that is then used for device specific reserved regions which commits say would be like a GPU or USB device. -Jon > > Wasn't the idea that we can do all this during the initial parsing, i.e. > > we don't even have iort_rmr_entry, we store them as iommu_resv_regions > > and simply kmemdup() or whatever at this point? > > > Hmm... Not yet. I removed struct iommu_rmr() from v4. But yes, it looks like > we can get rid of iort_rmr_entry as well. Will give it a go in next. > > Thanks, > Shameer > > > Robin. > > > > > + > > > + region = iommu_alloc_resv_region(rmr_desc->base_address, > > > + rmr_desc->length, > > > + prot, type); > > > + if (region) { > > > + region->fw_data.rmr.flags = e->flags; > > > + region->fw_data.rmr.sid = e->sid; > > > + list_add_tail(®ion->list, head); > > > + rmrs++; > > > + } > > > + } > > > + > > > + return (rmrs == 0) ? -ENODEV : 0; > > > +} > > > + > > > /** > > > * iort_iommu_msi_get_resv_regions - Reserved region driver helper > > > * @dev: Device from iommu_get_resv_regions() > > > @@ -1108,6 +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct > > device *dev, struct list_head *head) > > > const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > > > const u32 *input_id) > > > { return NULL; } > > > +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head > > *head) > > > +{ return -ENODEV; } > > > #endif > > > > > > static int nc_dma_get_range(struct device *dev, u64 *size) > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > index 229ec65d98be..f893d460cfa4 100644 > > > --- a/drivers/iommu/dma-iommu.c > > > +++ b/drivers/iommu/dma-iommu.c > > > @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); > > > int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > struct list_head *list) > > > { > > > + if (!is_of_node(iommu_fwnode)) > > > + return iort_iommu_get_rmrs(iommu_fwnode, list); > > > + > > > return -EINVAL; > > > } > > > EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > > > struct list_head *list) > > > { > > > + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); > > > } > > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > > > diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h > > > index 1a12baa58e40..e8c45fa59531 100644 > > > --- a/include/linux/acpi_iort.h > > > +++ b/include/linux/acpi_iort.h > > > @@ -39,6 +39,8 @@ const struct iommu_ops > > *iort_iommu_configure_id(struct device *dev, > > > const u32 *id_in); > > > int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head > > *head); > > > phys_addr_t acpi_iort_dma_get_max_cpu_address(void); > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > + struct list_head *list); > > > #else > > > static inline void acpi_iort_init(void) { } > > > static inline u32 iort_msi_map_id(struct device *dev, u32 id) > > > @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct device > > *dev, struct list_head *head) > > > > > > static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) > > > { return PHYS_ADDR_MAX; } > > > + > > > +static inline > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > + struct list_head *list) > > > +{ return -ENODEV; } > > > #endif > > > > > > #endif /* __ACPI_IORT_H__ */ > > >
On Tue, Jun 29, 2021 at 7:34 PM Jon Nettleton <jon@solid-run.com> wrote: > > On Mon, Jun 14, 2021 at 2:49 PM Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Robin Murphy [mailto:robin.murphy@arm.com] > > > Sent: 14 June 2021 12:23 > > > 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; > > > wanghuiqiang <wanghuiqiang@huawei.com> > > > Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory > > > regions > > > > > > On 2021-05-24 12:02, 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; > > > > + struct iommu_resv_region *region; > > > > + enum iommu_resv_type type; > > > > + struct acpi_iort_rmr_desc *rmr_desc; > > > > + > > > > + if (e->smmu != iommu) > > > > + continue; > > > > + > > > > + rmr_desc = e->rmr_desc; > > > > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > > > > + type = IOMMU_RESV_DIRECT_RELAXABLE; > > > > + else > > > > + type = IOMMU_RESV_DIRECT; > > > > > I have been looking at this a bit more and looking at the history of > RMR_REMAP_PERMITTED. Based on the history I have found it > seems to me like this would be the better options for prot. > > @@ -840,7 +840,7 @@ int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > return -ENODEV; > > list_for_each_entry(e, &iort_rmr_list, list) { > - int prot = IOMMU_READ | IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + int prot = IOMMU_READ | IOMMU_WRITE; > struct iommu_resv_region *region; > enum iommu_resv_type type; > struct acpi_iort_rmr_desc *rmr_desc; > @@ -849,10 +849,13 @@ int iort_iommu_get_rmrs(struct fwnode_handle > *iommu_fwnode, > continue; > > rmr_desc = e->rmr_desc; > - if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) { > type = IOMMU_RESV_DIRECT_RELAXABLE; > - else > + prot |= IOMMU_CACHE; > + } else { > type = IOMMU_RESV_DIRECT; > + prot |= IOMMU_MMIO; > + } > > region = iommu_alloc_resv_region(rmr_desc->base_address, > rmr_desc->length, > > any input on this? My reasoning is that IOMMU_RESV_DIRECT is specifically > referenced for things like MSI doorbell and in all the examples needs > IOMMU_MMIO, while REMAP_PERMITTED seems to be used for allocated > system memory that is then used for device specific reserved regions which > commits say would be like a GPU or USB device. > > -Jon > > > > Wasn't the idea that we can do all this during the initial parsing, i.e. > > > we don't even have iort_rmr_entry, we store them as iommu_resv_regions > > > and simply kmemdup() or whatever at this point? > > > > > > Hmm... Not yet. I removed struct iommu_rmr() from v4. But yes, it looks like > > we can get rid of iort_rmr_entry as well. Will give it a go in next. > > > > Thanks, > > Shameer > > > > > Robin. > > > > > > > + > > > > + region = iommu_alloc_resv_region(rmr_desc->base_address, > > > > + rmr_desc->length, > > > > + prot, type); > > > > + if (region) { > > > > + region->fw_data.rmr.flags = e->flags; > > > > + region->fw_data.rmr.sid = e->sid; > > > > + list_add_tail(®ion->list, head); > > > > + rmrs++; > > > > + } > > > > + } > > > > + > > > > + return (rmrs == 0) ? -ENODEV : 0; > > > > +} > > > > + > > > > /** > > > > * iort_iommu_msi_get_resv_regions - Reserved region driver helper > > > > * @dev: Device from iommu_get_resv_regions() > > > > @@ -1108,6 +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct > > > device *dev, struct list_head *head) > > > > const struct iommu_ops *iort_iommu_configure_id(struct device *dev, > > > > const u32 *input_id) > > > > { return NULL; } > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head > > > *head) > > > > +{ return -ENODEV; } > > > > #endif > > > > > > > > static int nc_dma_get_range(struct device *dev, u64 *size) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > > > index 229ec65d98be..f893d460cfa4 100644 > > > > --- a/drivers/iommu/dma-iommu.c > > > > +++ b/drivers/iommu/dma-iommu.c > > > > @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); > > > > int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > struct list_head *list) > > > > { > > > > + if (!is_of_node(iommu_fwnode)) > > > > + return iort_iommu_get_rmrs(iommu_fwnode, list); > > > > + > > > > return -EINVAL; > > > > } > > > > EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > > @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > > void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > > > > struct list_head *list) > > > > { > > > > + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); > > > > } > > > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > > > > > diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h > > > > index 1a12baa58e40..e8c45fa59531 100644 > > > > --- a/include/linux/acpi_iort.h > > > > +++ b/include/linux/acpi_iort.h > > > > @@ -39,6 +39,8 @@ const struct iommu_ops > > > *iort_iommu_configure_id(struct device *dev, > > > > const u32 *id_in); > > > > int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head > > > *head); > > > > phys_addr_t acpi_iort_dma_get_max_cpu_address(void); > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > + struct list_head *list); > > > > #else > > > > static inline void acpi_iort_init(void) { } > > > > static inline u32 iort_msi_map_id(struct device *dev, u32 id) > > > > @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct device > > > *dev, struct list_head *head) > > > > > > > > static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) > > > > { return PHYS_ADDR_MAX; } > > > > + > > > > +static inline > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > + struct list_head *list) > > > > +{ return -ENODEV; } > > > > #endif > > > > > > > > #endif /* __ACPI_IORT_H__ */ > > > > Ping. Any comments on this? -Jon
> -----Original Message----- > From: Jon Nettleton [mailto:jon@solid-run.com] > Sent: 04 July 2021 08:39 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: Robin Murphy <robin.murphy@arm.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; Linuxarm <linuxarm@huawei.com>; > steven.price@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; > yangyicong <yangyicong@huawei.com>; Sami.Mujawar@arm.com; > wanghuiqiang <wanghuiqiang@huawei.com> > Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve RMR memory > regions > > On Tue, Jun 29, 2021 at 7:34 PM Jon Nettleton <jon@solid-run.com> wrote: > > > > On Mon, Jun 14, 2021 at 2:49 PM Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Robin Murphy [mailto:robin.murphy@arm.com] > > > > Sent: 14 June 2021 12:23 > > > > 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; wanghuiqiang <wanghuiqiang@huawei.com> > > > > Subject: Re: [PATCH v5 3/8] ACPI/IORT: Add a helper to retrieve > > > > RMR memory regions > > > > > > > > On 2021-05-24 12:02, 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; > > > > > + struct iommu_resv_region *region; > > > > > + enum iommu_resv_type type; > > > > > + struct acpi_iort_rmr_desc *rmr_desc; > > > > > + > > > > > + if (e->smmu != iommu) > > > > > + continue; > > > > > + > > > > > + rmr_desc = e->rmr_desc; > > > > > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > > > > > + type = IOMMU_RESV_DIRECT_RELAXABLE; > > > > > + else > > > > > + type = IOMMU_RESV_DIRECT; > > > > > > > > I have been looking at this a bit more and looking at the history of > > RMR_REMAP_PERMITTED. Based on the history I have found it seems to > me > > like this would be the better options for prot. > > > > @@ -840,7 +840,7 @@ int iort_iommu_get_rmrs(struct fwnode_handle > *iommu_fwnode, > > return -ENODEV; > > > > list_for_each_entry(e, &iort_rmr_list, list) { > > - int prot = IOMMU_READ | IOMMU_WRITE | > IOMMU_NOEXEC | IOMMU_MMIO; > > + int prot = IOMMU_READ | IOMMU_WRITE; > > struct iommu_resv_region *region; > > enum iommu_resv_type type; > > struct acpi_iort_rmr_desc *rmr_desc; @@ -849,10 > > +849,13 @@ int iort_iommu_get_rmrs(struct fwnode_handle > *iommu_fwnode, > > continue; > > > > rmr_desc = e->rmr_desc; > > - if (e->flags & IOMMU_RMR_REMAP_PERMITTED) > > + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) { > > type = IOMMU_RESV_DIRECT_RELAXABLE; > > - else > > + prot |= IOMMU_CACHE; > > + } else { > > type = IOMMU_RESV_DIRECT; > > + prot |= IOMMU_MMIO; > > + } > > > > region = > iommu_alloc_resv_region(rmr_desc->base_address, > > rmr_desc->length, > > > > any input on this? My reasoning is that IOMMU_RESV_DIRECT is > > specifically referenced for things like MSI doorbell and in all the > > examples needs IOMMU_MMIO, while REMAP_PERMITTED seems to be used > for > > allocated system memory that is then used for device specific reserved > > regions which commits say would be like a GPU or USB device. I am Ok to make those changes but not sure we can make the above assumptions based on the way it is currently used. Hi Robin, Any thoughts? Thanks, Shameer > > > > -Jon > > > > > > Wasn't the idea that we can do all this during the initial parsing, i.e. > > > > we don't even have iort_rmr_entry, we store them as > > > > iommu_resv_regions and simply kmemdup() or whatever at this point? > > > > > > > > > Hmm... Not yet. I removed struct iommu_rmr() from v4. But yes, it > > > looks like we can get rid of iort_rmr_entry as well. Will give it a go in next. > > > > > > Thanks, > > > Shameer > > > > > > > Robin. > > > > > > > > > + > > > > > + region = > iommu_alloc_resv_region(rmr_desc->base_address, > > > > > + rmr_desc->length, > > > > > + prot, type); > > > > > + if (region) { > > > > > + region->fw_data.rmr.flags = e->flags; > > > > > + region->fw_data.rmr.sid = e->sid; > > > > > + list_add_tail(®ion->list, head); > > > > > + rmrs++; > > > > > + } > > > > > + } > > > > > + > > > > > + return (rmrs == 0) ? -ENODEV : 0; } > > > > > + > > > > > /** > > > > > * iort_iommu_msi_get_resv_regions - Reserved region driver helper > > > > > * @dev: Device from iommu_get_resv_regions() @@ -1108,6 > > > > > +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct > > > > device *dev, struct list_head *head) > > > > > const struct iommu_ops *iort_iommu_configure_id(struct device > *dev, > > > > > const u32 > *input_id) > > > > > { return NULL; } > > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct > > > > > +list_head > > > > *head) > > > > > +{ return -ENODEV; } > > > > > #endif > > > > > > > > > > static int nc_dma_get_range(struct device *dev, u64 *size) > > > > > diff --git a/drivers/iommu/dma-iommu.c > > > > > b/drivers/iommu/dma-iommu.c index 229ec65d98be..f893d460cfa4 > > > > > 100644 > > > > > --- a/drivers/iommu/dma-iommu.c > > > > > +++ b/drivers/iommu/dma-iommu.c > > > > > @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); > > > > > int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > > struct list_head *list) > > > > > { > > > > > + if (!is_of_node(iommu_fwnode)) > > > > > + return iort_iommu_get_rmrs(iommu_fwnode, list); > > > > > + > > > > > return -EINVAL; > > > > > } > > > > > EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > > > @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); > > > > > void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > > > > > struct list_head *list) > > > > > { > > > > > + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); > > > > > } > > > > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > > > > > > > diff --git a/include/linux/acpi_iort.h > > > > > b/include/linux/acpi_iort.h index 1a12baa58e40..e8c45fa59531 > > > > > 100644 > > > > > --- a/include/linux/acpi_iort.h > > > > > +++ b/include/linux/acpi_iort.h > > > > > @@ -39,6 +39,8 @@ const struct iommu_ops > > > > *iort_iommu_configure_id(struct device *dev, > > > > > const u32 *id_in); > > > > > int iort_iommu_msi_get_resv_regions(struct device *dev, struct > > > > > list_head > > > > *head); > > > > > phys_addr_t acpi_iort_dma_get_max_cpu_address(void); > > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > > + struct list_head *list); > > > > > #else > > > > > static inline void acpi_iort_init(void) { } > > > > > static inline u32 iort_msi_map_id(struct device *dev, u32 id) > > > > > @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct > > > > > device > > > > *dev, struct list_head *head) > > > > > > > > > > static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) > > > > > { return PHYS_ADDR_MAX; } > > > > > + > > > > > +static inline > > > > > +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, > > > > > + struct list_head *list) { return -ENODEV; } > > > > > #endif > > > > > > > > > > #endif /* __ACPI_IORT_H__ */ > > > > > > > Ping. Any comments on this? > > -Jon
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; + struct iommu_resv_region *region; + enum iommu_resv_type type; + struct acpi_iort_rmr_desc *rmr_desc; + + if (e->smmu != iommu) + continue; + + rmr_desc = e->rmr_desc; + if (e->flags & IOMMU_RMR_REMAP_PERMITTED) + type = IOMMU_RESV_DIRECT_RELAXABLE; + else + type = IOMMU_RESV_DIRECT; + + region = iommu_alloc_resv_region(rmr_desc->base_address, + rmr_desc->length, + prot, type); + if (region) { + region->fw_data.rmr.flags = e->flags; + region->fw_data.rmr.sid = e->sid; + list_add_tail(®ion->list, head); + rmrs++; + } + } + + return (rmrs == 0) ? -ENODEV : 0; +} + /** * iort_iommu_msi_get_resv_regions - Reserved region driver helper * @dev: Device from iommu_get_resv_regions() @@ -1108,6 +1156,8 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head) const struct iommu_ops *iort_iommu_configure_id(struct device *dev, const u32 *input_id) { return NULL; } +int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head *head) +{ return -ENODEV; } #endif static int nc_dma_get_range(struct device *dev, u64 *size) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 229ec65d98be..f893d460cfa4 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie); int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, struct list_head *list) { + if (!is_of_node(iommu_fwnode)) + return iort_iommu_get_rmrs(iommu_fwnode, list); + return -EINVAL; } EXPORT_SYMBOL(iommu_dma_get_rmrs); @@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs); void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, struct list_head *list) { + generic_iommu_put_resv_regions(iommu_fwnode->dev, list); } EXPORT_SYMBOL(iommu_dma_put_rmrs); diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h index 1a12baa58e40..e8c45fa59531 100644 --- a/include/linux/acpi_iort.h +++ b/include/linux/acpi_iort.h @@ -39,6 +39,8 @@ const struct iommu_ops *iort_iommu_configure_id(struct device *dev, const u32 *id_in); int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head); phys_addr_t acpi_iort_dma_get_max_cpu_address(void); +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, + struct list_head *list); #else static inline void acpi_iort_init(void) { } static inline u32 iort_msi_map_id(struct device *dev, u32 id) @@ -59,6 +61,11 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head) static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void) { return PHYS_ADDR_MAX; } + +static inline +int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, + struct list_head *list) +{ return -ENODEV; } #endif #endif /* __ACPI_IORT_H__ */
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(+) -- 2.17.1