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

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

[Shaoqin Huang]

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.

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.

I set the "use-legacy-x86-rom" property on arm to false, thus the arm won't load
the vgabios-ramfb.bin.

I want to set the "use-legacy-x86-rom" property to false by default, and only
set it to true on x86, but I didn't find the similiar thing like the
arm_virt_compat, so I didn't use this way.

Changelog:
---------
v2 -> v3:
  - Fix the underscore error.
  - Add a new patch to set the property in arm compatibility.
v1 -> v2:
  - Change the property name.

v2: https://lore.kernel.org/all/20250606070234.2063451-1-shahuang@redhat.com/
v1: https://lore.kernel.org/all/20250605030351.2056571-1-shahuang@redhat.com/

Shaoqin Huang (2):
  ramfb: Add property to control if load the romfile
  hw/arm: Add the romfile compatatibility

 hw/arm/virt.c                 | 3 +++
 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 +-
 8 files changed, 16 insertions(+), 7 deletions(-)

-- 
2.40.1

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

[Shaoqin Huang]

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.

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),
 };
 
 static void ramfb_class_initfn(ObjectClass *klass, void *data)
diff --git a/hw/display/ramfb-stubs.c b/hw/display/ramfb-stubs.c
index cf64733b10..b83551357b 100644
--- a/hw/display/ramfb-stubs.c
+++ b/hw/display/ramfb-stubs.c
@@ -8,7 +8,7 @@ void ramfb_display_update(QemuConsole *con, RAMFBState *s)
 {
 }
 
-RAMFBState *ramfb_setup(Error **errp)
+RAMFBState *ramfb_setup(bool romfile, Error **errp)
 {
     error_setg(errp, "ramfb support not available");
     return NULL;
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 8c0f907673..9a17d97d07 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -135,7 +135,7 @@ const VMStateDescription ramfb_vmstate = {
     }
 };
 
