Am 28. März 2024 15:54:21 UTC schrieb "Philippe Mathieu-Daudé"
<phi...@linaro.org>:
>x86_bios_rom_init() is the single non-PCI-machine call
>from pc_system_firmware_init(). Extract it to the caller.
>
>Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>---
> hw/i386/pc.c | 6 +++++-
> hw/i386/pc_sysfw.c | 5 +----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index f184808e3e..5b96daa414 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -956,7 +956,11 @@ void pc_memory_init(PCMachineState *pcms,
> }
>
> /* Initialize PC system firmware */
>- pc_system_firmware_init(pcms, rom_memory);
>+ if (pci_enabled) {
>+ pc_system_firmware_init(pcms, rom_memory);
>+ } else {
>+ x86_bios_rom_init(machine, "bios.bin", rom_memory, true);
>+ }
>
> option_rom_mr = g_malloc(sizeof(*option_rom_mr));
> memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
>diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>index 862a082b0a..541dcaef71 100644
>--- a/hw/i386/pc_sysfw.c
>+++ b/hw/i386/pc_sysfw.c
>@@ -202,10 +202,7 @@ void pc_system_firmware_init(PCMachineState *pcms,
> int i;
> BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)];
>
>- if (!pc_machine_is_pci_enabled(pcms)) {
>- x86_bios_rom_init(MACHINE(pcms), "bios.bin", rom_memory, true);
>- return;
>- }
>+ assert(pc_machine_is_pci_enabled(pcms));
AFAICS nothing refers to pci in the whole file any longer. The only reason for
checking pci_enabled before seems for filtering out the x86_bios_rom_init()
case. This has been moved to the caller. Can we thus drop the assert? This
allows for further removal of code in this patch and avoids superficial
barriers for reusing this code. Or do I miss something?
Anyway, this patch looks like good material on its own and could be tagged
independently.
With dropping the assert considered:
Reviewed-by: Bernhard Beschow <shen...@gmail.com>
>
> /* Map legacy -drive if=pflash to machine properties */
> for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
