[Qemu-devel] [PULL 0/3] vga: some cirrus fixes.

[Qemu-devel] [PULL 0/3] vga: some cirrus fixes.

[Gerd Hoffmann]

  Hi,

Small collection of cirrus fixes.

please pull,
  Gerd

The following changes since commit 9e99c5fd7060267a0331d900e73fdb36047bfe0c:

  tests: Fix unterminated string output visitor enum human string (2014-07-10 11:53:14 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-vga-20140711-1

for you to fetch changes up to d16136d22af0fcf0d651de04c9e3cbc7137cc6f9:

  cirrus: Fix host CPU blits (2014-07-11 10:17:02 +0200)

----------------------------------------------------------------
vga: some cirrus fixes.

----------------------------------------------------------------
Benjamin Herrenschmidt (2):
      cirrus: Fix build of debug code
      cirrus: Fix host CPU blits

Gonglei (1):
      cirrus_vga: adding sanity check for vram size

 hw/display/cirrus_vga.c     | 24 ++++++++++++++++++++----
 hw/display/cirrus_vga_rop.h |  3 +--
 2 files changed, 21 insertions(+), 6 deletions(-)

[Qemu-devel] [PULL 1/3] cirrus_vga: adding sanity check for vram size

[Gerd Hoffmann]

From: Gonglei <arei.gonglei@huawei.com>

when configure a invalid vram size for cirrus card, such as less
2 MB, which will crash qemu. Follow the real hardware, the cirrus
card has 4 MB video memory. Also for backward compatibility, accept
8 MB and 16 MB vram size.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 6fbe39d..52d039e 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2911,6 +2911,14 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp)
     ISACirrusVGAState *d = ISA_CIRRUS_VGA(dev);
     VGACommonState *s = &d->cirrus_vga.vga;
 
