Re: port-xen/51080: Support pvfb on xen domu (patch)

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Wed, Apr 20, 2016 at 10:20:00PM +0000, pablo%polvorin.com.ar@localhost wrote:
> >Description:
> The patch is available here http://blog.polvorin.com.ar/patches/pvfb.patch
> 
> is an attempt at supporting the virtual framebuffer on xen guest
> ( http://wiki.netbsd.org/projects/project/xen-domu-pvfb/ )
> 
> Basic functionality is in place, but reached a point where guidance/feedback would be very welcome in order to push this forward,
> this is the first piece of kernel code I attempt to write, and I'm not
> that familiar with netbsd to start with.
> 
> Need to review, at least
> 
> * Memory management (conversion to frame number to pass references to xen host).  Currently save/restore isn't working,  but neither do it  without this patch on my install.
> 
> * code just act as a kind of adapter between xenbus and genfb, wskbddev and wsmousedev, letting them attach to xenbus. Is that the way to go?
> 
> * The way I added the drivers to be used by autoconf

Hello,
this looks good, good work !
For the framebuffer resolution/depth, how does linux do ? Is it something it
gets from the domU config file via the store ? Or via the command line ?
AFAIK NetBSD's genfb doens't have a way to change the resolution from userland.
If there's no provision to set the parameters in the xenstore you could pass
it from the kernel command line, e.g. xenfb=1280x1024x32
This will have to be parsed in xen_parse_cmdline(), which then can be called
from xfb_xenbus.c.

Are you sure the framebuffer has a protocol entry in the xenstore ?
If not, you should probably remove it from xfb_xenbus_resume().

For the race condition you mention in xfb_xenbus_attach() I don't understant
how it can happens. The
genfb framebuffer at 0xffff,  size  , depth,  stride
line should always be there.
Are you sure it's not some uninitialised variable ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--