[Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-07-12 Thread Alon Levy 
Add two new IOs.
 QXL_IO_FLUSH_SURFACES - equivalent to update area for all surfaces, used
  to reduce vmexits from NumSurfaces to 1 on guest S3, S4 and resolution change 
(windows
  driver implementation is such that this is done on each of those occasions).
 QXL_IO_FLUSH_RELEASE - used to ensure anything on last_release is put on the 
release ring
  for the client to free.

Cc: Yonit Halperin yhalp...@redhat.com
---
 hw/qxl.c |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 1d6acce..a9cc1a3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -181,6 +181,13 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice 
*qxl, uint32_t id,
 }
 }
 
+#if SPICE_INTERFACE_QXL_MINOR = 1
+static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
+{
+qxl-ssd.worker-flush_surfaces_async(qxl-ssd.worker, 0);
+}
+#endif
+
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
uint32_t count)
 {
@@ -1195,6 +1202,7 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_DESTROY_PRIMARY_ASYNC:
 case QXL_IO_DESTROY_SURFACE_ASYNC:
 case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+case QXL_IO_FLUSH_SURFACES_ASYNC:
 #if SPICE_INTERFACE_QXL_MINOR  1
 fprintf(stderr, qxl: error: async not supported by libspice but guest 
driver used it\n);
 return;
@@ -1322,6 +1330,30 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 }
 qxl_spice_destroy_surface_wait(d, val, async);
 break;
+case QXL_IO_FLUSH_RELEASE: {
+QXLReleaseRing *ring = d-ram-release_ring;
+if (ring-prod - ring-cons + 1 == ring-num_items) {
+fprintf(stderr,
+ERROR: no flush, full release ring [p%d,%dc]\n,
+ring-prod, ring-cons);
+}
+qxl_push_free_res(d, 1 /* flush */);
+dprint(d, 1, QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n,
+qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res, d-last_release);
+break;
+}
+case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if SPICE_INTERFACE_QXL_MINOR = 1
+dprint(d, 1, QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n,
+   val, qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+   d-num_free_res);
+qxl_spice_flush_surfaces_async(d);
+#else
+dprint(d, 1, QXL_IO_FLUSH_SURFACES_ASYNC (%d) ignored, too old spice 
server\n,
+   val);
+#endif
+break;
 case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
 case QXL_IO_DESTROY_ALL_SURFACES:
 d-mode = QXL_MODE_UNDEFINED;
-- 
1.7.6

[Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Alon Levy 
Add two new IOs.
 QXL_IO_FLUSH_SURFACES - equivalent to update area for all surfaces, used
  to reduce vmexits from NumSurfaces to 1 on guest S3, S4 and resolution change 
(windows
  driver implementation is such that this is done on each of those occasions).
 QXL_IO_FLUSH_RELEASE - used to ensure anything on last_release is put on the 
release ring
  for the client to free.

Cc: Yonit Halperin yhalp...@redhat.com
---
 hw/qxl.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 29425a5..f158d45 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1247,6 +1247,30 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_DESTROY_ALL_SURFACES:
 qxl_spice_destroy_surfaces(d);
 break;
+case QXL_IO_FLUSH_SURFACES:
+dprint(d, 1, QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, res#=%d)\n,
+val, qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res);
+qemu_spice_stop(d-ssd);
+qemu_spice_start(d-ssd);
+dprint(d, 1, QXL_IO_FLUSH_SURFACES exit (%s, s#=%d, res#=%d,%p)\n,
+qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res, d-last_release);
+break;
+case QXL_IO_FLUSH_RELEASE: {
+QXLReleaseRing *ring = d-ram-release_ring;
+if (ring-prod - ring-cons + 1 == ring-num_items) {
+// TODO - return a value to the guest and let it loop?
+fprintf(stderr,
+ERROR: no flush, full release ring [p%d,%dc]\n,
+ring-prod, ring-cons);
+}
+qxl_push_free_res(d, 1 /* flush */);
+dprint(d, 1, QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n,
+qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res, d-last_release);
+break;
+}
 case QXL_IO_MEMSLOT_ADD_ASYNC:
 PANIC_ON(val = NUM_MEMSLOTS);
 PANIC_ON(d-guest_slots[val].active);
-- 
1.7.5.4

Re: [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Gerd Hoffmann 

  Hi,


+case QXL_IO_FLUSH_SURFACES:
+dprint(d, 1, QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, res#=%d)\n,
+val, qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res);
+qemu_spice_stop(d-ssd);
+qemu_spice_start(d-ssd);
+dprint(d, 1, QXL_IO_FLUSH_SURFACES exit (%s, s#=%d, res#=%d,%p)\n,
+qxl_mode_to_string(d-mode), d-guest_surfaces.count,
+d-num_free_res, d-last_release);
+break;


This should be async as we'll go sleep and wait for the spice server 
thread finish in qemu_spice_stop().



+case QXL_IO_FLUSH_RELEASE: {
+QXLReleaseRing *ring =d-ram-release_ring;
+if (ring-prod - ring-cons + 1 == ring-num_items) {
+// TODO - return a value to the guest and let it loop?

  
Hmm.

cheers,
  Gerd

Re: [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Alon Levy 
On Wed, Jun 29, 2011 at 03:06:33PM +0200, Gerd Hoffmann wrote:
   Hi,
 
 +case QXL_IO_FLUSH_SURFACES:
 +dprint(d, 1, QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, 
 res#=%d)\n,
 +val, qxl_mode_to_string(d-mode), d-guest_surfaces.count,
 +d-num_free_res);
 +qemu_spice_stop(d-ssd);
 +qemu_spice_start(d-ssd);
 +dprint(d, 1, QXL_IO_FLUSH_SURFACES exit (%s, s#=%d, res#=%d,%p)\n,
 +qxl_mode_to_string(d-mode), d-guest_surfaces.count,
 +d-num_free_res, d-last_release);
 +break;
 
 This should be async as we'll go sleep and wait for the spice server
 thread finish in qemu_spice_stop().

ok, so don't cherry-pick the ioport_to_string patch yet since I'm doing
s/FLUSH_SURFACES/FLUSH_SURFACES_ASYNC/

 
 +case QXL_IO_FLUSH_RELEASE: {
 +QXLReleaseRing *ring =d-ram-release_ring;
 +if (ring-prod - ring-cons + 1 == ring-num_items) {
 +// TODO - return a value to the guest and let it loop?
   
 Hmm.
 
 cheers,
   Gerd

Re: [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Gerd Hoffmann 

+case QXL_IO_FLUSH_RELEASE: {
+QXLReleaseRing *ring =d-ram-release_ring;
+if (ring-prod - ring-cons + 1 == ring-num_items) {
+// TODO - return a value to the guest and let it loop?

   
Hmm.

So the story goes: I wrote this, but didn't actually see this happen in 
practice,
particularily since the driver empties the release ring. The simplest would be 
to
replace it with some fprintf(stderr)


How do you think this could happen?  If there are no unprocessed 
requests in the pipeline (shouldn't be, all surfaces are flushed to 
device memory and destroyedv at that point) and the driver cares empty 
the release ring before calling this it should not happen, right?


cheers,
  Gerd

Re: [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Alon Levy 
On Wed, Jun 29, 2011 at 03:06:33PM +0200, Gerd Hoffmann wrote:
   Hi,
 
 +case QXL_IO_FLUSH_SURFACES:
 +dprint(d, 1, QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, 
 res#=%d)\n,
 +val, qxl_mode_to_string(d-mode), d-guest_surfaces.count,
 +d-num_free_res);
 +qemu_spice_stop(d-ssd);
 +qemu_spice_start(d-ssd);
 +dprint(d, 1, QXL_IO_FLUSH_SURFACES exit (%s, s#=%d, res#=%d,%p)\n,
 +qxl_mode_to_string(d-mode), d-guest_surfaces.count,
 +d-num_free_res, d-last_release);
 +break;
 
 This should be async as we'll go sleep and wait for the spice server
 thread finish in qemu_spice_stop().

Yeah, I meant to do that, forgot, thanks for catching it.

 
 +case QXL_IO_FLUSH_RELEASE: {
 +QXLReleaseRing *ring =d-ram-release_ring;
 +if (ring-prod - ring-cons + 1 == ring-num_items) {
 +// TODO - return a value to the guest and let it loop?
   
 Hmm.
So the story goes: I wrote this, but didn't actually see this happen in 
practice,
particularily since the driver empties the release ring. The simplest would be 
to
replace it with some fprintf(stderr)

 
 cheers,
   Gerd

Re: [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3S4 support

2011-06-29 Thread Alon Levy 
On Wed, Jun 29, 2011 at 04:50:10PM +0200, Gerd Hoffmann wrote:
 +case QXL_IO_FLUSH_RELEASE: {
 +QXLReleaseRing *ring =d-ram-release_ring;
 +if (ring-prod - ring-cons + 1 == ring-num_items) {
 +// TODO - return a value to the guest and let it loop?

 Hmm.
 So the story goes: I wrote this, but didn't actually see this happen in 
 practice,
 particularily since the driver empties the release ring. The simplest would 
 be to
 replace it with some fprintf(stderr)
 
 How do you think this could happen?  If there are no unprocessed
 requests in the pipeline (shouldn't be, all surfaces are flushed to
 device memory and destroyedv at that point) and the driver cares
 empty the release ring before calling this it should not happen,
 right?

Yes. The point was to check anyway, it should never happen with our driver,
but a check can catch an error I guess.

 
 cheers,
   Gerd