[Qemu-devel] [PATCH 0/3] qxl fb size and async fix

[Qemu-devel] [PATCH 0/3] qxl fb size and async fix

[Alon Levy]

Hi Gerd,

 Some additional patches to go on top of "hw/qxl: ignore guest from guestbug until reset".
 
 Tree (with previous patchset, without qxl info) available at
 
  git://people.freedesktop.org/~alon/qemu to-q/qxl/fb_async_fix.v1
 
 First two add a fb_size_mb property and use that to allow larger framebuffers,
 second bugs out on too large primary create request, and third resets the
 current_async to allow recovery from bad state via reset. I'm dropping the qxl
 info patch for now, lack of time.

Alon

Alon Levy (3):
  qxl: add fb_size_mb and fb_size
  qxl: refuse to create primary larger then fb size
  qxl: reset current_async on qxl_soft_reset

 hw/qxl.c |   42 +++++++++++++++++++++++++++++++-----------
 hw/qxl.h |    2 ++
 2 files changed, 33 insertions(+), 11 deletions(-)

-- 
1.7.10.1

[Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Alon Levy]

In preperation for supporting a larger framebuffer for multiple monitors
on a single card, add a property to qxl fb_size_mb, and corresponding
byte sized fb_size, and use instead of VGA_RAM_SIZE.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   31 ++++++++++++++++++++-----------
 hw/qxl.h |    2 ++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b5e53ce..a9b4fd1 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -114,20 +114,16 @@ static QXLMode qxl_modes[] = {
     QXL_MODE_EX(1600, 1200),
     QXL_MODE_EX(1680, 1050),
     QXL_MODE_EX(1920, 1080),
-#if VGA_RAM_SIZE >= (16 * 1024 * 1024)
     /* these modes need more than 8 MB video memory */
     QXL_MODE_EX(1920, 1200),
     QXL_MODE_EX(1920, 1440),
     QXL_MODE_EX(2048, 1536),
     QXL_MODE_EX(2560, 1440),
     QXL_MODE_EX(2560, 1600),
-#endif
-#if VGA_RAM_SIZE >= (32 * 1024 * 1024)
     /* these modes need more than 16 MB video memory */
     QXL_MODE_EX(2560, 2048),
     QXL_MODE_EX(2800, 2100),
     QXL_MODE_EX(3200, 2400),
-#endif
 };
 
 static PCIQXLDevice *qxl0;
