Re: [Xen-devel] Regression in OSSTest Windows install test case

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 15 July 2015 15:40
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Wei Liu; Jan Beulich; Andrew Cooper; Paul Durrant
> Subject: Regression in OSSTest Windows install test case
> 
> Hi all
> 
> The Windows install test case has been failing reliably in OSSTest for
> the last 6 runs.
> 
> The issue manifest as Windows hit blue screen with garbled text that
> cannot be recognised when booting. Xen doesn't complain about any
> failures.
> 
> I bisected that and found this commit is to be blamed.
> 
> commit 3bbaaec09b1b942f5624dee176da6e416d31f982
> Author: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Date:   Thu Jul 9 19:04:00 2015 +0200
> 
>     x86/hvm: unify stdvga mmio intercept with standard mmio intercept
> 
>     It's clear from the following check in hvmemul_rep_movs:
> 
>         if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
>              (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
>             return X86EMUL_UNHANDLEABLE;
> 
>     that mmio <-> mmio copy is not handled. This means the code in the
>     stdvga mmio intercept that explicitly handles mmio <-> mmio copy when
>     hvm_copy_to/from_guest_phys() fails is never going to be executed.
> 
>     This patch therefore adds a check in hvmemul_do_io_addr() to make sure
>     mmio <-> mmio is disallowed and then registers standard mmio intercept
> ops
>     in stdvga_init().
> 
>     With this patch all mmio and portio handled within Xen now goes through
>     process_io_intercept().
> 
>     Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
>     Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Tell me if you need more information or want me to test a patch. I do
> have test environment at hand.
> 

I think I found the semantic difference. Prior to this patch it would seem that 
stdvga buffered writes even when not in stdvga mode. When I tested your config 
(using cirrus), windows drops out of stdvga quite early in boot which slows 
down the splash screen to a crawl. When I just uncommented the single line 
'stdvga=1' then I got through the splash screen several orders of magnitude 
faster.
So, I think this (compile tested only) incremental patch should fix the test:

diff --git a/xen/arch/x86/hvm/stdvga.c b/xen/arch/x86/hvm/stdvga.c
index 6d22b22..d528155 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 *han
     };
     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

     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

         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 */

I can't say that I like the idea of unconditionally buffering VRAM writes, but 
I'll test patch and post it.

  Paul


> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel