Re: [PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev
On 2021-05-10 10:19, Shameerali Kolothum Thodi wrote: Hi Robin, -Original Message- From: Robin Murphy [mailto:robin.mur...@arm.com] Sent: 07 May 2021 11:02 To: Shameerali Kolothum Thodi ; linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; iommu@lists.linux-foundation.org Cc: Linuxarm ; lorenzo.pieral...@arm.com; j...@8bytes.org; wanghuiqiang ; Guohanjun (Hanjun Guo) ; steven.pr...@arm.com; sami.muja...@arm.com; j...@solid-run.com; eric.au...@redhat.com Subject: Re: [PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev On 2021-04-20 09:27, Shameer Kolothum wrote: Get RMR regions associated with a dev reserved so that there is a unity mapping for them in SMMU. Signed-off-by: Shameer Kolothum --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 + 1 file changed, 29 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 14e9c7034c04..8bacedf7bb34 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2531,6 +2531,34 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) return iommu_fwspec_add_ids(dev, args->args, 1); } +static bool arm_smmu_dev_has_rmr(struct arm_smmu_master *master, +struct iommu_rmr *e) +{ + int i; + + for (i = 0; i < master->num_sids; i++) { + if (e->sid == master->sids[i]) + return true; + } + + return false; +} + +static void arm_smmu_rmr_get_resv_regions(struct device *dev, + struct list_head *head) +{ + struct arm_smmu_master *master = dev_iommu_priv_get(dev); + struct arm_smmu_device *smmu = master->smmu; + struct iommu_rmr *rmr; + + list_for_each_entry(rmr, >rmr_list, list) { + if (!arm_smmu_dev_has_rmr(master, rmr)) + continue; + + iommu_dma_get_rmr_resv_regions(dev, rmr, head); + } +} + TBH I wouldn't have thought we need a driver-specific hook for this, or is it too painful to correlate fwspec->iommu_fwnode back to the relevant IORT node generically? From a quick look, I think I could get rid of the above with something like below, --8< +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, + struct iommu_rmr *e) +{ + int i; + + for (i = 0; i < fwspec->num_ids; i++) { +if (e->sid == fwspec->ids[i]) +return true; +} + +return false; +} + + +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_rmr *rmr; + + INIT_LIST_HEAD(_list); + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, _list)) + return; ... + list_for_each_entry(rmr, _list, list) { + + if (!iommu_dma_dev_has_rmr(fwspec, rmr) + continue; + ... + region = iommu_alloc_resv_region(rmr->base_address, +rmr->length, prot, +type); ... + } +} /** * iommu_dma_get_resv_regions - Reserved region driver helper * @dev: Device from iommu_get_resv_regions() @@ -188,10 +242,11 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) 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); } 8< But looking at the SMMUv2 code, the fwspec->ids is MASK:SID, so I am not sure the RMR sid can be compared directly to fwspec->ids above. Right? Or is there a better way here? Ah, but consider how the IDs got there in the first place ;) A mask will never be set on ACPI systems, since IORT (intentionally) only caters for straightforward mappings rather than arbitrary complexity, so the assumption of fwspec ID == SID is already baked in by virtue of arm_smmu_iort_xlate(). The IORT code is free to assume its own behaviour! Robin. Thanks, Shameer static void arm_smmu_get_resv_regions(struct device *dev, struct list_head *head) { @@ -2545,6 +2573,7 @@ static void arm_smmu_get_resv_regions(struct device *dev, list_add_tail(>list, head); iommu_dma_get_resv_regions(dev, head); + arm_smmu_rmr_get_resv_regions(dev, head); } static bool arm_smmu_dev_has_feature(struct device *dev, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev
Hi Robin, > -Original Message- > From: Robin Murphy [mailto:robin.mur...@arm.com] > Sent: 07 May 2021 11:02 > To: Shameerali Kolothum Thodi ; > linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; > iommu@lists.linux-foundation.org > Cc: Linuxarm ; lorenzo.pieral...@arm.com; > j...@8bytes.org; wanghuiqiang ; Guohanjun > (Hanjun Guo) ; steven.pr...@arm.com; > sami.muja...@arm.com; j...@solid-run.com; eric.au...@redhat.com > Subject: Re: [PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions > associated with a dev > > On 2021-04-20 09:27, Shameer Kolothum wrote: > > Get RMR regions associated with a dev reserved so that there is > > a unity mapping for them in SMMU. > > > > Signed-off-by: Shameer Kolothum > > > --- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 > + > > 1 file changed, 29 insertions(+) > > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > index 14e9c7034c04..8bacedf7bb34 100644 > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > > @@ -2531,6 +2531,34 @@ static int arm_smmu_of_xlate(struct device *dev, > struct of_phandle_args *args) > > return iommu_fwspec_add_ids(dev, args->args, 1); > > } > > > > +static bool arm_smmu_dev_has_rmr(struct arm_smmu_master *master, > > +struct iommu_rmr *e) > > +{ > > + int i; > > + > > + for (i = 0; i < master->num_sids; i++) { > > + if (e->sid == master->sids[i]) > > + return true; > > + } > > + > > + return false; > > +} > > + > > +static void arm_smmu_rmr_get_resv_regions(struct device *dev, > > + struct list_head *head) > > +{ > > + struct arm_smmu_master *master = dev_iommu_priv_get(dev); > > + struct arm_smmu_device *smmu = master->smmu; > > + struct iommu_rmr *rmr; > > + > > + list_for_each_entry(rmr, >rmr_list, list) { > > + if (!arm_smmu_dev_has_rmr(master, rmr)) > > + continue; > > + > > + iommu_dma_get_rmr_resv_regions(dev, rmr, head); > > + } > > +} > > + > > TBH I wouldn't have thought we need a driver-specific hook for this, or > is it too painful to correlate fwspec->iommu_fwnode back to the relevant > IORT node generically? From a quick look, I think I could get rid of the above with something like below, --8< +static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec, + struct iommu_rmr *e) +{ + int i; + + for (i = 0; i < fwspec->num_ids; i++) { +if (e->sid == fwspec->ids[i]) +return true; +} + +return false; +} + + +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_rmr *rmr; + + INIT_LIST_HEAD(_list); + if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, _list)) + return; ... + list_for_each_entry(rmr, _list, list) { + + if (!iommu_dma_dev_has_rmr(fwspec, rmr) + continue; + ... + region = iommu_alloc_resv_region(rmr->base_address, +rmr->length, prot, +type); ... + } +} /** * iommu_dma_get_resv_regions - Reserved region driver helper * @dev: Device from iommu_get_resv_regions() @@ -188,10 +242,11 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) 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); } 8< But looking at the SMMUv2 code, the fwspec->ids is MASK:SID, so I am not sure the RMR sid can be compared directly to fwspec->ids above. Right? Or is there a better way here? Thanks, Shameer > > > static void arm_smmu_get_resv_regions(struct device *dev, > > struct list_head *head) > > { > > @@ -2545,6 +2573,7 @@ static void arm_smmu_get_resv_regions(struct > device *dev, > > list_add_tail(>list, head); > > > > iommu_dma_get_resv_regions(dev, head); > > + arm_smmu_rmr_get_resv_regions(dev, head); > > } > > > > static bool arm_smmu_dev_has_feature(struct device *dev, > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev
On 2021-04-20 09:27, Shameer Kolothum wrote: Get RMR regions associated with a dev reserved so that there is a unity mapping for them in SMMU. Signed-off-by: Shameer Kolothum --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 + 1 file changed, 29 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 14e9c7034c04..8bacedf7bb34 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2531,6 +2531,34 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) return iommu_fwspec_add_ids(dev, args->args, 1); } +static bool arm_smmu_dev_has_rmr(struct arm_smmu_master *master, +struct iommu_rmr *e) +{ + int i; + + for (i = 0; i < master->num_sids; i++) { + if (e->sid == master->sids[i]) + return true; + } + + return false; +} + +static void arm_smmu_rmr_get_resv_regions(struct device *dev, + struct list_head *head) +{ + struct arm_smmu_master *master = dev_iommu_priv_get(dev); + struct arm_smmu_device *smmu = master->smmu; + struct iommu_rmr *rmr; + + list_for_each_entry(rmr, >rmr_list, list) { + if (!arm_smmu_dev_has_rmr(master, rmr)) + continue; + + iommu_dma_get_rmr_resv_regions(dev, rmr, head); + } +} + TBH I wouldn't have thought we need a driver-specific hook for this, or is it too painful to correlate fwspec->iommu_fwnode back to the relevant IORT node generically? Robin. static void arm_smmu_get_resv_regions(struct device *dev, struct list_head *head) { @@ -2545,6 +2573,7 @@ static void arm_smmu_get_resv_regions(struct device *dev, list_add_tail(>list, head); iommu_dma_get_resv_regions(dev, head); + arm_smmu_rmr_get_resv_regions(dev, head); } static bool arm_smmu_dev_has_feature(struct device *dev, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 08/10] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev
Get RMR regions associated with a dev reserved so that there is a unity mapping for them in SMMU. Signed-off-by: Shameer Kolothum --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 + 1 file changed, 29 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 14e9c7034c04..8bacedf7bb34 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2531,6 +2531,34 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) return iommu_fwspec_add_ids(dev, args->args, 1); } +static bool arm_smmu_dev_has_rmr(struct arm_smmu_master *master, +struct iommu_rmr *e) +{ + int i; + + for (i = 0; i < master->num_sids; i++) { + if (e->sid == master->sids[i]) + return true; + } + + return false; +} + +static void arm_smmu_rmr_get_resv_regions(struct device *dev, + struct list_head *head) +{ + struct arm_smmu_master *master = dev_iommu_priv_get(dev); + struct arm_smmu_device *smmu = master->smmu; + struct iommu_rmr *rmr; + + list_for_each_entry(rmr, >rmr_list, list) { + if (!arm_smmu_dev_has_rmr(master, rmr)) + continue; + + iommu_dma_get_rmr_resv_regions(dev, rmr, head); + } +} + static void arm_smmu_get_resv_regions(struct device *dev, struct list_head *head) { @@ -2545,6 +2573,7 @@ static void arm_smmu_get_resv_regions(struct device *dev, list_add_tail(>list, head); iommu_dma_get_resv_regions(dev, head); + arm_smmu_rmr_get_resv_regions(dev, head); } static bool arm_smmu_dev_has_feature(struct device *dev, -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu