From patchwork Thu Mar 29 20:56:46 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alon Levy X-Patchwork-Id: 149485 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6DA20B6EF4 for ; Fri, 30 Mar 2012 07:57:03 +1100 (EST) Received: from localhost ([::1]:40624 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDMOz-0000JF-EG for incoming@patchwork.ozlabs.org; Thu, 29 Mar 2012 16:57:01 -0400 Received: from eggs.gnu.org ([208.118.235.92]:37897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDMOr-0000Ix-Jj for qemu-devel@nongnu.org; Thu, 29 Mar 2012 16:56:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDMOp-0001N0-47 for qemu-devel@nongnu.org; Thu, 29 Mar 2012 16:56:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26663) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDMOo-0001Mu-Ox for qemu-devel@nongnu.org; Thu, 29 Mar 2012 16:56:51 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2TKunsX012062 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 29 Mar 2012 16:56:49 -0400 Received: from garlic.redhat.com (vpn-201-133.tlv.redhat.com [10.35.201.133]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q2TKul6A002228; Thu, 29 Mar 2012 16:56:47 -0400 From: Alon Levy To: qemu-devel@nongnu.org, kraxel@redhat.com Date: Thu, 29 Mar 2012 22:56:46 +0200 Message-Id: <1333054606-19847-1-git-send-email-alevy@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [RFC] qxl: don't panic on phys2virt X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Issues a qxl_guest_bug from qxl_phys2virt. Everywhere else will fail. qxl_phys2virt requires an additional argument because all it's possible return values are legit (well, I could use the fact it returns a pointer so it should be word aligned but I don't want to go there, this is totally internal). Signed-off-by: Alon Levy --- hw/qxl-logger.c | 38 +++++++++++++++++++---------- hw/qxl-render.c | 14 +++++++++-- hw/qxl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++---------- hw/qxl.h | 3 ++- 4 files changed, 99 insertions(+), 28 deletions(-) diff --git a/hw/qxl-logger.c b/hw/qxl-logger.c index 367aad1..1b8baee 100644 --- a/hw/qxl-logger.c +++ b/hw/qxl-logger.c @@ -100,12 +100,16 @@ static const char *qxl_v2n(const char *n[], size_t l, int v) } #define qxl_name(_list, _value) qxl_v2n(_list, ARRAY_SIZE(_list), _value) -static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id) +static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id, + int *error) { QXLImage *image; QXLImageDescriptor *desc; - image = qxl_phys2virt(qxl, addr, group_id); + image = qxl_phys2virt(qxl, addr, group_id, error); + if (*error) { + return; + } desc = &image->descriptor; fprintf(stderr, " (id %" PRIx64 " type %d flags %d width %d height %d", desc->id, desc->type, desc->flags, desc->width, desc->height); @@ -130,17 +134,19 @@ static void qxl_log_rect(QXLRect *rect) rect->left, rect->top); } -static void qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy, int group_id) +static void qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy, + int group_id, int *error) { fprintf(stderr, " src %" PRIx64, copy->src_bitmap); - qxl_log_image(qxl, copy->src_bitmap, group_id); + qxl_log_image(qxl, copy->src_bitmap, group_id, error); fprintf(stderr, " area"); qxl_log_rect(©->src_area); fprintf(stderr, " rop %d", copy->rop_descriptor); } -static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id) +static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id, + int *error) { fprintf(stderr, ": surface_id %d type %s effect %s", draw->surface_id, @@ -148,13 +154,13 @@ static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id) qxl_name(qxl_draw_effect, draw->effect)); switch (draw->type) { case QXL_DRAW_COPY: - qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id); + qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id, error); break; } } static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw, - int group_id) + int group_id, int *error) { fprintf(stderr, ": type %s effect %s", qxl_name(qxl_draw_type, draw->type), @@ -166,7 +172,7 @@ static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw, } switch (draw->type) { case QXL_DRAW_COPY: - qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id); + qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id, error); break; } } @@ -192,6 +198,7 @@ static void qxl_log_cmd_surface(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd) void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id) { QXLCursor *cursor; + int error; fprintf(stderr, ": %s", qxl_name(qxl_cursor_cmd, cmd->type)); @@ -202,7 +209,10 @@ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id) cmd->u.set.position.y, cmd->u.set.visible ? "yes" : "no", cmd->u.set.shape); - cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id); + cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id, &error); + if (error) { + return; + } fprintf(stderr, " type %s size %dx%d hot-spot +%d+%d" " unique 0x%" PRIx64 " data-size %d", qxl_name(spice_cursor_type, cursor->header.type), @@ -220,6 +230,7 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext) { bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT; void *data; + int error; if (!qxl->cmdlog) { return; @@ -230,13 +241,16 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext) qxl_name(qxl_type, ext->cmd.type), compat ? "(compat)" : ""); - data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id); + data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, &error); + if (error) { + return; + } switch (ext->cmd.type) { case QXL_CMD_DRAW: if (!compat) { - qxl_log_cmd_draw(qxl, data, ext->group_id); + qxl_log_cmd_draw(qxl, data, ext->group_id, &error); } else { - qxl_log_cmd_draw_compat(qxl, data, ext->group_id); + qxl_log_cmd_draw_compat(qxl, data, ext->group_id, &error); } break; case QXL_CMD_SURFACE: diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 28ab182..04eca5c 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -229,10 +229,16 @@ fail: /* called from spice server thread context only */ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) { - QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id); + int error; + QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, + &error); QXLCursor *cursor; QEMUCursor *c; + if (error) { + return; + } + if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) { return; } @@ -244,7 +250,11 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) } switch (cmd->type) { case QXL_CURSOR_SET: - cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id); + cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id, + &error); + if (error) { + return; + } if (cursor->chunk.data_size != cursor->data_size) { fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__); return; diff --git a/hw/qxl.c b/hw/qxl.c index db2318e..e7ffe99 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -382,15 +382,32 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl) /* * keep track of some command state, for savevm/loadvm. * called from spice server thread context only + * + * Returns 0 on success, 1 on failure. + * + * Failure is caused by: + * non initialized slots (qxl_phys2virt failure) + * illegal command + * surface id larger then NUM_SURFACES */ -static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext) +static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext) { + int error; + switch (le32_to_cpu(ext->cmd.type)) { case QXL_CMD_SURFACE: { - QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id); + QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, + &error); + if (error) { + return 1; + } uint32_t id = le32_to_cpu(cmd->surface_id); - PANIC_ON(id >= NUM_SURFACES); + + if (id >= NUM_SURFACES) { + qxl_guest_bug(qxl, "QXL_CMD_SURFACE id %d >= %d", id, NUM_SURFACES); + return 1; + } qemu_mutex_lock(&qxl->track_lock); if (cmd->type == QXL_SURFACE_CMD_CREATE) { qxl->guest_surfaces.cmds[id] = ext->cmd.data; @@ -407,7 +424,12 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext) } case QXL_CMD_CURSOR: { - QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id); + int error; + QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, + &error); + if (error) { + return 1; + } if (cmd->type == QXL_CURSOR_SET) { qemu_mutex_lock(&qxl->track_lock); qxl->guest_cursor = ext->cmd.data; @@ -416,6 +438,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext) break; } } + return 0; } /* spice display interface callbacks */ @@ -1087,25 +1110,45 @@ static void qxl_reset_surfaces(PCIQXLDevice *d) } /* can be also called from spice server thread context */ -void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id) +/* NULL is returned on error, but it can also be a legitimate return + * value, so this function needs to be changed to pass error by another + * argument */ +void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id, + int *error) { uint64_t phys = le64_to_cpu(pqxl); uint32_t slot = (phys >> (64 - 8)) & 0xff; uint64_t offset = phys & 0xffffffffffff; + assert(error); + *error = 1; switch (group_id) { case MEMSLOT_GROUP_HOST: return (void *)(intptr_t)offset; case MEMSLOT_GROUP_GUEST: - PANIC_ON(slot >= NUM_MEMSLOTS); - PANIC_ON(!qxl->guest_slots[slot].active); - PANIC_ON(offset < qxl->guest_slots[slot].delta); + if (slot >= NUM_MEMSLOTS) { + qxl_guest_bug(qxl, "slot too large %d >= %d", slot, NUM_MEMSLOTS); + return NULL; + } + if (!qxl->guest_slots[slot].active) { + qxl_guest_bug(qxl, "inactive slot %d\n", slot); + return NULL; + } + if (offset < qxl->guest_slots[slot].delta) { + qxl_guest_bug(qxl, "slot %d offset %"PRIu64" < delta %"PRIu64"\n", + slot, offset, qxl->guest_slots[slot].delta); + return NULL; + } offset -= qxl->guest_slots[slot].delta; - PANIC_ON(offset > qxl->guest_slots[slot].size) + if (offset > qxl->guest_slots[slot].size) { + qxl_guest_bug(qxl, "slot %d offset %"PRIu64" > size %"PRIu64"\n", + slot, offset, qxl->guest_slots[slot].size); + return NULL; + } + *error = 0; return qxl->guest_slots[slot].ptr + offset; - default: - PANIC_ON(1); } + return NULL; } static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl) @@ -1532,6 +1575,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl) { intptr_t vram_start; int i; + int error; if (qxl->mode != QXL_MODE_NATIVE && qxl->mode != QXL_MODE_COMPAT) { return; @@ -1554,11 +1598,13 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl) } cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i], - MEMSLOT_GROUP_GUEST); + MEMSLOT_GROUP_GUEST, &error); + assert(!error); assert(cmd->type == QXL_SURFACE_CMD_CREATE); surface_offset = (intptr_t)qxl_phys2virt(qxl, cmd->u.surface_create.data, - MEMSLOT_GROUP_GUEST); + MEMSLOT_GROUP_GUEST, &error); + assert(!error); surface_offset -= vram_start; surface_size = cmd->u.surface_create.height * abs(cmd->u.surface_create.stride); diff --git a/hw/qxl.h b/hw/qxl.h index 11a0db3..4a48d41 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -126,7 +126,8 @@ typedef struct PCIQXLDevice { #define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10 /* qxl.c */ -void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id); +void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id, + int *error); void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...); void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,