On Thu, 2020-05-28 at 13:19 +0200, Igor Mammedov wrote:
[..]
> > @@ -1334,6 +1335,31 @@ static void nvdimm_build_ssdt(GArray *table_offsets,
> > GArray *table_data,
> > free_aml_allocator();
> > }
> >
> > +void *nvdimm_build_srat(GArray *table_data)
> > +{
> > + AcpiSratMemoryAffinity *numamem = NULL;
> > + GSList *device_list = nvdimm_get_device_list();
> > +
> > + for (; device_list; device_list = device_list->next) {
> > + DeviceState *dev = device_list->data;
> I'd use Object here with OBJECT() cast and drop casts beolw in property
> getters
>
Done, that makes it much cleaner.
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 2e15f6848e..1461d8a718 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2428,6 +2428,16 @@ build_srat(GArray *table_data, BIOSLinker *linker,
> > MachineState *machine)
> > MEM_AFFINITY_ENABLED);
> > }
> > }
> > +
> > + if (machine->nvdimms_state->is_enabled) {
> > + void *ret;
> > +
> > + ret = nvdimm_build_srat(table_data);
> > + if (ret != NULL) {
> > + numamem = ret;
> > + }
>
> why do we need return value here and a test condition and assign 'ret' to
> numamem?
Ah I thought numamem was propagated through the different parts of the
build_srat flow, but I misread. You're right it is not needed, removing.
