Re: [Qemu-devel] [RFC qom-cpu v2 2/2] target-i386: Turn Haswell into subclass of SandyBridge

[Andreas Färber]

Am 12.12.2012 16:05, schrieb Eduardo Habkost:
> On Wed, Dec 12, 2012 at 03:47:49PM +0100, Andreas Färber wrote:
>> Am 12.12.2012 13:45, schrieb Eduardo Habkost:
>>> On Mon, Dec 10, 2012 at 11:59:32PM +0100, Andreas Färber wrote:
>>>>   ehabkost: "When adding the Haswell CPU model, I intended to make it
>>>>   a superset of the features present on the SandyBridge model"
>>>>
>>>> Inherit from SandyBridge to keep only the delta for Haswell.
>>>
>>> Most CPUs have a superset of the features of their predecessors. Are you
>>> simply using SandyBridge->Haswell as an example, or you think their
>>> relationship is special somehow?
>>>
>>> I believe we don't want to make externally-visible class inheritance,
>>> but probably just reuse constans or init functions internally. A Haswell
>>> CPU is not a type of SandyBridge CPU, it just happens to contain a
>>> superset of the features present in SandyBridge.
>>>
>>> I mean: Haswell also has a superset of features of 486, but we don't
>>> want to make the hierarchy look like the following, do we?
>>
>> I don't see why we would want to use a #define-based inheritence as
>> suggested for the PPRO when we have QOM. QOM inheritence reduces lines
>> of code significantly compared to just taking the values from elsewhere.
> 
> The reuse doesn't need to be #define-based (although maybe a
> #define-based approach would work too), it could be function-call-based.
> 
>>
>> For the Haswell you said what I quoted, for the other models I said I
>> need your or someone's help to verify whether a hierarchy such as below
>> is semantically right or just a coincidence. I was at least considering
>> an abstract intel-/amd-*-cpu to avoid repeating the three value
>> assignments over and over.
> 
> Creating X86IntelCPU and X86AMDCPU classes make sense to me, because
> Haswell is a kind of Intel CPu. Making Haswell a subclass of 486 (like
> below) wouldn't.
> 
>>
>> At this time I believe the parents of a type are not (yet) exposed via
>> QMP, just the "type" string property.
> 
> Even if they are not exposed externally, it's a confusing usage of
> inheritance for me. I mean: a Haswell CPU is not a type of 486 CPU, it's
> simply a different kind of CPU that happens to have a superset of the
> 486 features.

I concur with your is-a argument. My patch took a pragmatic approach.

I'd like to wait for some more review comments on how to share code
between models then - I remember Paul, Anthony and Alex discussing the
x86 models a while back on IRC, CC'ing. (Summary: reading CPU models
from config files has been dropped, we only have built-in models left -
now how to design name-class mappings below X86CPU)

Andreas

>>> - X86CPU
>>>   -> X86IntelCPU
>>>      -> 486
>>>         -> pentium
>>>            -> pentium2
>>>               -> pentium3
>>>                  -> Conroe
>>>                     -> Penryn
>>>                        -> Nehalem
>>>                           -> Westmere
>>>                              -> SandyBridge
>>>                                 -> Haswell

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg