Re: [PATCH v5 20/20] mac_{old, new}world: Pass MacOS VGA NDRV in card ROM instead of fw_cfg

[BALATON Zoltan]

On Fri, 28 Oct 2022, Mark Cave-Ayland wrote:
On 25/10/2022 22:31, BALATON Zoltan wrote:
OpenBIOS cannot run FCode ROMs yet but it can detect NDRV in VGA card
ROM and add it to the device tree for MacOS. Pass the NDRV this way
instead of via fw_cfg. This solves the problem with OpenBIOS also
adding the NDRV to ati-vga which it does not work with. This does not
need any changes to OpenBIOS as this NDRV ROM handling is already
there but this patch also allows simplifying OpenBIOS later to remove
the fw_cfg ndrv handling from the vga FCode and also drop the
vga-ndrv? option which is not needed any more as users can disable the
ndrv with -device VGA,romfile="" (or override it with their own NDRV
or ROM). Once FCode support is implemented in OpenBIOS, the proper
FCode ROM can be set the same way so this paves the way to remove some
hacks.

This is not correct though: in a real option ROM the NDRV is included as part 
of the ROM payload and is not a standalone file. The IEEE-1275 PCI 
specification gives the correct format for an option ROM which at minimum 
contains a header, and likely some additional FCode.

As the commit message says that does not work with OpenBIOS at the moment 
but passing the NDRV does. That it's not how real hardware works is not an 
argument after all real hardware does not have fw_cfg either and this way 
is much simpler than fw_cfg, it fixes the problem with ati-vga and it can 
be changed later to pass the real FCode ROM the same way so I think it's a 
better way to handle this now as what we have currently.

Isn't the immediate problem here that the NDRV handling in OpenBIOS needs to 
be improved so that it can be disabled for particular VGA devices such as 
ATI?

No change is needed to OpenBIOS (I've discussed it more in the reply to 
Howard on the list yesterday). With this patch only VGA device will have 
qemu_vga.ndrv so OpenBIOS won't add it for ati-vga. Also the fw_cfg and 
vga_ndrv? stuff can be removed from OpenBIOS after this patch as it's not 
nedeed any more thus simplifying the vga.fs FCode in OpenBIOS a lot.

Regards,
BALATON Zoltan

Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
  hw/ppc/mac_newworld.c | 18 ++++++------------
  hw/ppc/mac_oldworld.c | 18 ++++++------------
  2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index de4a7bae12..1d12bd85ed 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -526,18 +526,6 @@ static void ppc_core99_init(MachineState *machine)
      fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
      fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_NVRAM_ADDR, nvram_addr);
  -    /* MacOS NDRV VGA driver */
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
-    if (filename) {
-        gchar *ndrv_file;
-        gsize ndrv_size;
-
-        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
-            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
ndrv_size);
-        }
-        g_free(filename);
-    }
-
      qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
  }
  @@ -581,6 +569,11 @@ static int core99_kvm_type(MachineState *machine, 
const char *arg)
      return 2;
  }
  +static GlobalProperty props[] = {
+    /* MacOS NDRV VGA driver */
+    { "VGA", "romfile", NDRV_VGA_FILENAME },
+};
+
  static void core99_machine_class_init(ObjectClass *oc, void *data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
@@ -601,6 +594,7 @@ static void core99_machine_class_init(ObjectClass *oc, 
void *data)
  #endif
      mc->default_ram_id = "ppc_core99.ram";
      mc->ignore_boot_device_suffixes = true;
+    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
      fwc->get_dev_path = core99_fw_dev_path;
  }
  diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index eecc54da59..e7d35135d6 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -344,18 +344,6 @@ static void ppc_heathrow_init(MachineState *machine)
      fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
      fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
  -    /* MacOS NDRV VGA driver */
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
-    if (filename) {
-        gchar *ndrv_file;
-        gsize ndrv_size;
-
-        if (g_file_get_contents(filename, &ndrv_file, &ndrv_size, NULL)) {
-            fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
ndrv_size);
-        }
-        g_free(filename);
-    }
-
      qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
  }
  @@ -400,6 +388,11 @@ static int heathrow_kvm_type(MachineState *machine, 
const char *arg)
      return 2;
  }
  +static GlobalProperty props[] = {
+    /* MacOS NDRV VGA driver */
+    { "VGA", "romfile", NDRV_VGA_FILENAME },
+};
+
  static void heathrow_class_init(ObjectClass *oc, void *data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
@@ -420,6 +413,7 @@ static void heathrow_class_init(ObjectClass *oc, void 
*data)
      mc->default_display = "std";
      mc->ignore_boot_device_suffixes = true;
      mc->default_ram_id = "ppc_heathrow.ram";
+    compat_props_add(mc->compat_props, props, G_N_ELEMENTS(props));
      fwc->get_dev_path = heathrow_fw_dev_path;
  }



ATB,

Mark.