[PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[Bernhard Beschow]

As reported by Volker [1], commit 6f6ad2b24582 "hw/i386/pc: Confine system
flash handling to pc_sysfw" causes a regression when specifying the property
`-M pflash0` in the PCI PC machines:
  qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
Revert the commit for now until a solution is found.

Best regards,
Bernhard

[1] https://lore.kernel.org/qemu-devel/9e82a04b-f2c1-4e34-b4b6-46a0581b572f@t-online.de/

Bernhard Beschow (2):
  Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove
    it"
  Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

 include/hw/i386/pc.h |  2 ++
 hw/i386/pc.c         |  1 +
 hw/i386/pc_piix.c    |  1 +
 hw/i386/pc_sysfw.c   | 17 +++++++++++++----
 4 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.44.0

[PATCH 1/2] Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove it"

[Bernhard Beschow]

Commit 6f6ad2b24582 "hw/i386/pc: Confine system flash handling to pc_sysfw"
causes a regression when specifying the property `-M pflash0` in the PCI PC
machines:
  qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
In order to revert the commit, the commit below must be reverted first.

This reverts commit cb05cc16029bb0a61ac5279ab7b3b90dcf2aa69f.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_sysfw.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index b02e285579..b9c1eb352d 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -91,6 +91,18 @@ static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
     return PFLASH_CFI01(dev);
 }
 
+static void pc_system_flash_create(PCMachineState *pcms)
+{
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+
+    if (pcmc->pci_enabled) {
+        pcms->flash[0] = pc_pflash_create(pcms, "system.flash0",
+                                          "pflash0");
+        pcms->flash[1] = pc_pflash_create(pcms, "system.flash1",
+                                          "pflash1");
+    }
+}
+
 static void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 {
     char *prop_name;
@@ -198,8 +210,7 @@ void pc_system_firmware_init(PCMachineState *pcms,
         return;
     }
 
-    pcms->flash[0] = pc_pflash_create(pcms, "system.flash0", "pflash0");
-    pcms->flash[1] = pc_pflash_create(pcms, "system.flash1", "pflash1");
+    pc_system_flash_create(pcms);
 
     /* Map legacy -drive if=pflash to machine properties */
     for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
-- 
2.44.0

[PATCH 2/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[Bernhard Beschow]

Specifying the property `-M pflash0` results in a regression:
  qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
Revert the change for now until a solution is found.

This reverts commit 6f6ad2b24582593d8feb00434ce2396840666227.

Reported-by: Volker Rümelin <vr_qemu@t-online.de>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 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, 6 insertions(+), 4 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index e88468131a..0f9c1a45fc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -191,6 +191,8 @@ 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 f8eb684a49..2ad8de5097 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1733,6 +1733,7 @@ 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 ec7c07b362..34203927e1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -231,6 +231,7 @@ 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 b9c1eb352d..3efabbbab2 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);
 }
 
-static void pc_system_flash_create(PCMachineState *pcms)
+void pc_system_flash_create(PCMachineState *pcms)
 {
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 
@@ -103,7 +103,7 @@ static void pc_system_flash_create(PCMachineState *pcms)
     }
 }
 
-static void pc_system_flash_cleanup_unused(PCMachineState *pcms)
+void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 {
     char *prop_name;
     int i;
@@ -210,8 +210,6 @@ 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],
-- 
2.44.0

Re: [PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[Bernhard Beschow]

Am 26. Februar 2024 21:59:07 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>As reported by Volker [1], commit 6f6ad2b24582 "hw/i386/pc: Confine system
>
>flash handling to pc_sysfw" causes a regression when specifying the property
>
>`-M pflash0` in the PCI PC machines:
>
>  qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
>
>Revert the commit for now until a solution is found.
>

Ping

>
>
>Best regards,
>
>Bernhard
>
>
>
>[1] https://lore.kernel.org/qemu-devel/9e82a04b-f2c1-4e34-b4b6-46a0581b572f@t-online.de/
>
>
>
>Bernhard Beschow (2):
>
>  Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove
>
>    it"
>
>  Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"
>
>
>
> include/hw/i386/pc.h |  2 ++
>
> hw/i386/pc.c         |  1 +
>
> hw/i386/pc_piix.c    |  1 +
>
> hw/i386/pc_sysfw.c   | 17 +++++++++++++----
>
> 4 files changed, 17 insertions(+), 4 deletions(-)
>
>
>
>-- >
>2.44.0
>
>
>

Re: [PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[Michael S. Tsirkin]

On Thu, Feb 29, 2024 at 07:38:06PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 26. Februar 2024 21:59:07 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
> >As reported by Volker [1], commit 6f6ad2b24582 "hw/i386/pc: Confine system
> >
> >flash handling to pc_sysfw" causes a regression when specifying the property
> >
> >`-M pflash0` in the PCI PC machines:
> >
> >  qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
> >
> >Revert the commit for now until a solution is found.
> >
> 
> Ping

tagged, thanks!

> >
> >
> >Best regards,
> >
> >Bernhard
> >
> >
> >
> >[1] https://lore.kernel.org/qemu-devel/9e82a04b-f2c1-4e34-b4b6-46a0581b572f@t-online.de/
> >
> >
> >
> >Bernhard Beschow (2):
> >
> >  Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove
> >
> >    it"
> >
> >  Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"
> >
> >
> >
> > include/hw/i386/pc.h |  2 ++
> >
> > hw/i386/pc.c         |  1 +
> >
> > hw/i386/pc_piix.c    |  1 +
> >
> > hw/i386/pc_sysfw.c   | 17 +++++++++++++----
> >
> > 4 files changed, 17 insertions(+), 4 deletions(-)
> >
> >
> >
> >-- >
> >2.44.0
> >
> >
> >

Re: [PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

[Alex Williamson]

On Mon, 26 Feb 2024 22:59:07 +0100
Bernhard Beschow <shentey@gmail.com> wrote:

> As reported by Volker [1], commit 6f6ad2b24582 "hw/i386/pc: Confine system
> flash handling to pc_sysfw" causes a regression when specifying the property
> `-M pflash0` in the PCI PC machines:
>   qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
> Revert the commit for now until a solution is found.
> 
> Best regards,
> Bernhard
> 
> [1] https://lore.kernel.org/qemu-devel/9e82a04b-f2c1-4e34-b4b6-46a0581b572f@t-online.de/
> 
> Bernhard Beschow (2):
>   Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove
>     it"
>   Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"
> 
>  include/hw/i386/pc.h |  2 ++
>  hw/i386/pc.c         |  1 +
>  hw/i386/pc_piix.c    |  1 +
>  hw/i386/pc_sysfw.c   | 17 +++++++++++++----
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 

Please apply this, the original commits break my existing VMs.

Tested-by: Alex Williamson <alex.williamson@redhat.com>