[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192
From: |
Alon Levy |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192 |
Date: |
Tue, 15 Jan 2013 15:34:23 +0200 |
User-agent: |
Mutt/1.5.21 (2011-07-01) |
On Sun, Dec 23, 2012 at 10:31:38PM +0200, Alon Levy wrote:
> On Thu, Dec 13, 2012 at 01:05:48PM +0100, Gerd Hoffmann wrote:
> > On 12/13/12 12:36, Alon Levy wrote:
> > > This is a simpler solution to 869981, where migration breaks since qxl's
> > > rom bar size has changed. Instead of ignoring fields in QXLRom, which is
> > > what has
> > > actually changed, we remove some of the modes, a mechanism already
> > > accounted for by the guest. To reach exactly two pages and not one, we
> > > remove two out of four orientations, the remaining are normal and right
> > > turn (chosen arbitrarily). Leaving only normal would result in a single
> > > page which would also break migration.
> > >
> > > Added assertions to ensure changes in spice-protocol in the future
> > > causing increase or decrease of page size will result in failure at
> > > startup (could do this compile time also but not sure how).
> >
> > The assertions are not in the patch.
> >
> > > #define QXL_MODE_EX(x_res, y_res) \
> > > QXL_MODE_16_32(x_res, y_res, 0), \
> > > - QXL_MODE_16_32(y_res, x_res, 1), \
> > > - QXL_MODE_16_32(x_res, y_res, 2), \
> > > - QXL_MODE_16_32(y_res, x_res, 3)
> > > + QXL_MODE_16_32(x_res, y_res, 1)
> >
> > Why do you leave orientation = 1 in? Just to keep the size above 4K?
> > Shouldn't we just hardcode the rom size to 8k instead? Then assert that
> > everything fits into 8k? Or even better add a compile time check?
> >
> > While being at it it might be a good idea to move the mode table to a
> > fixed, large enougth offset (say 0x4096), so it doesn't move around
> > again in case we extend QXLRom one more time.
>
> This solution is breaking backward compatibility like Yonit pointed
> out. The fact that I can't produce a user that would break because of
> this doesn't prove there is no such user. I suggest we go back to the
> original patch I posted, breaking it into two like you requested. What
> do you say?
Ping?
>
> >
> > cheers,
> > Gerd
> >
>
- Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192,
Alon Levy <=
- Re: [Qemu-devel] [PATCH 2/2] qxl: change rom so that 4096 < size < 8192, Gerd Hoffmann, 2013/01/15
- [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Alon Levy, 2013/01/16
- [Qemu-devel] [PATCH v2 1/2] qxl: stop using non revision 4 rom fields for revision < 4, Alon Levy, 2013/01/16
- [Qemu-devel] [PATCH v2 2/2] qxl: change rom size to 8192, Alon Levy, 2013/01/16
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Gerd Hoffmann, 2013/01/17
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Alon Levy, 2013/01/17
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Gerd Hoffmann, 2013/01/17
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Alon Levy, 2013/01/20
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Gerd Hoffmann, 2013/01/21
- Re: [Qemu-devel] [PATCH v2 0/2] fix two revision related errors, Alon Levy, 2013/01/21