Re: [PATCH (for 4.6)] x86/hvm: Unconditionally buffer writes to VRAM

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/07/15 10:24, Paul Durrant wrote:
> When c/s 3bbaaec09 "unify stdvga mmio intercept with standard mmio
> intercept" was added, a small semantic change was made. Prior to
> this patch the hypervisor unconditionally sent all guest writes
> to the VGA aperture as buffered ioreqs, whereas after the patch it
> only does this when the VGA model is in 'stdvga' mode (sequencer
> register #7 == 0).
>
> When installing Windows 7 (64-bit) using the default QEMU VGA model
> (== cirrus), Windows leaves 'stdvga' mode early in boot and hence
> all further writes to the VGA aperture are done using synchronous
> ioreqs which slows down boot by several orders of magnitude (thanks
> to the elaborate splash screen that Windows presents). This can be
> viewed as a regression and so this patch re-instates previous
> buffering behaviour.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Tested-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

This is unfortunate, but should be returned to its previous behaviour.

Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>

> ---
>  xen/arch/x86/hvm/stdvga.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/stdvga.c b/xen/arch/x86/hvm/stdvga.c
> index 6d22b22..f50bff7 100644
> --- a/xen/arch/x86/hvm/stdvga.c
> +++ b/xen/arch/x86/hvm/stdvga.c
> @@ -441,7 +441,7 @@ static int stdvga_mem_write(const struct hvm_io_handler *handler,
>      };
>      struct hvm_ioreq_server *srv;
>  
> -    if ( !s->cache )
> +    if ( !s->cache || !s->stdvga )
>          goto done;
>  
>      /* Intercept mmio write */
> @@ -503,9 +503,6 @@ static bool_t stdvga_mem_accept(const struct hvm_io_handler *handler,
>  
>      spin_lock(&s->lock);
>  
> -    if ( !s->stdvga )
> -        goto reject;
> -
>      if ( p->dir == IOREQ_WRITE && p->count > 1 )
>      {
>          /*
> @@ -526,7 +523,7 @@ static bool_t stdvga_mem_accept(const struct hvm_io_handler *handler,
>  
>          goto reject;
>      }
> -    else if ( p->dir == IOREQ_READ && !s->cache )
> +    else if ( p->dir == IOREQ_READ && (!s->cache || !s->stdvga) )
>          goto reject;
>  
>      /* s->lock intentionally held */