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

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

On Fri, Jun 06, 2025 at 11:04:34AM +0200, 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 <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:

Don't we need to use the property in order to tie this to the
machine type ? Even if existing non-x86 machines are not using
this ROM, the fact that QEMU loaded it will impact the guest
memory layout which needs to be preserved across migration,
and thus machine type versions.


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