Re: [Xen-devel] [PATCH] x86: remove shadow dom0 command line option

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Mar 24, 2017 at 06:30:15PM +0000, Wei Liu wrote:
> On Fri, Mar 24, 2017 at 06:26:53PM +0000, Roger Pau Monné wrote:
> > On Fri, Mar 24, 2017 at 06:22:35PM +0000, Wei Liu wrote:
> > > On Fri, Mar 24, 2017 at 06:17:56PM +0000, Roger Pau Monné wrote:
> > > > On Fri, Mar 24, 2017 at 06:11:49PM +0000, Wei Liu wrote:
> > > > > On Fri, Mar 24, 2017 at 06:09:43PM +0000, George Dunlap wrote:
> > > > > > On 24/03/17 18:07, Roger Pau Monné wrote:
> > > > > > > On Fri, Mar 24, 2017 at 05:51:41PM +0000, Wei Liu wrote:
> > > > > > >> It has been broken for years and couldn't possibly be configured 
> > > > > > >> after
> > > > > > >> 4045953.
> > > > > > >>
> > > > > > >> Remove code and documentation.
> > > > > > >>
> > > > > > >> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > > > >> ---
> > > > > > >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > > > > >> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > > > >> Cc: Tim Deegan <tim@xxxxxxx>
> > > > > > >> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > > > > > >> ---
> > > > > > >>  docs/misc/xen-command-line.markdown | 13 +------------
> > > > > > >>  xen/arch/x86/dom0_build.c           | 10 +---------
> > > > > > >>  xen/arch/x86/pv/dom0_build.c        |  5 -----
> > > > > > >>  xen/arch/x86/setup.c                |  3 +--
> > > > > > >>  4 files changed, 3 insertions(+), 28 deletions(-)
> > > > > > >>
> > > > > > >> diff --git a/docs/misc/xen-command-line.markdown 
> > > > > > >> b/docs/misc/xen-command-line.markdown
> > > > > > >> index bdbdb8a53b..f86b1bb7d2 100644
> > > > > > >> --- a/docs/misc/xen-command-line.markdown
> > > > > > >> +++ b/docs/misc/xen-command-line.markdown
> > > > > > >> @@ -652,11 +652,6 @@ restrictions set up here. Note that the 
> > > > > > >> values to be specified here are
> > > > > > >>  ACPI PXM ones, not Xen internal node numbers. `relaxed` sets up 
> > > > > > >> vCPU
> > > > > > >>  affinities to prefer but be not limited to the specified 
> > > > > > >> node(s).
> > > > > > >>  
> > > > > > >> -### dom0\_shadow
> > > > > > >> -> `= <boolean>`
> > > > > > >> -
> > > > > > >> -This option is deprecated, please use `dom0=shadow` instead.
> > > > > > >> -
> > > > > > >>  ### dom0\_vcpus\_pin
> > > > > > >>  > `= <boolean>`
> > > > > > >>  
> > > > > > >> @@ -665,7 +660,7 @@ This option is deprecated, please use 
> > > > > > >> `dom0=shadow` instead.
> > > > > > >>  Pin dom0 vcpus to their respective pcpus
> > > > > > >>  
> > > > > > >>  ### dom0
> > > > > > >> -> `= List of [ pvh | shadow ]`
> > > > > > >> +> `= List of [ pvh ]`
> > > > > > > 
> > > > > > > The shadow option here needs to be kept, in case someone wants to 
> > > > > > > create a
> > > > > > > PVHv2 Dom0 with shadow instead of hap.
> > > > > > 
> > > > > > You can't change the meaning of "dom0=shadow" from "Make a PV dom0
> > > > > > running in autotranslate mode" to "Make a PVH dom0 running in shadow
> > > > > > mode rather than HAP". :-)
> > > > > 
> > > > > My thought as well.
> > > > > 
> > > > > The semantics can't change.
> > > > > 
> > > > > Shadow PVH dom0 is fine, but it should be added in another patch.
> > > > 
> > > > I'm not following this, the dom0=... option was added by me for PVHv2 
> > > > Dom0,
> > > > dom0=pvh,shadow has always been valid. What's the problem with that?
> > > > 
> > > > It's always been possible to specify dom0=pvh,shadow since PVHv2 Dom0 
> > > > was
> > > > introduced, there's no change here at all.
> > > 
> > > If dom0= has never been released, then we can change it however we like.
> > > 
> > > In that case, we need to update the docs to reflect that.
> > 
> > If what you want to do is make dom0=shadow imply pvh, I think that's wrong.
> > IMHO dom0=shadow alone shouldn't be valid, and should always be
> > dom0=shadow,pvh, as I noted in the comment I've made to the command line
> > documentation and the code that checks the dom0=... string.
> > 
> 
> But the doc as-is suggests it is either "pvh" or "shadow". Or did I
> misunderstand your intent?

Oh, then it's my fault for wording this incorrectly. This option takes a list
of the following sub-options, like the "iommu" option.

> What I suggest is the doc should be updated to
> 
>   = pvh[,shadow]

Not sure if we want to make the pvh sub-option mandatory, maybe at some point
we will also add pv specific sub-options to this list?

Roger.

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