Regression in OSSTest Windows install test case

Regression in OSSTest Windows install test case

[Wei Liu]

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@citrix.com>
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@citrix.com>
    Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Tell me if you need more information or want me to test a patch. I do
have test environment at hand.

Wei.

Re: Regression in OSSTest Windows install test case

[Andrew Cooper]

On 15/07/15 15:40, Wei Liu wrote:
> 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@citrix.com>
> 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@citrix.com>
>     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Tell me if you need more information or want me to test a patch. I do
> have test environment at hand.

Given the nature of the patch, I would not be unduly surprised that the
text is garbled.

Which windows and qemu are you using?  We have not encountered a single
failure like this in XenServer testing.

~Andrew

Re: Regression in OSSTest Windows install test case

[Wei Liu]

On Wed, Jul 15, 2015 at 03:43:09PM +0100, Andrew Cooper wrote:
> On 15/07/15 15:40, Wei Liu wrote:
> > 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@citrix.com>
> > 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@citrix.com>
> >     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > Tell me if you need more information or want me to test a patch. I do
> > have test environment at hand.
> 
> Given the nature of the patch, I would not be unduly surprised that the
> text is garbled.
> 
> Which windows and qemu are you using?  We have not encountered a single
> failure like this in XenServer testing.
> 

win7-x64.iso from OSSTest.

QEMU emulator version 2.2.1, Copyright (c) 2003-2008 Fabrice Bellard


> ~Andrew

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 15 July 2015 15:46
> To: Andrew Cooper
> Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Paul Durrant
> Subject: Re: Regression in OSSTest Windows install test case
> 
> On Wed, Jul 15, 2015 at 03:43:09PM +0100, Andrew Cooper wrote:
> > On 15/07/15 15:40, Wei Liu wrote:
> > > 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@citrix.com>
> > > 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@citrix.com>
> > >     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > >
> > > Tell me if you need more information or want me to test a patch. I do
> > > have test environment at hand.
> >
> > Given the nature of the patch, I would not be unduly surprised that the
> > text is garbled.
> >
> > Which windows and qemu are you using?  We have not encountered a
> single
> > failure like this in XenServer testing.
> >
> 
> win7-x64.iso from OSSTest.
> 
> QEMU emulator version 2.2.1, Copyright (c) 2003-2008 Fabrice Bellard
> 

Ok, could be a bad interaction with upstream QEMU then. I'll try to repro. Most of my dev testing was done with win7 32 bit and upstream QEMU though.

  Paul

> 
> > ~Andrew

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 15 July 2015 15:46
> To: Andrew Cooper
> Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Paul Durrant
> Subject: Re: Regression in OSSTest Windows install test case
> 
> On Wed, Jul 15, 2015 at 03:43:09PM +0100, Andrew Cooper wrote:
> > On 15/07/15 15:40, Wei Liu wrote:
> > > 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@citrix.com>
> > > 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@citrix.com>
> > >     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > >
> > > Tell me if you need more information or want me to test a patch. I do
> > > have test environment at hand.
> >
> > Given the nature of the patch, I would not be unduly surprised that the
> > text is garbled.
> >
> > Which windows and qemu are you using?  We have not encountered a
> single
> > failure like this in XenServer testing.
> >
> 
> win7-x64.iso from OSSTest.
> 
> QEMU emulator version 2.2.1, Copyright (c) 2003-2008 Fabrice Bellard
> 

BTW, does the xl cfg specify cirrus or stdvga?

  Paul

> 
> > ~Andrew

Re: Regression in OSSTest Windows install test case

[Wei Liu]

On Wed, Jul 15, 2015 at 03:49:44PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: 15 July 2015 15:46
> > To: Andrew Cooper
> > Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Paul Durrant
> > Subject: Re: Regression in OSSTest Windows install test case
> > 
> > On Wed, Jul 15, 2015 at 03:43:09PM +0100, Andrew Cooper wrote:
> > > On 15/07/15 15:40, Wei Liu wrote:
> > > > 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@citrix.com>
> > > > 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@citrix.com>
> > > >     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > >
> > > > Tell me if you need more information or want me to test a patch. I do
> > > > have test environment at hand.
> > >
> > > Given the nature of the patch, I would not be unduly surprised that the
> > > text is garbled.
> > >
> > > Which windows and qemu are you using?  We have not encountered a
> > single
> > > failure like this in XenServer testing.
> > >
> > 
> > win7-x64.iso from OSSTest.
> > 
> > QEMU emulator version 2.2.1, Copyright (c) 2003-2008 Fabrice Bellard
> > 
> 
> BTW, does the xl cfg specify cirrus or stdvga?
> 
>   Paul

Here is the config I use (slightly modified from the one in OSSTest). No
setting with regard to video card, so it should be the cirrus.

