Re: [Qemu-devel] [PATCH qom-next 08/12] target-i386: introduce cpu-model property for x86_cpu

[Igor Mammedov <imammedo@redhat.com>]

On 05/30/2012 05:22 PM, Andreas Färber wrote:
> Am 30.05.2012 00:10, schrieb Igor Mammedov:
>> it's probably intermidiate step till cpu modeled as
>> sub-classes. After then we probably could drop it.
>>
>> However it still could be used for overiding default
>> cpu subclasses definition, and probably renamed to
>> something like 'features'.
>>
>> v2:
>>   - remove accidential tcg_* init code move
>>
>> Signed-off-by: Igor Mammedov<imammedo@redhat.com>
>> ---
>>   cpu-defs.h           |    2 +-
>>   hw/pc.c              |   10 ----------
>>   target-i386/cpu.c    |   24 ++++++++++++++++++++++++
>>   target-i386/helper.c |   17 ++++++++++++-----
>>   4 files changed, 37 insertions(+), 16 deletions(-)
>
> For me this is still the big no-go in this series:
>
> * Moving the default cpu_model into cpu_x86_init() buys us nothing IMO,
> it duplicates the property setting code and differs from all other
> targets where the default CPU is the machine's decision.
Agreed.

>
> * As you rightly point out, we are heading towards sub-classes and that
> contradicts this two-step initialization. I don't see how this is an
> intermediate step?
It's not clear to me how sub-classes contradict with two-step initialization,
, could you elaborate more on this?

About cpu_model property being an intermediate step, I think it is
an easy and safe way to hide/isolate cpu implementation details
in cpu.c and use only QOM interface to create cpus.
Later with an introduction of sub-classes it could be reduced to
feature override functionality and when cpu features are converted to
proper properties then cpu-model property could be dropped altogether.

There are 2 problems I am solving with cpu-model property:
1 - APIC should be created before cpu becomes run-able i.e. before
x86_cpu_realize(). Now it's not so but cpu_reset() that is called after APIC
is created kind of 'fixes' issue. Moving APIC [11/12] creation into property
forces us to create it at the proper time.

2 - I'm not sure yet how to deal with APIC creation with sub-classes
introduction. Should it be created in an each sub-class initfn
/doubts: code duplication/ or in parent class /doubts: lack of APICless 486 cpu model/?

I could move APIC from cpu-model property in initfn right now and
create it there and in cpu-model property destroy APIC if cpu_def
doesn't have APIC feature /only 486cpu, may we ignore it?/ or if
feature is asked to be disabled via feature flag in cpu-model string
then just disable it as it's done for the rest of supported cpus
in real hw.


> I admit, I am the one to blame for not redoing the x86 CPU subclasses
> patches yet - major issue being the built-in vs. -cpudef split:
> Now that Eduardo refactored the config file reading, I wonder if we can
> outsource the built-in CPUs to the config file? If so, then I would
> appreciate one of you x86 experts to do that please (may need some
> rebasing when the occasional features get added/tweaked). Then I can
> base a series on top that initializes CPU subclasses in a consistent way
> rather than duplicating data for the builtins just to make -cpudef work
> or creating different intermediate subclasses for both types.
I'll ask Eduardo how I can help here.

>
> * To override CPU features you should think about how to set x86 CPU
> features in a QOM way (think QMP), then we can design the infrastructure
> for setting them through global properties (or whatever needed) around
> it. I honestly don't know what the requirements are in practice, so I
> can't make suggestions there without some more feedback.
>
> Regards,
> Andreas
>

-- 
-----
  Igor