Re: [PATCH v1 2/8]: PVH mmu changes

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 4 Oct 2012 14:54:47 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Fri, 2012-09-21 at 20:15 +0100, Mukesh Rathor wrote:
> > 
> > +int pvh_rem_xen_p2m(unsigned long spfn, int count)
> > +{
> > +       struct xen_remove_from_physmap xrp;
> > +       int i, rc;
> > +
> > +       for (i=0; i < count; i++) {
> > +               xrp.domid = DOMID_SELF;
> > +               xrp.gpfn = spfn+i;
> > +               rc =
> > HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> 
> It seems that the current implementation of this hypercall in the h/v
> doesn't handle unmapping of foreign pages, since it requires that the
> page be owned by the domain whose P2M it is manipulating.
> 
> How did you fix this on the hypervisor side? I'd like to make sure I
> do things consistently on the ARM side.
> 
> Talking with Tim it seems like having foreign mappings in a p2m is a
> bit tricky and needs a bit of careful thought WRT reference counting
> etc.

Hey Ian,

This is what I've (don't worry about "hybrid" word for now, i'll fix it):

casee XENMEM_remove_from_physmap:
    {
        struct xen_remove_from_physmap xrfp;
        struct page_info *page;
        struct domain *d;
        p2m_type_t p2mt;

        if ( copy_from_guest(&xrfp, arg, 1) )
            return -EFAULT;

        rc = rcu_lock_target_domain_by_id(xrfp.domid, &d);
        if ( rc != 0 )
            return rc;

        if ( xsm_remove_from_physmap(current->domain, d) )
        {
            rcu_unlock_domain(d);
            return -EPERM;
        }

        domain_lock(d);

        page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC);
        if ( page || (is_hybrid_vcpu(current) &&
                      (p2m_is_mmio(p2mt) || p2m_is_foreign(p2mt))) )
        {
            unsigned long argmfn = page ? page_to_mfn(page) : INVALID_MFN;
            guest_physmap_remove_page(d, xrfp.gpfn, argmfn, 0);
            if (page)
                put_page(page);
        }
        else {
            if ( is_hybrid_vcpu(current) )
                gdprintk(XENLOG_WARNING, "%s: Domain:%u gmfn:%lx invalid\n",
                         __FUNCTION__, current->domain->domain_id, xrfp.gpfn);
            rc = -ENOENT;
        }

        domain_unlock(d);

        rcu_unlock_domain(d);

        break;
    }