On 10/10/19 9:29 PM, Eduardo Habkost wrote:
> On Fri, Sep 06, 2019 at 07:11:57PM +0000, Moger, Babu wrote:
>> This is an effort to re-arrange few data structure for better
>> readability. Add X86CPUTopoInfo which will have all the topology
>> informations required to build the cpu topology. There is no
>> functional changes.
>>
>> Signed-off-by: Babu Moger <babu.mo...@amd.com>
>> ---
>> hw/i386/pc.c | 40 +++++++++++++++++++++++++++-------------
>> include/hw/i386/topology.h | 40 ++++++++++++++++++++++++++--------------
>> 2 files changed, 53 insertions(+), 27 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index ada445f8f3..95aab8e5e7 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -930,11 +930,15 @@ static uint32_t
>> x86_cpu_apic_id_from_index(PCMachineState *pcms,
>> {
>> MachineState *ms = MACHINE(pcms);
>> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> + X86CPUTopoInfo topo_info;
>> uint32_t correct_id;
>> static bool warned;
>>
>> - correct_id = x86_apicid_from_cpu_idx(pcms->smp_dies, ms->smp.cores,
>> - ms->smp.threads, cpu_index);
>> + topo_info.nr_dies = pcms->smp_dies;
>> + topo_info.nr_cores = ms->smp.cores;
>> + topo_info.nr_threads = ms->smp.threads;
>> +
>> + correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index);
>
> If you are using the struct in function calls, please make sure
> all fields are filled correctly, so we won't introduce bugs
> accidentally if we start using the new fields inside the topology
> functions.
>
> Alternatively, you can leave the struct without the numa_nodes
> and nr_sockets fields by now (because they are unused), and add
> the fields in another patch.
Yes. Separated the patches and added the new fields separately.
>
> Except for this, the patch looks good. However, I don't
> understand yet the use case for the `numa_nodes` field yet. I
> will probably comment on the `numa_nodes` field once I see the
> patches that use the new field.
>
