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. Literally two weeks ago [1] we had a situation where neither a comment nor the changelog that introduced a change explained why a decision was made, and the result was a heated argument between two maintainers about the safest way to change it, that was only resolved when I went through the mail archives and read through a fairly long and winding thread from 9 years ago. That shouldn't happen. The code + the changelog should give someone generally familiar with the codebase everything they need to understand what's going on, and why the change was made. -George [1] marc.info/?i=<689b8f75-20dd-24d9-bd5f-f03a8201b...@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