@@ -284,6 +280,7 @@ static inline uint32_t msb_mask(uint32_t val)
 static ram_addr_t qxl_rom_size(void)
 {
     uint32_t rom_size = sizeof(QXLRom) + sizeof(QXLModes) + sizeof(qxl_modes);
+
     rom_size = MAX(rom_size, TARGET_PAGE_SIZE);
     rom_size = msb_mask(rom_size * 2 - 1);
     return rom_size;
@@ -296,8 +293,9 @@ static void init_qxl_rom(PCIQXLDevice *d)
     uint32_t ram_header_size;
     uint32_t surface0_area_size;
     uint32_t num_pages;
-    uint32_t fb, maxfb = 0;
+    uint32_t fb;
     int i;
+    int n_modes = 0;
 
     memset(rom, 0, d->rom_size);
 
@@ -312,11 +310,13 @@ static void init_qxl_rom(PCIQXLDevice *d)
     rom->slots_end     = NUM_MEMSLOTS - 1;
     rom->n_surfaces    = cpu_to_le32(NUM_SURFACES);
 
-    modes->n_modes     = cpu_to_le32(ARRAY_SIZE(qxl_modes));
     for (i = 0; i < modes->n_modes; i++) {
         fb = qxl_modes[i].y_res * qxl_modes[i].stride;
-        if (maxfb < fb) {
-            maxfb = fb;
+        if (d->fb_size < fb) {
+            d->fb_size = fb;
+        }
+        if (fb <= d->fb_size) {
+            n_modes++;
         }
         modes->modes[i].id          = cpu_to_le32(i);
         modes->modes[i].x_res       = cpu_to_le32(qxl_modes[i].x_res);
@@ -327,11 +327,13 @@ static void init_qxl_rom(PCIQXLDevice *d)
         modes->modes[i].y_mili      = cpu_to_le32(qxl_modes[i].y_mili);
         modes->modes[i].orientation = cpu_to_le32(qxl_modes[i].orientation);
     }
-    if (maxfb < VGA_RAM_SIZE && d->id == 0)
-        maxfb = VGA_RAM_SIZE;
+    if (d->fb_size < VGA_RAM_SIZE && d->id == 0) {
+        d->fb_size = VGA_RAM_SIZE;
+    }
+    modes->n_modes     = cpu_to_le32(n_modes);
 
     ram_header_size    = ALIGN(sizeof(QXLRam), 4096);
-    surface0_area_size = ALIGN(maxfb, 4096);
+    surface0_area_size = ALIGN(d->fb_size, 4096);
     num_pages          = d->vga.vram_size;
     num_pages         -= ram_header_size;
     num_pages         -= surface0_area_size;
@@ -1720,6 +1722,12 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl, uint32_t ram_min_mb)
     if (qxl->vga.vram_size < ram_min_mb * 1024 * 1024) {
         qxl->vga.vram_size = ram_min_mb * 1024 * 1024;
     }
+    if (qxl->fb_size_mb != -1 &&
+        qxl->fb_size_mb * 1024 * 1024 < qxl->vga.vram_size) {
+        qxl->fb_size = qxl->fb_size_mb * 1024 * 1024;
+    } else {
+        qxl->fb_size = VGA_RAM_SIZE;
+    }
 
     /* vram32 (surfaces, 32bit, bar 1) */
     if (qxl->vram32_size_mb != -1) {
@@ -2052,6 +2060,7 @@ static Property qxl_properties[] = {
         DEFINE_PROP_UINT32("ram_size_mb",  PCIQXLDevice, ram_size_mb, -1),
         DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram32_size_mb, -1),
         DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, -1),
+        DEFINE_PROP_UINT32("fb_size_mb", PCIQXLDevice, fb_size_mb, -1),
         DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/qxl.h b/hw/qxl.h
index a4ab7cc..1b4ec16 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -84,6 +84,7 @@ typedef struct PCIQXLDevice {
     QXLReleaseInfo     *last_release;
     uint32_t           last_release_offset;
     uint32_t           oom_running;
+    uint32_t           fb_size;
 
     /* rom pci bar */
     QXLRom             shadow_rom;
@@ -105,6 +106,7 @@ typedef struct PCIQXLDevice {
     uint32_t          ram_size_mb;
     uint32_t          vram_size_mb;
     uint32_t          vram32_size_mb;
+    uint32_t          fb_size_mb;
 
     /* qxl_render_update state */
     int                render_update_cookie_num;
-- 
1.7.10.1

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Gerd Hoffmann]

  Hi,

> +        if (d->fb_size < fb) {
> +            d->fb_size = fb;
> +        }
> +        if (fb <= d->fb_size) {

This check doesn't make sense, it will always be true.

> +    if (qxl->fb_size_mb != -1 &&
> +        qxl->fb_size_mb * 1024 * 1024 < qxl->vga.vram_size) {
> +        qxl->fb_size = qxl->fb_size_mb * 1024 * 1024;
> +    } else {
> +        qxl->fb_size = VGA_RAM_SIZE;
> +    }

Reminds me that I have some pending work to make vga ram size
configurable which I should finish and repost ...

Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1

It probably makes sense to base this on top.

> +        DEFINE_PROP_UINT32("fb_size_mb", PCIQXLDevice, fb_size_mb, -1),

Maybe also rename this to vgamem_mb for consistency with standard vga.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Alon Levy]

On Mon, Jun 11, 2012 at 09:42:28AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > +        if (d->fb_size < fb) {
> > +            d->fb_size = fb;
> > +        }
> > +        if (fb <= d->fb_size) {
> 
> This check doesn't make sense, it will always be true.

My mistake. Will remove the first three lines, so fb_size setting will
be required to get larger modes.

> 
> > +    if (qxl->fb_size_mb != -1 &&
> > +        qxl->fb_size_mb * 1024 * 1024 < qxl->vga.vram_size) {
> > +        qxl->fb_size = qxl->fb_size_mb * 1024 * 1024;
> > +    } else {
> > +        qxl->fb_size = VGA_RAM_SIZE;
> > +    }
> 
> Reminds me that I have some pending work to make vga ram size
> configurable which I should finish and repost ...
> 
> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
> 
> It probably makes sense to base this on top.

So I can rebase on top and send the patches, with the understanding you
will do any later changes and repost the whole thing.

> 
> > +        DEFINE_PROP_UINT32("fb_size_mb", PCIQXLDevice, fb_size_mb, -1),
> 
> Maybe also rename this to vgamem_mb for consistency with standard vga.

Will change.

> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Gerd Hoffmann]

  Hi,

>> Reminds me that I have some pending work to make vga ram size
>> configurable which I should finish and repost ...
>>
>> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
>>
>> It probably makes sense to base this on top.
> 
> So I can rebase on top and send the patches, with the understanding you
> will do any later changes and repost the whole thing.

Yes, makes sense to put them into one series I think.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Alon Levy]

On Mon, Jun 11, 2012 at 10:29:59AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >> Reminds me that I have some pending work to make vga ram size
> >> configurable which I should finish and repost ...
> >>
> >> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
> >>
> >> It probably makes sense to base this on top.
> > 
> > So I can rebase on top and send the patches, with the understanding you
> > will do any later changes and repost the whole thing.
> 
> Yes, makes sense to put them into one series I think.

You dropped me from the to field, please don't.

Hope you didn't miss the one liner async_complete reset patch.

> 
> cheers,
>   Gerd
> 
> 

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Alon Levy]

On Mon, Jun 11, 2012 at 10:29:59AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >> Reminds me that I have some pending work to make vga ram size
> >> configurable which I should finish and repost ...
> >>
> >> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
> >>
> >> It probably makes sense to base this on top.
> > 
> > So I can rebase on top and send the patches, with the understanding you
> > will do any later changes and repost the whole thing.
> 
> Yes, makes sense to put them into one series I think.

git://people.freedesktop.org/~alon/qemu fb_async_fix.v2

> 
> cheers,
>   Gerd
> 
> 

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Gerd Hoffmann]

On 06/11/12 11:33, Alon Levy wrote:
> On Mon, Jun 11, 2012 at 10:29:59AM +0200, Gerd Hoffmann wrote:
>>   Hi,
>>
>>>> Reminds me that I have some pending work to make vga ram size
>>>> configurable which I should finish and repost ...
>>>>
>>>> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
>>>>
>>>> It probably makes sense to base this on top.
>>>
>>> So I can rebase on top and send the patches, with the understanding you
>>> will do any later changes and repost the whole thing.
>>
>> Yes, makes sense to put them into one series I think.
> 
> git://people.freedesktop.org/~alon/qemu fb_async_fix.v2

http://www.kraxel.org/cgit/qemu/log/?h=vga.2

Did some small fixups [no need for the -1 dance, the reason the other
properties do that is that we have one accepting byte values and one
accepting megabyte values].

looks ok to you?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/3] qxl: add fb_size_mb and fb_size

[Alon Levy]

On Mon, Jun 11, 2012 at 12:18:14PM +0200, Gerd Hoffmann wrote:
> On 06/11/12 11:33, Alon Levy wrote:
> > On Mon, Jun 11, 2012 at 10:29:59AM +0200, Gerd Hoffmann wrote:
> >>   Hi,
> >>
> >>>> Reminds me that I have some pending work to make vga ram size
> >>>> configurable which I should finish and repost ...
> >>>>
> >>>> Current state pushed to http://www.kraxel.org/cgit/qemu/log/?h=vga.1
> >>>>
> >>>> It probably makes sense to base this on top.
> >>>
> >>> So I can rebase on top and send the patches, with the understanding you
> >>> will do any later changes and repost the whole thing.
> >>
> >> Yes, makes sense to put them into one series I think.
> > 
> > git://people.freedesktop.org/~alon/qemu fb_async_fix.v2
> 
> http://www.kraxel.org/cgit/qemu/log/?h=vga.2
> 
> Did some small fixups [no need for the -1 dance, the reason the other
> properties do that is that we have one accepting byte values and one
> accepting megabyte values].
> 
> looks ok to you?

Looks like you squashed and removed the -1 (you're right of course, it
was just me doing a straight copy), other then that I didn't notice
anything. Looks good.

> 
> cheers,
>   Gerd

[Qemu-devel] [PATCH 2/3] qxl: refuse to create primary larger then fb size

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/qxl.c b/hw/qxl.c
index a9b4fd1..6596856 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1204,6 +1204,16 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
 {
     QXLDevSurfaceCreate surface;
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
+    int size;
+    int requested_height = le32_to_cpu(sc->height);
+    int requested_stride = le32_to_cpu(sc->stride);
+
+    size = abs(requested_stride) * requested_height;
+    if (size > qxl->fb_size) {
+        qxl_set_guest_bug(qxl, "%s: requested primary larger then framebuffer"
+                               " size", __func__);
+        return;
+    }
 
     if (qxl->mode == QXL_MODE_NATIVE) {
         qxl_set_guest_bug(qxl, "%s: nop since already in QXL_MODE_NATIVE",
-- 
1.7.10.1

[Qemu-devel] [PATCH 3/3] qxl: reset current_async on qxl_soft_reset

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/qxl.c b/hw/qxl.c
index 6596856..b18a59d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -988,6 +988,7 @@ static void qxl_soft_reset(PCIQXLDevice *d)
     trace_qxl_soft_reset(d->id);
     qxl_check_state(d);
     qxl_clear_guest_bug(d);
+    d->current_async = QXL_UNDEFINED_IO;
 
     if (d->id == 0) {
         qxl_enter_vga_mode(d);
-- 
1.7.10.1