Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Juergen Gross]

On 17/01/2019 12:59, Jan Beulich wrote:
>>>> On 17.01.19 at 10:14, <jgross@suse.com> wrote:
>> On 17/01/2019 10:08, Andrew Cooper wrote:
>>> On 17/01/2019 08:43, Roger Pau Monné wrote:
>>>> On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
>>>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of 
>> Xen.
>>>>>>>  
>>>>>>>  Specify the bit width of the DMA heap.
>>>>>>>  
>>>>>>> -### dom0 (x86)
>>>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>>>> +### dom0
>>>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>>>  
>>>>>>> -> Sub-options:
>>>>>>> +    Applicability: x86
>>>>>>>  
>>>>>>> -> `pvh`
>>>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>>>  
>>>>>>> -> Default: `false`
>>>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>>>> +    guest.  The default is PV.  In addition, the following requirements 
>> must
>>>>>>> +    be met:
>>>>>>>  
>>>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>>>> +        selected mode.
>>>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` 
>> enabled.
>>>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` 
>> enabled,
>>>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>>>  
>>>>>>> -> `shadow`
>>>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a 
>> PVH
>>>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or 
>> shadow
>>>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>>>  
>>>>>>> -> Default: `false`
>>>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out. 
>>  A
>>>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and 
>> the
>>>>>>> +    hardware is not HAP-capable.
>>>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>>>> which is intended to be meaningful to non-developers. But not to the
>>>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>>>> I'm not sure what else to say.  I object to purposefully omitting
>>>>> relevant information from our documentation.
>>>> Maybe it would be helpful to add some kind of tag that could
>>>> standardize the relationship between Kconfig options and command line
>>>> options?
>>>>
>>>>     Kconfig: SHADOW_PAGING
>>>>
>>>> Or similar. This would get the specific Kconfig details out of the
>>>> general description of the functionality, thus not harming readability
>>>> by non-expert users?
>>>>
>>>> Using such tag would require some explanation of it's meaning at the
>>>> top of the document.
>>>>
>>>>> Most people reading it, including non-developers, will know what Kconfig
>>>>> is and how to check, owing to at least a basic knowledge of Linux. 
>>>>> Those that don't will be capable of basic human interaction such as
>>>>> asking a question of someone more knowledgeable.
>>>> If the above is not suitable, I might suggest to reword the sentence
>>>> as:
>>>>
>>>> "This option is unavailable when the Kconfig `SHADOW_PAGING` option is
>>>> not selected at build time."
>>>>
>>>> Explicitly mentioning Kconfig and selected simplifies the language for
>>>> non-expert users IMO, and makes it clear this is exclusively a build
>>>> time decision. Note I'm not a native speaker, so my sense of easier to
>>>> understand could be completely wrong.
>>>
>>> I have a rewrite of the head of the document pending anyway which I hope
>>> to get sorted properly for 4.12
>>>
>>> While having a Kconfig: section would probably be fine for ~80% of the
>>> simple cases, it doesn't work for this patch.
>>>
>>> I guess the root of the issue is that I do not believe that phrasing the
>>> information like this makes it harder for non-expert users
>>> read/comprehend, and there definitely are a group of readers for which
>>> this information is relevant.
>>
>> In any case I'd prefer to spell out the complete config option (e.g.
>> CONFIG_FOO) in case we ever get a way to read the config from the
>> running hypervisor (FWIW I'm just writing a series for doing that).
> 
> Well, as expressed in earlier replies - I'm particularly against the
> CONFIG_ prefix, which no-one will find when grep-ing Kconfig
> files. This prefix is an implementation detail, to reduce the risk of
> name collisions. FWIW I'm very much in favor of going with either
> of the variants suggested by Roger; it is really secondary to me
> which of the two was chosen.

Is an admin looking for Xen command line parameters expected to grep
Kconfig files? A developer knowing about Kconfig files can be expected
to skip the CONFIG_ prefix when looking into those.

In case we don't want to add a way to get the config file from the just
running hypervisor we still have /boot/xen-*.config which includes the
CONFIG_ prefixes.

I'd decide to make the doc admin-friendly. The "implementation detail"
is visible to the admin/user and the un-prefixed config option is not.


Juergen

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 17/01/2019 12:26, Jan Beulich wrote:
>>>> On 17.01.19 at 13:15, <andrew.cooper3@citrix.com> wrote:
>> On 17/01/2019 11:20, Jan Beulich wrote:
>>>>>> On 16.01.19 at 20:51, <andrew.cooper3@citrix.com> wrote:
>>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>>>  
>>>>>>  Specify the bit width of the DMA heap.
>>>>>>  
>>>>>> -### dom0 (x86)
>>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>>> +### dom0
>>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>>  
>>>>>> -> Sub-options:
>>>>>> +    Applicability: x86
>>>>>>  
>>>>>> -> `pvh`
>>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>>>> +    be met:
>>>>>>  
>>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>>> +        selected mode.
>>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>>  
>>>>>> -> `shadow`
>>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>>>> +    hardware is not HAP-capable.
>>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>>> which is intended to be meaningful to non-developers. But not to the
>>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>>> I'm not sure what else to say.  I object to purposefully omitting
>>>> relevant information from our documentation.
>>> But I'm not asking to omit the information. I'm asking to present it
>>> in a way understandable to anyone, irrespective of their Kconfig
>>> knowledge.
>> You have literally contradicted yourself in your two replies here.
>>
>> Your latest reply suggests that you didn't mean what you actually wrote
>> earlier.  If this is the case, please take more care to get your point
>> across clearly.
> Hmm, apologies, my use CONFIG_* above was indeed ambiguous
> without the context implied by "mentioned elsewhere". As "mentioned
> elsewhere" I'm fine with any form of wording that names the option
> without making it a primary part of the text, and without the CONFIG_
> prefix. As also said elsewhere (but perhaps due to not being a native
> speaker) I also dislike your use of "compiled out" (and its inverse).
> Both parts get addressed by either of Roger's suggestions.

I've switched to phrasing things as enabled/disabled rather than
compiled in/out.

However, the CONFIG_ prefixes are staying, because as said by myself and
others in this thread, they are critical for the understanding of
non-expert users, who won't be aware of implicitly dropping or adding
the prefix depending on circumstance.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Jan Beulich]

>>> On 17.01.19 at 13:15, <andrew.cooper3@citrix.com> wrote:
> On 17/01/2019 11:20, Jan Beulich wrote:
>>>>> On 16.01.19 at 20:51, <andrew.cooper3@citrix.com> wrote:
>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>>  
>>>>>  Specify the bit width of the DMA heap.
>>>>>  
>>>>> -### dom0 (x86)
>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>> +### dom0
>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>  
>>>>> -> Sub-options:
>>>>> +    Applicability: x86
>>>>>  
>>>>> -> `pvh`
>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>  
>>>>> -> Default: `false`
>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>>> +    be met:
>>>>>  
>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>> +        selected mode.
>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>  
>>>>> -> `shadow`
>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>  
>>>>> -> Default: `false`
>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>>> +    hardware is not HAP-capable.
>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>> which is intended to be meaningful to non-developers. But not to the
>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>> I'm not sure what else to say.  I object to purposefully omitting
>>> relevant information from our documentation.
>> But I'm not asking to omit the information. I'm asking to present it
>> in a way understandable to anyone, irrespective of their Kconfig
>> knowledge.
> 
> You have literally contradicted yourself in your two replies here.
> 
> Your latest reply suggests that you didn't mean what you actually wrote
> earlier.  If this is the case, please take more care to get your point
> across clearly.

Hmm, apologies, my use CONFIG_* above was indeed ambiguous
without the context implied by "mentioned elsewhere". As "mentioned
elsewhere" I'm fine with any form of wording that names the option
without making it a primary part of the text, and without the CONFIG_
prefix. As also said elsewhere (but perhaps due to not being a native
speaker) I also dislike your use of "compiled out" (and its inverse).
Both parts get addressed by either of Roger's suggestions.

Jan



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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 17/01/2019 11:20, Jan Beulich wrote:
>>>> On 16.01.19 at 20:51, <andrew.cooper3@citrix.com> wrote:
>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/docs/misc/xen-command-line.pandoc
>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>  
>>>>  Specify the bit width of the DMA heap.
>>>>  
>>>> -### dom0 (x86)
>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>> +### dom0
>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>  
>>>> -> Sub-options:
>>>> +    Applicability: x86
>>>>  
>>>> -> `pvh`
>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>  
>>>> -> Default: `false`
>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>> +    be met:
>>>>  
>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>> +        selected mode.
>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>  
>>>> -> `shadow`
>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>  
>>>> -> Default: `false`
>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>> +    hardware is not HAP-capable.
>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>> which is intended to be meaningful to non-developers. But not to the
>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>> I'm not sure what else to say.  I object to purposefully omitting
>> relevant information from our documentation.
> But I'm not asking to omit the information. I'm asking to present it
> in a way understandable to anyone, irrespective of their Kconfig
> knowledge.

You have literally contradicted yourself in your two replies here.

