Re: [Xen-devel] Weird altp2m behaviour when switching early to a new view

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/09/2018 05:12 PM, George Dunlap wrote:
> The obvious place to look is the logdirtyvram functionality, which is
> used to make it easier for QEMU to figure out which bits of the display
> buffer have been modified.  One of the big reasons the altp2m
> functionality is still considered "experimental" is that these sorts of
> interactions were never carefully thought out.

George (and Alexey), thanks for the help!

I've managed to find out why this is happening, though I don't yet have
a solution for it. After much debugging, it turns out that the
"p2m_is_ram(p2mt)" test in hvm_hap_nested_page_fault() fails if I switch
to the new altp2m view fast enough, and that in turn disables the
logdirty processing gated on it:

/* Spurious fault? PoD and log-dirty also take this path. */
if ( p2m_is_ram(p2mt) )
{
    rc = 1;
    /*
     * Page log dirty is always done with order 0. If this mfn resides
     * in a large page, we do not change other pages type within that
     * large page.
     */
    if ( npfec.write_access )
    {
        paging_mark_pfn_dirty(currd, _pfn(gfn));
        /*
         * If p2m is really an altp2m, unlock here to avoid lock
         * ordering violation when the change below is propagated
         * from host p2m.
         */
        if ( ap2m_active )
            __put_gfn(p2m, gfn);
        p2m_change_type_one(currd, gfn, p2m_ram_logdirty, p2m_ram_rw);
        __put_gfn(ap2m_active ? hostp2m : p2m, gfn);

        goto out;
    }
    goto out_put_gfn;
}

I think this is likely because switching to the new EPT view loses
information stored when previously working with the default one, in this
case while that work has not finished (if that's the case, creating a
new altp2m view should also somehow inherit the history of view 0 - when
doing set_access() on view 0 the changes propagate to all existent
altp2m views, but I don't think that happens when a new view is created:
that starts with a clean slate).

Also related to this part of the altp2m design, I've also noticed that
creating a new EPT view with libxc involves this function:

int xc_altp2m_create_view(xc_interface *handle, uint32_t domid,
                          xenmem_access_t default_access,
                          uint16_t *view_id);

However, default_access is completely ignored. If this was meant to
reset all pages to that access it's trivial to wire it in into the
hypervisor (we could just iterate through all guest pages from 0 to
max_gpfn and set the access) - but in light of this issue that's not a
good thing to do. Or, rather, it was meant to specify the default_access
for new pages where it is not explicitly specified?

Either way, the argument is not used:

case HVMOP_altp2m_create_p2m:
    if ( !(rc = p2m_init_next_altp2m(d, &a.u.view.view)) )
        rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
    break;

That's the progress so far.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel