Re: [Xen-devel] [PATCH v6 5/5] domain: use PGC_extra domheap page for shared_info

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 17 March 2020 13:14
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Durrant, Paul <pdurrant@xxxxxxxxxxxx>; 
> Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; 
> George Dunlap
> <george.dunlap@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Konrad 
> Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>
> Subject: RE: [EXTERNAL] [PATCH v6 5/5] domain: use PGC_extra domheap page for 
> shared_info
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 10.03.2020 18:49, paul@xxxxxxx wrote:
> > From: Paul Durrant <pdurrant@xxxxxxxxxx>
> >
> > Currently shared_info is a shared xenheap page but shared xenheap pages
> > complicate future plans for live-update of Xen so it is desirable to,
> > where possible, not use them [1]. This patch therefore converts shared_info
> > into a PGC_extra domheap page. This does entail freeing shared_info during
> > domain_relinquish_resources() rather than domain_destroy() so care is
> > needed to avoid de-referencing a NULL shared_info pointer hence some
> > extra checks of 'is_dying' are needed.
> 
> If there's going to be agreement to follow this route, the implementation,
> with a really minor cosmetic adjustment - see below -, looks okay to me.
> Nevertheless I continue to dislike the implication from the extra care
> that's now needed. As I think I have said before, I'd like to have at
> least one other REST maintainer's opinion here.
> 

Ok, fair enough.

> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -260,9 +260,12 @@ void dump_pageframe_info(struct domain *d)
> >
> >      page_list_for_each ( page, &d->extra_page_list )
> >      {
> > -        printk("    ExtraPage %p: caf=%08lx, taf=%" PRtype_info "\n",
> > +        const char *tag = mfn_eq(page_to_mfn(page), d->shared_info.mfn) ?
> > +            "[SHARED INFO]" : "";
> 
> Please can this be " [SHARED INFO]" with ...
> 
> > +        printk("    ExtraPage %p: caf=%08lx, taf=%" PRtype_info " %s\n",
> 
> ... the blank before the final %s dropped here, such that we won't
> have a trailing blank in the output?

Sure.

  Paul

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