Re: [Xen-devel] pvgrub2 is merged

[Stefano Stabellini]

On Wed, 18 Dec 2013, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 18.12.2013 20:39, Stefano Stabellini wrote:
> > On Wed, 18 Dec 2013, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> >> On 17.12.2013 15:35, Fabio Fantoni wrote:
> >>> Il 17/12/2013 15:10, Fabio Fantoni ha scritto:
> >>>> Il 17/12/2013 15:08, Vladimir 'φ-coder/phcoder' Serbinenko ha scritto:
> >>>>>> Thanks.
> >>>>>> Now there is another error, probably introduced by xenfb support:
> >>>>>>
> >>>>> doesn't look like related to xenfb. Is it 64-bit or PAE guest?
> >>>>
> >>>> 64 bit
> >>>
> >>> I did "git reset --hard" to commit "Remove grub_bios_interrupt on
> >>> coreboot." and then I applied only
> >>> "grub-core/lib/x86_64/xen/relocator.S: Fix hypercall ABI violation."
> >>> commit.
> >>> Now the Sid domU boot correctly, therefore the regression is caused by
> >>> "xenfb" or "xen grants to v1" commit, should I find the exact commit
> >>> that causes that problem or these informations are enough for you?
> >>
> >> It's because of vfb. Apparently vfb framebuffer stays mapped as rw even
> >> after vfb shutdown
> >> address@hidden:15:52:40:~/grub2$ sudo xenstore-ls
> >> /local/domain/54/device/vfb
> >> 0 = ""
> >>  backend = "/local/domain/0/backend/vfb/54/0"
> >>  backend-id = "0"
> >>  state = "1"
> >> address@hidden:15:52:51:~/grub2$ sudo xenstore-ls
> >> /local/domain/0/backend/vfb/54/0
> >> frontend = "/local/domain/54/device/vfb/0"
> >> frontend-id = "54"
> >> online = "1"
> >> state = "2"
> >> domain = "grub"
> >> vnc = "1"
> >> vnclisten = "127.0.0.1"
> >> vncdisplay = "0"
> >> vncunused = "1"
> >> sdl = "0"
> >> opengl = "0"
> >> feature-resize = "1"
> >> hotplug-status = "connected"
> >>
> >> When I do "dry vfb": do everything except writing vfb state problem
> >> disappears. So my question would be:
> >> - how can I inspect how backend maps framebuffer pages?
> > 
> > There is only one xenfb backend: hw/display/xenfb.c in the QEMU source
> > tree.
> > 
> > 
> >> - Why does it map as rw and not ro? It doesn't need to write to 
> >> framebuffer?
> > 
> > Actually it is mapping it RO, see hw/display/xenfb.c:xenfb_map_fb
> > 
> ./tools/qemu-xen-dir-remote/hw/xenfb.c:
>     xenfb->pixels = xc_map_foreign_pages(xen_xc, xenfb->c.xendev.dom,
>                                        PROT_READ | PROT_WRITE, fbmfns, 
> xenfb->fbpages);

You are right, my bad.
I did a quick test and it should be safe to modify it to PROT_READ only.


> >> - How do I force it to drop the mapping?
> > 
> > Theoretically QEMU should drop the mapping at disconnect time:
> > 
> > hw/display/xenfb.c:fb_disconnect
> > 
> >     /*
> >      * FIXME: qemu can't un-init gfx display (yet?).
> >      *   Replacing the framebuffer with anonymous shared memory
> >      *   instead.  This releases the guest pages and keeps qemu happy.
> >      */
> >     fb->pixels = mmap(fb->pixels, fb->fbpages * XC_PAGE_SIZE,
> >                       PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANON,
> >                       -1, 0);
> > 
> Could this fail?

Yes and we don't check for the return value (-1 in case of error). Well spotted!
Do you want to submit a patch to fix both issues or should I do it?