Re: [PATCH v3 1/2] ramfb: Add property to control if load the romfile

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Jun 09, 2025 at 03:34:07AM -0400, Shaoqin Huang wrote:
> Now the ramfb will load the vgabios-ramfb.bin unconditionally, but only
> the x86 need the vgabios-ramfb.bin, this can cause that when use the
> release package on arm64 it can't find the vgabios-ramfb.bin.
> 
> Because only seabios will use the vgabios-ramfb.bin, load the rom logic
> is x86-specific. For other !x86 platforms, the edk2 ships an EFI driver
> for ramfb, so they don't need to load the romfile.

vgabios-ramfb.bin is just one of many VGA BIOS ROMs in QEMU

$ git grep vgabios hw/
../hw/display/ati.c:    k->romfile = "vgabios-ati.bin";
../hw/display/bochs-display.c:    k->romfile   = "vgabios-bochs-display.bin";
../hw/display/qxl.c:    k->romfile = "vgabios-qxl.bin";
../hw/display/ramfb.c:    rom_add_vga("vgabios-ramfb.bin");
../hw/display/vga-pci.c:    k->romfile = "vgabios-stdvga.bin";
../hw/display/vga_int.h:#define VGABIOS_FILENAME "vgabios.bin"
../hw/display/vga_int.h:#define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin"
../hw/display/virtio-vga.c:    pcidev_k->romfile = "vgabios-virtio.bin";
../hw/display/vmware_vga.c:    k->romfile = "vgabios-vmware.bin";
../hw/ppc/amigaone.c: * BIOS emulator in firmware cannot run QEMU vgabios and hangs on it, use
../hw/ppc/amigaone.c: * from http://www.nongnu.org/vgabios/ instead.
../hw/xen/xen_pt_graphics.c:static void *get_vgabios(XenPCIPassthroughState *s, int *size,
../hw/xen/xen_pt_graphics.c:    bios = get_vgabios(s, &bios_size, dev);


At least some of these devices are built into non-x86 system
emulators, and would show the same behaviour if the ROM is not
installed

$ qemu-system-aarch64  -machine virt -cpu max -device ati-vga
qemu-system-aarch64: -device ati-vga: failed to find romfile "vgabios-ati.bin"
$ qemu-system-aarch64  -machine virt -cpu max -device cirrus-vga
qemu-system-aarch64: -device cirrus-vga: failed to find romfile "vgabios-cirrus.bin"
$ qemu-system-aarch64  -machine virt -cpu max -device VGA
qemu-system-aarch64: -device VGA: failed to find romfile "vgabios-stdvga.bin"

Perhaps some of these devices are non-functional for other
reasons ?

None the less if the device is built for non-x86 targets, and
the ROM files contain x86-only code that is to be executed by
SeaBIOS only, then conceptually this fix should apply to all
devices use a VGA BIOS ROM, not just ramfb.

If we're introducing a property to control this usage, then
we should fix all devices at once, so we don't need to add
separate properties for other devices in future.

> 
> So add a new property use_legacy_x86_rom in both ramfb and vfio_pci
> device, because the vfio display also use the ramfb_setup() to load
> the vgabios-ramfb.bin file.
> 
> After have this property, the machine type can set the compatibility to
> not load the vgabios-ramfb.bin if the arch doesn't need it.
> 
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  hw/display/ramfb-standalone.c | 4 +++-
>  hw/display/ramfb-stubs.c      | 2 +-
>  hw/display/ramfb.c            | 6 ++++--
>  hw/vfio/display.c             | 4 ++--
>  hw/vfio/pci.c                 | 1 +
>  hw/vfio/pci.h                 | 1 +
>  include/hw/display/ramfb.h    | 2 +-
>  7 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
> index 1be106b57f..af1175bf96 100644
> --- a/hw/display/ramfb-standalone.c
> +++ b/hw/display/ramfb-standalone.c
> @@ -17,6 +17,7 @@ struct RAMFBStandaloneState {
>      QemuConsole *con;
>      RAMFBState *state;
>      bool migrate;
> +    bool use_legacy_x86_rom;
>  };
>  
>  static void display_update_wrapper(void *dev)
> @@ -39,7 +40,7 @@ static void ramfb_realizefn(DeviceState *dev, Error **errp)
>      RAMFBStandaloneState *ramfb = RAMFB(dev);
>  
>      ramfb->con = graphic_console_init(dev, 0, &wrapper_ops, dev);
> -    ramfb->state = ramfb_setup(errp);
> +    ramfb->state = ramfb_setup(ramfb->use_legacy_x86_rom, errp);
>  }
>  
>  static bool migrate_needed(void *opaque)
> @@ -62,6 +63,7 @@ static const VMStateDescription ramfb_dev_vmstate = {
>  
>  static const Property ramfb_properties[] = {
>      DEFINE_PROP_BOOL("x-migrate", RAMFBStandaloneState, migrate,  true),
> +    DEFINE_PROP_BOOL("use-legacy-x86-rom", RAMFBStandaloneState, use_legacy_x86_rom, true),

There are lots of ROMs, so this property name should include
some reference to vgabios, perhaps

  'use-legacy-vgabios-rom'


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|