Re: [Xen-devel] [PATCH v3 1/3] x86/mm: Introduce altp2m_get_gfn_type_access

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Apr 11, 2019 at 6:50 AM George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
>
> On 4/11/19 1:17 PM, Alexandru Stefan ISAILA wrote:
> >>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> >>> index b9bbb8f485..d38d7c29ca 100644
> >>> --- a/xen/arch/x86/mm/p2m.c
> >>> +++ b/xen/arch/x86/mm/p2m.c
> >>> @@ -2626,6 +2626,7 @@ int p2m_change_altp2m_gfn(struct domain *d, 
> >>> unsigned int idx,
> >>>       mfn_t mfn;
> >>>       unsigned int page_order;
> >>>       int rc = -EINVAL;
> >>> +    bool copied_from_hostp2m;
> >>>
> >>>       if ( idx >= MAX_ALTP2M || d->arch.altp2m_eptp[idx] == 
> >>> mfn_x(INVALID_MFN) )
> >>>           return rc;
> >>> @@ -2636,7 +2637,7 @@ int p2m_change_altp2m_gfn(struct domain *d, 
> >>> unsigned int idx,
> >>>       p2m_lock(hp2m);
> >>>       p2m_lock(ap2m);
> >>>
> >>> -    mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
> >>> +    mfn = altp2m_get_gfn_type_access(ap2m, old_gfn, &t, &a, &page_order, 
> >>> &copied_from_hostp2m);
> >>
> >> Before, if new_gfn was INVALID_GFN, then the host p2m wasn't read at
> >> all.  Now, the hostp2m will have __get_gfn_type_access() called with
> >> P2M_ALLOC | P2M_UNSHARE.  Is that change intentional, and if so, why?
> >
> > This has been requested by Tamas in v2.
>
> That's nice, but 1) you still haven't answered the question, and 2) it's
> not in the changelog.

What I requested was that if remapping is being done then both the old
and new gfn's should be unshared in the hostp2m for keeping things
consistent. The page type of old_gfn was already checked whether it's
p2m_ram_rw and bail if it wasn't so functionality-wise this just
simplifies things as a user don't have to request unsharing manually
before remapping. Now, if the new_gfn is invalid it shouldn't query
the hostp2m as that is effectively a request to remove the entry from
the altp2m. But provided that scenario is used only when removing
entries that were previously remapped/copied to the altp2m, those
entries already went through P2M_ALLOC | P2M_UNSHARE before, so it
won't have an affect. But it's also pointless.

Tamas

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