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

[Juergen Gross <jgross@xxxxxxxx>]

On 17/01/2019 12:59, Jan Beulich wrote:
>>>> On 17.01.19 at 10:14, <jgross@xxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel