[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image
Date: Sat, 17 Oct 2020 19:38:24 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 10/17/20 6:31 PM, BALATON Zoltan via wrote:
On Sat, 17 Oct 2020, Philippe Mathieu-Daudé wrote:
+Alistair for loader

On 10/17/20 5:47 PM, BALATON Zoltan via 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>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
v9: Revert change from v8, back to the same as v7 rebased on latest

  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 05e46ee6fe..0117ae17f5 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)
@@ -100,6 +102,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;
@@ -128,24 +131,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,
+    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);
+        /* Unfortunately, load_elf sign-extends reading elf32 */

Maybe this is what translate_fn() is for?

uint64_t oldworld_phys(void *opaque, uint64_t addr)
   return addr & UINT32_MAX;

Using as (untested):

       bios_size = load_elf(filename, NULL, oldworld_phys, NULL,
                            NULL, &bios_addr, NULL,
                            NULL, 1, PPC_ELF_MACHINE, 0, 0);

Please don't come up with any more great ideas for this patch unless you also propose a replacement and test it. This one works and we could just get this in as it is until the real problem with load_elf is fixed at which point all this can be removed so no need to be more sophisticated as the simple cast I have.

Zoltan, I'm not trying to block your patch to get merged,
I'm asking because I'm trying to understand how this API
is expected to be used.

As you can see in the original discussion:


problem is really in include/hw/elf_ops.h this is just a work around for that as I did not want to break anything I can't test so I'd rather fix it up here and let you fix load_elf then drop this cast. But unless you can do that before the freeze please don't hold up this patch any more.


+        bios_addr = (uint32_t)bios_addr;
+        if (bios_size <= 0) {
+            /* or load binary ROM image */
+            bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
+            bios_addr = PROM_BASE;
+        }
      } else {
          bios_size = -1;
-    if (bios_size < 0 || bios_size > BIOS_SIZE) {
+    if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
          error_report("could not load PowerPC bios '%s'", bios_name);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]