| Message ID | 43de5b58df37d8b8de037cb23c47ab8454caf37c.1613492577.git.rahul.singh@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm : smmuv3: Fix to handle multiple StreamIds per device. | expand |
Hi Rahul, > On 17 Feb 2021, at 10:05, Rahul Singh <Rahul.Singh@arm.com> wrote: > > SMMUv3 driver does not handle multiple StreamId if the master device > supports more than one StreamID. > > This bug was introduced when the driver was ported from Linux to XEN. > dt_device_set_protected(..) should be called from add_device(..) not > from the dt_xlate(..). > > Move dt_device_set_protected(..) from dt_xlate(..) to add_device(). > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Thanks a lot, this is fixing issues with multiple stream ids for one device :-) Cheers Bertrand > --- > This patch is a candidate for 4.15 as without this patch it is not possible to > assign multiple StreamIds to the same device when device is protected behind > SMMUv3. > --- > xen/drivers/passthrough/arm/smmu-v3.c | 29 ++++++++++----------------- > 1 file changed, 11 insertions(+), 18 deletions(-) > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > index 914cdc1cf4..53d150cdb6 100644 > --- a/xen/drivers/passthrough/arm/smmu-v3.c > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > @@ -2207,24 +2207,6 @@ static int arm_smmu_add_device(u8 devfn, struct device *dev) > */ > arm_smmu_enable_pasid(master); > > - return 0; > - > -err_free_master: > - xfree(master); > - dev_iommu_priv_set(dev, NULL); > - return ret; > -} > - > -static int arm_smmu_dt_xlate(struct device *dev, > - const struct dt_phandle_args *args) > -{ > - int ret; > - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > - > - ret = iommu_fwspec_add_ids(dev, args->args, 1); > - if (ret) > - return ret; > - > if (dt_device_is_protected(dev_to_dt(dev))) { > dev_err(dev, "Already added to SMMUv3\n"); > return -EEXIST; > @@ -2237,6 +2219,17 @@ static int arm_smmu_dt_xlate(struct device *dev, > dev_name(fwspec->iommu_dev), fwspec->num_ids); > > return 0; > + > +err_free_master: > + xfree(master); > + dev_iommu_priv_set(dev, NULL); > + return ret; > +} > + > +static int arm_smmu_dt_xlate(struct device *dev, > + const struct dt_phandle_args *args) > +{ > + return iommu_fwspec_add_ids(dev, args->args, 1); > } > > /* Probing and initialisation functions */ > -- > 2.17.1 >
+IanJ On Wed, 17 Feb 2021, Bertrand Marquis wrote: > Hi Rahul, > > > > On 17 Feb 2021, at 10:05, Rahul Singh <Rahul.Singh@arm.com> wrote: > > > > SMMUv3 driver does not handle multiple StreamId if the master device > > supports more than one StreamID. > > > > This bug was introduced when the driver was ported from Linux to XEN. > > dt_device_set_protected(..) should be called from add_device(..) not > > from the dt_xlate(..). > > > > Move dt_device_set_protected(..) from dt_xlate(..) to add_device(). > > > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> > > Thanks a lot, this is fixing issues with multiple stream ids for one device :-) Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > This patch is a candidate for 4.15 as without this patch it is not possible to > > assign multiple StreamIds to the same device when device is protected behind > > SMMUv3. I agree especially considering that the impact is limited to smmu-v3.c. > > --- > > xen/drivers/passthrough/arm/smmu-v3.c | 29 ++++++++++----------------- > > 1 file changed, 11 insertions(+), 18 deletions(-) > > > > diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c > > index 914cdc1cf4..53d150cdb6 100644 > > --- a/xen/drivers/passthrough/arm/smmu-v3.c > > +++ b/xen/drivers/passthrough/arm/smmu-v3.c > > @@ -2207,24 +2207,6 @@ static int arm_smmu_add_device(u8 devfn, struct device *dev) > > */ > > arm_smmu_enable_pasid(master); > > > > - return 0; > > - > > -err_free_master: > > - xfree(master); > > - dev_iommu_priv_set(dev, NULL); > > - return ret; > > -} > > - > > -static int arm_smmu_dt_xlate(struct device *dev, > > - const struct dt_phandle_args *args) > > -{ > > - int ret; > > - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > - > > - ret = iommu_fwspec_add_ids(dev, args->args, 1); > > - if (ret) > > - return ret; > > - > > if (dt_device_is_protected(dev_to_dt(dev))) { > > dev_err(dev, "Already added to SMMUv3\n"); > > return -EEXIST; > > @@ -2237,6 +2219,17 @@ static int arm_smmu_dt_xlate(struct device *dev, > > dev_name(fwspec->iommu_dev), fwspec->num_ids); > > > > return 0; > > + > > +err_free_master: > > + xfree(master); > > + dev_iommu_priv_set(dev, NULL); > > + return ret; > > +} > > + > > +static int arm_smmu_dt_xlate(struct device *dev, > > + const struct dt_phandle_args *args) > > +{ > > + return iommu_fwspec_add_ids(dev, args->args, 1); > > } > > > > /* Probing and initialisation functions */ > > -- > > 2.17.1 > > >
Hi, On 17/02/2021 23:23, Stefano Stabellini wrote: > +IanJ > > On Wed, 17 Feb 2021, Bertrand Marquis wrote: >> Hi Rahul, >> >> >>> On 17 Feb 2021, at 10:05, Rahul Singh <Rahul.Singh@arm.com> wrote: >>> >>> SMMUv3 driver does not handle multiple StreamId if the master device >>> supports more than one StreamID. >>> >>> This bug was introduced when the driver was ported from Linux to XEN. >>> dt_device_set_protected(..) should be called from add_device(..) not >>> from the dt_xlate(..). >>> >>> Move dt_device_set_protected(..) from dt_xlate(..) to add_device(). >>> >>> Signed-off-by: Rahul Singh <rahul.singh@arm.com> >> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> >> >> Thanks a lot, this is fixing issues with multiple stream ids for one device :-) > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > >>> --- >>> This patch is a candidate for 4.15 as without this patch it is not possible to >>> assign multiple StreamIds to the same device when device is protected behind >>> SMMUv3. > > I agree especially considering that the impact is limited to smmu-v3.c. SMMUv3 is in tech preview, so the risk is pretty low as we don't support it :). But, a release ack from Ian is not necessary until tonight (19th February) for bug fixes. So I have committed the patch. Cheers,
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index 914cdc1cf4..53d150cdb6 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -2207,24 +2207,6 @@ static int arm_smmu_add_device(u8 devfn, struct device *dev) */ arm_smmu_enable_pasid(master); - return 0; - -err_free_master: - xfree(master); - dev_iommu_priv_set(dev, NULL); - return ret; -} - -static int arm_smmu_dt_xlate(struct device *dev, - const struct dt_phandle_args *args) -{ - int ret; - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); - - ret = iommu_fwspec_add_ids(dev, args->args, 1); - if (ret) - return ret; - if (dt_device_is_protected(dev_to_dt(dev))) { dev_err(dev, "Already added to SMMUv3\n"); return -EEXIST; @@ -2237,6 +2219,17 @@ static int arm_smmu_dt_xlate(struct device *dev, dev_name(fwspec->iommu_dev), fwspec->num_ids); return 0; + +err_free_master: + xfree(master); + dev_iommu_priv_set(dev, NULL); + return ret; +} + +static int arm_smmu_dt_xlate(struct device *dev, + const struct dt_phandle_args *args) +{ + return iommu_fwspec_add_ids(dev, args->args, 1); } /* Probing and initialisation functions */
SMMUv3 driver does not handle multiple StreamId if the master device supports more than one StreamID. This bug was introduced when the driver was ported from Linux to XEN. dt_device_set_protected(..) should be called from add_device(..) not from the dt_xlate(..). Move dt_device_set_protected(..) from dt_xlate(..) to add_device(). Signed-off-by: Rahul Singh <rahul.singh@arm.com> --- This patch is a candidate for 4.15 as without this patch it is not possible to assign multiple StreamIds to the same device when device is protected behind SMMUv3. --- xen/drivers/passthrough/arm/smmu-v3.c | 29 ++++++++++----------------- 1 file changed, 11 insertions(+), 18 deletions(-)