Am 21.02.24 um 22:16 schrieb Philippe Mathieu-Daudé:
> From: Bernhard Beschow <shen...@gmail.com>
>
> Rather than distributing PC system flash handling across three files, let's
> confine it to one. Now, pc_system_firmware_init() creates, configures and
> cleans
> up the system flash which makes the code easier to understand. It also avoids
> the extra call to pc_system_flash_cleanup_unused() in the Xen case.
>
> Signed-off-by: Bernhard Beschow <shen...@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Message-ID: <20240208220349.4948-7-shen...@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
> include/hw/i386/pc.h | 2 --
> hw/i386/pc.c | 1 -
> hw/i386/pc_piix.c | 1 -
> hw/i386/pc_sysfw.c | 6 ++++--
> 4 files changed, 4 insertions(+), 6 deletions(-)
Hi Bernhard,
this patch breaks QEMU on my system.
./qemu-system-x86_64 -machine q35,pflash0=pflash0-storage -blockdev
driver=file,node-name=pflash0-storage,filename=/usr/share/qemu/ovmf-x86_64.bin,read-only=true
qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
I had to revert cb05cc1602 ("hw/i386/pc_sysfw: Inline
pc_system_flash_create() and remove it") and 6f6ad2b245 ("hw/i386/pc:
Confine system flash handling to pc_sysfw") to make it work again.
With best regards,
Volker
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 0a8a96600c..e8f4af5d5c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -193,8 +193,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq
> *i8259_irqs);
> #define TYPE_PORT92 "port92"
>
> /* pc_sysfw.c */
> -void pc_system_flash_create(PCMachineState *pcms);
> -void pc_system_flash_cleanup_unused(PCMachineState *pcms);
> void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
> bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
> int *data_len);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e526498164..1ee41a5e56 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1733,7 +1733,6 @@ static void pc_machine_initfn(Object *obj)
> #endif
> pcms->default_bus_bypass_iommu = false;
>
> - pc_system_flash_create(pcms);
> pcms->pcspk = isa_new(TYPE_PC_SPEAKER);
> object_property_add_alias(OBJECT(pcms), "pcspk-audiodev",
> OBJECT(pcms->pcspk), "audiodev");
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 34203927e1..ec7c07b362 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -231,7 +231,6 @@ static void pc_init1(MachineState *machine,
> assert(machine->ram_size == x86ms->below_4g_mem_size +
> x86ms->above_4g_mem_size);
>
> - pc_system_flash_cleanup_unused(pcms);
> if (machine->kernel_filename != NULL) {
> /* For xen HVM direct kernel boot, load linux here */
> xen_load_linux(pcms);
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index c8d9e71b88..b4c3833352 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -91,7 +91,7 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
> return PFLASH_CFI01(dev);
> }
>
> -void pc_system_flash_create(PCMachineState *pcms)
> +static void pc_system_flash_create(PCMachineState *pcms)
> {
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>
> @@ -103,7 +103,7 @@ void pc_system_flash_create(PCMachineState *pcms)
> }
> }
>
> -void pc_system_flash_cleanup_unused(PCMachineState *pcms)
> +static void pc_system_flash_cleanup_unused(PCMachineState *pcms)
> {
> char *prop_name;
> int i;
> @@ -212,6 +212,8 @@ void pc_system_firmware_init(PCMachineState *pcms,
> return;
> }
>
> + pc_system_flash_create(pcms);
> +
> /* Map legacy -drive if=pflash to machine properties */
> for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
> pflash_cfi01_legacy_drive(pcms->flash[i],
