Re: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter

[李强]

Hello Gerd,

> -----Original Message-----
> From: Qemu-devel
> [mailto:address@hidden On Behalf Of
> Gerd Hoffmann
> Sent: Thursday, March 16, 2017 5:31 PM
> To: address@hidden
> Cc: Gerd Hoffmann
> Subject: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter
> 
> Ok, we have this beast in the cirrus code which is not used at all by modern
> guests, except when you try to find security holes in qemu.  So, add an option
> to disable blitter altogether.  Guests released within the last ten years 
> should
> not show any rendering issues if you turn off blitter support.
> 
> There are no known bugs in the cirrus blitter code.  But in the past we hoped 
> a
> few times already that we've finally nailed the last issue.  So having some 
> easy
> way to mitigate in case yet another blitter issue shows up certainly makes me
> sleep a bit better at night.
> 
> For completeness:  The by far better way to mitigate is to switch away from
> cirrus and use stdvga instead.  Or something more modern like virtio-vga in
> case your guest has support for it.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Message-id: address@hidden
> ---
>  hw/display/cirrus_vga.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index
> 6ffe64f..326d511 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -205,6 +205,7 @@ typedef struct CirrusVGAState {
>      uint32_t cirrus_bank_base[2];
>      uint32_t cirrus_bank_limit[2];
>      uint8_t cirrus_hidden_palette[48];
> +    bool enable_blitter;
>      int cirrus_blt_pixelwidth;
>      int cirrus_blt_width;
>      int cirrus_blt_height;
> @@ -960,6 +961,10 @@ static void cirrus_bitblt_start(CirrusVGAState * s)  {
>      uint8_t blt_rop;
> 
> +    if (!s->enable_blitter) {
> +        goto bitblt_ignore;
> +    }
> +
>      s->vga.gr[0x31] |= CIRRUS_BLT_BUSY;
> 
>      s->cirrus_blt_width = (s->vga.gr[0x20] | (s->vga.gr[0x21] << 8)) + 1; @@
> -3024,6 +3029,8 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev,
> Error **errp)  static Property isa_cirrus_vga_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", struct ISACirrusVGAState,
>                         cirrus_vga.vga.vram_size_mb, 4),
> +    DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
> +                       cirrus_vga.enable_blitter, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> @@ -3093,6 +3100,8 @@ static void pci_cirrus_vga_realize(PCIDevice *dev,
> Error **errp)  static Property pci_vga_cirrus_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
>                         cirrus_vga.vga.vram_size_mb, 4),
> +    DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
> +                     cirrus_vga.enable_blitter, true),

The default is 'ENABLE'? I think there should be 'false'.


Thanks.

Qiang

>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> --
> 1.8.3.1
>