Re: [Xen-devel] [PATCH] xen/arm: define PAGE_HYPERVISOR as BUFFERABLE

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 4 Jun 2013, Ian Campbell wrote:
> On Tue, 2013-06-04 at 14:46 +0100, Stefano Stabellini wrote:
> > On Tue, 4 Jun 2013, Ian Campbell wrote:
> > > On Tue, 2013-06-04 at 14:29 +0100, Stefano Stabellini wrote:
> > > > On Fri, 10 May 2013, Stefano Stabellini wrote:
> > > > > Use stage 1 attribute indexes for PAGE_HYPERVISOR, the appriopriate 
> > > > > one
> > > > > for normal memory hypervisor mappings would be BUFFERABLE.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > > >
> > > > > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> > > > > index fd6946e..13f76fc 100644
> > > > > --- a/xen/include/asm-arm/page.h
> > > > > +++ b/xen/include/asm-arm/page.h
> > > > > @@ -58,7 +58,7 @@
> > > > >  #define DEV_WC        BUFFERABLE
> > > > >  #define DEV_CACHED    WRITEBACK
> > > > >  
> > > > > -#define PAGE_HYPERVISOR         (MATTR_MEM)
> > > > > +#define PAGE_HYPERVISOR         (BUFFERABLE)
> > > > >  #define MAP_SMALL_PAGES         PAGE_HYPERVISOR
> > > > >  
> > > > >  /*
> > > >  
> > > > This patch is completely wrong, fortunately it hasn't been applied.
> > > 
> > > Yes, it appears to have slipped through my net, sorry. (But good in this
> > > case!)
> > > 
> > > > 
> > > > Despite the name, BUFFERABLE means non-cacheable. Mapping a guest page
> > > > non-cacheable in the hypervisor would cause mismatch in memory
> > > > attributes.
> > > > 
> > > > We should map the guest page as "normal memory", that is WRITEBACK.
> > > 
> > > The hypervisor's own memory mappings are using WRITEALLOC not WRITEBACK
> > > (see mfn_to_xen_entry). Is one or the other wrong?
> > 
> > They are both appropriate. BTW WRITEALLOC is effectively what we have
> > now (MATTR_MEM is truncated to 0x7 that is WRITEALLOC).
> 
> OK. So should we switch to WRITEALLOC explicitly or would you make an
> argument for switching to WRITEBACK?

The only argument for using WRITEBACK instead of WRITEALLOC would be to
match Linux cache policy, but it's not a very good argument.


> Can we get all that in an update changelog with an S-o-b etc please.

OK

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