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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2020 14:48, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 06 March 2020 13:44
>> To: Paul Durrant <xadimgnik@xxxxxxxxx>
>> Cc: pdurrant@xxxxxxxx; 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>; 
>> 'Julien Grall' <julien@xxxxxxx>;
>> 'Wei Liu' <wl@xxxxxxx>; 'Konrad Rzeszutek Wilk' <konrad.wilk@xxxxxxxxxx>; 
>> 'Andrew Cooper'
>> <andrew.cooper3@xxxxxxxxxx>; Durrant, Paul <pdurrant@xxxxxxxxxxxx>; 'Ian 
>> Jackson'
>> <ian.jackson@xxxxxxxxxxxxx>; 'George Dunlap' <george.dunlap@xxxxxxxxxx>; 
>> 'Tim Deegan' <tim@xxxxxxx>;
>> 'Tamas K Lengyel' <tamas@xxxxxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; 
>> 'Roger Pau Monné'
>> <roger.pau@xxxxxxxxxx>
>> Subject: Re: [PATCH v3 5/6] mm: add 'is_special_page' macro...
>>
>> On 06.03.2020 13:35, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
>>>> Beulich
>>>> Sent: 06 March 2020 12:20
>>>> To: pdurrant@xxxxxxxx
>>>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Julien Grall 
>>>> <julien@xxxxxxx>; Wei Liu
>> <wl@xxxxxxx>;
>>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Andrew Cooper 
>>>> <andrew.cooper3@xxxxxxxxxx>; Paul
>>>> Durrant <pdurrant@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; 
>>>> George Dunlap
>>>> <george.dunlap@xxxxxxxxxx>; Tim Deegan <tim@xxxxxxx>; Tamas K Lengyel 
>>>> <tamas@xxxxxxxxxxxxx>; xen-
>>>> devel@xxxxxxxxxxxxxxxxxxxx; Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> Subject: Re: [Xen-devel] [PATCH v3 5/6] mm: add 'is_special_page' macro...
>>>>
>>>> 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?)
> 
> It's tricky because it would either mean breaking older (pre
> resource-mapping) QEMUs,

Didn't even qemu-trad get switched? (Anyway, not a big deal here,
just would have been nice if this large conditional could have
been shrunk a little in size.)

> or allowing the toolstack to allocate the 'special' pages with
> an extra flag to make them PGC_extra.

Doesn't sound impossible, but also not something we want to eagerly
go for.

Jan

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