Your latest reply suggests that you didn't mean what you actually wrote
earlier.  If this is the case, please take more care to get your point
across clearly.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 17/01/2019 09:14, Juergen Gross wrote:
> On 17/01/2019 10:08, Andrew Cooper wrote:
>> On 17/01/2019 08:43, Roger Pau Monné wrote:
>>> On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
>>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>>>  
>>>>>>  Specify the bit width of the DMA heap.
>>>>>>  
>>>>>> -### dom0 (x86)
>>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>>> +### dom0
>>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>>  
>>>>>> -> Sub-options:
>>>>>> +    Applicability: x86
>>>>>>  
>>>>>> -> `pvh`
>>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>>>> +    be met:
>>>>>>  
>>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>>> +        selected mode.
>>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>>  
>>>>>> -> `shadow`
>>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>>>> +    hardware is not HAP-capable.
>>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>>> which is intended to be meaningful to non-developers. But not to the
>>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>>> I'm not sure what else to say.  I object to purposefully omitting
>>>> relevant information from our documentation.
>>> Maybe it would be helpful to add some kind of tag that could
>>> standardize the relationship between Kconfig options and command line
>>> options?
>>>
>>>     Kconfig: SHADOW_PAGING
>>>
>>> Or similar. This would get the specific Kconfig details out of the
>>> general description of the functionality, thus not harming readability
>>> by non-expert users?
>>>
>>> Using such tag would require some explanation of it's meaning at the
>>> top of the document.
>>>
>>>> Most people reading it, including non-developers, will know what Kconfig
>>>> is and how to check, owing to at least a basic knowledge of Linux. 
>>>> Those that don't will be capable of basic human interaction such as
>>>> asking a question of someone more knowledgeable.
>>> If the above is not suitable, I might suggest to reword the sentence
>>> as:
>>>
>>> "This option is unavailable when the Kconfig `SHADOW_PAGING` option is
>>> not selected at build time."
>>>
>>> Explicitly mentioning Kconfig and selected simplifies the language for
>>> non-expert users IMO, and makes it clear this is exclusively a build
>>> time decision. Note I'm not a native speaker, so my sense of easier to
>>> understand could be completely wrong.
>> I have a rewrite of the head of the document pending anyway which I hope
>> to get sorted properly for 4.12
>>
>> While having a Kconfig: section would probably be fine for ~80% of the
>> simple cases, it doesn't work for this patch.
>>
>> I guess the root of the issue is that I do not believe that phrasing the
>> information like this makes it harder for non-expert users
>> read/comprehend, and there definitely are a group of readers for which
>> this information is relevant.
> In any case I'd prefer to spell out the complete config option (e.g.
> CONFIG_FOO) in case we ever get a way to read the config from the
> running hypervisor (FWIW I'm just writing a series for doing that).

I think having a Xen equivalent of /proc/config.gz is a good move,
irrespective of any documentation concerns.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Jan Beulich]

>>> On 17.01.19 at 10:14, <jgross@suse.com> wrote:
> On 17/01/2019 10:08, Andrew Cooper wrote:
>> On 17/01/2019 08:43, Roger Pau Monné wrote:
>>> On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
>>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of 
> Xen.
>>>>>>  
>>>>>>  Specify the bit width of the DMA heap.
>>>>>>  
>>>>>> -### dom0 (x86)
>>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>>> +### dom0
>>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>>  
>>>>>> -> Sub-options:
>>>>>> +    Applicability: x86
>>>>>>  
>>>>>> -> `pvh`
>>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>>> +    guest.  The default is PV.  In addition, the following requirements 
> must
>>>>>> +    be met:
>>>>>>  
>>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>>> +        selected mode.
>>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` 
> enabled.
>>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` 
> enabled,
>>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>>  
>>>>>> -> `shadow`
>>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a 
> PVH
>>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or 
> shadow
>>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>>  
>>>>>> -> Default: `false`
>>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out. 
>  A
>>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and 
> the
>>>>>> +    hardware is not HAP-capable.
>>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>>> which is intended to be meaningful to non-developers. But not to the
>>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>>> I'm not sure what else to say.  I object to purposefully omitting
>>>> relevant information from our documentation.
>>> Maybe it would be helpful to add some kind of tag that could
>>> standardize the relationship between Kconfig options and command line
>>> options?
>>>
>>>     Kconfig: SHADOW_PAGING
>>>
>>> Or similar. This would get the specific Kconfig details out of the
>>> general description of the functionality, thus not harming readability
>>> by non-expert users?
>>>
>>> Using such tag would require some explanation of it's meaning at the
>>> top of the document.
>>>
>>>> Most people reading it, including non-developers, will know what Kconfig
>>>> is and how to check, owing to at least a basic knowledge of Linux. 
>>>> Those that don't will be capable of basic human interaction such as
>>>> asking a question of someone more knowledgeable.
>>> If the above is not suitable, I might suggest to reword the sentence
>>> as:
>>>
>>> "This option is unavailable when the Kconfig `SHADOW_PAGING` option is
>>> not selected at build time."
>>>
>>> Explicitly mentioning Kconfig and selected simplifies the language for
>>> non-expert users IMO, and makes it clear this is exclusively a build
>>> time decision. Note I'm not a native speaker, so my sense of easier to
>>> understand could be completely wrong.
>> 
>> I have a rewrite of the head of the document pending anyway which I hope
>> to get sorted properly for 4.12
>> 
>> While having a Kconfig: section would probably be fine for ~80% of the
>> simple cases, it doesn't work for this patch.
>> 
>> I guess the root of the issue is that I do not believe that phrasing the
>> information like this makes it harder for non-expert users
>> read/comprehend, and there definitely are a group of readers for which
>> this information is relevant.
> 
> In any case I'd prefer to spell out the complete config option (e.g.
> CONFIG_FOO) in case we ever get a way to read the config from the
> running hypervisor (FWIW I'm just writing a series for doing that).

Well, as expressed in earlier replies - I'm particularly against the
CONFIG_ prefix, which no-one will find when grep-ing Kconfig
files. This prefix is an implementation detail, to reduce the risk of
name collisions. FWIW I'm very much in favor of going with either
of the variants suggested by Roger; it is really secondary to me
which of the two was chosen.

Jan


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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Jan Beulich]

