Series comparison

-[PATCH 0/6] hw/nvram/fw_cfg: Move PCI bus methods out
+[PATCH v2 0/7] hw/nvram/fw_cfg: Move PCI bus methods out
+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é (6):
+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 | 21 ++++++----------
+ 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         | 34 +++++--------------------
+ hw/nvram/fw_cfg.c         | 44 +++++++++-----------------------
  hw/pci/pci.c              | 53 +++++++++++++++++++++++++++++++++++++++
  system/vl.c               |  3 ++-
-files changed, 76 insertions(+), 46 deletions(-)
+files changed, 89 insertions(+), 54 deletions(-)
 --
 .45.2

-[PATCH 1/6] hw/nvram/fw_cfg: Rename fw_cfg_add_[file]_from_generator()
+[PATCH v2 1/7] hw/nvram/fw_cfg: Rename fw_cfg_add_[file]_from_generator()
 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 +-
 files changed, 6 insertions(+), 6 deletions(-)
 ...

-[PATCH 2/6] hw/nvram/fw_cfg: Pass QOM parent to fw_cfg_add_file_from_generator()
+[PATCH v2 2/7] hw/nvram/fw_cfg: Pass QOM parent to fw_cfg_add_file_from_generator()
 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 ++-
 files changed, 14 insertions(+), 10 deletions(-)
 ...

-New patch
+[PATCH v2 3/7] hw/nvram/fw_cfg: Skip FW_CFG_DATA_GENERATOR when no data to generate
+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 ++++++----
+files changed, 14 insertions(+), 9 deletions(-)
+diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/hw/nvram/fw_cfg.h
++++ b/include/hw/nvram/fw_cfg.h
+@@ -XXX,XX +XXX,XX @@ 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.
+@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
+--- a/hw/nvram/fw_cfg.c
++++ b/hw/nvram/fw_cfg.c
+@@ -XXX,XX +XXX,XX @@ 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) {
+@@ -XXX,XX +XXX,XX @@ 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;
+ }
+--
+.45.2

-[PATCH 3/6] hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE
+[PATCH v2 4/7] hw/pci: Have PCI_BUS implement TYPE_FW_CFG_DATA_GENERATOR_INTERFACE
-The FW_CFG_DATA_GENERATOR allows any object to produce
+The FW_CFG_DATA_GENERATOR interface allows any object to
-blob of data consumable by the fw_cfg device. Implement
+produce a blob of data consumable by the fw_cfg device.
-that for PCI bus objects.
+Implement that for PCI bus objects.
 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
 ---
  hw/pci/pci.c | 37 +++++++++++++++++++++++++++++++++++++
 file changed, 37 insertions(+)
 ...

-[PATCH 4/6] hw/pci: Add pci_bus_add_fw_cfg_extra_pci_roots() helper
+[PATCH v2 5/7] hw/pci: Add pci_bus_add_fw_cfg_extra_pci_roots() helper
 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 ++++++++++++++++
 files changed, 19 insertions(+)
 ...

-[PATCH 5/6] hw: Use pci_bus_add_fw_cfg_extra_pci_roots()
+[PATCH v2 6/7] hw: Use pci_bus_add_fw_cfg_extra_pci_roots()
 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 ++-
 files changed, 5 insertions(+), 3 deletions(-)
 ...

-[PATCH 6/6] hw/nvram/fw_cfg: Remove fw_cfg_add_extra_pci_roots()
+[PATCH v2 7/7] hw/nvram/fw_cfg: Remove fw_cfg_add_extra_pci_roots()
 ...
 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 -----------------------
 files changed, 32 deletions(-)
 ...

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é (6):
  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/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()

-- 
2.45.2

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>
---
 include/hw/nvram/fw_cfg.h | 6 +++---
 hw/nvram/fw_cfg.c         | 4 ++--
 system/vl.c               | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

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>
---
 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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -XXX,XX +XXX,XX @@ 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
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -XXX,XX +XXX,XX @@ 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

The FW_CFG_DATA_GENERATOR allows any object to produce
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 XXXXXXX..XXXXXXX 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -XXX,XX +XXX,XX @@
 #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"
@@ -XXX,XX +XXX,XX @@ 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;
@@ -XXX,XX +XXX,XX @@ 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 = {
@@ -XXX,XX +XXX,XX @@ 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

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>
---
 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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -XXX,XX +XXX,XX @@ 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

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>
---
 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 XXXXXXX..XXXXXXX 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -XXX,XX +XXX,XX @@ 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

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>
---
 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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -XXX,XX +XXX,XX @@
 #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
@@ -XXX,XX +XXX,XX @@ 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

Missing review: 3 & 4

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

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()

-- 
2.45.2

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(-)

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(-)

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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -XXX,XX +XXX,XX @@ 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.
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -XXX,XX +XXX,XX @@ 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) {
@@ -XXX,XX +XXX,XX @@ 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

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(+)

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(+)

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(-)

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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -XXX,XX +XXX,XX @@
 #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
@@ -XXX,XX +XXX,XX @@ 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