Re: [Xen-devel] [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 05 October 2018 15:51
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Brian Woods <brian.woods@xxxxxxx>; Suravee Suthikulpanit
> <suravee.suthikulpanit@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian Jackson
> <Ian.Jackson@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin
> Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>
> Subject: Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into
> has_iommu_pt() and need_iommu_pt_sync()
> 
> >>> On 04.10.18 at 12:45, <paul.durrant@xxxxxxxxxx> wrote:
> > The name 'need_iommu()' is a little confusing as it suggests a domain
> needs
> > to use the IOMMU but something might not be set up yet, when in fact it
> > represents a tri-state value (not a boolean as might be expected) where
> > -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> > been fully set up'.
> >
> > Two different meanings are also inferred from the macro it in various
> > places in the code:
> >
> > - Some callers want to test whether a domain has IOMMU mappings at all
> > - Some callers want to test whether they need to synchronize the
> domain's
> >   P2M and IOMMU mappings
> >
> > This patch replaces the 'need_iommu' tri-state value with a defined
> > enumeration and adds a boolean flag 'need_sync' to separate these
> meanings,
> > and places both of these in struct domain_iommu, rather than directly in
> > struct domain.
> > This patch also creates two new boolean macros:
> >
> > - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings,
> even
> >   if they are still under construction.
> > - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
> >   synchronization of the P2M and IOMMU mappings.
> >
> > All callers of need_iommu() are then modified to use the macro
> appropriate
> > to what they are trying to test, except for the instance in
> > xen/drivers/passthrough/pci.c:assign_device() which has simply been
> > removed since it appears to be unnecessary.
> >
> > NOTE: There are some callers of need_iommu() that strictly operate on
> >       the hardware domain. In some of these case a more global flag is
> >       used instead.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Acked-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> 
> Presumably as a result of leaving out patch 4, this patch had quite a
> bit of fuzz when applying. Would you please double check that I
> didn't screw things up?
> 

Certainly the commit looks fine. I'll re-base my PV-IOMMU series and re-test 
it... probably not until next week though.

  Paul

> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel