Re: [Xen-devel] [PATCH v3 13/13] xen/iommu: smmu: Advertise when the SMMU support coherent table walk

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Stefano,

On 06/02/2015 22:06, Stefano Stabellini wrote:
> On Fri, 30 Jan 2015, Julien Grall wrote:
> > When SMMU doesn't support coherent table walk, Xen may need to clean
> > updated PT (see commit 4c5f4cb "xen/arm: p2m: Clean cache PT when the
> > IOMMU doesn't support coherent walk").
> >
> > If one SMMU of the platform doesn't support coherent table walk, the
> > feature is disabled for the whole platform. This is because device is
> > assigned to a domain after the page table are populated.
> >
> > This could impact performance on domain which doesn't use device
> > passthrough. But, as the spec strongly recommends the support of this
> > feature for maintstream platform, I expect server will always have SMMUs
> > supporting coherent table walk. If not, we may need to enable this feature
> > per-domain.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >
> > ---
> >      I've just noticed that the support on the previous driver (i.e in
> >      Xen 4.5) may incorrectly expose this feature when all the SMMUs is
> >      not supporting coherent table walk.
> >
> >      I'm not sure, if I should send a patch for it.
> >
> >      Also I didn't squash this patch into "xen/iommu: smmu: Add Xen specific
> >      code to be able to use the driver" to help for review and to catch
> >      possible error in this patch.
> >
> >      Changes in v3:
> >          - Patch added
> > ---
> >   xen/drivers/passthrough/arm/smmu.c | 25 +++++++++++++++++++++++++
> >   1 file changed, 25 insertions(+)
> >
> > diff --git a/xen/drivers/passthrough/arm/smmu.c 
> > b/xen/drivers/passthrough/arm/smmu.c
> > index 373eee8..f4c7e49 100644
> > --- a/xen/drivers/passthrough/arm/smmu.c
> > +++ b/xen/drivers/passthrough/arm/smmu.c
> > @@ -577,6 +577,13 @@ struct arm_smmu_domain {
> >         spinlock_t                      lock;
> >   };
> >
> > +/*
> > + * Xen: Platform features. It indicates the list of features support by all the
> > + * SMMUs.
> > + * Actually we only care about coherent table walk.
> > + */
> > +static u32 platform_features = ARM_SMMU_FEAT_COHERENT_WALK;
> > +
> >   /* Xen: Dummy iommu_domain */
> >   struct iommu_domain
> >   {
> > @@ -2810,6 +2817,13 @@ static int arm_smmu_iommu_domain_init(struct domain *d)
> >
> >         domain_hvm_iommu(d)->arch.priv = xen_domain;
> >
> > +       /*
> > +        * The feature coherent walk can be enabled only when all SMMUs
> > +        * support it.
> > +        */
> > +       if (platform_features & ARM_SMMU_FEAT_COHERENT_WALK)
> > +               iommu_set_feature(d, IOMMU_FEAT_COHERENT_WALK);
> > +
> >         return 0;
> >   }
> As long as you are sure that arm_smmu_iommu_domain_init is called after
> all the iommus have been initialized by arm_smmu_dt_init, then this
> patch is correct.
We don't support SMMU hotplug on Xen :). All the SMMUs are initialize 
via iommu_setup before DOM0 is created.
Regards,

--
Julien Grall

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