[Xen-ia64-devel] GET_THIS_PADDR appears to be broken

[Horms <horms@xxxxxxxxxxxx>]

GET_THIS_PADDR() doesn't appear to work correclty
on xen-ia64-unstable.hg 15165:96331db61e47

Long-winded description of why

  cpu_data           = 0xf000000004410000
  ia64_tpa(cpu_data) = 0x0000000004410000
  __per_cpu_start    = 0x0003ffffffff0000

  ia64_set_kr(IA64_KR_PER_CPU_DATA,
              ia64_tpa(cpu_data) - (long) __per_cpu_start);
  ar.k3              = ia64_tpa(cpu_data) - __per_cpu_start;
                     = 0x0000000004410000 - 0xf000000004410000
                   = 0x0f00000004420000 # N.B Underflow

  #ifdef XEN
  #define GET_THIS_PADDR(reg, var)                        \
          mov        reg = IA64_KR(PER_CPU_DATA);;        \
          addl       reg = THIS_CPU(var) - PERCPU_ADDR, reg
  #else
  #define GET_THIS_PADDR(reg, var)                        \
          mov        reg = IA64_KR(PER_CPU_DATA);;        \
          addl       reg = THIS_CPU(var), reg
  #endif


  IA64_KR(PER_CPU_DATA)       = ar.k3
                              = 0x0f00000004420000
  THIS_CPU(ia64_mca_pal_base) = &per_cpu__ia64_mca_pal_bas
                              = 0xf0ffffffffff0320
  PERCPU_ADDR                 = 0xf0ffffffffff0000

  GET_THIS_PADDR(r2, ia64_mca_pal_base)
                           r2 = (THIS_CPU(ia64_mca_pal_base) - PERCPU_ADDR) +
                                IA64_KR(PER_CPU_DATA)
                              = (0xf0ffffffffff0320 - 0xf0ffffffffff0000) +
                                0x0f00000004420000
                              = 0x0000000000000320 + 0x0f00000004420000
                              = 0x0f00000004420320 # N.B Overflow

  On the machine in question 0x0f00000004420320 is not a valid
  physical address.

  But if we use the linux version of GET_THIS_PADDR, here is
  how things pan out.

  GET_THIS_PADDR(r2, ia64_mca_pal_base)
                           r2 = THIS_CPU(ia64_mca_pal_base) +
                                IA64_KR(PER_CPU_DATA)
                              = 0xf0ffffffffff0320 + 0x0f00000004420000
                              = 0x0000000004410320 # N.B Overflow

  By inspection I was able to determine that 0x0000000004410320 is the
  correct physical address.

  It can also be derived using the following calculation, which is
  in some ways a little clearer as it doesn't include underflow
  or overflow.

  r2 = (THIS_CPU(ia64_mca_pal_base) - PERCPU_ADDR) + ia64_tpa(cpu_data)
     = (0xf0ffffffffff0320 - 0xf0ffffffffff0000) + 0x0000000004410000
     = 0x0000000000000320 + 0x0000000004410000
     = 0x0000000004410320

Solution 1

Use the Linux variant of GET_THIS_PADDR(reg, var)

  #define GET_THIS_PADDR(reg, var)                        \
          mov        reg = IA64_KR(PER_CPU_DATA);;        \
          addl       reg = THIS_CPU(var), reg

This seems like a simple and logical solition that accodring to the
maths above should work. However, when I try to compile this
I get a relocation error.

ia64-unknown-linux-gnu-ld -g -T xen.lds.s -N \
                -Map map.out linux-xen/head.o 
/home/horms/work/xen/xen-ia64-unstable.hg/xen/common/built_in.o 
/home/horms/work/xen/xen-ia64-unstable.hg/xen/drivers/built_in.o 
/home/horms/work/xen/xen-ia64-unstable.hg/xen/arch/ia64/built_in.o \
                
/home/horms/work/xen/xen-ia64-unstable.hg/xen/common/symbols-dummy.o -o 
/home/horms/work/xen/xen-ia64-unstable.hg/xen/xen-syms
/home/horms/work/xen/xen-ia64-unstable.hg/xen/arch/ia64/linux-xen/mca_asm.S:271:
 relocation truncated to fit: IMM22 against symbol `per_cpu__ia64_mca_pal_base' 
defined in .data.percpu section in 
/home/horms/work/xen/xen-ia64-unstable.hg/xen/arch/ia64/built_in.o

I'm not really sure what that means, other than that
per_cpu__ia64_mca_pal_base aka THIS_CPU(ia64_mca_pal_base)
ought to be a 22bit integer, which it isn't.

I also noticed that the assembled code on xen and linux differ.
Though I'm no linker expert, so I don't understand why.

  Linux
  # objdump -D arch/ia64/kernel/built-in.o
  00000000000000f8 <per_cpu__ia64_mca_pal_base>:
  f8:   00 00 00 00 00 00                   break.i 0x0


  Xen
  # objdump -D arch/ia64/built_in.o
  00000000000000a0 <per_cpu__ia64_mca_pal_base>:
  a0:   00 00 00 00 00 00       [MII]       break.m 0x0

Solution 2

  Change the value that is stored in ar.k3
  from ia64_tpa(cpu_data) - __per_cpu_start
  to simply ia64_tpa(cpu_data)

  This reduces GET_THIS_PADDR() to
  (THIS_CPU(ia64_mca_pal_base) - PERCPU_ADDR) + ia64_tpa(cpu_data)
  which has been shown to be correct above.

  This isn't an entirely ideal solution as it uneccesarily
  diverges both ar.k3 and GET_THIS_PADDR() from the Linux versions,
  however testing shows that it does seem to work.

Previous Solutions

  Until 12448:efb346a02e70 there was a tpa based version of
  GET_THIS_PADDR().

   #define GET_THIS_PADDR(reg, var)             \
        movl    reg = THIS_CPU(var)             \
        tpa     reg = reg

  It did not appear to work in the tests I ran either.
  Even after adding ;; after THIS_CPU(var)
  But I didn't persevere for very long.


Index: xen-ia64-unstable.hg/xen/arch/ia64/linux-xen/setup.c
===================================================================
--- xen-ia64-unstable.hg.orig/xen/arch/ia64/linux-xen/setup.c   2007-06-27 
18:16:19.000000000 +0900
+++ xen-ia64-unstable.hg/xen/arch/ia64/linux-xen/setup.c        2007-06-27 
18:16:22.000000000 +0900
@@ -918,7 +918,11 @@ cpu_init (void)
         *   phys = ar.k3 + &per_cpu_var
         */
        ia64_set_kr(IA64_KR_PER_CPU_DATA,
+#ifdef XEN
+                   ia64_tpa(cpu_data));
+#else
                    ia64_tpa(cpu_data) - (long) __per_cpu_start);
+#endif
 
        get_max_cacheline_size();
 


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