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

[Cédric Le Goater]

On 6/5/25 05:03, 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.
So add a new property ramfb-romfile 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 <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/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
index 1be106b57f..2b96a49baa 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 ramfb_romfile;
  };
  
  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->ramfb_romfile, 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("ramfb-romfile", RAMFBStandaloneState, ramfb_romfile, 
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..56f10564f9 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->ramfb_romfile, 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->ramfb_romfile, errp);
          if (!vdev->dpy->ramfb) {
              return false;
          }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 7f1532fbed..bfdf365978 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("ramfb-romfile", VFIOPCIDevice, ramfb_romfile, 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..d567de8f10 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 ramfb_romfile;

The attribute 'ramfb_romfile' sounds like a file name. Could you please
add an 'enable' prefix or suffix.


Thanks,

C.

 
      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;