Patchwork [3/5] Process outstanding commands in the ring after changing capability bits

login
register
mail settings
Submitter Søren Sandmann
Date Sept. 3, 2012, 5:53 p.m.
Message ID <1346694835-23590-3-git-send-email-sandmann@cs.au.dk>
Download mbox | patch
Permalink /patch/181403/
State New
Headers show

Comments

Søren Sandmann - Sept. 3, 2012, 5:53 p.m.
From: Søren Sandmann Pedersen <ssp@redhat.com>

When a new client connects, there may be commands in the ring that it
can't understand, so we need to process these before forwarding new
commands to the client. By doing this after changing the capability
bits we ensure that the new client will never see a command that it
doesn't understand (under the assumption that the guest will read and
obey the capability bits).
---
 server/red_worker.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Alon Levy - Sept. 3, 2012, 6:31 p.m.
> From: Søren Sandmann Pedersen <ssp@redhat.com>
> 
> When a new client connects, there may be commands in the ring that it
> can't understand, so we need to process these before forwarding new
> commands to the client. By doing this after changing the capability
> bits we ensure that the new client will never see a command that it
> doesn't understand (under the assumption that the guest will read and
> obey the capability bits).


ACK.

We really should have some sort of fence mechanism for this. This patch will still work, since the command ring is 32 items long, so it should be relatively cheap to flush it (each item is a single comamnd. worse case can be 32*video_mem). There is still a race - the guest has to handle the interrupt before sending any new commands.

In the future we could introduce a new command called QXLFence and have the interrupt handler call a new io to return it just before pushing it to the ring. The fence would be used only in the server right now, but when the driver releases it it can use it to know all commands before it have been processed (note that it doesn't mean all those commands have been released).

> ---
>  server/red_worker.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 60b5471..f87967c 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9493,6 +9493,11 @@ static void
> on_new_display_channel_client(DisplayChannelClient *dcc)
>      }
>      red_channel_client_ack_zero_messages_window(&dcc->common.base);
>      if (worker->surfaces[0].context.canvas) {
> +        int ring_is_empty;
> +
> +        while (red_process_commands(worker, MAX_PIPE_SIZE,
> &ring_is_empty)) {
> +        }
> +
>          red_current_flush(worker, 0);
>          push_new_primary_surface(dcc);
>          red_push_surface_image(dcc, 0);
> --
> 1.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>

Patch

diff --git a/server/red_worker.c b/server/red_worker.c
index 60b5471..f87967c 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -9493,6 +9493,11 @@  static void on_new_display_channel_client(DisplayChannelClient *dcc)
     }
     red_channel_client_ack_zero_messages_window(&dcc->common.base);
     if (worker->surfaces[0].context.canvas) {
+        int ring_is_empty;
+
+        while (red_process_commands(worker, MAX_PIPE_SIZE, &ring_is_empty)) {
+        }
+        
         red_current_flush(worker, 0);
         push_new_primary_surface(dcc);
         red_push_surface_image(dcc, 0);