[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86: XENMAPSPACE_gmfn{,_batch,_range} want to special case idx == gpfn


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 21 Oct 2020 12:58:41 +0200
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Andrew Cooper" <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
  • Delivery-date: Wed, 21 Oct 2020 10:58:59 +0000
  • Ironport-sdr: Abx5GA9fO+utGWBYzMaxwLe+qjMt0rrBzzDV383h303FqESbIrdEq9BZqvUIZUodt6aPVybGnL BsxayAldXeSqpMVJfhjmzq5yCv+qCz/8Gc3Ds67QpL3tDgeRoekZDIWG2qbI0MNdGQeyHCozhD PBJyknQHZR7q6k19N2W15bCU8uckNOPnb19Hdj3CjOIEhJPgAhllApScmdrzx+y0tILg7IcDBh InVSVIRLzW7EKLW/TTjdyQ9T5d74O3LV3Y3ISB3zKG/m32zCD/7r7DFBcBQlbO8kIgPqYXCIK+ qFU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Oct 21, 2020 at 12:38:48PM +0200, Jan Beulich wrote:
> On 21.10.2020 11:39, Roger Pau Monné wrote:
> > On Fri, Oct 16, 2020 at 08:44:10AM +0200, Jan Beulich wrote:
> >> --- a/xen/arch/x86/mm.c
> >> +++ b/xen/arch/x86/mm.c
> >> @@ -4555,7 +4555,7 @@ int xenmem_add_to_physmap_one(
> >>          if ( is_special_page(mfn_to_page(prev_mfn)) )
> >>              /* Special pages are simply unhooked from this phys slot. */
> >>              rc = guest_physmap_remove_page(d, gpfn, prev_mfn, 
> >> PAGE_ORDER_4K);
> >> -        else
> >> +        else if ( !mfn_eq(mfn, prev_mfn) )
> >>              /* Normal domain memory is freed, to avoid leaking memory. */
> >>              rc = guest_remove_page(d, gfn_x(gpfn));
> > 
> > What about the access differing between the old and the new entries,
> > while pointing to the same mfn, would Xen install the new entry
> > successfully?
> 
> Yes - guest_physmap_add_page() doesn't get bypassed.

But will it succeed if the default access is different from the one
the installed entry currently has? Will it update the access bits
to match the new ones?

> 
> > Seems easier to me to use guest_physmap_remove_page in that case to
> > remove the entry from the p2m without freeing the page.
> 
> Why do any removal when none is really needed? I also don't see
> this fit the "special pages" clause and comment very well. I'd
> question the other way around whether guest_physmap_remove_page()
> needs calling at all (the instance above; the other one of course
> is needed).

Right, replying to my question above: it will succeed, since
guest_physmap_add_entry will overwrite the previous entry.

I agree, it looks like the guest_physmap_remove_page call done for
special pages is not really needed, as guest_physmap_add_entry would
already overwrite such entries and not free the associated mfn?

Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.