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: 16 July 2015 10:12
> To: Paul Durrant
> Cc: Wei Liu; xen-devel@xxxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxxx]
> > > Sent: 16 July 2015 10:03
> > > To: Paul Durrant
> > > Cc: Wei Liu; xen-devel@xxxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxxx>

Excellent. Thanks,

  Paul

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

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