[Qemu-devel] Mips target '-kernel' option bug

[Qemu-devel] Mips target '-kernel' option bug

[J. Mayer]

I failed to run Mips target test image on my amd64 machine and I now
found the reason of the bug:
the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
implicitelly assumes that the ram_addr_t is 32 bits long.
Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
load address then is over 4 GB. Then, when computing the initrd_offset,
the code always concludes that there's not enough RAM available to load
it at the top of the kernel.
I found 2 ways of fixing the bug, but I don't know which one is correct
in Mips execution environment.
The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
translating the kernel virtual address from 0x8000nnnn to the physical
one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
The second solution would be to explicitelly always cast the kernel_high
value to 32 bits.
As I do not really know if some Mips target specific constraints would
make one of the other solution prefered, I'd better let the specialist
choose !

The good news is that, once this issue is fixed, the Mips test images
run with the reverse-endian softmmu patch applied.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] Mips target '-kernel' option bug

[Thiemo Seufer]

J. Mayer wrote:
> I failed to run Mips target test image on my amd64 machine and I now
> found the reason of the bug:
> the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> implicitelly assumes that the ram_addr_t is 32 bits long.
> Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> load address then is over 4 GB. Then, when computing the initrd_offset,
> the code always concludes that there's not enough RAM available to load
> it at the top of the kernel.
> I found 2 ways of fixing the bug, but I don't know which one is correct
> in Mips execution environment.
> The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> translating the kernel virtual address from 0x8000nnnn to the physical
> one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> The second solution would be to explicitelly always cast the kernel_high
> value to 32 bits.
> As I do not really know if some Mips target specific constraints would
> make one of the other solution prefered, I'd better let the specialist
> choose !
> 
> The good news is that, once this issue is fixed, the Mips test images
> run with the reverse-endian softmmu patch applied.

I think this patch is the correct fix. Please test and comment.


Thiemo


Index: qemu-work/elf_ops.h
===================================================================
--- qemu-work.orig/elf_ops.h	2007-10-17 14:18:09.000000000 +0100
+++ qemu-work/elf_ops.h	2007-10-17 14:20:20.000000000 +0100
@@ -159,7 +159,7 @@
         goto fail;
 
     if (pentry)
-   	*pentry = (uint64_t)ehdr.e_entry;
+   	*pentry = (uint64_t)(elf_sword)ehdr.e_entry;
 
     glue(load_symbols, SZ)(&ehdr, fd, must_swab);
 
@@ -206,9 +206,9 @@
     }
     qemu_free(phdr);
     if (lowaddr)
-        *lowaddr = (uint64_t)low;
+        *lowaddr = (uint64_t)(elf_sword)low;
     if (highaddr)
-        *highaddr = (uint64_t)high;
+        *highaddr = (uint64_t)(elf_sword)high;
     return total_size;
  fail:
     qemu_free(data);
Index: qemu-work/loader.c
===================================================================
--- qemu-work.orig/loader.c	2007-10-17 14:18:09.000000000 +0100
+++ qemu-work/loader.c	2007-10-17 14:20:19.000000000 +0100
@@ -173,6 +173,7 @@
 
 #define SZ		32
 #define elf_word        uint32_t
+#define elf_sword        int32_t
 #define bswapSZs	bswap32s
 #include "elf_ops.h"
 
@@ -182,6 +183,7 @@
 #undef elf_sym
 #undef elf_note
 #undef elf_word
+#undef elf_sword
 #undef bswapSZs
 #undef SZ
 #define elfhdr		elf64_hdr
@@ -190,6 +192,7 @@
 #define elf_shdr	elf64_shdr
 #define elf_sym		elf64_sym
 #define elf_word        uint64_t
+#define elf_sword        int64_t
 #define bswapSZs	bswap64s
 #define SZ		64
 #include "elf_ops.h"

Re: [Qemu-devel] Mips target '-kernel' option bug

[Jocelyn Mayer]

On Wed, 2007-10-17 at 14:51 +0100, Thiemo Seufer wrote:
> J. Mayer wrote:
> > I failed to run Mips target test image on my amd64 machine and I now
> > found the reason of the bug:
> > the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> > implicitelly assumes that the ram_addr_t is 32 bits long.
> > Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> > load address then is over 4 GB. Then, when computing the initrd_offset,
> > the code always concludes that there's not enough RAM available to load
> > it at the top of the kernel.
> > I found 2 ways of fixing the bug, but I don't know which one is correct
> > in Mips execution environment.
> > The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> > translating the kernel virtual address from 0x8000nnnn to the physical
> > one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> > The second solution would be to explicitelly always cast the kernel_high
> > value to 32 bits.
> > As I do not really know if some Mips target specific constraints would
> > make one of the other solution prefered, I'd better let the specialist
> > choose !
> > 
> > The good news is that, once this issue is fixed, the Mips test images
> > run with the reverse-endian softmmu patch applied.
> 
> I think this patch is the correct fix. Please test and comment.

Thanks, I'll test it at home tonight.
To satisfy my curiosity, is there a specific reason to have a positive
VIRT_TO_PHYS_ADDEND ?

[...]

Re: [Qemu-devel] Mips target '-kernel' option bug

[Thiemo Seufer]

Jocelyn Mayer wrote:
> On Wed, 2007-10-17 at 14:51 +0100, Thiemo Seufer wrote:
> > J. Mayer wrote:
> > > I failed to run Mips target test image on my amd64 machine and I now
> > > found the reason of the bug:
> > > the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> > > implicitelly assumes that the ram_addr_t is 32 bits long.
> > > Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> > > load address then is over 4 GB. Then, when computing the initrd_offset,
> > > the code always concludes that there's not enough RAM available to load
> > > it at the top of the kernel.
> > > I found 2 ways of fixing the bug, but I don't know which one is correct
> > > in Mips execution environment.
> > > The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> > > translating the kernel virtual address from 0x8000nnnn to the physical
> > > one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> > > The second solution would be to explicitelly always cast the kernel_high
> > > value to 32 bits.
> > > As I do not really know if some Mips target specific constraints would
> > > make one of the other solution prefered, I'd better let the specialist
> > > choose !
> > > 
> > > The good news is that, once this issue is fixed, the Mips test images
> > > run with the reverse-endian softmmu patch applied.
> > 
> > I think this patch is the correct fix. Please test and comment.
> 
> Thanks, I'll test it at home tonight.
> To satisfy my curiosity, is there a specific reason to have a positive
> VIRT_TO_PHYS_ADDEND ?

So it wraps around correctly WRT the sign extension rules.


Thiemo

Re: [Qemu-devel] Mips target '-kernel' option bug

[Blue Swirl]

On 10/17/07, Jocelyn Mayer <l_indien@magic.fr> wrote:
> On Wed, 2007-10-17 at 14:51 +0100, Thiemo Seufer wrote:
> > J. Mayer wrote:
> > > I failed to run Mips target test image on my amd64 machine and I now
> > > found the reason of the bug:
> > > the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> > > implicitelly assumes that the ram_addr_t is 32 bits long.
> > > Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> > > load address then is over 4 GB. Then, when computing the initrd_offset,
> > > the code always concludes that there's not enough RAM available to load
> > > it at the top of the kernel.
> > > I found 2 ways of fixing the bug, but I don't know which one is correct
> > > in Mips execution environment.
> > > The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> > > translating the kernel virtual address from 0x8000nnnn to the physical
> > > one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> > > The second solution would be to explicitelly always cast the kernel_high
> > > value to 32 bits.
> > > As I do not really know if some Mips target specific constraints would
> > > make one of the other solution prefered, I'd better let the specialist
> > > choose !
> > >
> > > The good news is that, once this issue is fixed, the Mips test images
> > > run with the reverse-endian softmmu patch applied.
> >
> > I think this patch is the correct fix. Please test and comment.
>
> Thanks, I'll test it at home tonight.
> To satisfy my curiosity, is there a specific reason to have a positive
> VIRT_TO_PHYS_ADDEND ?

On Sparc, OpenBIOS image is loaded to a physical address that is
higher in the address space than the virtual address:
#define PROM_PADDR           0xff0000000ULL
#define PROM_VADDR           0xffd00000
and
#define PROM_ADDR            0x1fff0000000ULL
#define PROM_VADDR           0x000ffd00000ULL

Re: [Qemu-devel] Mips target '-kernel' option bug

[J. Mayer]

On Wed, 2007-10-17 at 22:06 +0300, Blue Swirl wrote:
> On 10/17/07, Jocelyn Mayer <l_indien@magic.fr> wrote:
> > On Wed, 2007-10-17 at 14:51 +0100, Thiemo Seufer wrote:
> > > J. Mayer wrote:
> > > > I failed to run Mips target test image on my amd64 machine and I now
> > > > found the reason of the bug:
> > > > the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> > > > implicitelly assumes that the ram_addr_t is 32 bits long.
> > > > Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> > > > load address then is over 4 GB. Then, when computing the initrd_offset,
> > > > the code always concludes that there's not enough RAM available to load
> > > > it at the top of the kernel.
> > > > I found 2 ways of fixing the bug, but I don't know which one is correct
> > > > in Mips execution environment.
> > > > The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> > > > translating the kernel virtual address from 0x8000nnnn to the physical
> > > > one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> > > > The second solution would be to explicitelly always cast the kernel_high
> > > > value to 32 bits.
> > > > As I do not really know if some Mips target specific constraints would
> > > > make one of the other solution prefered, I'd better let the specialist
> > > > choose !
> > > >
> > > > The good news is that, once this issue is fixed, the Mips test images
> > > > run with the reverse-endian softmmu patch applied.
> > >
> > > I think this patch is the correct fix. Please test and comment.
> >
> > Thanks, I'll test it at home tonight.
> > To satisfy my curiosity, is there a specific reason to have a positive
> > VIRT_TO_PHYS_ADDEND ?
> 
> On Sparc, OpenBIOS image is loaded to a physical address that is
> higher in the address space than the virtual address:
> #define PROM_PADDR           0xff0000000ULL
> #define PROM_VADDR           0xffd00000
> and
> #define PROM_ADDR            0x1fff0000000ULL
> #define PROM_VADDR           0x000ffd00000ULL

OK, thanks.
And the patch seems OK for me, it may be a good idea to commit it !

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] Mips target '-kernel' option bug

[Thiemo Seufer]

J. Mayer wrote:
> On Wed, 2007-10-17 at 22:06 +0300, Blue Swirl wrote:
> > On 10/17/07, Jocelyn Mayer <l_indien@magic.fr> wrote:
> > > On Wed, 2007-10-17 at 14:51 +0100, Thiemo Seufer wrote:
> > > > J. Mayer wrote:
> > > > > I failed to run Mips target test image on my amd64 machine and I now
> > > > > found the reason of the bug:
> > > > > the kernel loader code used in hw/mips_r4k.c and hw/mips_malta.c
> > > > > implicitelly assumes that the ram_addr_t is 32 bits long.
> > > > > Unfortunatelly, on 64 bits hosts, this won't be the case and the kernel
> > > > > load address then is over 4 GB. Then, when computing the initrd_offset,
> > > > > the code always concludes that there's not enough RAM available to load
> > > > > it at the top of the kernel.
> > > > > I found 2 ways of fixing the bug, but I don't know which one is correct
> > > > > in Mips execution environment.
> > > > > The first patch is to make the VIRT_TO_PHYS_ADDEND negative, thus
> > > > > translating the kernel virtual address from 0x8000nnnn to the physical
> > > > > one 0x0000nnnn (instead of 0x10000nnnn, when running on 64 bits hosts).
> > > > > The second solution would be to explicitelly always cast the kernel_high
> > > > > value to 32 bits.
> > > > > As I do not really know if some Mips target specific constraints would
> > > > > make one of the other solution prefered, I'd better let the specialist
> > > > > choose !
> > > > >
> > > > > The good news is that, once this issue is fixed, the Mips test images
> > > > > run with the reverse-endian softmmu patch applied.
> > > >
> > > > I think this patch is the correct fix. Please test and comment.
> > >
> > > Thanks, I'll test it at home tonight.
> > > To satisfy my curiosity, is there a specific reason to have a positive
> > > VIRT_TO_PHYS_ADDEND ?
> > 
> > On Sparc, OpenBIOS image is loaded to a physical address that is
> > higher in the address space than the virtual address:
> > #define PROM_PADDR           0xff0000000ULL
> > #define PROM_VADDR           0xffd00000
> > and
> > #define PROM_ADDR            0x1fff0000000ULL
> > #define PROM_VADDR           0x000ffd00000ULL
> 
> OK, thanks.
> And the patch seems OK for me, it may be a good idea to commit it !

Committed.


Thiemo