[Spice-devel] [PATCH] miniport/qxl: FillVideoMode: only allow word aligned strides

[Alon Levy]

> On Thu, Oct 11, 2012 at 12:23:21PM +0200, Alon Levy wrote:
> > This is a pixman requirement. If it isn't enforced spice crashes,
> > qemu
> > with it. That should be fixed, but there is no simple way to allow
> > for
> > non word aligned strides, and the only down side is that for 16 bit
> > modes we will require even widths. And we already have the same
> > implementation detail in the X driver, only there it is a multiple
> > of 8,
> > not 4.
> > ---
> >  miniport/qxl.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/miniport/qxl.c b/miniport/qxl.c
> > index 55e55d3..f177d91 100644
> > --- a/miniport/qxl.c
> > +++ b/miniport/qxl.c
> > @@ -469,17 +469,24 @@ VP_STATUS
> > FillVidModeInfo(VIDEO_MODE_INFORMATION *pMode, ULONG xres, ULONG
> > yres,
> >  /* Fills given video mode structure */
> >  VP_STATUS FillVidModeInfo(VIDEO_MODE_INFORMATION *pMode, ULONG
> >  xres, ULONG yres, ULONG bpp, ULONG index)
> >  {
> > +    unsigned bytes_pp = (bpp + 7) / 8;
> > +
> >      if (xres <= 0 || yres <= 0)
> >          return ERROR_INVALID_DATA;
> >  
> >      VideoPortZeroMemory(pMode, sizeof(VIDEO_MODE_INFORMATION));
> >  
> > +    /* Honor pixmap's stride % 4 == 0 limitation. Pixmap is used
> > by spice-server. */
> > +    if (xres * bytes_pp % 4 != 0) {
> > +        xres = ((xres * bytes_pp) & ~0x3) / bytes_pp;
> > +    }
> > +
> >      /*Common entries*/
> >      pMode->Length                       =
> >      sizeof(VIDEO_MODE_INFORMATION);
> >      pMode->ModeIndex                    = index;
> >      pMode->VisScreenWidth               = xres;
> >      pMode->VisScreenHeight              = yres;
> > -    pMode->ScreenStride                 = xres * ((bpp + 7) / 8);
> > +    pMode->ScreenStride                 = xres * bytes_pp;
> 
> Shouldn't we keep VisScreenWidth unchanged, and just round
> ScreenStride up
> so that it's a multiple of 4? Ie don't change xres at all, and set
> ScreenStride to (xres * bytes_pp + 3)/4.

Hmm, right - that should work, I'll try it, thanks.

> 
> Christophe
>