Re: [PATCH 7/7] video/vesa: adjust (not just) command line option handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.04.2021 15:49, Andrew Cooper wrote:
> On 27/04/2021 13:56, Jan Beulich wrote:
> 
> The grammar in the subject is very awkward.  The (not just) like that is
> weird.
> 
> If it were me, I'd phrase this as "minor adjustments to command line
> handling".

Well, the (not just) is intentionally there because there are changes
to stuff unrelated to command line option handling as well.

>> Document both options. Add section annotations to both variables holding
>> the parsed values as well as a few adjacent ones. Adjust the types of
>> font_height and vga_compat.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> In principle, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, with
> one note below.

Thanks.

> However, is there really any value in these options?  I can't see a case
> where their use will result in a less broken system.

Well, if we mis-detect VRAM size, the respective option might indeed
help. I'm less certain of the utility of the mapping option, the more
that now there's no possible (and implicit) effect on MTRRs anymore.

>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -2369,9 +2369,21 @@ cache-warming. 1ms (1000) has been measu
>>  ### vesa-map
>>  > `= <integer>`
>>  
>> +> Default: `0`
>> +
>> +Specify, in MiB, the portion of video RAM to actually remap.  This will be
>> +honored only when large enough to cover the space needed for the chosen 
>> video
>> +mode, and only when less than a non-zero value possibly specified through
>> +'vesa-ram'.
> 
> "and only when less than a non-zero value possibly specified" is
> confusing to follow.
> 
> What I think you mean is that vesa-map will be honoured when it is >=
> chosen video mode, and <= vesa-ram?

Yes. Any suggestion how to improve the wording without using >= and
<= ?

Jan