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 10:09
> 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 10:05:19AM +0100, Paul Durrant wrote:
> > > -----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'.
> 
> Hm, not exactly. inclusive (iommu_inclusive_mapping) right now maps
> everything except unusable regions. That's way more than just mapping
> reserved regions. If we want to keep this behaviour while introducing
> an option to map only reserved regions we need both an inclusive and a
> reserved option.
> 

Ok, how about:

inclusive -> all E820 ranges except unusable or ram
reserved -> all E820 reserved ranges

then

strict -> all ram ranges belonging to dom0
relaxed -> all ram ranges
none -> no ram ranges

The problem then is what does, say, reserved + no-inclusive mean? I guess we could have a flag for each non ram E820 range type?

  Paul


> Roger.

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