xen/drivers/passthrough/arm/smmu-v3.c | 29 ++++++++++----------------- 1 file changed, 11 insertions(+), 18 deletions(-)
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(-)
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 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, -- Julien Grall
© 2016 - 2024 Red Hat, Inc.