[v2] hw/display: load the correct ROM file for isa-vga device

[PATCH v2] hw/display: load the correct ROM file for isa-vga device

Posted by Liav Albani 3 years, 4 months ago

Apparently QEMU didn't load the correct ROM file when using the isa-vga
device on my development machine, which resulted in a display waiting to
be initialized by a guest OS kernel. With this fix, SeaBIOS is able to
print vital data to a text mode console during boot, which is useful in
case of failing to continue booting.

The build name of the vgabios.bin is changed too, to vgabios-isavga.bin
to ensure we always have that file when QEMU is installed as a package
or compiled from source.

Signed-off-by: Liav Albani <liavalb@gmail.com>
---
 hw/display/vga-isa.c | 2 +-
 hw/display/vga_int.h | 2 +-
 pc-bios/meson.build  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 46abbc5653..bcf646d012 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -84,7 +84,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
                                 VBE_DISPI_LFB_PHYSICAL_ADDRESS,
                                 &s->vram);
     /* ROM BIOS */
-    rom_add_vga(VGABIOS_FILENAME);
+    rom_add_vga(VGABIOS_ISAVGA_FILENAME);
 }
 
 static Property vga_isa_properties[] = {
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 305e700014..b63788e809 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -183,7 +183,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val);
 extern const uint8_t sr_mask[8];
 extern const uint8_t gr_mask[16];
 
-#define VGABIOS_FILENAME "vgabios.bin"
+#define VGABIOS_ISAVGA_FILENAME "vgabios-isavga.bin"
 #define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin"
 
 extern const MemoryRegionOps vga_mem_ops;
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 388e0db6e4..6af94a4a0a 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -29,7 +29,7 @@ blobs = [
   'bios-microvm.bin',
   'qboot.rom',
   'sgabios.bin',
-  'vgabios.bin',
+  'vgabios-isavga.bin',
   'vgabios-cirrus.bin',
   'vgabios-stdvga.bin',
   'vgabios-vmware.bin',
-- 
2.37.3

Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device

Posted by Liav Albani 3 years, 4 months ago

On 9/17/22 17:32, Liav Albani wrote:

> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index 388e0db6e4..6af94a4a0a 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -29,7 +29,7 @@ blobs = [
>     'bios-microvm.bin',
>     'qboot.rom',
>     'sgabios.bin',
> -  'vgabios.bin',
> +  'vgabios-isavga.bin',
>     'vgabios-cirrus.bin',
Well, it seems like this one doesn't want to be compiled now, so I'll 
need to dive deeper to figure out how to ensure it always produces the 
requested file.
>     'vgabios-stdvga.bin',
>     'vgabios-vmware.bin',

Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device

Posted by Liav Albani 3 years, 4 months ago

On 9/17/22 17:40, Liav Albani wrote:
> On 9/17/22 17:32, Liav Albani wrote:
>
>> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
>> index 388e0db6e4..6af94a4a0a 100644
>> --- a/pc-bios/meson.build
>> +++ b/pc-bios/meson.build
>> @@ -29,7 +29,7 @@ blobs = [
>>     'bios-microvm.bin',
>>     'qboot.rom',
>>     'sgabios.bin',
>> -  'vgabios.bin',
>> +  'vgabios-isavga.bin',
>>     'vgabios-cirrus.bin',
> Well, it seems like this one doesn't want to be compiled now, so I'll 
> need to dive deeper to figure out how to ensure it always produces the 
> requested file.

So apparently the problem on my development machine is that for some odd 
reason I don't have the "vgabios.bin" file, but I do have
the "vgabios-isavga.bin" file. According to my package manager (pacman), 
it is owned by the SeaBIOS package:

     usr/share/qemu/vgabios-isavga.bin is owned by extra/seabios 1.15.0-1

Other files in that directory are owned by the same package as well, for 
example:

     usr/share/qemu/vgabios-stdvga.bin is owned by extra/seabios 1.15.0-1
     usr/share/qemu/vgabios-virtio.bin is owned by extra/seabios 1.15.0-1
     usr/share/qemu/vgabios-vmware.bin is owned by extra/seabios 1.15.0-1

So I'm not sure what is the best approach here to fix this. It is 
definitely not a problem in QEMU because when I compile it from source I 
do get a "vgabios.bin" file in the build directory, but I do think that 
QEMU should try to use the "vgabios-isavga.bin" file when using the 
isa-vga device, so in some way we could fix it in QEMU, which also makes 
sense to ensure the filename is "vgabios-isavga.bin" for the isa-vga 
device and not plain "vgabios.bin", which really doesn't say much about 
the type of the graphics device.

>>     'vgabios-stdvga.bin',
>>     'vgabios-vmware.bin',

Re: [PATCH v2] hw/display: load the correct ROM file for isa-vga device

Posted by Philippe Mathieu-Daudé via 3 years, 4 months ago

+David

On 17/9/22 17:06, Liav Albani wrote:
> On 9/17/22 17:40, Liav Albani wrote:
>> On 9/17/22 17:32, Liav Albani wrote:
>>
>>> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
>>> index 388e0db6e4..6af94a4a0a 100644
>>> --- a/pc-bios/meson.build
>>> +++ b/pc-bios/meson.build
>>> @@ -29,7 +29,7 @@ blobs = [
>>>     'bios-microvm.bin',
>>>     'qboot.rom',
>>>     'sgabios.bin',
>>> -  'vgabios.bin',
>>> +  'vgabios-isavga.bin',
>>>     'vgabios-cirrus.bin',
>> Well, it seems like this one doesn't want to be compiled now, so I'll 
>> need to dive deeper to figure out how to ensure it always produces the 
>> requested file.
> 
> So apparently the problem on my development machine is that for some odd 
> reason I don't have the "vgabios.bin" file, but I do have
> the "vgabios-isavga.bin" file. According to my package manager (pacman), 
> it is owned by the SeaBIOS package:
> 
>      usr/share/qemu/vgabios-isavga.bin is owned by extra/seabios 1.15.0-1
> 
> Other files in that directory are owned by the same package as well, for 
> example:
> 
>      usr/share/qemu/vgabios-stdvga.bin is owned by extra/seabios 1.15.0-1
>      usr/share/qemu/vgabios-virtio.bin is owned by extra/seabios 1.15.0-1
>      usr/share/qemu/vgabios-vmware.bin is owned by extra/seabios 1.15.0-1
> 
> So I'm not sure what is the best approach here to fix this. It is 
> definitely not a problem in QEMU because when I compile it from source I 
> do get a "vgabios.bin" file in the build directory, but I do think that 
> QEMU should try to use the "vgabios-isavga.bin" file when using the 
> isa-vga device, so in some way we could fix it in QEMU, which also makes 
> sense to ensure the filename is "vgabios-isavga.bin" for the isa-vga 
> device and not plain "vgabios.bin", which really doesn't say much about 
> the type of the graphics device.

This seems due to a recent Arch Linux change in SeaBIOS package:
https://github.com/archlinux/svntogit-packages/commit/ef2a5da4f1aa3fa45eea88c83eee01e89974145f
(so not related to mainstream QEMU project).

Regards,

Phil.