Re: [Xen-devel] Xen 3.4.x and request-abs-pointer

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 5 Jul 2010, John Haxby wrote:
> A little while ago Jeremy Fitzhardinge was having some problem with 
> absolute pointers.  This turned out to be a very simple fix in qemu-xen: 
> wait for the frontend to be connected before connecting the backend.  
> I've attached that original patch, if there's going to be a xen-3.4.4 it 
> would be nice if this was included.
> 
> jch
> 

That patch was recently reverted because it was the wrong fix:


>From stefano.stabellini@xxxxxxxxxxxxx Thu Jul  1 12:36:06 2010
Date: Thu, 1 Jul 2010 12:32:15 +0100
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian
 Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Keir Fraser <Keir.Fraser@xxxxxxxxxxxxx>,
        Anthony Perard <anthony.perard@xxxxxxxxxx>, Keir, Gerd Hoffmann
        <kraxel@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1 of 1] mini-os: PV fronted MUST be in 
XenbusStateConnected not XenbusStateInitialized during init

Thank you for the patch Konrad.

I think this fix shows us that 805ed3b20492d2f4bb465bfda65cedd286e23209
was the wrong fix:

commit 805ed3b20492d2f4bb465bfda65cedd286e23209
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Fri May 21 15:46:55 2010 +0100

    Wait for frontend state Connected before connecting the backend

    The frontend of the framebuffer set a value
    (request-abs-pointer) and go
    to the state Connected.  The backend must read this value
    only when the
    frontend has the state Connected.


The problem was that the backend can be sure that the linux xenfb
frontend wrote request-abs-pointer only after the frontend state is
Connected.
In order to do that properly we need a new hook in qemu xen_backend: we
should probably rename the current connect hook to initialise and create
a new connect hook that would be implemented by xenfb to read
request-abs-pointer.



On Wed, 30 Jun 2010, Konrad Rzeszutek Wilk wrote:
> # HG changeset patch
> # User Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> # Date 1277908182 14400
> # Node ID 76e7d0258f65e4ab1b105cd70a96f2a411d5ca22
> # Parent  a24dbfcbdf695f49867d5881ea20ab40f18aea98
> mini-os: PV fronted MUST be in XenbusStateConnected not 
> XenbusStateInitialized during init.
> 
> With the QEMU change 805ed3b20492d2f4bb465bfda65cedd286e23209
> ("Wait for frontend state Connected before connecting the backend"),
> where QEMU backend (say VNC framebuffer) will ONLY call ops->connect
> (which when successfull, will move the backend state from XenbusStateInitWait
> to XenbusStateConnected) when the frontend state is in XenbusStateConnected.
> 
> Previous to that git commit it would call ops->connect also
> if the frontend was in XenbusStateInitialized state.
> 
> Without this patch, the MiniOS (in my case pvgrub) would hang
> in the fbfront and kbfront threads waiting for the backend to change
> its state from InitWait to Connected. Which the backend would not do
> as the ops->connect in QEMU was never called.
> 
> The c/s 21260 "mini-os: Revert 21106:b20f897d6010 "Fix xenbus initialisation"
> fixes this for the blkfront and netfront. Unfortunately
> it did not fix it for the fbfront, kbdfront and pcifront which
> this patch does.
> 
> The patch fixes pv-grub hanging at:
> "
> ******************* FBFRONT for device/vfb/0 **********
> 
> 
> ******************* KBDFRONT for device/vkbd/0 **********
> " and makes it now go to:
> "
> ******************* FBFRONT for device/vfb/0 **********
> 
> 
> ******************* KBDFRONT for device/vkbd/0 **********
> 
> 
> backend at /local/domain/0/backend/vkbd/11/0
> /local/domain/0/backend/vkbd/11/0 connected
> ************************** KBDFRONT
> Thread "kbdfront" exited.
> backend at /local/domain/0/backend/vfb/11/0
> /local/domain/0/backend/vfb/11/0 connected
> "
> and you use VNC to see the GRUB menu.
> 
> diff -r a24dbfcbdf69 -r 76e7d0258f65 extras/mini-os/fbfront.c
> --- a/extras/mini-os/fbfront.c        Tue Jun 22 07:19:38 2010 +0100
> +++ b/extras/mini-os/fbfront.c        Wed Jun 30 10:29:42 2010 -0400
> @@ -124,7 +124,12 @@
>      }
>  
>      snprintf(path, sizeof(path), "%s/state", nodename);
> -    err = xenbus_switch_state(xbt, path, XenbusStateInitialised);
> +    /* In the past we would have made this Initialized. But with a QEMU
> +     * update 805ed3b that requires the frontend to be Connected state
> +     * to progress the backend to move from InitWait to Connected.
> +     * The QEMU bug fix was meant _only_ for XenFB but it affects every
> +     * device. */
> +    err = xenbus_switch_state(xbt, path, XenbusStateConnected);
>      if (err) {
>          printk("error writing initialized: %s\n", err);
>          free(err);
> @@ -485,7 +490,12 @@
>      }
>  
>      snprintf(path, sizeof(path), "%s/state", nodename);
> -    err = xenbus_switch_state(xbt, path, XenbusStateInitialised);
> +    /* In the past we would have made this Initialized. But with a QEMU
> +     * update 805ed3b that requires the frontend to be Connected state
> +     * to progress the backend to move from InitWait to Connected.
> +     * The QEMU bug fix was meant _only_ for XenFB but it affects every
> +     * device. */
> +    err = xenbus_switch_state(xbt, path, XenbusStateConnected);
>      if (err) {
>          message = "switching state";
>          goto abort_transaction;
> diff -r a24dbfcbdf69 -r 76e7d0258f65 extras/mini-os/pcifront.c
> --- a/extras/mini-os/pcifront.c       Tue Jun 22 07:19:38 2010 +0100
> +++ b/extras/mini-os/pcifront.c       Wed Jun 30 10:29:42 2010 -0400
> @@ -204,7 +204,12 @@
>      }
>  
>      snprintf(path, sizeof(path), "%s/state", nodename);
> -    err = xenbus_switch_state(xbt, path, XenbusStateInitialised);
> +    /* In the past we would have made this Initialized. But with a QEMU
> +     * update 805ed3b that requires the frontend to be Connected state
> +     * to progress the backend to move from InitWait to Connected.
> +     * The QEMU bug fix was meant _only_ for XenFB but it affects every
> +     * device. */
> +    err = xenbus_switch_state(xbt, path, XenbusStateConnected);
>      if (err) {
>          message = "switching state";
>          goto abort_transaction;
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel