Re: [Qemu-devel] [PATCH 2/3] ramfb enhancement

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 5/9/19 10:58 AM, Hou Qiming wrote:
>

The subject for this patch may be:

     [PATCH V2 2/3] hw/display/ramfb: don't allow resolution change 
until vm reset

> Only allow one resolution change per guest boot, which prevents a
> crash when the guest writes garbage to the configuration space (e.g.
> when rebooting).
>

It is actually a cool feature, changing the resolution following a 
display window
resize, but sadly is not stable enough. Let's hope it will be fixed someday.

> Signed-off-by: HOU Qiming <hqm03ster@gmail.com 
> <mailto:hqm03ster@gmail.com>>
> ---
>  hw/display/ramfb.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
> index c27fcc7..fa6296b 100644
> --- a/hw/display/ramfb.c
> +++ b/hw/display/ramfb.c
> @@ -31,6 +31,7 @@ struct RAMFBState {
>      uint32_t width, height;
>      hwaddr addr, length;
>      struct RAMFBCfg cfg;
> +    bool locked;
>  };
>
>  static void qemu_unmap_displaysurface_guestmem(pixman_image_t *image,
> @@ -73,11 +74,11 @@ static DisplaySurface 
> *qemu_create_displaysurface_guestmem(
>  static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
>  {
>      RAMFBState *s = dev;
> -    uint32_t fourcc, format;
> +    uint32_t fourcc, format, width, height;
>      hwaddr stride, addr, length;
>
> -    s->width  = be32_to_cpu(s->cfg.width);
> -    s->height = be32_to_cpu(s->cfg.height);
> +    width     = be32_to_cpu(s->cfg.width);
> +    height    = be32_to_cpu(s->cfg.height);
>      stride    = be32_to_cpu(s->cfg.stride);
>      fourcc    = be32_to_cpu(s->cfg.fourcc);
>      addr      = be64_to_cpu(s->cfg.addr);
> @@ -85,9 +86,16 @@ static void ramfb_fw_cfg_write(void *dev, off_t 
> offset, size_t len)
>      format    = qemu_drm_format_to_pixman(fourcc);
>
>      fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
> -            s->width, s->height, addr);
> +            width, height, addr);
> +    if (s->locked) {
> +        fprintf(stderr, "%s: resolution locked, change rejected\n", 
> __func__);
> +        return;
> +    }
> +    s->locked = true;
>      s->addr = addr;
>      s->length = length;
> +    s->width = width;
> +    s->height = height;
>      s->ds = qemu_create_displaysurface_guestmem(s->width, s->height,
>                                                  format, stride, s->addr);
>  }
> @@ -107,6 +115,13 @@ void ramfb_display_update(QemuConsole *con, 
> RAMFBState *s)
>      dpy_gfx_update_full(con);
>  }
>
> +static void ramfb_reset(void *opaque)
> +{
> +    RAMFBState *s = (RAMFBState *)opaque;
> +    s->locked = false;
> +    memset(&s->cfg, 0, sizeof(s->cfg));
> +}
> +
>  RAMFBState *ramfb_setup(Error **errp)
>  {
>      FWCfgState *fw_cfg = fw_cfg_find();
> @@ -119,9 +134,12 @@ RAMFBState *ramfb_setup(Error **errp)
>
>      s = g_new0(RAMFBState, 1);
>
> +    s->locked = false;
> +
>      rom_add_vga("vgabios-ramfb.bin");
>      fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
>                               NULL, ramfb_fw_cfg_write, s,
>                               &s->cfg, sizeof(s->cfg), false);
> +    qemu_register_reset(ramfb_reset, s);
>      return s;
>  }
> -- 
> 2.17.1
>
>