From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qb6T9-0000Hw-WF for qemu-devel@nongnu.org; Mon, 27 Jun 2011 03:42:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qb6T7-0003v9-Rq for qemu-devel@nongnu.org; Mon, 27 Jun 2011 03:42:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5526) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qb6T7-0003uu-7V for qemu-devel@nongnu.org; Mon, 27 Jun 2011 03:42:53 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p5R7gpFX001995 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 27 Jun 2011 03:42:51 -0400 Message-ID: <4E083476.2030509@redhat.com> Date: Mon, 27 Jun 2011 10:42:46 +0300 From: yhalperi MIME-Version: 1.0 References: <1308920577-31569-1-git-send-email-alevy@redhat.com> <1308920577-31569-13-git-send-email-alevy@redhat.com> In-Reply-To: <1308920577-31569-13-git-send-email-alevy@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qxl: add primary_created state, change mode lifetimes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: qemu-devel@nongnu.org, kraxel@redhat.com On 06/24/2011 04:02 PM, Alon Levy wrote: > Currently we define the following relation between modes and primary surface > existance: > > QXL_MODE_COMPAT > QXL_MODE_NATIVE primary exists > QXL_MODE_UNDEFINED > QXL_MODE_VGA primary doesn't exist > > This, coupled with disallowing various IO's when not in QXL_MODE_NATIVE or COMPAT > means that the S3 implementation gets more complex - This is wrong: > DESTROY_SURFACES > SURFACE_RELEASE > .. wakeup > PRIMARY_CREATE<-- error, we are already in NATIVE_MODE, not allowed! > Shouldn't the mode change to QXL_MODE_UNDEFINED on DESTROY_SURFACES? (I can see it currently doesn't, but it seems like a mistake) > This patch adds an explicit primary_created status, changed by: > CREATE_PRIMARY, DESTROY_PRIMARY, DESTROY_SURFACES > > This patch also changes the lifetime of QXL_MODE_NATIVE. > CREATE_PRIMARY - enter QXL_MODE_NATIVE > vga io - exit QXL_MODE_NATIVE > > anything else doesn't effect it, specifically DESTROY_PRIMARY. > --- > hw/qxl.c | 38 +++++++++++++++++++++++++++----------- > hw/qxl.h | 1 + > 2 files changed, 28 insertions(+), 11 deletions(-) > > diff --git a/hw/qxl.c b/hw/qxl.c > index fab0208..51a5270 100644 > --- a/hw/qxl.c > +++ b/hw/qxl.c > @@ -666,6 +666,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d) > return; > } > dprint(d, 1, "%s\n", __FUNCTION__); > + d->primary_created = 1; > qemu_spice_create_host_primary(&d->ssd); > d->mode = QXL_MODE_VGA; > memset(&d->ssd.dirty, 0, sizeof(d->ssd.dirty)); > @@ -677,10 +678,9 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d) > return; > } > dprint(d, 1, "%s\n", __FUNCTION__); > - if (d->mode != QXL_MODE_UNDEFINED) { > - d->mode = QXL_MODE_UNDEFINED; > - qemu_spice_destroy_primary_surface(&d->ssd, 0); > - } > + d->mode = QXL_MODE_UNDEFINED; > + d->primary_created = 0; > + qemu_spice_destroy_primary_surface(&d->ssd, 0); > } > > static void qxl_set_irq(PCIQXLDevice *d) > @@ -780,7 +780,9 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) > dprint(qxl, 1, "%s\n", __FUNCTION__); > if (qxl->mode != QXL_MODE_UNDEFINED) { > qxl->mode = QXL_MODE_UNDEFINED; > - qemu_spice_destroy_primary_surface(&qxl->ssd, 0); > + if (qxl->primary_created) { > + qemu_spice_destroy_primary_surface(&qxl->ssd, 0); > + } > } > qxl_soft_reset(qxl); > } > @@ -912,8 +914,10 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm, > { > QXLSurfaceCreate *sc =&qxl->guest_primary.surface; > > - assert(qxl->mode != QXL_MODE_NATIVE); > - qxl_exit_vga_mode(qxl); > + if (qxl->mode != QXL_MODE_NATIVE) { > + qxl_exit_vga_mode(qxl); > + } > + assert(!qxl->primary_created); > > dprint(qxl, 1, "%s: %dx%d\n", __FUNCTION__, > le32_to_cpu(sc->width), le32_to_cpu(sc->height)); > @@ -933,6 +937,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm, > surface->flags |= QXL_SURF_FLAG_KEEP_DATA; > } > > + qxl->primary_created = 1; > qxl->mode = QXL_MODE_NATIVE; > qxl->cmdflags = 0; > > @@ -1156,15 +1161,19 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val) > case QXL_IO_DESTROY_PRIMARY: > PANIC_ON(val != 0); > dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode)); > - if (d->mode != QXL_MODE_UNDEFINED) { > - d->mode = QXL_MODE_UNDEFINED; > + if (d->primary_created) { > + d->primary_created = 0; > qemu_spice_destroy_primary_surface(&d->ssd, 0); > } > break; > case QXL_IO_DESTROY_SURFACE_WAIT: > + if (val == 0) { > + d->primary_created = 0; > + } > qemu_spice_destroy_surface_wait(&d->ssd, val); > break; > case QXL_IO_DESTROY_ALL_SURFACES: > + d->primary_created = 0; > qemu_spice_destroy_surfaces(&d->ssd); > break; > case QXL_IO_FLUSH_SURFACES: > @@ -1209,8 +1218,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val) > case QXL_IO_DESTROY_PRIMARY_ASYNC: > PANIC_ON(val != 0); > dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC\n"); > - if (d->mode != QXL_MODE_UNDEFINED) { > - d->mode = QXL_MODE_UNDEFINED; > + if (d->primary_created) { > + d->primary_created = 0; > async = qemu_mallocz(sizeof(*async)); > goto async_common; > } else { > @@ -1218,13 +1227,20 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val) > break; > } > case QXL_IO_UPDATE_AREA_ASYNC: > + dprint(d, 1, "QXL_IO_UPDATE_AREA_ASYNC %d\n", val); > async = qemu_mallocz(sizeof(*async)); > async->update_area = d->ram->update_area; > async->update_surface = d->ram->update_surface; > goto async_common; > case QXL_IO_NOTIFY_OOM_ASYNC: > + goto async_common; > case QXL_IO_DESTROY_SURFACE_ASYNC: > + if (val == 0) { > + d->primary_created = 0; > + } > + goto async_common; > case QXL_IO_DESTROY_ALL_SURFACES_ASYNC: > + d->primary_created = 0; > async = qemu_mallocz(sizeof(*async)); > async_common: > async->port = io_port; > diff --git a/hw/qxl.h b/hw/qxl.h > index 584f02b..893f8d8 100644 > --- a/hw/qxl.h > +++ b/hw/qxl.h > @@ -23,6 +23,7 @@ typedef struct PCIQXLDevice { > uint32_t guestdebug; > uint32_t cmdlog; > enum qxl_mode mode; > + uint32_t primary_created; > uint32_t cmdflags; > int generation; > uint32_t revision;