[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hi Jan,

On 06/03/2020 13:44, Jan Beulich wrote:
On 06.03.2020 13:35, Paul Durrant wrote:
-----Original Message-----
From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
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 
<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

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

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.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.