Re: [PATCH v2 2/5] iommu: introduce dom0-iommu option

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 03 August 2018 09:52
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>; Kevin Tian <kevin.tian@intel.com>;
> Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> option
> 
> On Fri, Aug 03, 2018 at 09:35:58AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On
> Behalf
> > > Of Roger Pau Monné
> > > Sent: 03 August 2018 09:14
> > > To: Jan Beulich <JBeulich@suse.com>
> > > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini
> > > <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>; George Dunlap
> > > <George.Dunlap@citrix.com>; Andrew Cooper
> > > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Tim
> > > (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Suravee
> > > Suthikulpanit <suravee.suthikulpanit@amd.com>; xen-devel <xen-
> > > devel@lists.xenproject.org>; Brian Woods <brian.woods@amd.com>
> > > Subject: Re: [Xen-devel] [PATCH v2 2/5] iommu: introduce dom0-iommu
> > > option
> > >
> > > On Thu, Aug 02, 2018 at 02:23:23AM -0600, Jan Beulich wrote:
> > > > >>> On 02.08.18 at 09:46, <kevin.tian@intel.com> wrote:
> > > > >>  From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> > > > >> Sent: Wednesday, August 1, 2018 7:04 PM
> > > > >> --- a/docs/misc/xen-command-line.markdown
> > > > >> +++ b/docs/misc/xen-command-line.markdown
> > > > >> @@ -1150,12 +1150,18 @@ detection of systems known to
> misbehave
> > > > >> upon accesses to that port.
> > > > >>
> > > > >>  > `dom0-passthrough`
> > > > >>
> > > > >> +> **WARNING: This command line option is deprecated, and
> > > superseded
> > > > >> by
> > > > >> +> _dom0-iommu=none_ - using both options in combination is
> > > > >> undefined.**
> > > > >> +
> > > > >
> > > > > in patch description you said 'supersede'... is it better to say that
> > > > > new dom0_iommu is favored if both options are specified than
> > > > > saying 'undefined'?
> > > >
> > > > That would complicate handling (perhaps just slightly, but anyway),
> > > > since we'd have to maintain a second boolean. Without that the
> > > > order on the command line determines behavior. (And I see that in
> > > > the end you've figured that out.)
> > > >
> > > > >> @@ -1198,6 +1204,32 @@ detection of systems known to
> misbehave
> > > upon
> > > > >> accesses to that port.
> > > > >>
> > > > >>  >> Enable IOMMU debugging code (implies `verbose`).
> > > > >>
> > > > >> +### dom0-iommu
> > > > >> +> `= List of [ none | strict | relaxed ]`
> > > > >> +
> > > > >> +> Sub-options are of boolean kind and can be prefixed with `no-` to
> > > effect
> > > > >> the
> > > > >> +> inverse meaning.
> > > > >
> > > > > not important comment, but doesn't "no-none" sound weird? :-)
> > > >
> > > > Only positive (true) values should be permitted for I think all of
> > > > these. I didn't look at the patch yes, so perhaps that's already
> > > > the case.
> > >
> > > For the current set of options introduced in this patch none, strict
> > > or relaxed it doesn't make much sense to allow the no- prefix.
> > >
> > > For options added in later patches (inclusive and reserved) it makes
> > > sense to allow the no- prefix, so that a user can do
> > > 'dom0-iommu=no-inclusive' in order to change the default value.
> > >
> >
> > But what does that mean? 'no-inclusive' clearly means you don't get the
> inclusive map, but what do you get instead? Strict? None?
> 
> IMO you always either get no iommu mappings at all (none), only Dom0
> assigned RAM regions (strict) or all RAM except the regions used by
> Xen (relaxed). Those options control what RAM gets mapped into the
> iommu page tables.
> 
> Then you can use inclusive to even get more mappings of non-RAM
> regions, but that can be used in conjunction with either strict or
> relaxed and should allow the usage of the no- prefix.
> 
> So if you use dom0-iommu=no-inclusive you get the default relaxed RAM
> mappings and that's all.
> 
> I hope this makes sense, I will try to clarify the documentation.
> 

Actually I wonder whether we should rename 'inclusive' to 'reserved'. Essentially 'none', 'strict' or 'relaxed' are all about mappings of RAM, and then we need to decide whether to map the E820 reserved regions. So I think the inclusive map as it stands today is equivalent to 'relaxed' + 'reserved'.

  Paul

> Roger.

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