[PATCH v2 0/7] hw/nvram/fw_cfg: Move PCI bus methods out

[PATCH v2 0/7] hw/nvram/fw_cfg: Move PCI bus methods out

[Philippe Mathieu-Daudé]

Missing review: 3 & 4

Since v1:
- Patch 3 is new
- Added danpb R-b tags

In order to keep fw_cfg device model clean, remove the PCI
bus specific code. Instead, the equivalent functionality is
implemented within the PCI_BUS object in hw/pci/,
implementing TYPE_FW_CFG_DATA_GENERATOR_INTERFACE.

Philippe Mathieu-Daudé (7):
  hw/nvram/fw_cfg: Rename fw_cfg_add_[file]_from_generator()
  hw/nvram/fw_cfg: Pass QOM parent to fw_cfg_add_file_from_generator()
  hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate
  hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE
  hw/pci: Add pci_bus_add_fw_cfg_extra_pci_roots() helper
  hw: Use pci_bus_add_fw_cfg_extra_pci_roots()
  hw/nvram/fw_cfg: Remove fw_cfg_add_extra_pci_roots()

 include/hw/nvram/fw_cfg.h | 32 +++++++++++------------
 include/hw/pci/pci.h      |  3 +++
 hw/arm/virt.c             |  3 ++-
 hw/hppa/machine.c         |  2 +-
 hw/i386/pc.c              |  3 ++-
 hw/nvram/fw_cfg.c         | 44 +++++++++-----------------------
 hw/pci/pci.c              | 53 +++++++++++++++++++++++++++++++++++++++
 system/vl.c               |  3 ++-
 8 files changed, 89 insertions(+), 54 deletions(-)

-- 
2.45.2

[PATCH v2 1/7] hw/nvram/fw_cfg: Rename fw_cfg_add_[file]_from_generator()

[Philippe Mathieu-Daudé]

fw_cfg_add_from_generator() is adding a 'file' entry,
so rename as fw_cfg_add_file_from_generator() for
clarity. Besides, we might introduce generators for
other entry types.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/hw/nvram/fw_cfg.h | 6 +++---
 hw/nvram/fw_cfg.c         | 4 ++--
 system/vl.c               | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index fa426776192..14e68966c59 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -291,7 +291,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
                          size_t len);
 
 /**
- * fw_cfg_add_from_generator:
+ * fw_cfg_add_file_from_generator:
  * @s: fw_cfg device being modified
  * @filename: name of new fw_cfg file item
  * @gen_id: name of object implementing FW_CFG_DATA_GENERATOR interface
@@ -307,8 +307,8 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
  *
  * Returns: %true on success, %false on error.
  */
-bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename,
-                               const char *gen_id, Error **errp);
+bool fw_cfg_add_file_from_generator(FWCfgState *s, const char *filename,
+                                    const char *gen_id, Error **errp);
 
 /**
  * fw_cfg_add_extra_pci_roots:
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b644577734c..fe3b86135a7 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1027,8 +1027,8 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
     return NULL;
 }
 
-bool fw_cfg_add_from_generator(FWCfgState *s, const char *filename,
-                               const char *gen_id, Error **errp)
+bool fw_cfg_add_file_from_generator(FWCfgState *s, const char *filename,
+                                    const char *gen_id, Error **errp)
 {
     FWCfgDataGeneratorClass *klass;
     GByteArray *array;
diff --git a/system/vl.c b/system/vl.c
index 2f855d83fbb..f103532a9a1 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1184,7 +1184,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
         buf = g_memdup(str, size);
     } else if (nonempty_str(gen_id)) {
-        if (!fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp)) {
+        if (!fw_cfg_add_file_from_generator(fw_cfg, name, gen_id, errp)) {
             return -1;
         }
         return 0;
-- 
2.45.2

[PATCH v2 2/7] hw/nvram/fw_cfg: Pass QOM parent to fw_cfg_add_file_from_generator()

[Philippe Mathieu-Daudé]

Currently fw_cfg_add_file_from_generator() is restricted
to command line created objects which reside in the
'/objects' QOM container. In order to extend to other
types of containers, pass the QOM parent by argument.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/hw/nvram/fw_cfg.h | 10 ++++++----
 hw/nvram/fw_cfg.c         | 11 ++++++-----
 system/vl.c               |  3 ++-
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 14e68966c59..fcb06f18cc3 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -294,11 +294,12 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
  * fw_cfg_add_file_from_generator:
  * @s: fw_cfg device being modified
  * @filename: name of new fw_cfg file item
- * @gen_id: name of object implementing FW_CFG_DATA_GENERATOR interface
+ * @part: name of object implementing FW_CFG_DATA_GENERATOR interface
+ * @parent: the object in which to resolve the @part
  * @errp: pointer to a NULL initialized error object
  *
  * Add a new NAMED fw_cfg item with the content generated from the
- * @gen_id object. The data generated by the @gen_id object is copied
+ * @part object. The data generated by the @part object is copied
  * into the data structure of the fw_cfg device.
  * The next available (unused) selector key starting at FW_CFG_FILE_FIRST
  * will be used; also, a new entry will be added to the file directory
@@ -307,8 +308,9 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
  *
  * Returns: %true on success, %false on error.
  */
