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

[Mark Cave-Ayland]

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.

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?

> Signed-off-by: BALATON Zoltan <balaton@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.