On 6/6/25 2:04 AM, Philippe Mathieu-Daudé wrote:
On 6/6/25 09:52, Daniel P. Berrangé wrote:
On Fri, Jun 06, 2025 at 03:02:34AM -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.

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.

Can you make this a series, with an additional patch that updates the
current in-dev machine types to use this new property, so we're clear
about the proposed usage.


Signed-off-by: Shaoqin Huang <shahu...@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/vfio/pci.c b/hw/vfio/pci.c
index 7f1532fbed..4e4759c954 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),
   };

Alternatively with target-info API:

-- >8 --
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 8c0f907673d..689f10625f8 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -12,6 +12,7 @@
    */

   #include "qemu/osdep.h"
+#include "qemu/target-info.h"
   #include "qapi/error.h"
   #include "hw/loader.h"
   #include "hw/display/ramfb.h"
@@ -147,7 +148,15 @@ RAMFBState *ramfb_setup(Error **errp)

       s = g_new0(RAMFBState, 1);

-    rom_add_vga("vgabios-ramfb.bin");
+    switch (target_system_arch()) {
+    case SYS_EMU_TARGET_I386:
+    case SYS_EMU_TARGET_X86_64:
+        rom_add_vga("vgabios-ramfb.bin");
+        break;
+    default:
+        break;
+    }
+
       fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
                                NULL, ramfb_fw_cfg_write, s,
                                &s->cfg, sizeof(s->cfg), false);
---

Recent work event introduces target_base_FOO() so that'd be:

-- >8 --
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 8c0f907673d..2aa3b309010 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -12,6 +12,7 @@
    */

   #include "qemu/osdep.h"
+#include "qemu/target-info.h"
   #include "qapi/error.h"
   #include "hw/loader.h"
   #include "hw/display/ramfb.h"
@@ -147,7 +148,10 @@ RAMFBState *ramfb_setup(Error **errp)

       s = g_new0(RAMFBState, 1);

-    rom_add_vga("vgabios-ramfb.bin");
+    if (target_base_x86()) {
+        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);
---

Unfortunately I had to focus on more urgent stuff so this isn't
merged yet (besides I hurt a finger yesterday and am now typing
slower). I hope I'd be able to respin that next week.

In case it's too much effort to respin all the series in a close future, maybe it could be a good idea to just send target_system_arch(), so that people can start using it. From there, people can add target_base_FOO() easily on a as needed basis, without any hard conflict issue.

Thanks,
Pierrick

Reply via email to