Re: [PATCH v2] port/xen: map memory directly in privcmd PRIVCMD_MMAPBATCH

[Cherry G. Mathew <cherry%zyx.in@localhost>]

>>>>> "Roger" == Roger Pau Monne <roger.pau%citrix.com@localhost> writes:

    Roger> Manuel Bouyer wrote:
    >> On Tue, Jun 26, 2012 at 10:18:59AM +0100, Roger Pau Monne wrote:
    >>>> OK, so if I understand it properly - qemu maps pages that will
    >>>> be 0x3f800 in guest's physical space.
    >>> Yes, this is supposedly going to be the video region of the
    >>> guest.
    >>> 
    >>>> Let says it's mapped at BASE+0x3f800 in qemu's virtual space (I
    >>>> assume qemu actually uses a linear mapping of the guest's
    >>>> physical space).  - later something (the hypervisor ?) moves
    >>>> this page to 0xf0000 in guest's physical space. In which way ?
    >>>> does it remap the underlying mfn to a different pfn ?
    >>> Then, when Qemu starts, the guests writes the BAR, and Qemu
    >>> detects that the guest wants the video ram at address 0xf0000,
    >>> since this is different from the previous address (0x3f800),
    >>> Qemu calls xc_domain_add_to_physmap(..., idx=0x3f800,
    >>> gpfn=0xf0000), thus moving that gfns in the guest p2m.
    >>> 
    >>> With current NetBSD implementation, we didn't notice this
    >>> change, and the NetBSD kernel still thinks this pages in Qemu
    >>> address space are mapped to 0x3f800, so when Qemu tries to write
    >>> to this pages in it's address space, NetBSD kernel hits the
    >>> pagefault and we try to map 0x3f800, getting an error from the
    >>> hypervisor, because this gfn (0x3f800) is no longer accessible,
    >>> we should instead try to map the new one (0xf0000).
    >>> 
    >>>> - In qemu's virtual space the underlying pages are still mapped
    >>>> at BASE+0x3f800, when it should now be BASE+0xf0000. How does
    >>>> the proposed patch help there ? making the mapping wired won't
    >>>> make it move automatically, unless there's some magic
    >>>> happening.
    >>> Since this are gfns, they are not "real" addresses, they are
    >>> just in the p2m guest table handled by the hypervisor. The
    >>> underlying machine addresses don't change, the only thing that
    >>> changes are the gfns that Xen assigns to them. So if NetBSD
    >>> kernel maps the memory directly in the call to
    >>> xc_map_foreign_bulk, we don't care that the gfns change, since
    >>> the machine addresses behind those gfn are going to be the same,
    >>> and we will already have them mapped in Qemu address space
    >>> correctly.
    >> 
    >> But this still looks bogus to me. We let the kernel live with
    >> wrong informations. I think qemu should instruct the kernel that
    >> things changed.

    Roger> With the proposed patch, the kernel doesn't store the gfns
    Roger> anymore, since the mapping is done directly, and no pagefault
    Roger> handler is set. So the information (or the lack of it) stored
    Roger> by the NetBSD kernel is correct.

I'm still missing something here, but I think something is not right
here. I *think* qemu or NetBSD is mixing up the guest's gmfns (pfns) and
dom0 mfns here. 

- qemu queries the hypervisor for the guest pfn (aka gmfn) via: [1]
   xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_IOREQ_PFN, &ioreq_pfn);
- It then maps it into the dom0 qemu pmap via: [2]
 state->shared_page = xc_map_foreign_range(xen_xc, xen_domid,
   XC_PAGE_SIZE, PROT_READ|PROT_WRITE, ioreq_pfn);

Note that NetBSD is assuming that ioreq_pfn is a dom0 mfn at this point!

At this point, as if by magic, the dom0 mfns point to the correct
hvm ioreq_pfns. I'm guessing this is because the dom0 p2m table is
identity mapped ? hmmmmmmmmmmmmmmmm....... not good!

- next qemu updates the *guest* p2m by calling: [3]
  rc = xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn,
  idx, gpfn);

I'm not sure how this mapping is working at all. I think NetBSD's
privcmd.c needs to be updating its own p2m during the foreign domain
page mappings by considering them to be gmfns (ie; guest pfns).

My terminology of page/machine frames is from:
http://xenbits.xen.org/hg/xen-unstable.hg/file/cd4dd23a831d/xen/include/asm-x86/mm.h#l431

Cheers,
-- 
Cherry



[1] 
http://git.qemu.org/?p=qemu.git;a=blob;f=xen-all.c;h=b5220cc6a31ca8dcf524fee6b2266b5eed9881c6;hb=HEAD#l1099
[2]
http://git.qemu.org/?p=qemu.git;a=blob;f=xen-all.c;h=b5220cc6a31ca8dcf524fee6b2266b5eed9881c6;hb=HEAD#l1101
[3] 
http://git.qemu.org/?p=qemu.git;a=blob;f=xen-all.c;h=b5220cc6a31ca8dcf524fee6b2266b5eed9881c6;hb=HEAD#l301