| Message ID | 20170513094731.3676-7-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/05/17 10:47, shameer wrote: > This moves the SW MSI reserve region allocation to probe fn. Why? > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com> > --- > drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 770cc9e..e7a8a50 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -619,6 +619,9 @@ struct arm_smmu_device { > > /* IOMMU core code handle */ > struct iommu_device iommu; > + > + /* MSI Reserve region */ > + struct iommu_resv_region *msi_region; > }; > > /* SMMU private data for each master */ > @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) > static void arm_smmu_get_resv_regions(struct device *dev, > struct list_head *head) > { > - struct iommu_resv_region *region; > - int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > - > - region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, > - prot, IOMMU_RESV_SW_MSI); > - if (!region) > - return; > + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > + struct arm_smmu_device *smmu = smmu_domain->smmu; There's never any need for the domain dance if you don't need the domain, just pull the smmu out of the device's iommu_fwspec. > - list_add_tail(®ion->list, head); > + if (smmu && smmu->msi_region) > + list_add_tail(&smmu->msi_region->list, head); Have you considered what happens for the second and subsequent domains allocated on this SMMU? I can't see that ending well. Robin. > iommu_dma_get_resv_regions(dev, head); > } > @@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev, > { > struct iommu_resv_region *entry, *next; > > - list_for_each_entry_safe(entry, next, head, list) > + list_for_each_entry_safe(entry, next, head, list) { > + if (entry->type == IOMMU_RESV_SW_MSI || > + entry->type == IOMMU_RESV_MSI) > + continue; > + > kfree(entry); > + } > } > > static struct iommu_ops arm_smmu_ops = { > @@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, > return ret; > } > > +static struct iommu_resv_region *arm_smmu_alloc_msi_region( > + struct arm_smmu_device *smmu) > +{ > + struct iommu_resv_region *region; > + int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > + > + region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, > + prot, IOMMU_RESV_SW_MSI); > + return region; > +} > + > static int arm_smmu_device_probe(struct platform_device *pdev) > { > int irq, ret; > @@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > if (irq > 0) > smmu->gerr_irq = irq; > > + smmu->msi_region = arm_smmu_alloc_msi_region(smmu); > + > if (dev->of_node) { > ret = arm_smmu_device_dt_probe(pdev, smmu); > } else { >
> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@arm.com] > Sent: Tuesday, May 16, 2017 2:28 PM > To: Shameerali Kolothum Thodi; will.deacon@arm.com; > mark.rutland@arm.com; lorenzo.pieralisi@arm.com; hanjun.guo@linaro.org > Cc: devicetree@vger.kernel.org; Gabriele Paoloni; John Garry; Linuxarm; > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; Wangzhou > (B); Guohanjun (Hanjun Guo); linux-arm-kernel@lists.infradead.org; > devel@acpica.org > Subject: Re: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc > functions > > On 13/05/17 10:47, shameer wrote: > > This moves the SW MSI reserve region allocation to probe fn. > > Why? Sure, I will modify the commit message to mention about the next patch where we will add the quirk to modify the default SW MSI region with a HW MSI region on Hip06/07 platforms. > > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com> > > --- > > drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++--- > ------ > > 1 file changed, 27 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu- > v3.c > > index 770cc9e..e7a8a50 100644 > > --- a/drivers/iommu/arm-smmu-v3.c > > +++ b/drivers/iommu/arm-smmu-v3.c > > @@ -619,6 +619,9 @@ struct arm_smmu_device { > > > > /* IOMMU core code handle */ > > struct iommu_device iommu; > > + > > + /* MSI Reserve region */ > > + struct iommu_resv_region *msi_region; > > }; > > > > /* SMMU private data for each master */ > > @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device > *dev, struct of_phandle_args *args) > > static void arm_smmu_get_resv_regions(struct device *dev, > > struct list_head *head) > > { > > - struct iommu_resv_region *region; > > - int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; > > - > > - region = iommu_alloc_resv_region(MSI_IOVA_BASE, > MSI_IOVA_LENGTH, > > - prot, IOMMU_RESV_SW_MSI); > > - if (!region) > > - return; > > + struct iommu_domain *domain = > iommu_get_domain_for_dev(dev); > > + struct arm_smmu_domain *smmu_domain = > to_smmu_domain(domain); > > + struct arm_smmu_device *smmu = smmu_domain->smmu; > > There's never any need for the domain dance if you don't need the > domain, just pull the smmu out of the device's iommu_fwspec. Ok. That makes sense. Thanks. > > - list_add_tail(®ion->list, head); > > + if (smmu && smmu->msi_region) > > + list_add_tail(&smmu->msi_region->list, head); > > Have you considered what happens for the second and subsequent domains > allocated on this SMMU? I can't see that ending well. Yes, I probably missed this part. I will recheck this. Thanks, Shameer
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 770cc9e..e7a8a50 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -619,6 +619,9 @@ struct arm_smmu_device { /* IOMMU core code handle */ struct iommu_device iommu; + + /* MSI Reserve region */ + struct iommu_resv_region *msi_region; }; /* SMMU private data for each master */ @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) static void arm_smmu_get_resv_regions(struct device *dev, struct list_head *head) { - struct iommu_resv_region *region; - int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; - - region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, - prot, IOMMU_RESV_SW_MSI); - if (!region) - return; + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + struct arm_smmu_device *smmu = smmu_domain->smmu; - list_add_tail(®ion->list, head); + if (smmu && smmu->msi_region) + list_add_tail(&smmu->msi_region->list, head); iommu_dma_get_resv_regions(dev, head); } @@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev, { struct iommu_resv_region *entry, *next; - list_for_each_entry_safe(entry, next, head, list) + list_for_each_entry_safe(entry, next, head, list) { + if (entry->type == IOMMU_RESV_SW_MSI || + entry->type == IOMMU_RESV_MSI) + continue; + kfree(entry); + } } static struct iommu_ops arm_smmu_ops = { @@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev, return ret; } +static struct iommu_resv_region *arm_smmu_alloc_msi_region( + struct arm_smmu_device *smmu) +{ + struct iommu_resv_region *region; + int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO; + + region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, + prot, IOMMU_RESV_SW_MSI); + return region; +} + static int arm_smmu_device_probe(struct platform_device *pdev) { int irq, ret; @@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) if (irq > 0) smmu->gerr_irq = irq; + smmu->msi_region = arm_smmu_alloc_msi_region(smmu); + if (dev->of_node) { ret = arm_smmu_device_dt_probe(pdev, smmu); } else {
This moves the SW MSI reserve region allocation to probe fn. Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com> --- drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)