Re: [RFC] x86/HVM: stdvga caching mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/09/2024 11:33 am, Jan Beulich wrote:
> Hello,
>
> I happened to spot a ~14y old revert of the crucial hunk of the ~16y old
> 551ceee97513 ("x86, hvm: stdvga cache always on") in our patch set,
> supposedly to deal with text mode corruption when Linux is booted without
> any "vga=" option, and when - after the GUI is up - the console is
> switched back to one of the text mode ones.
>
> My immediate reaction was that we shouldn't be carrying such privately.
> Yet after some playing with it I'm now at the point where I'm wondering
> why we have that caching mode in the first place. It looks to hardly ever
> come into use:
> 1) As of 68e1183411be ("libxc: introduce a xc_dom_arch for hvm-3.0-x86_32
>    guests") caching mode is disabled from start-of-day, due the disabling
>    being unconditional in hvm/save.c:arch_hvm_load(). That can of course
>    be worked around, but then 2).
> 2) In the course of setting up VGA, REP STOS (maybe REP MOVS) are
>    apparently used by both SeaBIOS and ROMBIOS, as can be derived from
>    stdvga_mem_accept() always hitting the "if ( p->dir == IOREQ_WRITE &&
>    p->count > 1 )" path while BIOS initializes.
>
> Further:
> 3) 551ceee97513 ("x86, hvm: stdvga cache always on") bumped the maximum
>    range of "mapped" VRAM from 64k to 128k, yet without growing vram_page[].
>    Afaict in mode 0 (full 128k accessible at A0000-BFFFF) vram_get{b,l}()
>    now misbehave.
> 4) d1b531631515 ("x86/hvm: unconditionally buffer writes to VRAM") likely
>    went too far (or not far enough) in bypassing write handling, yet then
>    still allowing reads to be serviced from possibly stale cache, when
>    ->stdvga goes first off and later back on, without ->cache changing
>    state.
> 5) 22a1fbb575df ("x86/hvm: make sure stdvga cache cannot be re-enabled")
>    likely went too far. Surely there are cases (VRAM clearing at the very
>    least) after which VRAM state is known again, and hence caching could
>    in principle be re-enabled.
>
> Before I go and try to fix all of the above, I'd like to collect views
> towards simply ripping out that caching mode, vastly simplifying the
> source file in question.

STDVGA caching is primarily (exclusively?) an optimisation for Windows XP.

WinXP was written pre-virt, and wastes an awful lot of time rendering
its boot animation with IN/OUT.

The "caching" (really, putting them in the bufioreq ring, rather than
blocking for qemu on every access) made a good 10-20s improvement in VM
boot performance if memory serves, not to mention dom0 load when booting
multiple VMs in parallel.

But, this wouldn't be the first time its utility has been called into
question, and it's getting harder and harder to justify keeping some
WinXP compatibility around.

~Andrew