Re: [Xen-devel] [PATCH v3 5/6] mm: add 'is_special_page' macro...

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2020 15:26, Julien Grall wrote:
> On 06/03/2020 13:44, Jan Beulich wrote:
>> On 06.03.2020 13:35, Paul Durrant wrote:
>>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
>>>> Beulich
>>>> Sent: 06 March 2020 12:20
>>>>
>>>> On 05.03.2020 13:45, pdurrant@xxxxxxxx wrote:
>>>>> --- a/xen/arch/x86/mm/shadow/common.c
>>>>> +++ b/xen/arch/x86/mm/shadow/common.c
>>>>> @@ -2087,19 +2087,22 @@ static int sh_remove_all_mappings(struct domain 
>>>>> *d, mfn_t gmfn, gfn_t gfn)
>>>>>            * The qemu helper process has an untyped mapping of this dom's 
>>>>> RAM
>>>>>            * and the HVM restore program takes another.
>>>>>            * Also allow one typed refcount for
>>>>> -         * - Xen heap pages, to match share_xen_page_with_guest(),
>>>>> -         * - ioreq server pages, to match prepare_ring_for_helper().
>>>>> +         * - special pages, which are explicitly referenced and mapped by
>>>>> +         *   Xen.
>>>>> +         * - ioreq server pages, which may be special pages or normal
>>>>> +         *   guest pages with an extra reference taken by
>>>>> +         *   prepare_ring_for_helper().
>>>>>            */
>>>>>           if ( !(shadow_mode_external(d)
>>>>>                  && (page->count_info & PGC_count_mask) <= 3
>>>>>                  && ((page->u.inuse.type_info & PGT_count_mask)
>>>>> -                   == (is_xen_heap_page(page) ||
>>>>> +                   == (is_special_page(page) ||
>>>>>                          (is_hvm_domain(d) && is_ioreq_server_page(d, 
>>>>> page))))) )
>>>>
>>>> Shouldn't you delete most of this line, after the previous patch
>>>> converted the ioreq server pages to PGC_extra ones?
>>>
>>> I thought that too originally but then I realise we still have to
>>> cater for the 'legacy' emulators that still require IOREQ server
>>> pages to be mapped through the p2m, in which case they will not
>>> be PGC_extra pages.
>>
>> Oh, indeed. (I don't suppose we can ever do away with this legacy
>> mechanism?)
>>
>>>> Also I notice this construct is used by x86 code only - is there
>>>> a particular reason it doesn't get placed in an x86 header (at
>>>> least for the time being)?
>>>
>>> PGC_extra pages are common so maybe it is better off defined here
>>> so it is available to ARM code?
>>
>> To be honest, my question was mainly based on me being puzzled that
>> Arm (or common) code doesn't need any such adjustment. As a result
>> I'm wondering whether that's just "luck" (in which case I'd agree
>> the placement could remain as is), or whether there's a deeper
>> reason behind that, largely guaranteeing Arm would also never need
>> it.
> 
> is_special_page() is used in features that are not supported on Arm yet 
> (e.g migration). So we will need it in the future and therefore the 
> helper should be defined in common header.

Okay then, thanks for clarifying.

Jan

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