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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 06 March 2020 13:52
> 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>; '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 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.)

Yes, trad is switched but I thought our position was that we supported use of 
arbitrary distro versions of QEMU in which case any compat code really does 
have to stick around for a very long time :-(

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

No, probably not worth it just to save doing this test.

  Paul



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