Re: [PATCH v2] ramfb: Add property to control if load the romfile

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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