-bool fw_cfg_add_file_from_generator(FWCfgState *s, const char *filename,
-                                    const char *gen_id, Error **errp);
+bool fw_cfg_add_file_from_generator(FWCfgState *s,
+                                    Object *parent, const char *part,
+                                    const char *filename, Error **errp);
 
 /**
  * fw_cfg_add_extra_pci_roots:
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index fe3b86135a7..b94cd27bd85 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1027,22 +1027,23 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
     return NULL;
 }
 
-bool fw_cfg_add_file_from_generator(FWCfgState *s, const char *filename,
-                                    const char *gen_id, Error **errp)
+bool fw_cfg_add_file_from_generator(FWCfgState *s,
+                                    Object *parent, const char *part,
+                                    const char *filename, Error **errp)
 {
     FWCfgDataGeneratorClass *klass;
     GByteArray *array;
     Object *obj;
     gsize size;
 
-    obj = object_resolve_path_component(object_get_objects_root(), gen_id);
+    obj = object_resolve_path_component(parent, part);
     if (!obj) {
-        error_setg(errp, "Cannot find object ID '%s'", gen_id);
+        error_setg(errp, "Cannot find object ID '%s'", part);
         return false;
     }
     if (!object_dynamic_cast(obj, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE)) {
         error_setg(errp, "Object ID '%s' is not a '%s' subclass",
-                   gen_id, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE);
+                   part, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE);
         return false;
     }
     klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj);
diff --git a/system/vl.c b/system/vl.c
index f103532a9a1..4a370da624a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1184,7 +1184,8 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
         buf = g_memdup(str, size);
     } else if (nonempty_str(gen_id)) {
-        if (!fw_cfg_add_file_from_generator(fw_cfg, name, gen_id, errp)) {
+        if (!fw_cfg_add_file_from_generator(fw_cfg, object_get_objects_root(),
+                                            gen_id, name, errp)) {
             return -1;
         }
         return 0;
-- 
2.45.2

[PATCH v2 3/7] hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate

[Philippe Mathieu-Daudé]

Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to
return NULL when there is nothing to generate. In that case
fw_cfg_add_file_from_generator() will not add any item and
return %true.

Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/nvram/fw_cfg.h | 13 ++++++++-----
 hw/nvram/fw_cfg.c         | 10 ++++++----
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index fcb06f18cc3..5211018fd8f 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass {
      * @obj: the object implementing this interface
      * @errp: pointer to a NULL-initialized error object
      *
-     * Returns: reference to a byte array containing the data on success,
-     *          or NULL on error.
+     * Returns: NULL on failure (errp set if not NULL).
+     *          A byte array containing the data (if any,
+     *          otherwise NULL) on success.
      *
      * The caller should release the reference when no longer
      * required.
@@ -298,14 +299,16 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
  * @parent: the object in which to resolve the @part
  * @errp: pointer to a NULL initialized error object
  *
- * Add a new NAMED fw_cfg item with the content generated from the
- * @part object. The data generated by the @part object is copied
- * into the data structure of the fw_cfg device.
+ * If the @part object generates content, add a new NAMED fw_cfg item with it.
+ * The data generated by the @part object is copied into the data structure of
+ * the fw_cfg device.
  * The next available (unused) selector key starting at FW_CFG_FILE_FIRST
  * will be used; also, a new entry will be added to the file directory
  * structure residing at key value FW_CFG_FILE_DIR, containing the item name,
  * data size, and assigned selector key value.
  *
+ * If the @part object does not generate content, no fw_cfg item is added.
+ *
  * Returns: %true on success, %false on error.
  */
 bool fw_cfg_add_file_from_generator(FWCfgState *s,
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b94cd27bd85..7e1065e5f50 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1031,10 +1031,10 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
                                     Object *parent, const char *part,
                                     const char *filename, Error **errp)
 {
+    ERRP_GUARD();
     FWCfgDataGeneratorClass *klass;
     GByteArray *array;
     Object *obj;
-    gsize size;
 
     obj = object_resolve_path_component(parent, part);
     if (!obj) {
@@ -1048,11 +1048,13 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
     }
     klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj);
     array = klass->get_data(obj, errp);
