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

[Jan Beulich <jbeulich@xxxxxxxx>]

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?)

>> 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.

Jan

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