Re: [Xen-devel] [PATCH 2/4] iommu: generalize iommu_inclusive_mapping

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 31 July 2018 09:16
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Kevin Tian <kevin.tian@xxxxxxxxx>;
> Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> George Dunlap <George.Dunlap@xxxxxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Tim
> (Xen.org) <tim@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>; Jan Beulich
> <jbeulich@xxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH 2/4] iommu: generalize
> iommu_inclusive_mapping
> 
> On Tue, Jul 31, 2018 at 08:18:36AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf
> > > Of Roger Pau Monne
> > > Sent: 27 July 2018 16:32
> > > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini
> > > <sstabellini@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; George Dunlap
> > > <George.Dunlap@xxxxxxxxxx>; Andrew Cooper
> > > <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> Tim
> > > (Xen.org) <tim@xxxxxxx>; Julien Grall <julien.grall@xxxxxxx>; Jan
> Beulich
> > > <jbeulich@xxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > > Subject: [Xen-devel] [PATCH 2/4] iommu: generalize
> > > iommu_inclusive_mapping
> > >
> > > Introduce a new iommu=inclusive generic option that supersedes
> > > iommu_inclusive_mapping. This should be a non-functional change on
> > > Intel hardware, while AMD hardware will gain the same functionality of
> > > mapping almost everything below the 4GB boundary.
> > >
> > > Note that is a noop for ARM hardware.
> > >
> > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > ---
> > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: Julien Grall <julien.grall@xxxxxxx>
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > Cc: Tim Deegan <tim@xxxxxxx>
> > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > > ---
> > >  docs/misc/xen-command-line.markdown   | 14 ++++++
> > >  xen/drivers/passthrough/arm/iommu.c   |  4 ++
> > >  xen/drivers/passthrough/iommu.c       |  6 +++
> > >  xen/drivers/passthrough/vtd/extern.h  |  2 -
> > >  xen/drivers/passthrough/vtd/iommu.c   |  6 ---
> > >  xen/drivers/passthrough/vtd/x86/vtd.c | 66 +------------------------
> > >  xen/drivers/passthrough/x86/iommu.c   | 70
> > > +++++++++++++++++++++++++++
> > >  xen/include/xen/iommu.h               |  2 +
> > >  8 files changed, 97 insertions(+), 73 deletions(-)
> > >
> > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> > > command-line.markdown
> > > index 65b4754418..91a8bfc9a6 100644
> > > --- a/docs/misc/xen-command-line.markdown
> > > +++ b/docs/misc/xen-command-line.markdown
> > > @@ -1198,6 +1198,17 @@ detection of systems known to misbehave
> upon
> > > accesses to that port.
> > >
> > >  >> Enable IOMMU debugging code (implies `verbose`).
> > >
> > > +> `inclusive`
> >
> > This is a dom0 (or hwdom) specific setting so perhaps dom0-inclusive?
> >
> > Actually the dom0 iommu options are starting to get unwieldy as they are
> conflated with the general host iommu options so I think it may be
> worthwhile splitting things out into a separate 'dom0-iommu=' top level
> parameter at this stage. (My reasons are slightly selfish as I intend to add
> another dom0 iommu option to give it just reserved regions, to avoid
> unnecessary set-up if we know it will be using PV-IOMMU).
> 
> Mapping just the reserved regions is what I actually do for PVH with
> iommu=inclusive (patch 4/4), so maybe it would make sense to speak about
> the
> naming here in order to use the same naming for PV and PVH.
> 
> TBH I don't really like the dom0- prefix, the command line iommu
> options either apply to all domains or Dom0 only, having
> domu-inclusive for example makes no sense IMO.

No, I think there are some options that you may want to apply to dom0 only, but 
these are more like the dom0_mem or dom0_max_vpus options. Particularly, the 
inclusive option is probably something that is only desirable for dom0. Clearly 
dom0-passthrough and dom0-strict are already defined to relate to dom0 only, 
and options such as 'reserved' should only be specific on the command line in 
relation to dom0 IMO. For other domains such an option should be specified via 
xl.cfg.

  Paul

> 
> Maybe the current iommu_inclusive_mapping could be mapped to
> iommu=inclusive-mapping instead of iommu=inclusive and the new option
> could be added as iommu=reserved-mapping?
> 
> That would work for PVH AFAICT.
> 
> Thanks, Roger.

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