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

[Alexander Graf]

On 12/12/2012 06:29 PM, Andreas Färber wrote:
> 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.

Considering that x86 is very straight forward in its CPU definitions, I 
think we can easily make this a flat model based on CPUID features + a 
few variables.

If you really want to model something, I would rather say

386
  -> 386SX
  -> 386DX
opteron-G3
  -> phenom-9550
  -> phenom-9560

Any inheritance above that level sounds excessive to me.


Alex

> 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