>>> On 16.01.19 at 20:51, <andrew.cooper3@citrix.com> wrote:
> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>> --- a/docs/misc/xen-command-line.pandoc
>>> +++ b/docs/misc/xen-command-line.pandoc
>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>  
>>>  Specify the bit width of the DMA heap.
>>>  
>>> -### dom0 (x86)
>>> -> `= List of [ pvh | shadow | verbose ]`
>>> +### dom0
>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>  
>>> -> Sub-options:
>>> +    Applicability: x86
>>>  
>>> -> `pvh`
>>> +Controls for how dom0 is constructed on x86 systems.
>>>  
>>> -> Default: `false`
>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>> +    guest.  The default is PV.  In addition, the following requirements must
>>> +    be met:
>>>  
>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>> +        selected mode.
>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>> +        and the hardware must have VT-x/SVM extensions available.
>>>  
>>> -> `shadow`
>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>  
>>> -> Default: `false`
>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>> +    hardware is not HAP-capable.
>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>> which is intended to be meaningful to non-developers. But not to the
>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
> 
> I'm not sure what else to say.  I object to purposefully omitting
> relevant information from our documentation.

But I'm not asking to omit the information. I'm asking to present it
in a way understandable to anyone, irrespective of their Kconfig
knowledge.

Jan



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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Juergen Gross]

On 17/01/2019 10:08, Andrew Cooper wrote:
> On 17/01/2019 08:43, Roger Pau Monné wrote:
>> On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
>>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>>> --- a/docs/misc/xen-command-line.pandoc
>>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>>  
>>>>>  Specify the bit width of the DMA heap.
>>>>>  
>>>>> -### dom0 (x86)
>>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>>> +### dom0
>>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>>  
>>>>> -> Sub-options:
>>>>> +    Applicability: x86
>>>>>  
>>>>> -> `pvh`
>>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>>  
>>>>> -> Default: `false`
>>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>>> +    be met:
>>>>>  
>>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>>> +        selected mode.
>>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>>  
>>>>> -> `shadow`
>>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>>  
>>>>> -> Default: `false`
>>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>>> +    hardware is not HAP-capable.
>>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>>> which is intended to be meaningful to non-developers. But not to the
>>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>>> I'm not sure what else to say.  I object to purposefully omitting
>>> relevant information from our documentation.
>> Maybe it would be helpful to add some kind of tag that could
>> standardize the relationship between Kconfig options and command line
>> options?
>>
>>     Kconfig: SHADOW_PAGING
>>
>> Or similar. This would get the specific Kconfig details out of the
>> general description of the functionality, thus not harming readability
>> by non-expert users?
>>
>> Using such tag would require some explanation of it's meaning at the
>> top of the document.
>>
>>> Most people reading it, including non-developers, will know what Kconfig
>>> is and how to check, owing to at least a basic knowledge of Linux. 
>>> Those that don't will be capable of basic human interaction such as
>>> asking a question of someone more knowledgeable.
>> If the above is not suitable, I might suggest to reword the sentence
>> as:
>>
>> "This option is unavailable when the Kconfig `SHADOW_PAGING` option is
>> not selected at build time."
>>
>> Explicitly mentioning Kconfig and selected simplifies the language for
>> non-expert users IMO, and makes it clear this is exclusively a build
>> time decision. Note I'm not a native speaker, so my sense of easier to
>> understand could be completely wrong.
> 
> I have a rewrite of the head of the document pending anyway which I hope
> to get sorted properly for 4.12
> 
> While having a Kconfig: section would probably be fine for ~80% of the
> simple cases, it doesn't work for this patch.
> 
> I guess the root of the issue is that I do not believe that phrasing the
> information like this makes it harder for non-expert users
> read/comprehend, and there definitely are a group of readers for which
> this information is relevant.