-    if (!array) {
+    if (*errp) {
         return false;
     }
-    size = array->len;
-    fw_cfg_add_file(s, filename, g_byte_array_free(array, FALSE), size);
+    if (array) {
+        fw_cfg_add_file(s, filename, g_byte_array_free(array, FALSE),
+                        array->len);
+    }
 
     return true;
 }
-- 
2.45.2

Re: [PATCH v2 3/7] hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate

[Daniel P. Berrangé]

On Fri, Dec 13, 2024 at 02:33:48PM +0100, Philippe Mathieu-Daudé wrote:
> Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to
> return NULL when there is nothing to generate. In that case
> fw_cfg_add_file_from_generator() will not add any item and
> return %true.
> 
> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/nvram/fw_cfg.h | 13 ++++++++-----
>  hw/nvram/fw_cfg.c         | 10 ++++++----
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index fcb06f18cc3..5211018fd8f 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass {
>       * @obj: the object implementing this interface
>       * @errp: pointer to a NULL-initialized error object
>       *
> -     * Returns: reference to a byte array containing the data on success,
> -     *          or NULL on error.
> +     * Returns: NULL on failure (errp set if not NULL).
> +     *          A byte array containing the data (if any,
> +     *          otherwise NULL) on success.

Bit confusing wording, lets say

  Returns: A byte array containing data to add, or NULL without
           @errp set if no data is required, or NULL with @errp
	   set on failure.

With that change:

  Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/7] hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate

[Philippe Mathieu-Daudé]

On 13/12/24 14:48, Daniel P. Berrangé wrote:
> On Fri, Dec 13, 2024 at 02:33:48PM +0100, Philippe Mathieu-Daudé wrote:
>> Allow the FW_CFG_DATA_GENERATOR interface get_data() handler to
>> return NULL when there is nothing to generate. In that case
>> fw_cfg_add_file_from_generator() will not add any item and
>> return %true.
>>
>> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/nvram/fw_cfg.h | 13 ++++++++-----
>>   hw/nvram/fw_cfg.c         | 10 ++++++----
>>   2 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
>> index fcb06f18cc3..5211018fd8f 100644
>> --- a/include/hw/nvram/fw_cfg.h
>> +++ b/include/hw/nvram/fw_cfg.h
>> @@ -30,8 +30,9 @@ struct FWCfgDataGeneratorClass {
>>        * @obj: the object implementing this interface
>>        * @errp: pointer to a NULL-initialized error object
>>        *
>> -     * Returns: reference to a byte array containing the data on success,
>> -     *          or NULL on error.
>> +     * Returns: NULL on failure (errp set if not NULL).
>> +     *          A byte array containing the data (if any,
>> +     *          otherwise NULL) on success.
> 
> Bit confusing wording, lets say
> 
>    Returns: A byte array containing data to add, or NULL without
>             @errp set if no data is required, or NULL with @errp
> 	   set on failure.

Thank you!

> 
> With that change:
> 
>    Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> With regards,
> Daniel

[PATCH v2 4/7] hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE

[Philippe Mathieu-Daudé]

The FW_CFG_DATA_GENERATOR interface allows any object to
produce a blob of data consumable by the fw_cfg device.
Implement that for PCI bus objects.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci/pci.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 1416ae202c3..8844251eceb 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -46,6 +46,7 @@
 #include "hw/pci/msix.h"
 #include "hw/hotplug.h"
 #include "hw/boards.h"
+#include "hw/nvram/fw_cfg.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
 #include "pci-internal.h"
@@ -216,11 +217,41 @@ static uint16_t pcibus_numa_node(PCIBus *bus)
     return NUMA_NODE_UNASSIGNED;
 }
 
+static GByteArray *pci_bus_fw_cfg_gen_data(Object *obj, Error **errp)
+{
+    PCIBus *bus = PCI_BUS(obj);
+    GByteArray *byte_array;
+    uint64_t extra_hosts = 0;
+
+    if (!bus) {
+        return NULL;
+    }
+
+    QLIST_FOREACH(bus, &bus->child, sibling) {
+        /* look for expander root buses */
+        if (pci_bus_is_root(bus)) {
+            extra_hosts++;
+        }
+    }
+
+    if (!extra_hosts) {
+        return NULL;
+    }
+    extra_hosts = cpu_to_le64(extra_hosts);
+
+    byte_array = g_byte_array_new();
+    g_byte_array_append(byte_array,
+                        (const void *)&extra_hosts, sizeof(extra_hosts));
+
+    return byte_array;
+}
+
 static void pci_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
     PCIBusClass *pbc = PCI_BUS_CLASS(klass);
     ResettableClass *rc = RESETTABLE_CLASS(klass);
+    FWCfgDataGeneratorClass *fwgc = FW_CFG_DATA_GENERATOR_CLASS(klass);
 
     k->print_dev = pcibus_dev_print;
     k->get_dev_path = pcibus_get_dev_path;
@@ -232,6 +263,8 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
 
     pbc->bus_num = pcibus_num;
     pbc->numa_node = pcibus_numa_node;
+
+    fwgc->get_data = pci_bus_fw_cfg_gen_data;
 }
 
 static const TypeInfo pci_bus_info = {
@@ -240,6 +273,10 @@ static const TypeInfo pci_bus_info = {
     .instance_size = sizeof(PCIBus),
     .class_size = sizeof(PCIBusClass),
     .class_init = pci_bus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_FW_CFG_DATA_GENERATOR_INTERFACE },
+        { }
+    }
 };
 
 static const TypeInfo cxl_interface_info = {
-- 
2.45.2

Re: [PATCH v2 4/7] hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE

[Daniel P. Berrangé]

On Fri, Dec 13, 2024 at 02:33:49PM +0100, Philippe Mathieu-Daudé wrote:
> The FW_CFG_DATA_GENERATOR interface allows any object to
> produce a blob of data consumable by the fw_cfg device.
> Implement that for PCI bus objects.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/pci/pci.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 5/7] hw/pci: Add pci_bus_add_fw_cfg_extra_pci_roots() helper

[Philippe Mathieu-Daudé]

pci_bus_add_fw_cfg_extra_pci_roots() calls the fw_cfg
API with PCI bus specific arguments.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/hw/pci/pci.h |  3 +++
 hw/pci/pci.c         | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c0717e31219..603c456c3a8 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -297,6 +297,9 @@ int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 uint32_t pci_bus_get_slot_reserved_mask(PCIBus *bus);
 void pci_bus_set_slot_reserved_mask(PCIBus *bus, uint32_t mask);
 void pci_bus_clear_slot_reserved_mask(PCIBus *bus, uint32_t mask);
+bool pci_bus_add_fw_cfg_extra_pci_roots(FWCfgState *fw_cfg,
+                                        PCIBus *bus,
+                                        Error **errp);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
 static inline int pci_swizzle(int slot, int pin)
 {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8844251eceb..bf0a1840dbe 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -217,6 +217,22 @@ static uint16_t pcibus_numa_node(PCIBus *bus)
     return NUMA_NODE_UNASSIGNED;
 }
 
+bool pci_bus_add_fw_cfg_extra_pci_roots(FWCfgState *fw_cfg,
+                                        PCIBus *bus,
+                                        Error **errp)
+{
+    Object *obj;
+
+    if (!bus) {
+        return true;
+    }
+    obj = OBJECT(bus);
+
+    return fw_cfg_add_file_from_generator(fw_cfg, obj->parent,
+                                          object_get_canonical_path_component(obj),
+                                          "etc/extra-pci-roots", errp);
+}
+
 static GByteArray *pci_bus_fw_cfg_gen_data(Object *obj, Error **errp)
 {
     PCIBus *bus = PCI_BUS(obj);
-- 
2.45.2

[PATCH v2 6/7] hw: Use pci_bus_add_fw_cfg_extra_pci_roots()

[Philippe Mathieu-Daudé]

We want to remove fw_cfg_add_extra_pci_roots() which introduced
PCI bus knowledge within the generic hw/nvram/fw_cfg.c file.
Replace the calls by the pci_bus_add_fw_cfg_extra_pci_roots()
which is a 1:1 equivalent, but using correct API.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/arm/virt.c     | 3 ++-
 hw/hppa/machine.c | 2 +-
 hw/i386/pc.c      | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3bd9dd0f863..333eaf67ea3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1750,7 +1750,8 @@ void virt_machine_done(Notifier *notifier, void *data)
         exit(1);
     }
 
