* [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message
@ 2014-01-20 10:44 Alon Levy
2014-01-20 10:44 ` [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset Alon Levy
2014-01-20 11:55 ` [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Markus Armbruster
0 siblings, 2 replies; 7+ messages in thread
From: Alon Levy @ 2014-01-20 10:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Signed-off-by: Alon Levy <alevy@redhat.com>
---
hw/display/qxl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..f6af470 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1367,7 +1367,8 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
size = abs(requested_stride) * requested_height;
if (size > qxl->vgamem_size) {
qxl_set_guest_bug(qxl, "%s: requested primary larger then framebuffer"
- " size", __func__);
+ " size %d > %d", __func__, size,
+ qxl->vgamem_size);
return;
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset
2014-01-20 10:44 [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Alon Levy
@ 2014-01-20 10:44 ` Alon Levy
2014-01-21 10:56 ` Gerd Hoffmann
2014-01-20 11:55 ` [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Markus Armbruster
1 sibling, 1 reply; 7+ messages in thread
From: Alon Levy @ 2014-01-20 10:44 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Without this we occasionally trigger an assert at
hw/pci/pci.c:pcibus_reset that asserts the irq_count is zero on reset.
This has become a problem with the new drm driver for linux, since doing
a reboot from console causes a race between console updates that set the
irq and the reset assertion that the irq is clear.
Signed-off-by: Alon Levy <alevy@redhat.com>
---
hw/display/qxl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index f6af470..e164d96 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1126,6 +1126,7 @@ static void qxl_reset_state(PCIQXLDevice *d)
d->num_free_res = 0;
d->last_release = NULL;
memset(&d->ssd.dirty, 0, sizeof(d->ssd.dirty));
+ qxl_update_irq(d);
}
static void qxl_soft_reset(PCIQXLDevice *d)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message
2014-01-20 10:44 [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Alon Levy
2014-01-20 10:44 ` [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset Alon Levy
@ 2014-01-20 11:55 ` Markus Armbruster
2014-01-20 13:29 ` [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison Alon Levy
1 sibling, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2014-01-20 11:55 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel, kraxel
Alon Levy <alevy@redhat.com> writes:
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
> hw/display/qxl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index e4f172e..f6af470 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1367,7 +1367,8 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
> size = abs(requested_stride) * requested_height;
> if (size > qxl->vgamem_size) {
> qxl_set_guest_bug(qxl, "%s: requested primary larger then framebuffer"
> - " size", __func__);
> + " size %d > %d", __func__, size,
> + qxl->vgamem_size);
> return;
> }
Shouldn't you use %u or %PRIu32 to print uint32_t qxl->vgamem_size?
Hmm, we're comparing int size to uint32_t vgamem_size, not nice. Should
size be unsigned?
Since you touch the message anyway, you could fix "larger then" to
"larger than".
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison
2014-01-20 11:55 ` [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Markus Armbruster
@ 2014-01-20 13:29 ` Alon Levy
2014-01-20 13:52 ` Markus Armbruster
2014-01-20 13:53 ` Peter Maydell
0 siblings, 2 replies; 7+ messages in thread
From: Alon Levy @ 2014-01-20 13:29 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, armbru
Fix signed to unsigned comparison in qxl_create_guest_primary and add
the size of the framebuffer to the error message used when setting the
guest bug state (which causes a complete guess blackout until reset, so
it helps if it is verbose).
Signed-off-by: Alon Levy <alevy@redhat.com>
---
hw/display/qxl.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..b799b51 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1360,14 +1360,15 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
{
QXLDevSurfaceCreate surface;
QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
- int size;
+ uint32_t 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__);
+ qxl_set_guest_bug(qxl, "%s: requested primary larger than framebuffer"
+ " size %u > %u", __func__, size,
+ qxl->vgamem_size);
return;
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison
2014-01-20 13:29 ` [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison Alon Levy
@ 2014-01-20 13:52 ` Markus Armbruster
2014-01-20 13:53 ` Peter Maydell
1 sibling, 0 replies; 7+ messages in thread
From: Markus Armbruster @ 2014-01-20 13:52 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel, kraxel
Alon Levy <alevy@redhat.com> writes:
> Fix signed to unsigned comparison in qxl_create_guest_primary and add
> the size of the framebuffer to the error message used when setting the
> guest bug state (which causes a complete guess blackout until reset, so
> it helps if it is verbose).
>
> Signed-off-by: Alon Levy <alevy@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison
2014-01-20 13:29 ` [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison Alon Levy
2014-01-20 13:52 ` Markus Armbruster
@ 2014-01-20 13:53 ` Peter Maydell
1 sibling, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2014-01-20 13:53 UTC (permalink / raw)
To: Alon Levy; +Cc: Markus Armbruster, QEMU Developers, Gerd Hoffmann
On 20 January 2014 13:29, Alon Levy <alevy@redhat.com> wrote:
> Fix signed to unsigned comparison in qxl_create_guest_primary and add
> the size of the framebuffer to the error message used when setting the
> guest bug state (which causes a complete guess blackout until reset, so
> it helps if it is verbose).
>
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
> hw/display/qxl.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index e4f172e..b799b51 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1360,14 +1360,15 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
> {
> QXLDevSurfaceCreate surface;
> QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
> - int size;
> + uint32_t size;
> int requested_height = le32_to_cpu(sc->height);
> int requested_stride = le32_to_cpu(sc->stride);
>
> size = abs(requested_stride) * requested_height;
This looks a bit dubious -- the multiply is still going
to be done with signed arithmetic, so if it's possible
we might overflow so as to require a uint32_t size
then we've already hit undefined behaviour. Also, if
the multiply overflows 32 bits the check will not
catch this. Probably better to do this as a 64 bit
multiply.
Is requested_height really supposed to be signed?
Why is requested_stride an int that needs to go
through abs()? What is the behaviour supposed to be
if it is the minimum integer (in which case abs(x)
is undefined)?
> if (size > qxl->vgamem_size) {
> - qxl_set_guest_bug(qxl, "%s: requested primary larger then framebuffer"
> - " size", __func__);
> + qxl_set_guest_bug(qxl, "%s: requested primary larger than framebuffer"
> + " size %u > %u", __func__, size,
> + qxl->vgamem_size);
> return;
> }
PRIu32 is more portable for printing uint32_t types.
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset
2014-01-20 10:44 ` [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset Alon Levy
@ 2014-01-21 10:56 ` Gerd Hoffmann
0 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2014-01-21 10:56 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
On Mo, 2014-01-20 at 12:44 +0200, Alon Levy wrote:
> Without this we occasionally trigger an assert at
> hw/pci/pci.c:pcibus_reset that asserts the irq_count is zero on reset.
>
> This has become a problem with the new drm driver for linux, since
> doing
> a reboot from console causes a race between console updates that set
> the
> irq and the reset assertion that the irq is clear.
>
Added to spice patch queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-01-21 10:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-20 10:44 [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Alon Levy
2014-01-20 10:44 ` [Qemu-devel] [PATCH 2/2] qxl: clear irq on reset Alon Levy
2014-01-21 10:56 ` Gerd Hoffmann
2014-01-20 11:55 ` [Qemu-devel] [PATCH 1/2] hw/display/qxl: improve framebuffer error message Markus Armbruster
2014-01-20 13:29 ` [Qemu-devel] [PATCH v2] hw/display/qxl: fix signed to unsigned comparison Alon Levy
2014-01-20 13:52 ` Markus Armbruster
2014-01-20 13:53 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.