Am 25. Februar 2024 13:03:46 UTC schrieb "Volker Rümelin" <vr_q...@t-online.de>:
>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,
Hi Volker,
>
>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.
In my tests I've followed
https://wiki.archlinux.org/title/QEMU#Booting_in_UEFI_mode for both pc and q35
machine, and therefore missed the machine properties. If there is no way to fix
it with compat properties or the like, I propose to revert the two patches
until a different solution is found.
Best regards,
Bernhard
>
>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],
>