+    /* follow real hardware, cirrus card emulated has 4 MB video memory.
+       Also accept 8 MB/16 MB for backward compatibility. */
+    if (s->vram_size_mb != 4 && s->vram_size_mb != 8 &&
+        s->vram_size_mb != 16) {
+        error_setg(errp, "Invalid cirrus_vga ram size '%u'",
+                   s->vram_size_mb);
+        return;
+    }
     vga_common_init(s, OBJECT(dev), true);
     cirrus_init_common(&d->cirrus_vga, OBJECT(dev), CIRRUS_ID_CLGD5430, 0,
                        isa_address_space(isadev),
@@ -2957,6 +2965,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
      int16_t device_id = pc->device_id;
 
+     /* follow real hardware, cirrus card emulated has 4 MB video memory.
+       Also accept 8 MB/16 MB for backward compatibility. */
+     if (s->vga.vram_size_mb != 4 && s->vga.vram_size_mb != 8 &&
+         s->vga.vram_size_mb != 16) {
+         error_report("Invalid cirrus_vga ram size '%u'",
+                      s->vga.vram_size_mb);
+         return -1;
+     }
      /* setup VGA */
      vga_common_init(&s->vga, OBJECT(dev), true);
      cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
-- 
1.8.3.1

[Qemu-devel] [PULL 2/3] cirrus: Fix build of debug code

[Gerd Hoffmann]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Use PRIu64 to print uint64_t

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 52d039e..db330e9 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2059,7 +2059,7 @@ static void cirrus_vga_mem_write(void *opaque,
 	}
     } else {
 #ifdef DEBUG_CIRRUS
-        printf("cirrus: mem_writeb " TARGET_FMT_plx " value %02x\n", addr,
+        printf("cirrus: mem_writeb " TARGET_FMT_plx " value 0x%02" PRIu64 "\n", addr,
                mem_value);
 #endif
     }
@@ -2594,7 +2594,7 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
 	break;
     case 0x3c5:
 #ifdef DEBUG_VGA_REG
-	printf("vga: write SR%x = 0x%02x\n", s->sr_index, val);
+	printf("vga: write SR%x = 0x%02" PRIu64 "\n", s->sr_index, val);
 #endif
 	cirrus_vga_write_sr(c, val);
         break;
@@ -2619,7 +2619,7 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
 	break;
     case 0x3cf:
 #ifdef DEBUG_VGA_REG
-	printf("vga: write GR%x = 0x%02x\n", s->gr_index, val);
+	printf("vga: write GR%x = 0x%02" PRIu64 "\n", s->gr_index, val);
 #endif
 	cirrus_vga_write_gr(c, s->gr_index, val);
 	break;
@@ -2630,7 +2630,7 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     case 0x3b5:
     case 0x3d5:
 #ifdef DEBUG_VGA_REG
-	printf("vga: write CR%x = 0x%02x\n", s->cr_index, val);
+	printf("vga: write CR%x = 0x%02"PRIu64"\n", s->cr_index, val);
 #endif
 	cirrus_vga_write_cr(c, val);
 	break;
-- 
1.8.3.1

[Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Gerd Hoffmann]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Commit b2eb849d4b1fdb6f35d5c46958c7f703cf64cfef
"CVE-2007-1320 - Cirrus LGD-54XX "bitblt" heap overflow" broke
cpu to video blits.

When the ROP function is called from cirrus_bitblt_cputovideo_next(),
we pass 0 for the pitch but only operate on one line at a time. The
added test was tripping because after the initial substraction, the
pitch becomes negative. Make the test only trip when the height is
larger than one (ie. the pitch is actually used).

This fixes HW cursor support in Windows NT4.0 (which otherwise was
a white rectangle) and general display of icons in that OS when using
8bpp mode.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga_rop.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h
index 9c7bb09..0925a00 100644
--- a/hw/display/cirrus_vga_rop.h
+++ b/hw/display/cirrus_vga_rop.h
@@ -52,8 +52,7 @@ glue(cirrus_bitblt_rop_fwd_, ROP_NAME)(CirrusVGAState *s,
     dstpitch -= bltwidth;
     srcpitch -= bltwidth;
 
-    if (dstpitch < 0 || srcpitch < 0) {
-        /* is 0 valid? srcpitch == 0 could be useful */
+    if (bltheight > 1 && (dstpitch < 0 || srcpitch < 0)) {
         return;
     }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Peter Lieven]

Hi Benjamin,

On 11.07.2014 12:24, Gerd Hoffmann wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> Commit b2eb849d4b1fdb6f35d5c46958c7f703cf64cfef
> "CVE-2007-1320 - Cirrus LGD-54XX "bitblt" heap overflow" broke
> cpu to video blits.
>
> When the ROP function is called from cirrus_bitblt_cputovideo_next(),
> we pass 0 for the pitch but only operate on one line at a time. The
> added test was tripping because after the initial substraction, the
> pitch becomes negative. Make the test only trip when the height is
> larger than one (ie. the pitch is actually used).
>
> This fixes HW cursor support in Windows NT4.0 (which otherwise was
> a white rectangle) and general display of icons in that OS when using
> 8bpp mode.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hw/display/cirrus_vga_rop.h | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h
> index 9c7bb09..0925a00 100644
> --- a/hw/display/cirrus_vga_rop.h
> +++ b/hw/display/cirrus_vga_rop.h
> @@ -52,8 +52,7 @@ glue(cirrus_bitblt_rop_fwd_, ROP_NAME)(CirrusVGAState *s,
>       dstpitch -= bltwidth;
>       srcpitch -= bltwidth;
>   
> -    if (dstpitch < 0 || srcpitch < 0) {
> -        /* is 0 valid? srcpitch == 0 could be useful */
> +    if (bltheight > 1 && (dstpitch < 0 || srcpitch < 0)) {
>           return;
>       }
>   

it seems you have digged into the cirrus code recently. Have you an idea how to
fix the issue with the graphics corruption for cirrus vga and recent X Server versions?

E.g. take an Ubuntu 14.04 Desktop CD, boot it into live mode and open terminal.

I have tried to debug it a little, but I have no clue how to solve this. I tried to get
hands on a real hardware Cirrus Logic Graphics card and test if this happens there as well,
but I had no chance to get one.

Peter

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Benjamin Herrenschmidt]

On Mon, 2014-07-14 at 09:24 +0200, Peter Lieven wrote:
> it seems you have digged into the cirrus code recently. Have you an idea how to
> fix the issue with the graphics corruption for cirrus vga and recent X Server versions?
> 
> E.g. take an Ubuntu 14.04 Desktop CD, boot it into live mode and open terminal.
> 
> I have tried to debug it a little, but I have no clue how to solve this. I tried to get
> hands on a real hardware Cirrus Logic Graphics card and test if this happens there as well,
> but I had no chance to get one.

I *think* it is a X server side bug. They broke 24bpp packed pixel
format. I've seen patches submitted to the Xorg list fairly recently by
somebody from canonical.

You can test by creating an xorg.conf to force the display to 16bpp
instead of 24 and see if that helps.

Cheers,
Ben.

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Benjamin Herrenschmidt]

On Mon, 2014-07-14 at 17:29 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2014-07-14 at 09:24 +0200, Peter Lieven wrote:
> > it seems you have digged into the cirrus code recently. Have you an idea how to
> > fix the issue with the graphics corruption for cirrus vga and recent X Server versions?
> > 
> > E.g. take an Ubuntu 14.04 Desktop CD, boot it into live mode and open terminal.
> > 
> > I have tried to debug it a little, but I have no clue how to solve this. I tried to get
> > hands on a real hardware Cirrus Logic Graphics card and test if this happens there as well,
> > but I had no chance to get one.
> 
> I *think* it is a X server side bug. They broke 24bpp packed pixel
> format. I've seen patches submitted to the Xorg list fairly recently by
> somebody from canonical.
> 
> You can test by creating an xorg.conf to force the display to 16bpp
> instead of 24 and see if that helps.

Look for patches from "Robert Ancell <robert.ancell@canonical.com>" on
the xorg-devel list. In fact I'd suggest you test and report if that
works for you, that might give Keithp an incentive for actually merging
them :-)

Cheers,
Ben.

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Peter Lieven]

On 14.07.2014 09:29, Benjamin Herrenschmidt wrote:
> On Mon, 2014-07-14 at 09:24 +0200, Peter Lieven wrote:
>> it seems you have digged into the cirrus code recently. Have you an idea how to
>> fix the issue with the graphics corruption for cirrus vga and recent X Server versions?
>>
>> E.g. take an Ubuntu 14.04 Desktop CD, boot it into live mode and open terminal.
>>
>> I have tried to debug it a little, but I have no clue how to solve this. I tried to get
>> hands on a real hardware Cirrus Logic Graphics card and test if this happens there as well,
>> but I had no chance to get one.
> I *think* it is a X server side bug. They broke 24bpp packed pixel
> format. I've seen patches submitted to the Xorg list fairly recently by
> somebody from canonical.
>
> You can test by creating an xorg.conf to force the display to 16bpp
> instead of 24 and see if that helps.

Will do, thanks for the pointer.

Do you see a way to work around this in the graphics driver. E.g. blacklisting 24bpp modes
etc. Even if X.Org people actually merge the fixes there are 2 years of Linux releases out
there that have corrupted graphics.

Peter

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Gerd Hoffmann]

  Hi,

> Do you see a way to work around this in the graphics driver. E.g. blacklisting 24bpp modes
> etc. Even if X.Org people actually merge the fixes there are 2 years of Linux releases out
> there that have corrupted graphics.

For anything using kernel 3.14+ IMO the answer to pretty much *any* gfx
issue is "use stdvga instead of cirrus".  3.14 got a kms driver for the
qemu stdvga (bochsdrm), so the modesetting driver and all the modern
stuff such as root-less X server works with the stdvga too.  And the
constrains cirrus has (due to emulating existing, old hardware) are
gone.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Peter Lieven]

On 14.07.2014 11:53, Gerd Hoffmann wrote:
>    Hi,
>
>> Do you see a way to work around this in the graphics driver. E.g. blacklisting 24bpp modes
>> etc. Even if X.Org people actually merge the fixes there are 2 years of Linux releases out
>> there that have corrupted graphics.
> For anything using kernel 3.14+ IMO the answer to pretty much *any* gfx
> issue is "use stdvga instead of cirrus".  3.14 got a kms driver for the
> qemu stdvga (bochsdrm), so the modesetting driver and all the modern
> stuff such as root-less X server works with the stdvga too.  And the
> constrains cirrus has (due to emulating existing, old hardware) are
> gone.

Thats good news. For older Linuxs it seems that also -vga vmware is a good alternative.
The only thing that is a bit odd is that the X.Org driver automatically switches to the maximum
resolution at start. I will eventually add a patch to limit the maximum resolution on the qemu command
line.

-vga std was also a good choice, but for Win2012(R2) the maximum resolution is at 1024x768 so I
had to switch to -vga vmware there as well.

Peter

Re: [Qemu-devel] [PULL 3/3] cirrus: Fix host CPU blits

[Gerd Hoffmann]

  Hi,

> > For anything using kernel 3.14+ IMO the answer to pretty much *any* gfx
> > issue is "use stdvga instead of cirrus".  3.14 got a kms driver for the
> > qemu stdvga (bochsdrm), so the modesetting driver and all the modern
> > stuff such as root-less X server works with the stdvga too.  And the
> > constrains cirrus has (due to emulating existing, old hardware) are
> > gone.
> 
> Thats good news. For older Linuxs it seems that also -vga vmware is a good alternative.
> The only thing that is a bit odd is that the X.Org driver automatically switches to the maximum
> resolution at start.

Hmm, it should not to that.  bochsdrmfb explicitly orders 1024x768 to
the top of the list to make it the default, while keeping the other
resolutions available so you can easily switch via xrandr if you want.

Note that it also might be some desktop component switching via xrandr,
not the xorg modesetting driver.  Xorg logfile will probably tell.

cheers,
  Gerd

Re: [Qemu-devel] [PULL 0/3] vga: some cirrus fixes.

[Peter Maydell]

On 11 July 2014 11:24, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Small collection of cirrus fixes.
>
> please pull,
>   Gerd
>
> The following changes since commit 9e99c5fd7060267a0331d900e73fdb36047bfe0c:
>
>   tests: Fix unterminated string output visitor enum human string (2014-07-10 11:53:14 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-vga-20140711-1
>
> for you to fetch changes up to d16136d22af0fcf0d651de04c9e3cbc7137cc6f9:
>
>   cirrus: Fix host CPU blits (2014-07-11 10:17:02 +0200)
>
> ----------------------------------------------------------------
> vga: some cirrus fixes.
>
> ----------------------------------------------------------------
> Benjamin Herrenschmidt (2):
>       cirrus: Fix build of debug code
>       cirrus: Fix host CPU blits
>
> Gonglei (1):
>       cirrus_vga: adding sanity check for vram size

Applied, thanks.

-- PMM