---
name        = 'win'
memory = 512
vif         = [ 'type=ioemu,mac=5a:36:0e:98:50:33' ]
#
on_poweroff = 'destroy'
on_reboot   = 'preserve'
on_crash    = 'preserve'
#
vcpus = 2
#
kernel      = 'hvmloader'
builder     = 'hvm'
#
disk        = [
                'phy:/dev/DATA/win,hda,w',              
'file:/data/win7-x64.iso,hdc:cdrom,r'
              ]
#
usb=1
usbdevice='tablet'
#
#stdvga=1
keymap='en-gb';
#
sdl=0
opengl=0
vnc=1
vncunused=1
vncdisplay=0
vnclisten='0.0.0.0'

#
#boot = 'dc'
boot = 'c'
device_model_version='qemu-xen'
serial='file:/dev/stderr'

#
viridian=1


> 
> > 
> > > ~Andrew

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 15 July 2015 15:54
> To: Paul Durrant
> Cc: Wei Liu; Andrew Cooper; xen-devel@lists.xenproject.org; Jan Beulich
> Subject: Re: Regression in OSSTest Windows install test case
> 
> On Wed, Jul 15, 2015 at 03:49:44PM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > > Sent: 15 July 2015 15:46
> > > To: Andrew Cooper
> > > Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Paul Durrant
> > > Subject: Re: Regression in OSSTest Windows install test case
> > >
> > > On Wed, Jul 15, 2015 at 03:43:09PM +0100, Andrew Cooper wrote:
> > > > On 15/07/15 15:40, Wei Liu wrote:
> > > > > 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@citrix.com>
> > > > > 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@citrix.com>
> > > > >     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > > >
> > > > > Tell me if you need more information or want me to test a patch. I do
> > > > > have test environment at hand.
> > > >
> > > > Given the nature of the patch, I would not be unduly surprised that the
> > > > text is garbled.
> > > >
> > > > Which windows and qemu are you using?  We have not encountered a
> > > single
> > > > failure like this in XenServer testing.
> > > >
> > >
> > > win7-x64.iso from OSSTest.
> > >
> > > QEMU emulator version 2.2.1, Copyright (c) 2003-2008 Fabrice Bellard
> > >
> >
> > BTW, does the xl cfg specify cirrus or stdvga?
> >
> >   Paul
> 
> Here is the config I use (slightly modified from the one in OSSTest). No
> setting with regard to video card, so it should be the cirrus.
> 
> ---
> name        = 'win'
> memory = 512

^^^^ Wow. That's ambitious for Windows 7!

  Paul

> vif         = [ 'type=ioemu,mac=5a:36:0e:98:50:33' ]
> #
> on_poweroff = 'destroy'
> on_reboot   = 'preserve'
> on_crash    = 'preserve'
> #
> vcpus = 2
> #
> kernel      = 'hvmloader'
> builder     = 'hvm'
> #
> disk        = [
>                 'phy:/dev/DATA/win,hda,w',
> 'file:/data/win7-x64.iso,hdc:cdrom,r'
>               ]
> #
> usb=1
> usbdevice='tablet'
> #
> #stdvga=1
> keymap='en-gb';
> #
> sdl=0
> opengl=0
> vnc=1
> vncunused=1
> vncdisplay=0
> vnclisten='0.0.0.0'
> 
> #
> #boot = 'dc'
> boot = 'c'
> device_model_version='qemu-xen'
> serial='file:/dev/stderr'
> 
> #
> viridian=1
> 
> 
> >
> > >
> > > > ~Andrew

Re: Regression in OSSTest Windows install test case

[Wei Liu]

On Wed, Jul 15, 2015 at 03:55:24PM +0100, Paul Durrant wrote:
[...]
> > Here is the config I use (slightly modified from the one in OSSTest). No
> > setting with regard to video card, so it should be the cirrus.
> > 
> > ---
> > name        = 'win'
> > memory = 512
> 
> ^^^^ Wow. That's ambitious for Windows 7!
> 

I notice that too. But the installation seems to finish in a reasonable
time. :-P

Wei.

Re: Regression in OSSTest Windows install test case

[Fabio Fantoni]

Il 15/07/2015 16:57, Wei Liu ha scritto:
> On Wed, Jul 15, 2015 at 03:55:24PM +0100, Paul Durrant wrote:
> [...]
>>> Here is the config I use (slightly modified from the one in OSSTest). No
>>> setting with regard to video card, so it should be the cirrus.

Cirrus driver is not available for windows 7, support of it is near 
abandonment in upstream if I remember good what I read one year ago (or 
more) in qemu-devel, is better use stdvga instead in windows >=vista.
A had also strange graphic and performance problem with cirrus on 
windows 7 2-3 years ago, since then I have not set the cirrus on vm 
windows 7.
Try to vga="stdvga"

>>>
>>> ---
>>> name        = 'win'
>>> memory = 512
>> ^^^^ Wow. That's ambitious for Windows 7!
>>
> I notice that too. But the installation seems to finish in a reasonable
> time. :-P