-    fw_cfg_add_extra_pci_roots(vms->bus, vms->fw_cfg);
+    pci_bus_add_fw_cfg_extra_pci_roots(vms->fw_cfg, vms->bus,
+                                       &error_abort);
 
     virt_acpi_setup(vms);
     virt_build_smbios(vms);
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index a31dc32a9f7..4e673353225 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -240,7 +240,7 @@ static FWCfgState *create_fw_cfg(MachineState *ms, PCIBus *pci_bus,
                     g_memdup2(qemu_version, sizeof(qemu_version)),
                     sizeof(qemu_version));
 
-    fw_cfg_add_extra_pci_roots(pci_bus, fw_cfg);
+    pci_bus_add_fw_cfg_extra_pci_roots(fw_cfg, pci_bus, &error_abort);
 
     return fw_cfg;
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99b9b105e26..92047ce8c9d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -631,7 +631,8 @@ void pc_machine_done(Notifier *notifier, void *data)
     /* set the number of CPUs */
     x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
-    fw_cfg_add_extra_pci_roots(pcms->pcibus, x86ms->fw_cfg);
+    pci_bus_add_fw_cfg_extra_pci_roots(x86ms->fw_cfg, pcms->pcibus,
+                                       &error_abort);
 
     acpi_setup();
     if (x86ms->fw_cfg) {
-- 
2.45.2

[PATCH v2 7/7] hw/nvram/fw_cfg: Remove fw_cfg_add_extra_pci_roots()

[Philippe Mathieu-Daudé]

Now that all uses of fw_cfg_add_extra_pci_roots() have been
converted to the newer pci_bus_add_fw_cfg_extra_pci_roots(),
we can remove that bogus method. hw/nvram/fw_cfg must
stay generic. Device specific entries have to be implemented
using TYPE_FW_CFG_DATA_GENERATOR_INTERFACE.

This mostly reverts commit 0abd38885ac0fcdb08653922f339849cad387961
("fw_cfg: Refactor extra pci roots addition").

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/hw/nvram/fw_cfg.h |  9 ---------
 hw/nvram/fw_cfg.c         | 23 -----------------------
 2 files changed, 32 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 5211018fd8f..152e049819b 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -315,15 +315,6 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
                                     Object *parent, const char *part,
                                     const char *filename, Error **errp);
 
-/**
- * fw_cfg_add_extra_pci_roots:
- * @bus: main pci root bus to be scanned from
- * @s: fw_cfg device being modified
- *
- * Add a new fw_cfg item...
- */
-void fw_cfg_add_extra_pci_roots(PCIBus *bus, FWCfgState *s);
-
 FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
                                 AddressSpace *dma_as);
 FWCfgState *fw_cfg_init_mem(hwaddr ctl_addr, hwaddr data_addr);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7e1065e5f50..d56feca6bfe 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -41,7 +41,6 @@
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "hw/acpi/aml-build.h"
-#include "hw/pci/pci_bus.h"
 #include "hw/loader.h"
 
 #define FW_CFG_FILE_SLOTS_DFLT 0x20
@@ -1059,28 +1058,6 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
     return true;
 }
 
-void fw_cfg_add_extra_pci_roots(PCIBus *bus, FWCfgState *s)
-{
-    int extra_hosts = 0;
-
-    if (!bus) {
-        return;
-    }
-
-    QLIST_FOREACH(bus, &bus->child, sibling) {
-        /* look for expander root buses */
-        if (pci_bus_is_root(bus)) {
-            extra_hosts++;
-        }
-    }
-
-    if (extra_hosts && s) {
-        uint64_t *val = g_malloc(sizeof(*val));
-        *val = cpu_to_le64(extra_hosts);
-        fw_cfg_add_file(s, "etc/extra-pci-roots", val, sizeof(*val));
-    }
-}
-
 static void fw_cfg_machine_reset(void *opaque)
 {
     MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-- 
2.45.2

Re: [PATCH v2 0/7] hw/nvram/fw_cfg: Move PCI bus methods out

[Philippe Mathieu-Daudé]

On 13/12/24 14:33, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (7):
>    hw/nvram/fw_cfg: Rename fw_cfg_add_[file]_from_generator()
>    hw/nvram/fw_cfg: Pass QOM parent to fw_cfg_add_file_from_generator()
>    hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate
>    hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE
>    hw/pci: Add pci_bus_add_fw_cfg_extra_pci_roots() helper
>    hw: Use pci_bus_add_fw_cfg_extra_pci_roots()
>    hw/nvram/fw_cfg: Remove fw_cfg_add_extra_pci_roots()

Series queued.