In any case I'd prefer to spell out the complete config option (e.g.
CONFIG_FOO) in case we ever get a way to read the config from the
running hypervisor (FWIW I'm just writing a series for doing that).


Juergen

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 17/01/2019 08:43, Roger Pau Monné wrote:
> On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
>> On 16/01/2019 11:52, Jan Beulich wrote:
>>>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/docs/misc/xen-command-line.pandoc
>>>> +++ b/docs/misc/xen-command-line.pandoc
>>>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>>>  
>>>>  Specify the bit width of the DMA heap.
>>>>  
>>>> -### dom0 (x86)
>>>> -> `= List of [ pvh | shadow | verbose ]`
>>>> +### dom0
>>>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>>>  
>>>> -> Sub-options:
>>>> +    Applicability: x86
>>>>  
>>>> -> `pvh`
>>>> +Controls for how dom0 is constructed on x86 systems.
>>>>  
>>>> -> Default: `false`
>>>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>>>> +    guest.  The default is PV.  In addition, the following requirements must
>>>> +    be met:
>>>>  
>>>> -Flag that makes a dom0 boot in PVHv2 mode.
>>>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>>>> +        selected mode.
>>>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>>>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>>>> +        and the hardware must have VT-x/SVM extensions available.
>>>>  
>>>> -> `shadow`
>>>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>>>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>>>> +    paging.  The default is HAP when available, and shadow otherwise.
>>>>  
>>>> -> Default: `false`
>>>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>>>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>>>> +    hardware is not HAP-capable.
>>> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
>>> which is intended to be meaningful to non-developers. But not to the
>>> degree of NAK-ing the whole thing, if everyone else disagrees with me.
>> I'm not sure what else to say.  I object to purposefully omitting
>> relevant information from our documentation.
> Maybe it would be helpful to add some kind of tag that could
> standardize the relationship between Kconfig options and command line
> options?
>
>     Kconfig: SHADOW_PAGING
>
> Or similar. This would get the specific Kconfig details out of the
> general description of the functionality, thus not harming readability
> by non-expert users?
>
> Using such tag would require some explanation of it's meaning at the
> top of the document.
>
>> Most people reading it, including non-developers, will know what Kconfig
>> is and how to check, owing to at least a basic knowledge of Linux. 
>> Those that don't will be capable of basic human interaction such as
>> asking a question of someone more knowledgeable.
> If the above is not suitable, I might suggest to reword the sentence
> as:
>
> "This option is unavailable when the Kconfig `SHADOW_PAGING` option is
> not selected at build time."
>
> Explicitly mentioning Kconfig and selected simplifies the language for
> non-expert users IMO, and makes it clear this is exclusively a build
> time decision. Note I'm not a native speaker, so my sense of easier to
> understand could be completely wrong.

I have a rewrite of the head of the document pending anyway which I hope
to get sorted properly for 4.12

While having a Kconfig: section would probably be fine for ~80% of the
simple cases, it doesn't work for this patch.

I guess the root of the issue is that I do not believe that phrasing the
information like this makes it harder for non-expert users
read/comprehend, and there definitely are a group of readers for which
this information is relevant.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Roger Pau Monné]

On Wed, Jan 16, 2019 at 07:51:33PM +0000, Andrew Cooper wrote:
> On 16/01/2019 11:52, Jan Beulich wrote:
> >>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
> >> --- a/docs/misc/xen-command-line.pandoc
> >> +++ b/docs/misc/xen-command-line.pandoc
> >> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
> >>  
> >>  Specify the bit width of the DMA heap.
> >>  
> >> -### dom0 (x86)
> >> -> `= List of [ pvh | shadow | verbose ]`
> >> +### dom0
> >> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
> >>  
> >> -> Sub-options:
> >> +    Applicability: x86
> >>  
> >> -> `pvh`
> >> +Controls for how dom0 is constructed on x86 systems.
> >>  
> >> -> Default: `false`
> >> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
> >> +    guest.  The default is PV.  In addition, the following requirements must
> >> +    be met:
> >>  
> >> -Flag that makes a dom0 boot in PVHv2 mode.
> >> +    *   The dom0 kernel selected by the boot loader must be capable of the
> >> +        selected mode.
> >> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
> >> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
> >> +        and the hardware must have VT-x/SVM extensions available.
> >>  
> >> -> `shadow`
> >> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
> >> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
> >> +    paging.  The default is HAP when available, and shadow otherwise.
> >>  
> >> -> Default: `false`
> >> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
> >> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
> >> +    hardware is not HAP-capable.
> > As mentioned elsewhere, I object to adding CONFIG_* into this doc,
> > which is intended to be meaningful to non-developers. But not to the
> > degree of NAK-ing the whole thing, if everyone else disagrees with me.
> 
> I'm not sure what else to say.  I object to purposefully omitting
> relevant information from our documentation.

Maybe it would be helpful to add some kind of tag that could
standardize the relationship between Kconfig options and command line
options?

    Kconfig: SHADOW_PAGING

Or similar. This would get the specific Kconfig details out of the
general description of the functionality, thus not harming readability
by non-expert users?

Using such tag would require some explanation of it's meaning at the
top of the document.

> Most people reading it, including non-developers, will know what Kconfig
> is and how to check, owing to at least a basic knowledge of Linux. 
> Those that don't will be capable of basic human interaction such as
> asking a question of someone more knowledgeable.

If the above is not suitable, I might suggest to reword the sentence
as:

"This option is unavailable when the Kconfig `SHADOW_PAGING` option is
not selected at build time."

