Re: [PATCH 3/6] hw/acpi/viot: build array of PCI host bridges before generating VIOT ACPI table

[Ani Sinha]

On Wed, May 18, 2022 at 4:39 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Perform the generation of the VIOT ACPI table in 2 separate passes: the first 
> pass
> enumerates all of the PCI host bridges and adds the min_bus and max_bus 
> information
> to an array.
>
> Once this is done the VIOT table header is generated using the size of the 
> array
> to calculate the node count, which means it is no longer necessary to use a
> sub-array to hold the PCI host bridge range information along with viommu_off.
>
> Finally the PCI host bridge array is iterated again to add the required 
> entries
> to the final VIOT ACPI table.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Ani Sinha <ani@anisinha.ca>

> ---
>  hw/acpi/viot.c | 42 ++++++++++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
> index 662124812f..ce3b7b8c75 100644
> --- a/hw/acpi/viot.c
> +++ b/hw/acpi/viot.c
> @@ -10,10 +10,9 @@
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_host.h"
>
> -struct viot_pci_ranges {
> -    GArray *blob;
> -    size_t count;
> -    uint16_t output_node;
> +struct viot_pci_host_range {
> +    int min_bus;
> +    int max_bus;
>  };
>
>  static void build_pci_host_range(GArray *table_data, int min_bus, int 
> max_bus,
> @@ -44,8 +43,7 @@ static void build_pci_host_range(GArray *table_data, int 
> min_bus, int max_bus,
>  /* Build PCI range for a given PCI host bridge */
>  static int pci_host_bridges(Object *obj, void *opaque)
>  {
> -    struct viot_pci_ranges *pci_ranges = opaque;
> -    GArray *blob = pci_ranges->blob;
> +    GArray *pci_host_ranges = opaque;
>
>      if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
>          PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
> @@ -55,9 +53,11 @@ static int pci_host_bridges(Object *obj, void *opaque)
>
>              pci_bus_range(bus, &min_bus, &max_bus);
>
> -            build_pci_host_range(blob, min_bus, max_bus,
> -                                 pci_ranges->output_node);
> -            pci_ranges->count++;
> +            struct viot_pci_host_range pci_host_range = {
> +                .min_bus = min_bus,
> +                .max_bus = max_bus,
> +            };
> +            g_array_append_val(pci_host_ranges, pci_host_range);
>          }
>      }
>
> @@ -78,19 +78,19 @@ void build_viot(MachineState *ms, GArray *table_data, 
> BIOSLinker *linker,
>      int viommu_off = 48;
>      AcpiTable table = { .sig = "VIOT", .rev = 0,
>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
> -    struct viot_pci_ranges pci_ranges = {
> -        .output_node = viommu_off,
> -        .blob = g_array_new(false, true /* clear */, 1),
> -    };
> +    GArray *pci_host_ranges =  g_array_new(false, true,
> +                                           sizeof(struct 
> viot_pci_host_range));
> +    struct viot_pci_host_range *pci_host_range;
> +    int i;
>
>      /* Build the list of PCI ranges that this viommu manages */
>      object_child_foreach_recursive(OBJECT(ms), pci_host_bridges,
> -                                   &pci_ranges);
> +                                   pci_host_ranges);
>
>      /* ACPI table header */
>      acpi_table_begin(&table, table_data);
>      /* Node count */
> -    build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
> +    build_append_int_noprefix(table_data, pci_host_ranges->len + 1, 2);
>      /* Node offset */
>      build_append_int_noprefix(table_data, viommu_off, 2);
>      /* Reserved */
> @@ -111,9 +111,15 @@ void build_viot(MachineState *ms, GArray *table_data, 
> BIOSLinker *linker,
>      build_append_int_noprefix(table_data, 0, 8);
>
>      /* PCI ranges found above */
> -    g_array_append_vals(table_data, pci_ranges.blob->data,
> -                        pci_ranges.blob->len);
> -    g_array_free(pci_ranges.blob, true);
> +    for (i = 0; i < pci_host_ranges->len; i++) {
> +        pci_host_range = &g_array_index(pci_host_ranges,
> +                                        struct viot_pci_host_range, i);
> +
> +        build_pci_host_range(table_data, pci_host_range->min_bus,
> +                             pci_host_range->max_bus, viommu_off);
> +    }
> +
> +    g_array_free(pci_host_ranges, true);
>
>      acpi_table_end(linker, &table);
>  }
> --
> 2.20.1
>