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".
> 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 <jbeul...@suse.com>
In principle, Acked-by: Andrew Cooper <andrew.coop...@citrix.com>, with
one note below.
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.
>
> --- 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?
~Andrew
> +
> ### vesa-ram
> > `= <integer>`
>
> +> Default: `0`
> +
> +This allows to override the amount of video RAM, in MiB, determined to be
> +present.
> +
> ### vga
> > `= ( ask | current | text-80x<rows> | gfx-<width>x<height>x<depth> |
> mode-<mode> )[,keep]`
>
> --- a/xen/drivers/video/vesa.c
> +++ b/xen/drivers/video/vesa.c
> @@ -19,17 +19,17 @@
>
> static void lfb_flush(void);
>
> -static unsigned char *lfb;
> -static const struct font_desc *font;
> -static bool_t vga_compat;
> +static unsigned char *__read_mostly lfb;
> +static const struct font_desc *__initdata font;
> +static bool __initdata vga_compat;
>
> -static unsigned int vram_total;
> +static unsigned int __initdata vram_total;
> integer_param("vesa-ram", vram_total);
>
> -static unsigned int vram_remap;
> +static unsigned int __initdata vram_remap;
> integer_param("vesa-map", vram_remap);
>
> -static int font_height;
> +static unsigned int __initdata font_height;
> static int __init parse_font_height(const char *s)
> {
> if ( simple_strtoul(s, &s, 10) == 8 && (*s++ == 'x') )
>