Years ago on my tests system with few ram I tried to decrease ram as 
possible but I saw that with windows 7 (even if 32 bit) with < 1 gb made 
occasional problems that wasted my tests.
About performance for run tests (after windows install even if basic) 2 
gb of ram at least is needed for don't waste large amount of time on 
massive numbers of tests.
I saw many occasional windows tests fail on "[Xen-devel] [xen-unstable 
test] ....: regressions - FAIL" that I was unable to reproduce doing 
same tests (windows install, shutdown, save/restore) and that blocked 
push for days even if I not saw "real bug/regression" was found.
If these are only caused by "bad" xl cfg values rare to find on real 
users domUs I think is bad and should be improved.

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

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 15 July 2015 15:40
> To: xen-devel@lists.xenproject.org
> 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@citrix.com>
> 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@citrix.com>
>     Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> 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.

Re: Regression in OSSTest Windows install test case

[Wei Liu]

On Thu, Jul 16, 2015 at 09:39:42AM +0100, Paul Durrant wrote:
[...]
> 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.
> 

Thank you for the fix. It fixes the problem for me.

Tested-by: Wei Liu <wei.liu2@citrix.com>

>   Paul
> 
> 



> > Wei.

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 16 July 2015 10:03
> To: Paul Durrant
> Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Andrew Cooper
> Subject: Re: Regression in OSSTest Windows install test case
> 
> On Thu, Jul 16, 2015 at 09:39:42AM +0100, Paul Durrant wrote:
> [...]
> > 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.
> >
> 
> Thank you for the fix. It fixes the problem for me.

Cool. There's actually a small bug-ette in it. The line:

else if ( p->dir == IOREQ_READ && !s->cache && !s->stdvga )

should be:

else if ( p->dir == IOREQ_READ && (!s->cache || !s->stdvga) )

I don't think that will affect the behaviour in this case though since I very much doubt Windows is reading VRAM at this stage. Could you perhaps re-test with that change?

Cheers,

  Paul

> 
> Tested-by: Wei Liu <wei.liu2@citrix.com>
> 
> >   Paul
> >
> >
> 
> 
> 
> > > Wei.

Re: Regression in OSSTest Windows install test case

[Wei Liu]

On Thu, Jul 16, 2015 at 10:06:35AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: 16 July 2015 10:03
> > To: Paul Durrant
> > Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Andrew Cooper
> > Subject: Re: Regression in OSSTest Windows install test case
> > 
> > On Thu, Jul 16, 2015 at 09:39:42AM +0100, Paul Durrant wrote:
> > [...]
> > > 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.
> > >
> > 
> > Thank you for the fix. It fixes the problem for me.
> 
> Cool. There's actually a small bug-ette in it. The line:
> 
> else if ( p->dir == IOREQ_READ && !s->cache && !s->stdvga )
> 
> should be:
> 
> else if ( p->dir == IOREQ_READ && (!s->cache || !s->stdvga) )
> 
> I don't think that will affect the behaviour in this case though since I very much doubt Windows is reading VRAM at this stage. Could you perhaps re-test with that change?
> 

It works. So again:

 Tested-by: Wei Liu <wei.liu2@citrix.com>

> Cheers,
> 
>   Paul
> 
> > 
> > Tested-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > >   Paul
> > >
> > >
> > 
> > 
> > 
> > > > Wei.

Re: Regression in OSSTest Windows install test case

[Paul Durrant]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 16 July 2015 10:12
> To: Paul Durrant
> Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Andrew Cooper
> Subject: Re: Regression in OSSTest Windows install test case
> 
> On Thu, Jul 16, 2015 at 10:06:35AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > > Sent: 16 July 2015 10:03
> > > To: Paul Durrant
> > > Cc: Wei Liu; xen-devel@lists.xenproject.org; Jan Beulich; Andrew Cooper
> > > Subject: Re: Regression in OSSTest Windows install test case
> > >
> > > On Thu, Jul 16, 2015 at 09:39:42AM +0100, Paul Durrant wrote:
> > > [...]
> > > > 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.
> > > >
> > >
> > > Thank you for the fix. It fixes the problem for me.
> >
> > Cool. There's actually a small bug-ette in it. The line:
> >
> > else if ( p->dir == IOREQ_READ && !s->cache && !s->stdvga )
> >
> > should be:
> >
> > else if ( p->dir == IOREQ_READ && (!s->cache || !s->stdvga) )
> >
> > I don't think that will affect the behaviour in this case though since I very
> much doubt Windows is reading VRAM at this stage. Could you perhaps re-
> test with that change?
> >
> 
> It works. So again:
> 
>  Tested-by: Wei Liu <wei.liu2@citrix.com>

Excellent. Thanks,

  Paul

> 
> > Cheers,
> >
> >   Paul
> >
> > >
> > > Tested-by: Wei Liu <wei.liu2@citrix.com>
> > >
> > > >   Paul
> > > >
> > > >
> > >
> > >
> > >
> > > > > Wei.