RE: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@xxxxxxx]
> Sent: Tuesday, May 16, 2017 2:28 PM
> To: Shameerali Kolothum Thodi; will.deacon@xxxxxxx;
> mark.rutland@xxxxxxx; lorenzo.pieralisi@xxxxxxx; hanjun.guo@xxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx; Gabriele Paoloni; John Garry; Linuxarm;
> linux-acpi@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; Wangzhou
> (B); Guohanjun (Hanjun Guo); linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> devel@xxxxxxxxxx
> 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@xxxxxxxxxx>
> > ---
> >  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(&region->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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux