Re: [PATCH v3 07/18] machine: Add a new function init_apicid_fn in MachineClass

[Babu Moger]

On 1/29/20 3:14 AM, Igor Mammedov wrote:
> On Tue, 28 Jan 2020 13:45:31 -0600
> Babu Moger <address@hidden> wrote:
> 
>> On 1/28/20 10:29 AM, Igor Mammedov wrote:
>>> On Tue, 03 Dec 2019 18:37:42 -0600
>>> Babu Moger <address@hidden> wrote:
>>>   
>>>> Add a new function init_apicid_fn in MachineClass to initialize the mode
>>>> specific handlers to decode the apic ids.
>>>>
>>>> Signed-off-by: Babu Moger <address@hidden>
>>>> ---
>>>>  include/hw/boards.h |    1 +
>>>>  vl.c                |    3 +++
>>>>  2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>>> index d4fab218e6..ce5aa365cb 100644
>>>> --- a/include/hw/boards.h
>>>> +++ b/include/hw/boards.h
>>>> @@ -238,6 +238,7 @@ struct MachineClass {
>>>>                                                           unsigned 
>>>> cpu_index);
>>>>      const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>>>>      int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
>>>> +    void (*init_apicid_fn)(MachineState *ms);  
>>> it's x86 specific, so why it wasn put into PCMachineClass?  
>>
>> Yes. It is x86 specific for now. I tried to make it generic function so
>> other OSes can use it if required(like we have done in
>> possible_cpu_arch_ids). It initializes functions required to build the
>> apicid for each CPUs. We need these functions much early in the
>> initialization. It should be initialized before parse_numa_opts or
>> machine_run_board_init(in v1.c) which are called from generic context. We
>> cannot use PCMachineClass at this time.
> 
> could you point to specific patches in this series that require
> apic ids being initialized before parse_numa_opts and elaborate why?
> 
> we already have possible_cpu_arch_ids() which could be called very
> early and calculates APIC IDs in x86 case, so why not reuse it?

Forgot to respond to this. The possible_cpu_arch_ids does not use the numa
information to build the apic id. We cannot re-use it without changing it
drastically.

> 
>>
>>>
>>>   
>>>>  };
>>>>  
>>>>  /**
>>>> diff --git a/vl.c b/vl.c
>>>> index a42c24a77f..b6af604e11 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -4318,6 +4318,9 @@ int main(int argc, char **argv, char **envp)
>>>>      current_machine->cpu_type = machine_class->default_cpu_type;
>>>>      if (cpu_option) {
>>>>          current_machine->cpu_type = parse_cpu_option(cpu_option);
>>>> +        if (machine_class->init_apicid_fn) {
>>>> +            machine_class->init_apicid_fn(current_machine);
>>>> +        }
>>>>      }
>>>>      parse_numa_opts(current_machine);
>>>>  
>>>>
>>>>  
>>>   
>>
>