Re: [Xen-devel] [PATCH 1/7] xen/arm: SMMU: Implement the add_device callback in SMMU

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

> -----Original Message-----
> From: Stefano Stabellini [mailto:sstabellini@xxxxxxxxxx]
> Sent: 2017年7月4日 5:58
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxx; sstabellini@xxxxxxxxxx; Steve Capper
> <Steve.Capper@xxxxxxx>; Kaly Xin <Kaly.Xin@xxxxxxx>; Julien Grall
> <Julien.Grall@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Xen-devel] [PATCH 1/7] xen/arm: SMMU: Implement the add_device
> callback in SMMU
> 
> On Fri, 30 Jun 2017, Wei Chen wrote:
> > This add_device callback function is taking care of adding a device
> > to SMMU and make sure it is fully prepare to be used by the SMMU
> > afterwards.
> >
> > In previous code, we don't implement the add_device callback in
> > iommu_ops for ARM SMMU. We placed the work of add_device to
> > assign_device callback. The function assign_device should not care
> > about adding the device to an iommu_group. It might not even be
> > able to decide how to do that. In this patch, we move this work
> > back to add_device callback.
> >
> > This add_device callback is only called while we are handling all
> > devices for constructing the Domain0.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >  xen/drivers/passthrough/arm/smmu.c | 34 +++++++++++++++++++++++-----------
> >  1 file changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> > index 74c09b0..2efa52d 100644
> > --- a/xen/drivers/passthrough/arm/smmu.c
> > +++ b/xen/drivers/passthrough/arm/smmu.c
> > @@ -2591,6 +2591,26 @@ static void arm_smmu_destroy_iommu_domain(struct
> iommu_domain *domain)
> >     xfree(domain);
> >  }
> >
> > +static int arm_smmu_xen_add_device(u8 devfn, struct device*dev)
> > +{
> > +   if (dt_device_is_protected(dev->of_node)) {
> > +           if (!dev->archdata.iommu) {
> > +                   dev->archdata.iommu = xzalloc(struct 
> > arm_smmu_xen_device);
> > +                   if (!dev->archdata.iommu)
> > +                           return -ENOMEM;
> > +           }
> > +
> > +           if (!dev_iommu_group(dev))
> > +                   return arm_smmu_add_device(dev);
> > +   }
> > +
> > +   /*
> > +    * Return 0 if the device is not protected to follow the behavior
> > +    * of PCI add device.
> 
> What does this comment mean?
> 

While I was looking at the iommu_add_device which is used by PCI counterpart.
I found it will always return 0 even for device that are not protected by an 
IOMMU.
I would much prefer to keep platform devices have similar behavior as PCI 
devices.

So while I was implementing iommu_add_dt_device and arm_smmu_xen_add_device, I
returned 0 if the device is not protected by IOMMU.

> 
> > +    */
> > +   return 0;
> > +}
> > +
> >  static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
> >                            struct device *dev, u32 flag)
> >  {
> > @@ -2600,17 +2620,8 @@ static int arm_smmu_assign_dev(struct domain *d, u8
> devfn,
> >
> >     xen_domain = dom_iommu(d)->arch.priv;
> >
> > -   if (!dev->archdata.iommu) {
> > -           dev->archdata.iommu = xzalloc(struct arm_smmu_xen_device);
> > -           if (!dev->archdata.iommu)
> > -                   return -ENOMEM;
> > -   }
> > -
> > -   if (!dev_iommu_group(dev)) {
> > -           ret = arm_smmu_add_device(dev);
> > -           if (ret)
> > -                   return ret;
> > -   }
> > +   if (!dev_iommu_group(dev))
> > +       return -ENODEV;
> >
> >     spin_lock(&xen_domain->lock);
> >
> > @@ -2784,6 +2795,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
> >      .teardown = arm_smmu_iommu_domain_teardown,
> >      .iotlb_flush = arm_smmu_iotlb_flush,
> >      .iotlb_flush_all = arm_smmu_iotlb_flush_all,
> > +    .add_device = arm_smmu_xen_add_device,
> >      .assign_device = arm_smmu_assign_dev,
> >      .reassign_device = arm_smmu_reassign_dev,
> >      .map_page = arm_smmu_map_page,
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel
> >

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel