On Fri, 20 Dec 2013, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 19.12.2013 12:54, Stefano Stabellini wrote:
> > 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
> >>>> phcoder@debian: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"
> >>>> phcoder@debian: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?
> > 
> I'm fine with you doing it.

Done, see:

http://marc.info/?l=qemu-devel&m=138901099512700&w=2