Re: [Xen-devel] [EXTERNAL][PATCH v3 2/6] x86 / p2m: remove page_list check in p2m_alloc_table

From: Paul Durrant <xadimgnik@gmail.com>
To: "'Jan Beulich'" <jbeulich@suse.com>
Cc: "'Wei Liu'" <wl@xen.org>,
	"'Andrew Cooper'" <andrew.cooper3@citrix.com>,
	"'George Dunlap'" <george.dunlap@citrix.com>,
	xen-devel@lists.xenproject.org, pdurrant@amzn.com,
	"'Roger Pau Monné'" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [EXTERNAL][PATCH v3 2/6] x86 / p2m: remove page_list check in p2m_alloc_table
Date: Fri, 6 Mar 2020 13:11:08 -0000	[thread overview]
Message-ID: <007a01d5f3b8$b3e3f210$1babd630$@xen.org> (raw)
In-Reply-To: <3e5296fe-4162-8f1e-7497-57359334a902@suse.com>

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 06 March 2020 13:07
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: pdurrant@amzn.com; xen-devel@lists.xenproject.org; Andrew Cooper <andrew.cooper3@citrix.com>;
> George Dunlap <george.dunlap@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné <roger.pau@citrix.com>
> Subject: RE: [EXTERNAL][PATCH v3 2/6] x86 / p2m: remove page_list check in p2m_alloc_table
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 06.03.2020 13:50, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 06 March 2020 12:47
> >> To: Durrant, Paul <pdurrant@amazon.co.uk>
> >> Cc: pdurrant@amzn.com; xen-devel@lists.xenproject.org; Andrew Cooper <andrew.cooper3@citrix.com>;
> >> George Dunlap <george.dunlap@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>
> >> Subject: Re: [PATCH v3 2/6] x86 / p2m: remove page_list check in p2m_alloc_table
> >>
> >> On 06.03.2020 13:07, Durrant, Paul wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: 06 March 2020 11:46
> >>>> To: pdurrant@amzn.com
> >>>> Cc: xen-devel@lists.xenproject.org; Durrant, Paul <pdurrant@amazon.co.uk>; Andrew Cooper
> >>>> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Wei Liu <wl@xen.org>;
> Roger
> >> Pau
> >>>> Monné <roger.pau@citrix.com>
> >>>> Subject: Re: [PATCH v3 2/6] x86 / p2m: remove page_list check in p2m_alloc_table
> >>>>
> >>>> On 05.03.2020 13:45, pdurrant@amzn.com wrote:
> >>>>> From: Paul Durrant <pdurrant@amazon.com>
> >>>>>
> >>>>> There does not seem to be any justification for refusing to create the
> >>>>> domain's p2m table simply because it may have assigned pages.
> >>>>
> >>>> I think there is: If any such allocation had happened before, how
> >>>> would it be represented in the domain's p2m?
> >>>
> >>> Insertion into the p2m is a separate action from page allocation. Why should they be linked?
> >>
> >> They are, because of how XENMEM_populate_physmap works. Yes,
> >> they _could_ be separate steps, but that's only a theoretical
> >> consideration.
> >
> > Then surely the check should be in the XENMEM_populate_physmap code?
> 
> How that? populate-physmap can be called any number of times. We
> can't refuse a 2nd call there just because a 1st one had happened
> already. Or did you mean the inverse check (i.e. that there
> already is a p2m)?

Yes, I mean check the p2m has been initialized there.

> This surely wouldn't be a bad idea, as
> otherwise both ept_get_entry() and p2m_pt_get_entry() would
> blindly map MFN 0. But adding such a check wouldn't eliminate
> the reason to also have the check that you're proposing to drop.
> 

Why not? Anywhere assuming the existence of a p2m ought to check for it; I still can't see why initialising the p2m after having allocated pages (PGC_extra or otherwise) is inherently wrong.

  Paul

> Jan

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