Re: [Xen-ia64-devel] pickled code

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

I forgot to memtion an important thing.
Xen needs to change both reference count(count member) and
its page owner(_domain member) atomically.
Some codes depends on the fact that count and _domain are packed in 64bit.
e.g. steal_page().


On Thu, Sep 21, 2006 at 10:34:04AM +0900, Isaku Yamahata wrote:
> 
> Hi Jes.
> 
> It tries to keep struct page_info compact layout by avoiding
> padding before _domain member.
> Please notice u32 count_info member in front of _domain.
> 
> struct domain is allocated from xen heap,
> i.e. the range [PAGE_OFFSET, PAGE_OFFSET + 64MB).
> It means that struct domain* can be expressed as
> PAGE_OFFSET + 32bit offset(=machine phsyiacal address).
> 
> pickle          PAGE_OFFSET + 32bit offset => 32 bit offset
>                 i.e. discarding most significant 32bit.
>                 It can be don by casting u64 to u32 because
>                 Xen assumes little endian.
>         
> unpickle        32bit offset(=machine phsyical address)
>                          => PAGE_OFFSET + 32 bit offset
>                 i.e. __va()
> 
> 
> On Wed, Sep 20, 2006 at 04:21:56PM +0200, Jes Sorensen wrote:
> > Hi,
> > 
> > I found another interesting issue in the code - the way the 'pickle'
> > functions work just cannot be right. There is no way one should ever
> > try and truncate the output of __pa() to u32 or expect to be able to
> > run __va() on a u32 and obtain any level of usable output.
> > 
> > I have to admit I have zero clue what the pickle code is trying to
> > achieve, but I am at least fairly confident that something needs to
> > be done in this space :(
> > 
> > Cheers,
> > Jes
> > 
> 
> > Truncating __va() and __pa() output to u32 is simply not going to work.
> > Some platforms don't even have any physical memory within the 4GB window.
> > 
> > Signed-off-by: Jes Sorensen <jes@xxxxxxx>
> > 
> > 
> > diff -r 3e4fa8b5b245 xen/arch/ia64/xen/mm.c
> > --- a/xen/arch/ia64/xen/mm.c        Tue Sep 12 11:43:22 2006 -0600
> > +++ b/xen/arch/ia64/xen/mm.c        Wed Sep 20 15:47:09 2006 +0200
> > @@ -213,7 +213,7 @@ static void
> >  static void
> >  try_to_clear_PGC_allocate(struct domain* d, struct page_info* page)
> >  {
> > -    u32 _d, _nd;
> > +    u64 _d, _nd;
> >      u64 x, nx, y;
> >  
> >      _d = pickle_domptr(d);
> > @@ -1329,7 +1349,7 @@ steal_page(struct domain *d, struct page
> >  #if 0 /* if big endian */
> >  # error "implement big endian version of steal_page()"
> >  #endif
> > -    u32 _d, _nd;
> > +    u64 _d, _nd;
> >      u64 x, nx, y;
> >  
> >      if (page_get_owner(page) != d) {
> > diff -r 3e4fa8b5b245 xen/include/asm-ia64/mm.h
> > --- a/xen/include/asm-ia64/mm.h     Tue Sep 12 11:43:22 2006 -0600
> > +++ b/xen/include/asm-ia64/mm.h     Wed Sep 20 14:12:43 2006 +0200
> > @@ -53,7 +53,7 @@ struct page_info
> >          /* Page is in use: ((count_info & PGC_count_mask) != 0). */
> >          struct {
> >              /* Owner of this page (NULL if page is anonymous). */
> > -            u32 _domain; /* pickled format */
> > +            u64 _domain; /* pickled format */
> >              /* Type reference count and various PGT_xxx flags and fields. 
> > */
> >              unsigned long type_info;
> >          } __attribute__ ((packed)) inuse;
> > @@ -125,10 +125,10 @@ struct page_info
> >  #define IS_XEN_HEAP_FRAME(_pfn) ((page_to_maddr(_pfn) < xenheap_phys_end) \
> >                              && (page_to_maddr(_pfn) >= xen_pstart))
> >  
> > -static inline struct domain *unpickle_domptr(u32 _d)
> > +static inline struct domain *unpickle_domptr(u64 _d)
> >  { return (_d == 0) ? NULL : __va(_d); }
> >  static inline u32 pickle_domptr(struct domain *_d)
> > -{ return (_d == NULL) ? 0 : (u32)__pa(_d); }
> > +{ return (_d == NULL) ? 0 : (u64)__pa(_d); }
> >  
> >  #define page_get_owner(_p) (unpickle_domptr((_p)->u.inuse._domain))
> >  #define page_set_owner(_p, _d)     ((_p)->u.inuse._domain = 
> > pickle_domptr(_d))
> 
> > _______________________________________________
> > Xen-ia64-devel mailing list
> > Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-ia64-devel
> 
> -- 
> yamahata
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel