[prev in list] [next in list] [prev in thread] [next in thread]
List: qemu-devel
Subject: Re: [PATCH v7 1/8] mac_oldworld: Allow loading binary ROM image
From: BALATON Zoltan <balaton () eik ! bme ! hu>
Date: 2020-06-30 21:45:42
Message-ID: alpine.BSF.2.22.395.2006302249091.46417 () zero ! eik ! bme ! hu
[Download RAW message or body]
On Tue, 30 Jun 2020, Mark Cave-Ayland wrote:
> On 29/06/2020 19:55, BALATON Zoltan wrote:
> > The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
> > the rom region and fall back to loading a binary image with -bios if
> > loading ELF image failed. This allows testing emulation with a ROM
> > image from real hardware as well as using an ELF OpenBIOS image.
> >
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> > v4: use load address from ELF to check if ROM is too big
> >
> > hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
> > 1 file changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> > index f8c204ead7..baf3da6f90 100644
> > --- a/hw/ppc/mac_oldworld.c
> > +++ b/hw/ppc/mac_oldworld.c
> > @@ -59,6 +59,8 @@
> > #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
> >
> > #define GRACKLE_BASE 0xfec00000
> > +#define PROM_BASE 0xffc00000
> > +#define PROM_SIZE (4 * MiB)
> >
> > static void fw_cfg_boot_set(void *opaque, const char *boot_device,
> > Error **errp)
> > @@ -99,6 +101,7 @@ static void ppc_heathrow_init(MachineState *machine)
> > SysBusDevice *s;
> > DeviceState *dev, *pic_dev;
> > BusState *adb_bus;
> > + uint64_t bios_addr;
> > int bios_size;
> > unsigned int smp_cpus = machine->smp.cpus;
> > uint16_t ppc_boot_device;
> > @@ -127,24 +130,32 @@ static void ppc_heathrow_init(MachineState *machine)
> >
> > memory_region_add_subregion(sysmem, 0, machine->ram);
> >
> > - /* allocate and load BIOS */
> > - memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
> > + /* allocate and load firmware ROM */
> > + memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
> > &error_fatal);
> > + memory_region_add_subregion(sysmem, PROM_BASE, bios);
> >
> > - if (bios_name == NULL)
> > + if (!bios_name) {
> > bios_name = PROM_FILENAME;
> > + }
> > filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> > - memory_region_add_subregion(sysmem, PROM_ADDR, bios);
> > -
> > - /* Load OpenBIOS (ELF) */
> > if (filename) {
> > - bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
> > - 1, PPC_ELF_MACHINE, 0, 0);
> > + /* Load OpenBIOS (ELF) */
> > + bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
> > + NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
> > + if (bios_size <= 0) {
> > + /* or load binary ROM image */
> > + bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
> > + bios_addr = PROM_BASE;
> > + } else {
> > + /* load_elf sets high 32 bits for some reason, strip those */
> > + bios_addr &= 0xffffffffULL;
>
> Repeating my earlier comment from v5: something is wrong here if you need to \
> manually strip the high bits. If you compare with SPARC32 which uses the same \
> approach, there is no such strip required - have a look there to try and figure out \
> what's going on here.
OK, the problem here is this:
$ gdb qemu-system-ppc
(gdb) b mac_oldworld.c:146
Breakpoint 1 at 0x416770: file hw/ppc/mac_oldworld.c, line 146.
(gdb) r
Thread 1 "qemu-system-ppc" hit Breakpoint 1, ppc_heathrow_init \
(machine=0x555556863800) at hw/ppc/mac_oldworld.c:146 146 filename = \
qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); (gdb) n
147 if (filename) {
149 bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
151 if (bios_size <= 0) {
(gdb) p bios_size
$1 = 755500
(gdb) p/x bios_addr
$2 = 0xfffffffffff00000
this happens within load_elf that I don't feel like wanting to debug but
causes problem when we use it to calculate bios size later here:
- if (bios_size < 0 || bios_size > BIOS_SIZE) {
+ if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
unless we strip it down to expected 32 bits. This could be some unwanted
size extension or whatnot but I don't have time to dig deeper.
Now lets see what sun4m does:
$ gdb qemu-system-sparc
(gdb) b sun4m.c:721
Breakpoint 1 at 0x1fc0e6: file hw/sparc/sun4m.c, line 721.
(gdb) r
Thread 1 "qemu-system-spa" hit Breakpoint 1, prom_init (addr=1879048192, \
bios_name=0x555555b3c60d "openbios-sparc32") at hw/sparc/sun4m.c:721 721 filename \
= qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); (gdb) p/x addr
$1 = 0x70000000
(gdb) n
722 if (filename) {
723 ret = load_elf(filename, NULL,
726 if (ret < 0 || ret > PROM_SIZE_MAX) {
(gdb) p ret
$2 = 847872
(gdb) p/x addr
$3 = 0x70000000
Hmm, does not happen here, the difference is that this calls load_elf with
addr already initialised so maybe load_elf only sets low 32 bits? By the
way, sun4m does not use the returned addr so even if it was wrong it would
not be noticed,
Maybe initialising addr before calling load_elf in mac_oldworld,c could
fix this so we can get rid of the fix up? Unfortunately not:
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
SysBusDevice *s;
DeviceState *dev, *pic_dev;
BusState *adb_bus;
- uint64_t bios_addr;
+ uint64_t bios_addr = 0;
int bios_size;
unsigned int smp_cpus = machine->smp.cpus;
uint16_t ppc_boot_device;
$ gdb qemu-system-ppc
[...]
Thread 1 "qemu-system-ppc" hit Breakpoint 1, ppc_heathrow_init \
(machine=0x555556863800) at hw/ppc/mac_oldworld.c:146 146 filename = \
qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); (gdb) p bios_addr
$1 = 0
(gdb) n
147 if (filename) {
149 bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
151 if (bios_size <= 0) {
(gdb) p/x bios_addr
$2 = 0xfffffffffff00000
Could this be something about openbios-ppc? I don't know. I give up
investigating further at this point and let someone else find out.
Any ideas?
Regards,
BALATON Zoltan
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic