Re: [RFC v1 09/10] i386: split cpu.c and defer x86 models registration

[Claudio Fontana]

On 11/10/20 11:04 AM, Daniel P. Berrangé wrote:
> On Tue, Nov 10, 2020 at 10:40:04AM +0100, Claudio Fontana wrote:
>> On 11/9/20 7:03 PM, Daniel P. Berrangé wrote:
>>> On Mon, Nov 09, 2020 at 06:27:54PM +0100, Claudio Fontana wrote:
>>>> split cpu.c into:
>>>>
>>>> cpu.c            cpuid and common x86 cpu functionality
>>>> host-cpu.c       host x86 cpu functions and "host" cpu type
>>>> kvm-cpu-type.c   KVM x86 cpu type
>>>> hvf-cpu-type.c   HVF x86 cpu type
>>>> tcg-cpu-type.c   TCG x86 cpu type
>>>>
>>>> Defer the x86 models registration to MODULE_INIT_ACCEL_CPU,
>>>> so that accel-specific types can be used as parent types for all
>>>> cpu models. Use the generic TYPE_X86_CPU only if no
>>>> accel-specific specialization is enabled.
>>>
>>> Can you give more info on why this is needed and/or desirable ?
>>
>> Hello Daniel, there is a pointer to the overall higher level motivation in 
>> the cover letter.
>>
>> But I am not pushing for this specific mechanism to be used, as mentioned in 
>> the cover letter.
>>
>> If we need another mechanism to achieve that (not delaying the x86 model 
>> registration and make them inherit from the specialized class), but 
>> something else,
>> I would be happy to get additional ideas.
>>
>>>
>>> Dynamically changing the class hierarchy of CPUs at runtime feels
>>> like a rather suspicious approach to me
>>
>> TYPE_X86_CPU is base type is registered as usual.
>> New accel-specialized types are defined (TYPE_TCG_CPU, TYPE_KVM_CPU, 
>> TYPE_HVF_CPU), also using normal type registration.
>>
>> The missing step is how to adapt all the cpu models to use the functionality.
> 
> If I understand the problem correctly, we have two distinct axis of
> configurability
> 
>  - the CPU model definitions (Nehalem, Broadwell, Skylake, host, max)
>  - the accelerator CPU implementations (tcg, kvm, hvf).
> 
> At runtime any pair of objects from these two axis can be combined.
> 
> We're trying to avoid defining classes for the combinatorial expansion
> of these axis.
> 
> This patch series encodes these two axis in a single class hierarchy,
> with the CPU implementations being a parent of the CPU model definitions.
> It avoids the combinatorial expansion, by taking the approach of dynamically
> defining the parent/child relation between CPU impl and CPU defintion at
> runtime  baed on the choosen accelerator impl.
> 
> The fully static way to deal with this problem is to accept that distinct
> axis should be represented as distinct class hierarchies.
> 
> ie, we should have one class hierarchy for CPU model definitions, and
> one class hierarchy  for accelerator CPU implementations.
> 
> So at runtime we then get two object instances - a CPU implementation
> and a CPU definition. The CPU implementation object should have a
> property which is a link to the desired CPU definition.
> 
> 
>> The accelerator that is finally chosen to be used is only known at a 
>> specific point in the qemu initialization.
>> This point of time I defined as MODULE_INIT_ACCEL_CPU.
>>
>> That is the time when we know how the CPU should actually really behave (how 
>> it should be realized, etc).
>>
>> In this series I realized this by registering the cpu models only at 
>> MODULE_INIT_ACCEL_CPU time, and not earlier.
>> But maybe there is a better idea on how to do it, and I am all ears.
>>
>> .
>>>
>>> It is contrary to work we've been doing recently to try to make all
>>> classes be fully statically defined by getting rid of dynamic properties,
>>> such that introspection of classes does not depend on other CLI flags
>>> you might have passed.
>>
>> Understood, this goes against other requirements.
>>
>> The dynamism introduced here is to register the cpu models at 
>> MODULE_INIT_ACCEL_CPU time instead of MODULE_INIT_QOM time.
>> As a result, for any chosen accelerator, the type tree and class tree is 
>> identical.
> 
> For introspection the goal is that the type tree and class tree is
> identical for a *binary*, not an accelerator within a binary.
> 
> 
> Regards,
> Daniel
> 

Thanks Daniel for your comments, I am going to think these through and see if I 
can build a model like you suggest
(with the cpu implementation axis separate from the cpu model definitions axis),

as a possible other way forward.

Ciao,

Claudio