-RAMFBState *ramfb_setup(Error **errp)
+RAMFBState *ramfb_setup(bool romfile, Error **errp)
 {
     FWCfgState *fw_cfg = fw_cfg_find();
     RAMFBState *s;
@@ -147,7 +147,9 @@ RAMFBState *ramfb_setup(Error **errp)
 
     s = g_new0(RAMFBState, 1);
 
-    rom_add_vga("vgabios-ramfb.bin");
+    if (romfile) {
+        rom_add_vga("vgabios-ramfb.bin");
+    }
     fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
                              NULL, ramfb_fw_cfg_write, s,
                              &s->cfg, sizeof(s->cfg), false);
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index ea87830fe0..8bfd8eb1e3 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -365,7 +365,7 @@ static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
                                           &vfio_display_dmabuf_ops,
                                           vdev);
     if (vdev->enable_ramfb) {
-        vdev->dpy->ramfb = ramfb_setup(errp);
+        vdev->dpy->ramfb = ramfb_setup(vdev->use_legacy_x86_rom, errp);
         if (!vdev->dpy->ramfb) {
             return false;
         }
@@ -494,7 +494,7 @@ static bool vfio_display_region_init(VFIOPCIDevice *vdev, Error **errp)
                                           &vfio_display_region_ops,
                                           vdev);
     if (vdev->enable_ramfb) {
-        vdev->dpy->ramfb = ramfb_setup(errp);
+        vdev->dpy->ramfb = ramfb_setup(vdev->use_legacy_x86_rom, errp);
         if (!vdev->dpy->ramfb) {
             return false;
         }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 7f1532fbed..ff0d93fae0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3564,6 +3564,7 @@ static const TypeInfo vfio_pci_dev_info = {
 
 static const Property vfio_pci_dev_nohotplug_properties[] = {
     DEFINE_PROP_BOOL("ramfb", VFIOPCIDevice, enable_ramfb, false),
+    DEFINE_PROP_BOOL("use-legacy-x86-rom", VFIOPCIDevice, use_legacy_x86_rom, true),
     DEFINE_PROP_ON_OFF_AUTO("x-ramfb-migrate", VFIOPCIDevice, ramfb_migrate,
                             ON_OFF_AUTO_AUTO),
 };
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index d94ecaba68..713956157e 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -177,6 +177,7 @@ struct VFIOPCIDevice {
     bool no_kvm_ioeventfd;
     bool no_vfio_ioeventfd;
     bool enable_ramfb;
+    bool use_legacy_x86_rom;
     OnOffAuto ramfb_migrate;
     bool defer_kvm_irq_routing;
     bool clear_parent_atomics_on_exit;
diff --git a/include/hw/display/ramfb.h b/include/hw/display/ramfb.h
index a7e0019144..172aa6dc89 100644
--- a/include/hw/display/ramfb.h
+++ b/include/hw/display/ramfb.h
@@ -6,7 +6,7 @@
 /* ramfb.c */
 typedef struct RAMFBState RAMFBState;
 void ramfb_display_update(QemuConsole *con, RAMFBState *s);
-RAMFBState *ramfb_setup(Error **errp);
+RAMFBState *ramfb_setup(bool romfile, Error **errp);
 
 extern const VMStateDescription ramfb_vmstate;
 
-- 
2.40.1

[PATCH v3 2/2] hw/arm: Add the romfile compatatibility

[Shaoqin Huang]

On arm64, it doesn't use the vgabios-ramfb.bin, so set the property
"use-legacy-x86-rom" to false, thus the ramfb won't load the
vgabios-ramfb.bin.

This can mitigate the problem that on release version the qemu can't
find the vgabios-ramfb.bin if it use the ramfb.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
---
 hw/arm/virt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a96452f17a..5f94f7a2ca 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -38,6 +38,7 @@
 #include "hw/arm/primecell.h"
 #include "hw/arm/virt.h"
 #include "hw/block/flash.h"
+#include "hw/vfio/pci.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
 #include "hw/vfio/vfio-amd-xgbe.h"
 #include "hw/display/ramfb.h"
@@ -90,6 +91,8 @@
 
 static GlobalProperty arm_virt_compat[] = {
     { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
+    { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "false" },
+    { TYPE_VFIO_PCI, "use-legacy-x86-rom", "false" },
 };
 static const size_t arm_virt_compat_len = G_N_ELEMENTS(arm_virt_compat);
 
-- 
2.40.1

Re: [PATCH v3 2/2] hw/arm: Add the romfile compatatibility

[Peter Maydell]

On Mon, 9 Jun 2025 at 08:34, Shaoqin Huang <shahuang@redhat.com> wrote:
>
> On arm64, it doesn't use the vgabios-ramfb.bin, so set the property
> "use-legacy-x86-rom" to false, thus the ramfb won't load the
> vgabios-ramfb.bin.
>
> This can mitigate the problem that on release version the qemu can't
> find the vgabios-ramfb.bin if it use the ramfb.
>
> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> ---
>  hw/arm/virt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a96452f17a..5f94f7a2ca 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -38,6 +38,7 @@
>  #include "hw/arm/primecell.h"
>  #include "hw/arm/virt.h"
>  #include "hw/block/flash.h"
> +#include "hw/vfio/pci.h"
>  #include "hw/vfio/vfio-calxeda-xgmac.h"
>  #include "hw/vfio/vfio-amd-xgbe.h"
>  #include "hw/display/ramfb.h"
> @@ -90,6 +91,8 @@
>
>  static GlobalProperty arm_virt_compat[] = {
>      { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> +    { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "false" },
> +    { TYPE_VFIO_PCI, "use-legacy-x86-rom", "false" },

I think we should find a way to make this default to "false"
and only be set "true" for x86. Otherwise every single non-x86
board that ever adds support for ramfb and virtio will have
to add these two lines, which is a source of future bugs.
(Whereas if you forget to mark a new x86 board as needing
the legacy rom you'll find out about it pretty quickly.)

thanks
-- PMM

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

[Daniel P. Berrangé]

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 :|

Re: [PATCH v3 2/2] hw/arm: Add the romfile compatatibility

[Daniel P. Berrangé]

On Mon, Jun 09, 2025 at 09:48:36AM +0100, Peter Maydell wrote:
> On Mon, 9 Jun 2025 at 08:34, Shaoqin Huang <shahuang@redhat.com> wrote:
> >
> > On arm64, it doesn't use the vgabios-ramfb.bin, so set the property
> > "use-legacy-x86-rom" to false, thus the ramfb won't load the
> > vgabios-ramfb.bin.
> >
> > This can mitigate the problem that on release version the qemu can't
> > find the vgabios-ramfb.bin if it use the ramfb.
> >
> > Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
> > ---
> >  hw/arm/virt.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index a96452f17a..5f94f7a2ca 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -38,6 +38,7 @@
> >  #include "hw/arm/primecell.h"
> >  #include "hw/arm/virt.h"
> >  #include "hw/block/flash.h"
> > +#include "hw/vfio/pci.h"
> >  #include "hw/vfio/vfio-calxeda-xgmac.h"
> >  #include "hw/vfio/vfio-amd-xgbe.h"
> >  #include "hw/display/ramfb.h"
> > @@ -90,6 +91,8 @@
> >
> >  static GlobalProperty arm_virt_compat[] = {
> >      { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
> > +    { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "false" },
> > +    { TYPE_VFIO_PCI, "use-legacy-x86-rom", "false" },
> 
> I think we should find a way to make this default to "false"
> and only be set "true" for x86. Otherwise every single non-x86
> board that ever adds support for ramfb and virtio will have
> to add these two lines, which is a source of future bugs.
> (Whereas if you forget to mark a new x86 board as needing
> the legacy rom you'll find out about it pretty quickly.)

Yes, going forward we this to default to true only on x86.

For non-x86, historical versioned machine types will need
likely it set to true, in order to avoid the memory layout
being changed IIUC.


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 :|

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

[Gerd Hoffmann]

  Hi,

> $ 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 ?

Anything with pci memory bars is problematic on arm (which is one of the
reasons why ramfb exists in the first place).

> 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.

Note that non-x86 drivers for some of these devices exist, we have at
least roms/QemuMacDrivers which includes a driver for (IIRC) stdvga.

The ipxe roms are x86-only too btw.  We could make them multi-arch at
least for EFI platforms, but given that edk2 ships a virtio-net driver
and the recently added EFI archs (aarch64, riscv64, loongarch64) are all
younger than virtio-net there is little reason to do so.

> 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.

All pci devices already have a romfile property.  So for most devices
this is a matter of setting this property via machine compat properties.

ramfb is a bit different here because it is not a PCI device, so we
can't control things with the existing property and need a new one.

take care,
  Gerd

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

[Shaoqin Huang]

Hi Gerd, Daniel,

Sorry for the late reply.

On 6/10/25 2:47 PM, Gerd Hoffmann wrote:
>    Hi,
> 
>> $ 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 ?
> 
> Anything with pci memory bars is problematic on arm (which is one of the
> reasons why ramfb exists in the first place).
> 
>> 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.
> 
> Note that non-x86 drivers for some of these devices exist, we have at
> least roms/QemuMacDrivers which includes a driver for (IIRC) stdvga.
> 
> The ipxe roms are x86-only too btw.  We could make them multi-arch at
> least for EFI platforms, but given that edk2 ships a virtio-net driver
> and the recently added EFI archs (aarch64, riscv64, loongarch64) are all
> younger than virtio-net there is little reason to do so.
> 
>> 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.
> 
> All pci devices already have a romfile property.  So for most devices
> this is a matter of setting this property via machine compat properties.
> 
> ramfb is a bit different here because it is not a PCI device, so we
> can't control things with the existing property and need a new one.

So I guess I don't need to add property to all these devices and keep 
the current code is fine?

Thanks,
Shaoqin

> 
> take care,
>    Gerd
> 

-- 
Shaoqin

Re: [PATCH v3 2/2] hw/arm: Add the romfile compatatibility

[Shaoqin Huang]

On 6/9/25 5:10 PM, Daniel P. Berrangé wrote:
> On Mon, Jun 09, 2025 at 09:48:36AM +0100, Peter Maydell wrote:
>> On Mon, 9 Jun 2025 at 08:34, Shaoqin Huang <shahuang@redhat.com> wrote:
>>>
>>> On arm64, it doesn't use the vgabios-ramfb.bin, so set the property
>>> "use-legacy-x86-rom" to false, thus the ramfb won't load the
>>> vgabios-ramfb.bin.
>>>
>>> This can mitigate the problem that on release version the qemu can't
>>> find the vgabios-ramfb.bin if it use the ramfb.
>>>
>>> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
>>> ---
>>>   hw/arm/virt.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index a96452f17a..5f94f7a2ca 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -38,6 +38,7 @@
>>>   #include "hw/arm/primecell.h"
>>>   #include "hw/arm/virt.h"
>>>   #include "hw/block/flash.h"
>>> +#include "hw/vfio/pci.h"
>>>   #include "hw/vfio/vfio-calxeda-xgmac.h"
>>>   #include "hw/vfio/vfio-amd-xgbe.h"
>>>   #include "hw/display/ramfb.h"
>>> @@ -90,6 +91,8 @@
>>>
>>>   static GlobalProperty arm_virt_compat[] = {
>>>       { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
>>> +    { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "false" },
>>> +    { TYPE_VFIO_PCI, "use-legacy-x86-rom", "false" },
>>
>> I think we should find a way to make this default to "false"
>> and only be set "true" for x86. Otherwise every single non-x86
>> board that ever adds support for ramfb and virtio will have
>> to add these two lines, which is a source of future bugs.
>> (Whereas if you forget to mark a new x86 board as needing
>> the legacy rom you'll find out about it pretty quickly.)
> 
> Yes, going forward we this to default to true only on x86.
> 
> For non-x86, historical versioned machine types will need
> likely it set to true, in order to avoid the memory layout
> being changed IIUC.

Ok, I will set this to be true by default only on x86.

Thanks,
Shaoqin

> 
> 
> With regards,
> Daniel

-- 
Shaoqin