Explicitly mentioning Kconfig and selected simplifies the language for
non-expert users IMO, and makes it clear this is exclusively a build
time decision. Note I'm not a native speaker, so my sense of easier to
understand could be completely wrong.

Thanks, Roger.

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 16/01/2019 10:53, Roger Pau Monné wrote:
> On Wed, Jan 16, 2019 at 09:00:44AM +0000, Andrew Cooper wrote:
>> Update to the latest metadata style, and discuss the options more
>> completely where appropriate.
>>
>> Drop the redundant comment beside parse_dom0_param() - it is already out
>> of sync with the main documentation.  Also drop the individual
>> documentation for deprecated options which refer to their newer
>> versions, for the same reason.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
> Just one comment.
>
> [...]
>> +*   The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up
>> +    identity IOMMU mappings for all non-RAM regions below 4GB except for
>> +    unusable ranges, and ranges belonging to Xen.
>> +
>> +    Typically, some devices in a system use bits of RAM for communication, and
>> +    these areas should be listed as reserved in the E820 table and identified
>> +    via RMRR or IVMD entries in the APCI tables, so Xen can ensure that they
>> +    are identity-mapped in the IOMMU.  However, some firmware makes mistakes,
>> +    and this option is a coarse-grain workaround for those errors.
>> +
>> +    Where possible, finer grain corrections should be made with the `rmrr=`,
>> +    `ivrs_hpet=` or `ivrs_ioapic=` command line options.
>> +
>> +    This option is enabled by default on x86 systems, and invalid on ARM
>> +    systems.
>> +
>> +*   The `map-reserved` functionality is very similar to `map-inclusive`, but is
>> +    applicable to both x86 PV and PVH dom0's, and represents a subset of the
>> +    correction by only mapping reserved memory regions rather than all non-RAM
>> +    regions.
> Should you add "This option is enabled by default on x86 systems, and
> invalid on ARM systems." here?

What I listed here where the differences from map-inclusive, so the ARM
bit is covered IMO.

However, this bit of text doesn't survive past patch 6 anyway, so it
really doesn't matter.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

On 16/01/2019 11:52, Jan Beulich wrote:
>>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>>  
>>  Specify the bit width of the DMA heap.
>>  
>> -### dom0 (x86)
>> -> `= List of [ pvh | shadow | verbose ]`
>> +### dom0
>> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>>  
>> -> Sub-options:
>> +    Applicability: x86
>>  
>> -> `pvh`
>> +Controls for how dom0 is constructed on x86 systems.
>>  
>> -> Default: `false`
>> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
>> +    guest.  The default is PV.  In addition, the following requirements must
>> +    be met:
>>  
>> -Flag that makes a dom0 boot in PVHv2 mode.
>> +    *   The dom0 kernel selected by the boot loader must be capable of the
>> +        selected mode.
>> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
>> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
>> +        and the hardware must have VT-x/SVM extensions available.
>>  
>> -> `shadow`
>> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
>> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
>> +    paging.  The default is HAP when available, and shadow otherwise.
>>  
>> -> Default: `false`
>> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
>> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
>> +    hardware is not HAP-capable.
> As mentioned elsewhere, I object to adding CONFIG_* into this doc,
> which is intended to be meaningful to non-developers. But not to the
> degree of NAK-ing the whole thing, if everyone else disagrees with me.

I'm not sure what else to say.  I object to purposefully omitting
relevant information from our documentation.

Most people reading it, including non-developers, will know what Kconfig
is and how to check, owing to at least a basic knowledge of Linux. 
Those that don't will be capable of basic human interaction such as
asking a question of someone more knowledgeable.

~Andrew

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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Jan Beulich]

>>> On 16.01.19 at 10:00, <andrew.cooper3@citrix.com> wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
>  
>  Specify the bit width of the DMA heap.
>  
> -### dom0 (x86)
> -> `= List of [ pvh | shadow | verbose ]`
> +### dom0
> +    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
>  
> -> Sub-options:
> +    Applicability: x86
>  
> -> `pvh`
> +Controls for how dom0 is constructed on x86 systems.
>  
> -> Default: `false`
> +*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
> +    guest.  The default is PV.  In addition, the following requirements must
> +    be met:
>  
> -Flag that makes a dom0 boot in PVHv2 mode.
> +    *   The dom0 kernel selected by the boot loader must be capable of the
> +        selected mode.
> +    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
> +    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
> +        and the hardware must have VT-x/SVM extensions available.
>  
> -> `shadow`
> +*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
> +    guest, and controls whether dom0 uses hardware assisted paging, or shadow
> +    paging.  The default is HAP when available, and shadow otherwise.
>  
> -> Default: `false`
> +    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
> +    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
> +    hardware is not HAP-capable.

As mentioned elsewhere, I object to adding CONFIG_* into this doc,
which is intended to be meaningful to non-developers. But not to the
degree of NAK-ing the whole thing, if everyone else disagrees with me.

Jan



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

Re: [PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Roger Pau Monné]

On Wed, Jan 16, 2019 at 09:00:44AM +0000, Andrew Cooper wrote:
> Update to the latest metadata style, and discuss the options more
> completely where appropriate.
> 
> Drop the redundant comment beside parse_dom0_param() - it is already out
> of sync with the main documentation.  Also drop the individual
> documentation for deprecated options which refer to their newer
> versions, for the same reason.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Just one comment.

[...]
> +*   The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up
> +    identity IOMMU mappings for all non-RAM regions below 4GB except for
> +    unusable ranges, and ranges belonging to Xen.
> +
> +    Typically, some devices in a system use bits of RAM for communication, and
> +    these areas should be listed as reserved in the E820 table and identified
> +    via RMRR or IVMD entries in the APCI tables, so Xen can ensure that they
> +    are identity-mapped in the IOMMU.  However, some firmware makes mistakes,
> +    and this option is a coarse-grain workaround for those errors.
> +
> +    Where possible, finer grain corrections should be made with the `rmrr=`,
> +    `ivrs_hpet=` or `ivrs_ioapic=` command line options.
> +
> +    This option is enabled by default on x86 systems, and invalid on ARM
> +    systems.
> +
> +*   The `map-reserved` functionality is very similar to `map-inclusive`, but is
> +    applicable to both x86 PV and PVH dom0's, and represents a subset of the
> +    correction by only mapping reserved memory regions rather than all non-RAM
> +    regions.

Should you add "This option is enabled by default on x86 systems, and
invalid on ARM systems." here?

Thanks, Roger.

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

[PATCH v3 1/7] docs: Improve documentation for dom0= and dom0-iommu=

[Andrew Cooper]

Update to the latest metadata style, and discuss the options more
completely where appropriate.

Drop the redundant comment beside parse_dom0_param() - it is already out
of sync with the main documentation.  Also drop the individual
documentation for deprecated options which refer to their newer
versions, for the same reason.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Juergen Gross <jgross@suse.com>

v2:
 * Fix statement of defaults
 * Tweak wording.  In particular, expand the description of passthrough.
v3:
 * Rebase over dom0=verbose
---
 docs/misc/xen-command-line.pandoc | 130 ++++++++++++++++++++------------------
 xen/arch/x86/dom0_build.c         |   6 --
 2 files changed, 67 insertions(+), 69 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index d39bcee..243193d 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -636,61 +636,83 @@ trace feature is only enabled in debugging builds of Xen.
 
 Specify the bit width of the DMA heap.
 
-### dom0 (x86)
-> `= List of [ pvh | shadow | verbose ]`
+### dom0
+    = List of [ pvh=<bool>, shadow=<bool>, verbose=<bool> ]
 
-> Sub-options:
+    Applicability: x86
 
-> `pvh`
+Controls for how dom0 is constructed on x86 systems.
 
-> Default: `false`
+*   The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH
+    guest.  The default is PV.  In addition, the following requirements must
+    be met:
 
-Flag that makes a dom0 boot in PVHv2 mode.
+    *   The dom0 kernel selected by the boot loader must be capable of the
+        selected mode.
+    *   For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled.
+    *   For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled,
+        and the hardware must have VT-x/SVM extensions available.
 
-> `shadow`
+*   The `shadow` boolean is only applicable when dom0 is constructed as a PVH
+    guest, and controls whether dom0 uses hardware assisted paging, or shadow
+    paging.  The default is HAP when available, and shadow otherwise.
 
-> Default: `false`
+    This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out.  A
+    PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the
+    hardware is not HAP-capable.
 
-Flag that makes a dom0 use shadow paging. Only works when "pvh" is
-enabled.
+*   The `verbose` boolean is intended for diagnostics, and prints out extra
+    information during the dom0 build.  It defaults to false.
 
-> `verbose`
+### dom0-iommu
+    = List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>,
+                map-reserved=<bool> ]
 
-> Default: `false`
+Controls for the dom0 IOMMU setup.
 
-Print debug information during dom0 build.
+*   The `passthrough` boolean controls whether IOMMU translation functionality
+    is disabled for devices in dom0 (`passthrough=1`) or whether the IOMMU is
+    used to ensure that dom0 can only DMA to its permitted areas of RAM
+    (`passthrough=0`).
 
