Re: [Qemu-devel] [PATCH] numa: Introduce MachineClass::auto_enable_numa for implicit NUMA node

[Tao Xu <tao3.xu@intel.com>]

On 8/8/2019 1:52 AM, Eduardo Habkost wrote:
> On Tue, Aug 06, 2019 at 02:50:55PM +0200, Igor Mammedov wrote:
>> On Mon,  5 Aug 2019 15:13:02 +0800
>> Tao Xu <tao3.xu@intel.com> wrote:
>>
>>> Add MachineClass::auto_enable_numa field. When it is true, a NUMA node
>>> is expected to be created implicitly.
>>>
>>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>>> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> [...]
>>> +    mc->auto_enable_numa = true;
>>
>> this will always create a numa node (that will affect not only RAM but
>> also all other components that depends on numa state (like CPUs)),
>> where as spapr_populate_memory() was only faking numa node in DT for RAM.
>> It makes non-numa configuration impossible.
>> Seeing David's ACK on the patch it might be fine, but I believe
>> commit message should capture that and explain why the change in
>> behavior is fine.
> 
> After a quick look, all spapr code seems to have the same
> behavior when nb_numa_nodes==0 and nb_numa_nodes==1, but I'd like
> to be sure.
> 
> David and/or Tao Xu: do you confirm there's no ABI change at all
> on spapr after implicitly creating a NUMA node?
> 
Even without this patch and HMAT patch, if without numa configuration, 
global nb_numa_nodes is always existing and default is 0, so nb_nodes 
will be auto set to 1, so from my point of view, this patch will not 
change ABI.

And I would also want to listen David's opinion.
>>
>>>       smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>>>       smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>> index 2eb9a0b4e0..4a350b87d2 100644
>>> --- a/include/hw/boards.h
>>> +++ b/include/hw/boards.h
>>> @@ -220,6 +220,7 @@ struct MachineClass {
>>>       bool smbus_no_migration_support;
>>>       bool nvdimm_supported;
>>>       bool numa_mem_supported;
>>> +    bool auto_enable_numa;
>>>   
>>>       HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>>>                                              DeviceState *dev);
>>
>