Re: [Xen-devel] [PATCH] Elf loader fixes

[Joe Bonasera <joe.bonasera@xxxxxxx>]

This is a good start as the PHYS vs. VIRT stuff in the ELF loader
is all a bit overloaded. However, I believe these changes aren't
quite complete and for example would break the released OpenSolaris Xen
client.  It has multiple PT_LOAD sections in the Elf file,
some with p_vaddr == p_paddr on purpose and some which don't. We rely on a
"boot loader" (ie grub or domain builder) that only cares about p_paddr.
The identity mapped PT_LOAD section contains the OS entry point and
has the code to remap the other of the sections to the final VA by
creating/installing new page table entries.

For example, I think the xc_load_elf.c change:

@@ -189,7 +189,18 @@

         for ( done = 0; done < phdr->p_filesz; done += chunksz )
         {
-            pa = (phdr->p_paddr + done) - dsi->v_start;
+            if (phdr->p_paddr == phdr->p_vaddr) {
+                /*
+                 * Bug compatibility alert: In older linux kernels
+                 * p_paddr is broken, it doesn't contain the physical
+                 * address but instead is identical to p_vaddr.  Thus
+                 * we can't use it directly, instead we'll guess it
+                 * using dsi->v_start.
+                 */
+                pa = (phdr->p_vaddr + done) - dsi->v_start;
+            } else {
+                pa = (phdr->p_paddr + done);
+            }
             va = xc_map_foreign_range(
                 xch, dom, PAGE_SIZE, PROT_WRITE, parray[pa>>PAGE_SHIFT]);
             chunksz = phdr->p_filesz - done;

needs to have the line:
        pa = (phdr->p_paddr + done);
be more like:
        pa = (phdr->p_paddr + done) - kernstart;

or better yet add a dsi->p_start and dsi->p_end to use. The same
applies to your change to xen/common/elf.c for dom0.

To save you downloading OpenSolaris. Here's sample values from
the domU/dom0 ELF image:

In the xenguest section we currently specify VIRT_BASE=0x40000000, as
there was no PHYS_BASE=. In the flavor of your other changes, I'd expect
you could add PHYS_BASE= and OpenSolaris would change to use that.

  e_entry:            0x40800000

Program Header[0]:
    p_vaddr:      0x40800000      p_flags:    [ PF_X  PF_W  PF_R ]
    p_paddr:      0x40800000      p_type:     [ PT_LOAD ]
    p_filesz:     0xe95c          p_memsz:    0xe95c
    p_offset:     0xd4            p_align:    0

Program Header[1]:
    p_vaddr:      0xfb400000      p_flags:    [ PF_X  PF_R ]
    p_paddr:      0x40000000      p_type:     [ PT_LOAD ]
    p_filesz:     0x2aa362        p_memsz:    0x2aa362
    p_offset:     0xea40          p_align:    0

Program Header[2]:
    p_vaddr:      0xfb800000      p_flags:    [ PF_X  PF_W  PF_R ]
    p_paddr:      0x40400000      p_type:     [ PT_LOAD ]
    p_filesz:     0x16515         p_memsz:    0x94a44
    p_offset:     0x2b8dc0        p_align:    0

Here's sample values we use for the 64 bit Xen OS image:

  e_entry:            0x40800000

Program Header[0]:
    p_vaddr:      0x40800000      p_flags:    [ PF_X  PF_W  PF_R ]
    p_paddr:      0x40800000      p_type:     [ PT_LOAD ]
    p_filesz:     0xed28          p_memsz:    0xed28
    p_offset:     0x190           p_align:    0

Program Header[1]:
    p_vaddr:      0xfffffffffb800000  p_flags:    [ PF_X  PF_R ]
    p_paddr:      0x40000000      p_type:     [ PT_LOAD ]
    p_filesz:     0x39adca        p_memsz:    0x39adca
    p_offset:     0xeec0          p_align:    0

Program Header[2]:
    p_vaddr:      0xfffffffffbc00000  p_flags:    [ PF_X  PF_W  PF_R ]
    p_paddr:      0x40400000      p_type:     [ PT_LOAD ]
    p_filesz:     0x20fe9         p_memsz:    0xe36c0
    p_offset:     0x3a9cc0        p_align:    0


Joe

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