WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 2/2] PV framebuffer

To: Steven Smith <sos22-xen@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/2] PV framebuffer
From: Markus Armbruster <armbru@xxxxxxxxxx>
Date: Tue, 14 Nov 2006 15:01:30 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, sos22@xxxxxxxxxxxxx
Delivery-date: Tue, 14 Nov 2006 06:01:50 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20061112142045.GC2014@xxxxxxxxx> (Steven Smith's message of "Sun, 12 Nov 2006 14:20:45 +0000")
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <87lkmjptqq.fsf@xxxxxxxxxxxxxxxxx> <20061112142045.GC2014@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux)
Steven Smith <sos22-xen@xxxxxxxxxxxxx> writes:

>> PV framebuffer frontend.  Derived from http://hg.codemonkey.ws/vncfb
> This is the backend, isn't it?
>
>> diff -r 2e35cf028ff0 tools/python/xen/xend/XendDevices.py
>> --- a/tools/python/xen/xend/XendDevices.py   Thu Nov 09 15:43:24 2006 +0000
>> +++ b/tools/python/xen/xend/XendDevices.py   Thu Nov 09 17:58:26 2006 +0100
>> @@ -41,6 +41,8 @@ class XendDevices:
>>          'irq': irqif.IRQController,
>>          'usb': usbif.UsbifController,
>>          'tap': BlktapController,
>> +        'vfb': vfbif.VfbifController,
>> +        'vkbd': vfif.VkbdifController,
> Typo?  Should this be:
>
>   +        'vkbd': vfbif.VkbdifController,
>
> ?

Yes.  Thanks!

>> diff -r 2e35cf028ff0 tools/xenfb/vncfb.c
>> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
>> +++ b/tools/xenfb/vncfb.c       Thu Nov 09 10:18:58 2006 +0100
>
>> +int main(int argc, char **argv)
>> +{
>> +    rfbScreenInfoPtr server;
>> +    char *fake_argv[7] = { "vncfb", "-rfbport", "5901", 
>> +                               "-desktop", "xen-vncfb", 
>> +                               "-listen", "0.0.0.0" };
> HVM VNC server defaults to listening on 127.0.0.1 now.  You might want
> to make this do the same thing.

Okay.

>> diff -r 2e35cf028ff0 tools/xenfb/xenfb.c
>> --- /dev/null        Thu Jan 01 00:00:00 1970 +0000
>> +++ b/tools/xenfb/xenfb.c    Thu Nov 09 10:22:36 2006 +0100
>
>> +static int xenfb_hotplug(struct xs_handle *xsh, const char *backdir)
>> +{
>> +    if (xenfb_xs_printf(xsh, backdir, "hotplug-status", "connected"))
>> +            return -1;
>> +
>> +    if (!xs_watch(xsh, backdir, ""))
>> +            return -1;
>> +
>> +    switch (xenfb_wait_for_state(xsh, backdir,
>> +#if 1 /* TODO fudging state to permit restarting; to be removed */
>> +                    (1 << XenbusStateInitialising)
>> +                    | (1 << XenbusStateInitWait)
>> +                    | (1 << XenbusStateConnected)
>> +#else
>> +                    1 << XenbusStateInitialising,
>> +#endif
> I'm not sure what you're trying to do here.  It looks like you're
> waiting for the backend to go to state Initialising (or InitWait, or
> Connected), but this is the backend, so if it isn't there already
> you're going to wait forever.

Part of the crude hacks that let me kill and restart the backend
without disturbing the front end.

Here we wait for xend to create the initial xenstore nodes for us, by
waiting for the transition to XenbusStateInitialising.  If we
reconnect, we're already past that state.

Should go away when we implement reconnect cleanly.

>> +bool xenfb_attach_dom(struct xenfb *xenfb_pub, int domid)
>> +{
> ...
>> +    if (xenfb_hotplug(xsh, vfb_backdir) < 0)
>> +            goto error;
>> +    if (xenfb_hotplug(xsh, vkbd_backdir) < 0)
>> +            goto error;
>> +
>> +    if (xenfb_xs_printf(xsh, vkbd_backdir, "feature-abs-pointer", "1"))
>> +            goto error;
>> +    if (xenfb_xs_printf(xsh, vfb_backdir, "state", "%d",
>> +                        XenbusStateInitWait))
>> +            goto error;
>> +    if (xenfb_xs_printf(xsh, vkbd_backdir, "state", "%d",
>> +                        XenbusStateInitWait))
>> +            goto error;
>> +
> I'd probably reorder this a little to look more like this:
>
> (1) Set feature-abs-pointer
> (2) Set state to InitWait
> (3) Set hotplug status
>
> The only actual *required* constraint is (1) before (2), so that the
> frontend doesn't initialise before we've set the feature and
> potentially miss it.
>
> (2) before (3) is kind of nice, in that it makes sure that the backend
> is ready before xend unpauses the domain, and so the frontend'll be
> able to connect the first time it tries, but that's a lot less
> important here than for e.g. block devices.

Based on our previous discussions, I designed the startup protocol
this way:

    backend                          frontend
    ------------------------------------------------------------------
    hotplug_status = connected
    [makes xend populate xenstore, set fe and be state = Initialising]
    wait for be state = Initialising
      [i.e. wait for xend]
    write xs: feature-abs-pointer    write xs: feature-update
    be state = InitWait              fe state = Initialised
    ------------------------------ sync ------------------------------
    wait for fe state = Initialised  wait for be state = InitWait
    ------------------------------ sync ------------------------------
    read xs: feature-update          read xs: feature-abs-pointer
    write xs: request-update         write xs: request-abs-pointer
    be state = Connected             fe state = Connected
    ------------------------------ sync ------------------------------
    wait for fe state = Connected    wait for be state = Connected
    ------------------------------ sync ------------------------------
    read xs: request-abs-pointer     read xs: request-update

The symmetry made sense to me.

If this is not the right way to do it, please explain it to me once
again.  A diagram like the one I provided above might be more precise
and thus useful than prose, even if it's just as crude as mine.

>> + error:
>> +    serrno = errno;
>> +    if (xenfb->fb)
>> +            munmap(xenfb->fb, xenfb->fb_len);
>> +    if (fbmfns)
>> +            munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE);
>> +        if (xenfb->kbd_info)
>> +            munmap(xenfb->kbd_info, XC_PAGE_SIZE);
>> +        if (xenfb->fb_info)
>> +            munmap(xenfb->fb_info, XC_PAGE_SIZE);
>> +        if (xenfb->kbd_port);
>> +    xc_evtchn_unbind(xenfb->evt_xch, xenfb->kbd_port);
>> +        if (xenfb->fbdev_port)
>> +            xc_evtchn_unbind(xenfb->evt_xch, xenfb->fbdev_port);
>> +        if (xsh) 
>> +            xs_daemon_close(xsh);
>> +        errno = serrno;
> It'd be good to set an error in xenbus when this fails.  Look at
> xenbus_dev_fatal and _dev_error in drivers/xen/xenbus/xenbus_client.c
> to see how to do this.

Thanks, I'll look into that.

>> +        return false;
>> +}
>> +
>
> Steven.

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