Re: [Qemu-devel] [PATCH 08/10] qxl: add vgamem_size_mb and vgamem_size

[Alon Levy <alevy@redhat.com>]

On Tue, Jun 12, 2012 at 09:51:12AM +0200, Gerd Hoffmann wrote:
> From: Alon Levy <alevy@redhat.com>
> 

Please don't push this, it breaks both drivers, leaving n_modes 0 in the
rom. I'll send a fixed version.

> In preperation for supporting a larger framebuffer for multiple monitors
> on a single card, add a property to qxl vgamem_size_mb, and corresponding
> byte sized vgamem_size, and use instead of VGA_RAM_SIZE.
> 
> [ kraxel: simplify property handling, add sanity checks ]
> [ kraxel: fix mode copying ]
> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/qxl.c |   72 ++++++++++++++++++++++++++++++++++++-------------------------
>  hw/qxl.h |    2 +
>  2 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index c40cf55..c9028dd 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -27,8 +27,6 @@
>  
>  #include "qxl.h"
>  
> -#define VGA_RAM_SIZE (8192 * 1024)
> -
>  /*
>   * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
>   * such can be changed by the guest, so to avoid a guest trigerrable
> @@ -116,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;
> @@ -286,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;
> @@ -298,8 +293,8 @@ 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;
> -    int i;
> +    uint32_t fb;
> +    int i, n;
>  
>      memset(rom, 0, d->rom_size);
>  
> @@ -314,26 +309,25 @@ 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++) {
> +    for (i = 0, n = 0; i < modes->n_modes; i++) {
>          fb = qxl_modes[i].y_res * qxl_modes[i].stride;
> -        if (maxfb < fb) {
> -            maxfb = fb;
> +        if (fb > d->vgamem_size) {
> +            continue;
>          }
> -        modes->modes[i].id          = cpu_to_le32(i);
> -        modes->modes[i].x_res       = cpu_to_le32(qxl_modes[i].x_res);
> -        modes->modes[i].y_res       = cpu_to_le32(qxl_modes[i].y_res);
> -        modes->modes[i].bits        = cpu_to_le32(qxl_modes[i].bits);
> -        modes->modes[i].stride      = cpu_to_le32(qxl_modes[i].stride);
> -        modes->modes[i].x_mili      = cpu_to_le32(qxl_modes[i].x_mili);
> -        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;
> +        modes->modes[n].id          = cpu_to_le32(i);
> +        modes->modes[n].x_res       = cpu_to_le32(qxl_modes[i].x_res);
> +        modes->modes[n].y_res       = cpu_to_le32(qxl_modes[i].y_res);
> +        modes->modes[n].bits        = cpu_to_le32(qxl_modes[i].bits);
> +        modes->modes[n].stride      = cpu_to_le32(qxl_modes[i].stride);
> +        modes->modes[n].x_mili      = cpu_to_le32(qxl_modes[i].x_mili);
> +        modes->modes[n].y_mili      = cpu_to_le32(qxl_modes[i].y_mili);
> +        modes->modes[n].orientation = cpu_to_le32(qxl_modes[i].orientation);
> +        n++;
> +    }
> +    modes->n_modes     = cpu_to_le32(n);
>  
>      ram_header_size    = ALIGN(sizeof(QXLRam), 4096);
> -    surface0_area_size = ALIGN(maxfb, 4096);
> +    surface0_area_size = ALIGN(d->vgamem_size, 4096);
>      num_pages          = d->vga.vram_size;
>      num_pages         -= ram_header_size;
>      num_pages         -= surface0_area_size;
> @@ -1205,6 +1199,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->vgamem_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",
> @@ -1714,14 +1718,20 @@ static DisplayChangeListener display_listener = {
>      .dpy_refresh = display_refresh,
>  };
>  
> -static void qxl_init_ramsize(PCIQXLDevice *qxl, uint32_t ram_min_mb)
> +static void qxl_init_ramsize(PCIQXLDevice *qxl)
>  {
> -    /* vga ram (bar 0) */
> +    /* vga mode framebuffer / primary surface (bar 0, first part) */
> +    if (qxl->vgamem_size_mb < 8) {
> +        qxl->vgamem_size_mb = 8;
> +    }
> +    qxl->vgamem_size = qxl->vgamem_size_mb * 1024 * 1024;
> +
> +    /* vga ram (bar 0, total) */
>      if (qxl->ram_size_mb != -1) {
>          qxl->vga.vram_size = qxl->ram_size_mb * 1024 * 1024;
>      }
> -    if (qxl->vga.vram_size < ram_min_mb * 1024 * 1024) {
> -        qxl->vga.vram_size = ram_min_mb * 1024 * 1024;
> +    if (qxl->vga.vram_size < qxl->vgamem_size * 2) {
> +        qxl->vga.vram_size = qxl->vgamem_size * 2;
>      }
>  
>      /* vram32 (surfaces, 32bit, bar 1) */
> @@ -1744,6 +1754,7 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl, uint32_t ram_min_mb)
>          qxl->vram32_size = 4096;
>          qxl->vram_size = 4096;
>      }
> +    qxl->vgamem_size = msb_mask(qxl->vgamem_size * 2 - 1);
>      qxl->vga.vram_size = msb_mask(qxl->vga.vram_size * 2 - 1);
>      qxl->vram32_size = msb_mask(qxl->vram32_size * 2 - 1);
>      qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1);
> @@ -1855,7 +1866,7 @@ static int qxl_init_primary(PCIDevice *dev)
>      PortioList *qxl_vga_port_list = g_new(PortioList, 1);
>  
>      qxl->id = 0;
> -    qxl_init_ramsize(qxl, 32);
> +    qxl_init_ramsize(qxl);
>      vga->vram_size_mb = qxl->vga.vram_size >> 20;
>      vga_common_init(vga);
>      vga_init(vga, pci_address_space(dev), pci_address_space_io(dev), false);
> @@ -1878,7 +1889,7 @@ static int qxl_init_secondary(PCIDevice *dev)
>      PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
>  
>      qxl->id = device_id++;
> -    qxl_init_ramsize(qxl, 16);
> +    qxl_init_ramsize(qxl);
>      memory_region_init_ram(&qxl->vga.vram, "qxl.vgavram", qxl->vga.vram_size);
>      vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev);
>      qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram);
> @@ -2056,6 +2067,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("vgamem_mb", PCIQXLDevice, vgamem_size_mb, 8),
>          DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/qxl.h b/hw/qxl.h
> index a4ab7cc..172baf6 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           vgamem_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          vgamem_size_mb;
>  
>      /* qxl_render_update state */
>      int                render_update_cookie_num;
> -- 
> 1.7.1
> 
>