-### dom0-iommu
-> `= List of [ passthrough | strict | map-inclusive ]`
-
-This list of booleans controls the iommu usage by Dom0:
-
-* `passthrough`: disables DMA remapping for Dom0. Default is `false`. Note that
-  this option is hard coded to `false` for a PVH Dom0 and any attempt to
-  overwrite it from the command line is ignored.
-
-* `strict`: sets up DMA remapping only for the RAM Dom0 actually got assigned.
-  Default is `false` which means Dom0 will get mappings for all the host
-  RAM except regions in use by Xen. Note that this option is hard coded to
-  `true` for a PVH Dom0 and any attempt to overwrite it from the command line
-  is ignored.
-
-* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB
-  except for unusable ranges. Use this to work around firmware issues providing
-  incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU
-  accesses for Dom0, with this option all pages up to 4GB, not marked as
-  unusable in the E820 table, will get a mapping established. Note that this
-  option is only applicable to a PV Dom0 and is enabled by default on Intel
-  hardware.
-
-* `map-reserved`: sets up DMA remapping for all the reserved regions in the
-  memory map for Dom0. Use this to work around firmware issues providing
-  incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU
-  accesses for Dom0, all memory regions marked as reserved in the memory map
-  that don't overlap with any MMIO region from emulated devices will be
-  identity mapped. This option maps a subset of the memory that would be
-  mapped when using the `map-inclusive` option. This option is available to all
-  Dom0 modes and is enabled by default on Intel hardware.
+    This option is only applicable to x86 PV dom0's, and defaults to false.
+
+    Some older Intel VT-d hardware isn't capable of disabling translation
+    functionality on a per-device basis, and will cause this option to be
+    ignored and assumed to be 0.  Similar behaviour on such systems is only
+    available by fully disabling all IOMMUs.
+
+    This option is hardwired to false for x86 PVH dom0's (where a non-identity
+    transform is required for dom0 to function), and is ignored for ARM.
+
+*   The `strict` boolean is applicable to x86 PV dom0's only and defaults to
+    false.  It controls whether dom0 can have IOMMU mappings for all domain
+    RAM in the system, or only for its allocated RAM (and grant mappings etc.)
+
+    This option is hardwired to true for x86 PVH dom0's (as RAM belonging to
+    other domains in the system don't live in a compatible address space), and
+    is ignored for ARM.
+
+*   The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up
+    identity IOMMU mappings for all non-RAM regions below 4GB except for
+    unusable ranges, and ranges belonging to Xen.
+
+    Typically, some devices in a system use bits of RAM for communication, and
+    these areas should be listed as reserved in the E820 table and identified
+    via RMRR or IVMD entries in the APCI tables, so Xen can ensure that they
+    are identity-mapped in the IOMMU.  However, some firmware makes mistakes,
+    and this option is a coarse-grain workaround for those errors.
+
+    Where possible, finer grain corrections should be made with the `rmrr=`,
+    `ivrs_hpet=` or `ivrs_ioapic=` command line options.
+
+    This option is enabled by default on x86 systems, and invalid on ARM
+    systems.
+
+*   The `map-reserved` functionality is very similar to `map-inclusive`, but is
+    applicable to both x86 PV and PVH dom0's, and represents a subset of the
+    correction by only mapping reserved memory regions rather than all non-RAM
+    regions.
 
 ### dom0_ioports_disable (x86)
 > `= List of <hex>-<hex>`
@@ -1181,20 +1203,11 @@ detection of systems known to misbehave upon accesses to that port.
 > **WARNING: This command line option is deprecated, and superseded by
 > _dom0-iommu=passthrough_ - using both options in combination is undefined.**
 
-> Default: `false`
-
->> Control whether to disable DMA remapping for Dom0.
-
 > `dom0-strict`
 
 > **WARNING: This command line option is deprecated, and superseded by
 > _dom0-iommu=strict_ - using both options in combination is undefined.**
 
-> Default: `false`
-
->> Control whether to set up DMA remapping only for the memory Dom0 actually
->> got assigned. Implies `no-dom0-passthrough`.
-
 > `amd-iommu-perdev-intremap`
 
 > Default: `true`
@@ -1241,21 +1254,12 @@ Specify the timeout of the device IOTLB invalidation in milliseconds.
 By default, the timeout is 1000 ms. When you see error 'Queue invalidate
 wait descriptor timed out', try increasing this value.
 
-### iommu_inclusive_mapping (VT-d)
+### iommu_inclusive_mapping
 > `= <boolean>`
 
 **WARNING: This command line option is deprecated, and superseded by
 _dom0-iommu=map-inclusive_ - using both options in combination is undefined.**
 
-> Default: `true`
-
-Use this to work around firmware issues providing incorrect RMRR entries.
-Rather than only mapping RAM pages for IOMMU accesses for Dom0, with this
-option all pages up to 4GB, not marked as unusable in the E820 table, will
-get a mapping established. Note that this option is only applicable to a
-PV dom0. Also note that if `dom0-strict` mode is enabled then conventional
-RAM pages not assigned to dom0 will not be mapped.
-
 ### irq_ratelimit (x86)
 > `= <integer>`
 
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index c0bc022..7f6ee7f 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -283,12 +283,6 @@ bool __initdata opt_dom0_shadow;
 bool __initdata dom0_pvh;
 bool __initdata dom0_verbose;
 
-/*
- * List of parameters that affect Dom0 creation:
- *
- *  - pvh               Create a PVHv2 Dom0.
- *  - shadow            Use shadow paging for Dom0.
- */
 static int __init parse_dom0_param(const char *s)
 {
     const char *ss;
-- 
2.1.4


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