| Message ID | 20210805080724.480-10-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
On 2021-08-05 09:07, Shameer Kolothum wrote: > Get ACPI IORT RMR regions associated with a dev reserved > so that there is a unity mapping for them in SMMU. This feels like most of it belongs in the IORT code rather than iommu-dma (which should save the temporary list copy as well). Robin. > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > --- > drivers/iommu/dma-iommu.c | 56 +++++++++++++++++++++++++++++++++++---- > 1 file changed, 51 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 1b6e27475279..c1ae0c3d4b33 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -207,22 +207,68 @@ void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > } > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, > + struct iommu_resv_region *e) > +{ > + int i; > + > + for (i = 0; i < fwspec->num_ids; i++) { > + if (e->fw_data.rmr.sid == fwspec->ids[i]) > + return true; > + } > + > + return false; > +} > + > +static void iommu_dma_get_rmr_resv_regions(struct device *dev, > + struct list_head *list) > +{ > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > + struct list_head rmr_list; > + struct iommu_resv_region *rmr, *tmp; > + > + INIT_LIST_HEAD(&rmr_list); > + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, &rmr_list)) > + return; > + > + if (dev_is_pci(dev)) { > + struct pci_dev *pdev = to_pci_dev(dev); > + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); > + > + if (!host->preserve_config) > + return; > + } > + > + list_for_each_entry_safe(rmr, tmp, &rmr_list, list) { > + if (!iommu_dma_dev_has_rmr(fwspec, rmr)) > + continue; > + > + /* Remove from iommu RMR list and add to dev resv_regions */ > + list_del_init(&rmr->list); > + list_add_tail(&rmr->list, list); > + } > + > + iommu_dma_put_rmrs(fwspec->iommu_fwnode, &rmr_list); > +} > + > /** > * iommu_dma_get_resv_regions - Reserved region driver helper > * @dev: Device from iommu_get_resv_regions() > * @list: Reserved region list from iommu_get_resv_regions() > * > * IOMMU drivers can use this to implement their .get_resv_regions callback > - * for general non-IOMMU-specific reservations. Currently, this covers GICv3 > - * ITS region reservation on ACPI based ARM platforms that may require HW MSI > - * reservation. > + * for general non-IOMMU-specific reservations. Currently this covers, > + * -GICv3 ITS region reservation on ACPI based ARM platforms that may > + * require HW MSI reservation. > + * -Any ACPI IORT RMR memory range reservations (IORT spec rev E.b) > */ > void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) > { > > - if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) > + if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) { > iort_iommu_msi_get_resv_regions(dev, list); > - > + iommu_dma_get_rmr_resv_regions(dev, list); > + } > } > EXPORT_SYMBOL(iommu_dma_get_resv_regions); > >
On Fri, Oct 8, 2021 at 3:10 PM Robin Murphy <robin.murphy@arm.com> wrote: > > On 2021-08-05 09:07, Shameer Kolothum wrote: > > Get ACPI IORT RMR regions associated with a dev reserved > > so that there is a unity mapping for them in SMMU. > > This feels like most of it belongs in the IORT code rather than > iommu-dma (which should save the temporary list copy as well). See previous comment. The original intent was for device-tree to also be able to use these mechanisms to create RMR's and support them in the SMMU. -Jon > > Robin. > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > --- > > drivers/iommu/dma-iommu.c | 56 +++++++++++++++++++++++++++++++++++---- > > 1 file changed, 51 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > > index 1b6e27475279..c1ae0c3d4b33 100644 > > --- a/drivers/iommu/dma-iommu.c > > +++ b/drivers/iommu/dma-iommu.c > > @@ -207,22 +207,68 @@ void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, > > } > > EXPORT_SYMBOL(iommu_dma_put_rmrs); > > > > +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, > > + struct iommu_resv_region *e) > > +{ > > + int i; > > + > > + for (i = 0; i < fwspec->num_ids; i++) { > > + if (e->fw_data.rmr.sid == fwspec->ids[i]) > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static void iommu_dma_get_rmr_resv_regions(struct device *dev, > > + struct list_head *list) > > +{ > > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > + struct list_head rmr_list; > > + struct iommu_resv_region *rmr, *tmp; > > + > > + INIT_LIST_HEAD(&rmr_list); > > + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, &rmr_list)) > > + return; > > + > > + if (dev_is_pci(dev)) { > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); > > + > > + if (!host->preserve_config) > > + return; > > + } > > + > > + list_for_each_entry_safe(rmr, tmp, &rmr_list, list) { > > + if (!iommu_dma_dev_has_rmr(fwspec, rmr)) > > + continue; > > + > > + /* Remove from iommu RMR list and add to dev resv_regions */ > > + list_del_init(&rmr->list); > > + list_add_tail(&rmr->list, list); > > + } > > + > > + iommu_dma_put_rmrs(fwspec->iommu_fwnode, &rmr_list); > > +} > > + > > /** > > * iommu_dma_get_resv_regions - Reserved region driver helper > > * @dev: Device from iommu_get_resv_regions() > > * @list: Reserved region list from iommu_get_resv_regions() > > * > > * IOMMU drivers can use this to implement their .get_resv_regions callback > > - * for general non-IOMMU-specific reservations. Currently, this covers GICv3 > > - * ITS region reservation on ACPI based ARM platforms that may require HW MSI > > - * reservation. > > + * for general non-IOMMU-specific reservations. Currently this covers, > > + * -GICv3 ITS region reservation on ACPI based ARM platforms that may > > + * require HW MSI reservation. > > + * -Any ACPI IORT RMR memory range reservations (IORT spec rev E.b) > > */ > > void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) > > { > > > > - if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) > > + if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) { > > iort_iommu_msi_get_resv_regions(dev, list); > > - > > + iommu_dma_get_rmr_resv_regions(dev, list); > > + } > > } > > EXPORT_SYMBOL(iommu_dma_get_resv_regions); > > > >
On 2021-10-09 08:07, Jon Nettleton wrote: > On Fri, Oct 8, 2021 at 3:10 PM Robin Murphy <robin.murphy@arm.com> wrote: >> >> On 2021-08-05 09:07, Shameer Kolothum wrote: >>> Get ACPI IORT RMR regions associated with a dev reserved >>> so that there is a unity mapping for them in SMMU. >> >> This feels like most of it belongs in the IORT code rather than >> iommu-dma (which should save the temporary list copy as well). > > See previous comment. The original intent was for device-tree to also > be able to use these mechanisms to create RMR's and support them > in the SMMU. Can you clarify how code behind an "if (!is_of_node(...))" check alongside other IORT-specific code is expected to be useful for DT? Yes, iommu_dma_get_resv_regions() itself wants to end up serving as an abstraction layer, but that still doesn't mean it has to do much more than dispatch into firmware-specific backends as appropriate. Robin.
> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@arm.com] > Sent: 11 October 2021 16:01 > To: Jon Nettleton <jon@solid-run.com> > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel <linux-arm-kernel@lists.infradead.org>; ACPI Devel Maling > List <linux-acpi@vger.kernel.org>; Linux IOMMU > <iommu@lists.linux-foundation.org>; Linuxarm <linuxarm@huawei.com>; > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Joerg Roedel > <joro@8bytes.org>; Will Deacon <will@kernel.org>; wanghuiqiang > <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com>; Steven Price <steven.price@arm.com>; Sami > Mujawar <Sami.Mujawar@arm.com>; Eric Auger <eric.auger@redhat.com>; > yangyicong <yangyicong@huawei.com> > Subject: Re: [PATCH v7 9/9] iommu/dma: Reserve any RMR regions associated > with a dev > > On 2021-10-09 08:07, Jon Nettleton wrote: > > On Fri, Oct 8, 2021 at 3:10 PM Robin Murphy <robin.murphy@arm.com> > wrote: > >> > >> On 2021-08-05 09:07, Shameer Kolothum wrote: > >>> Get ACPI IORT RMR regions associated with a dev reserved > >>> so that there is a unity mapping for them in SMMU. > >> > >> This feels like most of it belongs in the IORT code rather than > >> iommu-dma (which should save the temporary list copy as well). > > > > See previous comment. The original intent was for device-tree to also > > be able to use these mechanisms to create RMR's and support them > > in the SMMU. > > Can you clarify how code behind an "if (!is_of_node(...))" check > alongside other IORT-specific code is expected to be useful for DT? > > Yes, iommu_dma_get_resv_regions() itself wants to end up serving as an > abstraction layer, but that still doesn't mean it has to do much more > than dispatch into firmware-specific backends as appropriate. (Resending as I accidently replied earlier from our internal ML id. Sorry) The way I thought about is as below, 1. iommu_dma_get_resv_regions() will invoke the common iommu_dma_get_rmr_resv_regions(). Yes, the if (!is_of_node(...)) is not required here. 2. iommu_dma_get_rmr_resv_regions() calls iommu_dma_get_rmrs(). iommu_dma_get_rmrs() has the (!is_of_node(...)) check to call into IORT or DT specific functions to retrieve the RMR reserve regions associated with a given iommu_fwnode. 3. The common iommu_dma_get_rmr_resv_regions() further checks for PCI host preserve_config and whether the returned RMR list actually has any dev specific region to reserve or not. So the only firmware specific backend is handled inside the iommu_dma_get_rmrs() and that is also called from the SMMU driver probe to install bypass SIDs. Anyway, if the eventual DT implementation or further IORT spec changes makes this abstraction irrelevant I am Ok to move this into the IORT code. Thanks, Shameer
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 1b6e27475279..c1ae0c3d4b33 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -207,22 +207,68 @@ void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode, } EXPORT_SYMBOL(iommu_dma_put_rmrs); +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, + struct iommu_resv_region *e) +{ + int i; + + for (i = 0; i < fwspec->num_ids; i++) { + if (e->fw_data.rmr.sid == fwspec->ids[i]) + return true; + } + + return false; +} + +static void iommu_dma_get_rmr_resv_regions(struct device *dev, + struct list_head *list) +{ + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + struct list_head rmr_list; + struct iommu_resv_region *rmr, *tmp; + + INIT_LIST_HEAD(&rmr_list); + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, &rmr_list)) + return; + + if (dev_is_pci(dev)) { + struct pci_dev *pdev = to_pci_dev(dev); + struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus); + + if (!host->preserve_config) + return; + } + + list_for_each_entry_safe(rmr, tmp, &rmr_list, list) { + if (!iommu_dma_dev_has_rmr(fwspec, rmr)) + continue; + + /* Remove from iommu RMR list and add to dev resv_regions */ + list_del_init(&rmr->list); + list_add_tail(&rmr->list, list); + } + + iommu_dma_put_rmrs(fwspec->iommu_fwnode, &rmr_list); +} + /** * iommu_dma_get_resv_regions - Reserved region driver helper * @dev: Device from iommu_get_resv_regions() * @list: Reserved region list from iommu_get_resv_regions() * * IOMMU drivers can use this to implement their .get_resv_regions callback - * for general non-IOMMU-specific reservations. Currently, this covers GICv3 - * ITS region reservation on ACPI based ARM platforms that may require HW MSI - * reservation. + * for general non-IOMMU-specific reservations. Currently this covers, + * -GICv3 ITS region reservation on ACPI based ARM platforms that may + * require HW MSI reservation. + * -Any ACPI IORT RMR memory range reservations (IORT spec rev E.b) */ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) { - if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) + if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) { iort_iommu_msi_get_resv_regions(dev, list); - + iommu_dma_get_rmr_resv_regions(dev, list); + } } EXPORT_SYMBOL(iommu_dma_get_resv_regions);
Get ACPI IORT RMR regions associated with a dev reserved so that there is a unity mapping for them in SMMU. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- drivers/iommu/dma-iommu.c | 56 +++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-)