>> +static void build_srat_generic_initiator_affinity(GArray *table_data, int
>> node,
>> + PCIDeviceHandle *handle,
>> + GenericAffinityFlags
>> flags)
>> +{
>> + build_append_int_noprefix(table_data, 5, 1); /* Type */
>> + build_append_int_noprefix(table_data, 32, 1); /* Length */
>> + build_append_int_noprefix(table_data, 0, 1); /* Reserved */
>> + build_append_int_noprefix(table_data, 1, 1); /* Device Handle Type
>> */
>> + build_append_int_noprefix(table_data, node, 4); /* Proximity Domain */
>> + build_append_int_noprefix(table_data, handle->segment, 2);
>> + build_append_int_noprefix(table_data, handle->bdf, 2);
>> + build_append_int_noprefix(table_data, handle->res0, 4);
>> + build_append_int_noprefix(table_data, handle->res1, 8);
>
> Why are we storing reserved fields in the PCIDeviceHandle? This
> function is already specific to building a PCI Device Handle, so we
> could just loop build_append_byte() with a fixed zero value here.
Good point, will make the change.
>> +void build_srat_generic_initiator(GArray *table_data)
>> +{
>> + GSList *gi_list, *list = acpi_generic_initiator_get_list();
>> + for (gi_list = list; gi_list; gi_list = gi_list->next) {
>> + AcpiGenericInitiator *gi = gi_list->data;
>> + Object *o;
>> + int count;
>> +
>> + if (gi->node == MAX_NODES) {
>> + continue;
>> + }
>
> Why do we have uninitialized AcpiGenericInitiator objects lingering?
Right, we don't need the check.
>> +
>> + o = object_resolve_path_type(gi->device, TYPE_VFIO_PCI_NOHOTPLUG,
>> NULL);
>
> TYPE_PCI_DEVICE? Maybe you could check hotpluggable from the device
> class, but certainly the generic code should not be dependent on being
> a vfio-pci-nohotplug device.
Understood.
> The spec also supports an ACPI object
> description, so should this be build_srat_generic_pci_initiator()?
Sure, makes sense.
>> + if (!o) {
>> + continue;
>> + }
>> +
>> + for (count = 0; count < gi->node_count; count++) {
>> + PCIDeviceHandle dev_handle = {0};
>> + PCIDevice *pci_dev = PCI_DEVICE(o);
>> +
>> + dev_handle.bdf = pci_dev->devfn;
>
> Where does the bus part of the bdf get filled in?
Good catch, should have code to added the bus.
>> + build_srat_generic_initiator_affinity(table_data,
>> + gi->node + count,
>> &dev_handle,
>> + GEN_AFFINITY_ENABLED);
>
> Seems like the code that built the AcpiGenericInitiator object should
> supply the flags. In fact the flag GEN_AFFINITY_ENABLED might be a
> better indicator to populate the SRAT with the GI than the node value.
Yeah, sure.
