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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2020 14:11, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 06 March 2020 13:07
>> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
>> Cc: pdurrant@xxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
>> <andrew.cooper3@xxxxxxxxxx>;
>> George Dunlap <george.dunlap@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau 
>> Monné <roger.pau@xxxxxxxxxx>
>> 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@xxxxxxxx>
>>>> Sent: 06 March 2020 12:47
>>>> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
>>>> Cc: pdurrant@xxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
>>>> <andrew.cooper3@xxxxxxxxxx>;
>>>> George Dunlap <george.dunlap@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau 
>>>> Monné
>> <roger.pau@xxxxxxxxxx>
>>>> 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@xxxxxxxx>
>>>>>> Sent: 06 March 2020 11:46
>>>>>> To: pdurrant@xxxxxxxx
>>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Durrant, Paul 
>>>>>> <pdurrant@xxxxxxxxxxxx>; Andrew Cooper
>>>>>> <andrew.cooper3@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>; 
>>>>>> Wei Liu <wl@xxxxxxx>;
>> Roger
>>>> Pau
>>>>>> Monné <roger.pau@xxxxxxxxxx>
>>>>>> Subject: Re: [PATCH v3 2/6] x86 / p2m: remove page_list check in 
>>>>>> p2m_alloc_table
>>>>>>
>>>>>> On 05.03.2020 13:45, pdurrant@xxxxxxxx wrote:
>>>>>>> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>>>>>>>
>>>>>>> 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;

As said - I agree this wouldn't be a bad thing to do. It would
be a requirement if paging_enable() wasn't called from
hvm_domain_initialise(), but via a distinct domctl. But since
it is, there's no way to invoke populate-physmap on a domain
without its p2m root table already allocated.

> I still can't see why initialising the p2m after having allocated
> pages (PGC_extra or otherwise) is inherently wrong.

"inherently" as in "from an abstract pov" - yes. But within the
constraints of the hypercalls available - no. Yet what gets
checked has to be of practical use, not just of theoretical one.
I.e. I'd be fine to see the check go away when a viable
alternative mechanism to allocate and _then_ populate p2m gets
introduced.

Jan

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