Re: [Xen-devel] [PATCH] xen/balloon: do not modify the p2m for auto_translate guests

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 13 Dec 2013, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 11, 2013 at 04:58:42PM +0000, Stefano Stabellini wrote:
> > decrease_reservation doesn't modify the p2m for auto_translate guests,
> > but increase_reservation does.
> > Fix that by avoiding any p2m modifications in both increase_reservation
> > and decrease_reservation for auto_translated guests.
> > 
> > Avoid allocating or using scratch pages for auto_translated guests.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> >  drivers/xen/balloon.c |   63 
> > ++++++++++++++++++++++++++-----------------------
> >  1 file changed, 34 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> > index 55ea73f..4c02e2b 100644
> > --- a/drivers/xen/balloon.c
> > +++ b/drivers/xen/balloon.c
> > @@ -350,17 +350,19 @@ static enum bp_state increase_reservation(unsigned 
> > long nr_pages)
> >  
> >             pfn = page_to_pfn(page);
> >  
> > -           set_phys_to_machine(pfn, frame_list[i]);
> > -
> >  #ifdef CONFIG_XEN_HAVE_PVMMU
> > -           /* Link back into the page tables if not highmem. */
> > -           if (xen_pv_domain() && !PageHighMem(page)) {
> > -                   int ret;
> > -                   ret = HYPERVISOR_update_va_mapping(
> > -                           (unsigned long)__va(pfn << PAGE_SHIFT),
> > -                           mfn_pte(frame_list[i], PAGE_KERNEL),
> > -                           0);
> > -                   BUG_ON(ret);
> > +           if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> > +                   set_phys_to_machine(pfn, frame_list[i]);
> > +
> > +                   /* Link back into the page tables if not highmem. */
> > +                   if (!PageHighMem(page)) {
> > +                           int ret;
> > +                           ret = HYPERVISOR_update_va_mapping(
> > +                                           (unsigned long)__va(pfn << 
> > PAGE_SHIFT),
> > +                                           mfn_pte(frame_list[i], 
> > PAGE_KERNEL),
> > +                                           0);
> > +                           BUG_ON(ret);
> > +                   }
> >             }
> >  #endif
> >  
> > @@ -378,7 +380,6 @@ static enum bp_state decrease_reservation(unsigned long 
> > nr_pages, gfp_t gfp)
> >     enum bp_state state = BP_DONE;
> >     unsigned long  pfn, i;
> >     struct page   *page;
> > -   struct page   *scratch_page;
> >     int ret;
> >     struct xen_memory_reservation reservation = {
> >             .address_bits = 0,
> > @@ -411,27 +412,29 @@ static enum bp_state decrease_reservation(unsigned 
> > long nr_pages, gfp_t gfp)
> >  
> >             scrub_page(page);
> >  
> > +#ifdef CONFIG_XEN_HAVE_PVMMU
> >             /*
> >              * Ballooned out frames are effectively replaced with
> >              * a scratch frame.  Ensure direct mappings and the
> >              * p2m are consistent.
> >              */
> > -           scratch_page = get_balloon_scratch_page();
> > -#ifdef CONFIG_XEN_HAVE_PVMMU
> > -           if (xen_pv_domain() && !PageHighMem(page)) {
> > -                   ret = HYPERVISOR_update_va_mapping(
> > -                           (unsigned long)__va(pfn << PAGE_SHIFT),
> > -                           pfn_pte(page_to_pfn(scratch_page),
> > -                                   PAGE_KERNEL_RO), 0);
> > -                   BUG_ON(ret);
> > -           }
> > -#endif
> >             if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> >                     unsigned long p;
> > +                   struct page   *scratch_page = 
> > get_balloon_scratch_page();
> > +
> > +                   if (!PageHighMem(page)) {
> 
> How come you removed the 'xen_pv_domain' part?

I thought that it's meaningless here. What we care about is whether it
is an auto_translated guests or not, everything else is irrelevant.
It also goes in the direction of using specific tests for things instead
of catch all tests like xen_pv_domain that don't mean much anymore.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel