[Qemu-devel] ARM QOM conversion / class hierarchy

[Qemu-devel] ARM QOM conversion / class hierarchy

[Peter Maydell]

(I can't find the relevant patches in the mailing list at
this point. I'm talking about this tree from Andreas:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu-arm )

So in an IRC discussion yesterday we didn't seem to make much headway
on what the right class hierarchy is here. There seem to be two basic
options:

(1) subclass per CPU type
This is what Andreas' tree does at the moment, so there's an ARMCPU
which is a subclass of CPUState, and then a lot of subclasses of
that, one per CPU ("arm926", "arm1026", "cortex-a8", "cortex-a9", etc).
Mostly these subclasses just arrange for different reset values for
various registers, setting feature bits, etc.

(2) no subclasses for CPU types
This approach would just have a single ARMCPU, and then -cpu foo
is syntactic sugar for setting a lot of qom properties.

Option two looks kind of nice, but I'm not sure whether it would
actually work. I think you could do 95% of what you need for a
different CPU that way (lots of properties for "value of ID_MMFR1",
"value of "ID_MMFR2", "reset value of SCTLR", etc etc, plus properties
for all our existing ARM_FEATURE_*, and some new ones where we're
currently keying off "which CPU is this?" rather than using a feature
bit, or just failing to distinguish things which aren't really
common to all CPUs). But I'm not sure how you'd handle the genuinely
implementation specific cp15 registers (eg cp15 crn=15). We could
have a property which says "is this a 926/1026/1176/A8/A9/..." (or
equivalently, key off the relevant fields of the main ID register) but
it doesn't seem very OO-like to have one class whose behaviour changes
based on an integer that's basically defining an ad-hoc sub-type...

Regardless, it seems to me that we ought to be doing this the
same way for all our target CPUs. I don't know whether mips/x86/
etc have any preference one way or the other.

-- PMM

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Paul Brook]

> Option two looks kind of nice, but I'm not sure whether it would
> actually work. I think you could do 95% of what you need for a
> different CPU that way (lots of properties for "value of ID_MMFR1",
> "value of "ID_MMFR2", "reset value of SCTLR", etc etc, plus properties
> for all our existing ARM_FEATURE_*, and some new ones where we're
> currently keying off "which CPU is this?" rather than using a feature
> bit, or just failing to distinguish things which aren't really
> common to all CPUs). But I'm not sure how you'd handle the genuinely
> implementation specific cp15 registers (eg cp15 crn=15). We could
> have a property which says "is this a 926/1026/1176/A8/A9/..." (or
> equivalently, key off the relevant fields of the main ID register) but
> it doesn't seem very OO-like to have one class whose behaviour changes
> based on an integer that's basically defining an ad-hoc sub-type...

IIUC the "proper OO" solution to this requires multiple inheritance, which we 
don't have.  The problem with subtyping is we can use it for at most one 
characteristic.  Everything else ends up being pushed into a common base class 
and controlled by feature bits (or equivalent).

If we're going to use the class hierachy to implement functionality then there 
are other candidates.  Given the primary purpose of QOM is [IMO] to handle 
interaction between devices, the external interface exposed by the core seems 
like a better candidate for subclassing.  i.e. conventional ARM cores with IRQ 
and FIQ inputs[1] v.s. M profile devices where the core exception model is 
intimately tied to the interrupt controller.

Paul

[1] This still applies to things like the Cortex-A9.  In practice ARM may sell 
you an SMP "cluster", but logically it's still a couple of normal cores and an 
interrupt controller.

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Peter Maydell]

On 20 March 2012 14:08, Paul Brook <paul@codesourcery.com> wrote:
>> Option two looks kind of nice, but I'm not sure whether it would
>> actually work. I think you could do 95% of what you need for a
>> different CPU that way (lots of properties for "value of ID_MMFR1",
>> "value of "ID_MMFR2", "reset value of SCTLR", etc etc, plus properties
>> for all our existing ARM_FEATURE_*, and some new ones where we're
>> currently keying off "which CPU is this?" rather than using a feature
>> bit, or just failing to distinguish things which aren't really
>> common to all CPUs). But I'm not sure how you'd handle the genuinely
>> implementation specific cp15 registers (eg cp15 crn=15). We could
>> have a property which says "is this a 926/1026/1176/A8/A9/..." (or
>> equivalently, key off the relevant fields of the main ID register) but
>> it doesn't seem very OO-like to have one class whose behaviour changes
>> based on an integer that's basically defining an ad-hoc sub-type...
>
> IIUC the "proper OO" solution to this requires multiple inheritance, which we
> don't have.  The problem with subtyping is we can use it for at most one
> characteristic.  Everything else ends up being pushed into a common base class
> and controlled by feature bits (or equivalent).

> If we're going to use the class hierachy to implement functionality then there
> are other candidates.  Given the primary purpose of QOM is [IMO] to handle
> interaction between devices, the external interface exposed by the core seems
> like a better candidate for subclassing.  i.e. conventional ARM cores with IRQ
> and FIQ inputs[1] v.s. M profile devices where the core exception model is
> intimately tied to the interrupt controller.

Yes, I think I'd agree there. So should we just have an init function
that provides the implementation-specific cp15 registers based on the value
provided in the QOM property for the main ID register?

> [1] This still applies to things like the Cortex-A9.  In practice ARM may sell
> you an SMP "cluster", but logically it's still a couple of normal cores and an
> interrupt controller.

Yes, this should be implemented by object composition (ie QOM child objects
inside a container). The lines blur rather with the A15, though, where for
instance the generic timer hangs off cp15 but injects interrupts into the GIC,
not directly into the core. (And even for the A9 the coupling is pretty close,
for instance the BE8 byte-swapping, which you might think is a property of the
core, doesn't apply to accesses to the private peripherals (timers, gic, etc).)

-- PMM

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Paul Brook]

> > If we're going to use the class hierachy to implement functionality then
> > there are other candidates.  Given the primary purpose of QOM is [IMO]
> > to handle interaction between devices, the external interface exposed by
> > the core seems like a better candidate for subclassing.  i.e.
> > conventional ARM cores with IRQ and FIQ inputs[1] v.s. M profile devices
> > where the core exception model is intimately tied to the interrupt
> > controller.
> 
> Yes, I think I'd agree there. So should we just have an init function
> that provides the implementation-specific cp15 registers based on the value
> provided in the QOM property for the main ID register?

Something like that, yes.  I'm not convinced the main ID register is the right 
property to use, but for actual implementation specific bits (rather than bits 
where an implementation picks one of a few common options) I guess we don't 
have any alternative but enumerating the implementations we support.

Paul

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Peter Maydell]

On 20 March 2012 16:06, Paul Brook <paul@codesourcery.com> wrote:
>> Yes, I think I'd agree there. So should we just have an init function
>> that provides the implementation-specific cp15 registers based on the value
>> provided in the QOM property for the main ID register?
>
> Something like that, yes.  I'm not convinced the main ID register is the right
> property to use, but for actual implementation specific bits (rather than bits
> where an implementation picks one of a few common options) I guess we don't
> have any alternative but enumerating the implementations we support.

Mmm, the disgusting thing the TI925T has where it can programmatically
change the value of its main ID register does somewhat argue against using
it for this.

-- PMM

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Michael Roth]

On Tue, Mar 20, 2012 at 12:12:45PM +0000, Peter Maydell wrote:
> (I can't find the relevant patches in the mailing list at
> this point. I'm talking about this tree from Andreas:
> http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu-arm )
> 
> So in an IRC discussion yesterday we didn't seem to make much headway
> on what the right class hierarchy is here. There seem to be two basic
> options:
> 
> (1) subclass per CPU type
> This is what Andreas' tree does at the moment, so there's an ARMCPU
> which is a subclass of CPUState, and then a lot of subclasses of
> that, one per CPU ("arm926", "arm1026", "cortex-a8", "cortex-a9", etc).
> Mostly these subclasses just arrange for different reset values for
> various registers, setting feature bits, etc.
> 
> (2) no subclasses for CPU types
> This approach would just have a single ARMCPU, and then -cpu foo
> is syntactic sugar for setting a lot of qom properties.
> 
> Option two looks kind of nice, but I'm not sure whether it would
> actually work. I think you could do 95% of what you need for a
> different CPU that way (lots of properties for "value of ID_MMFR1",
> "value of "ID_MMFR2", "reset value of SCTLR", etc etc, plus properties
> for all our existing ARM_FEATURE_*, and some new ones where we're
> currently keying off "which CPU is this?" rather than using a feature
> bit, or just failing to distinguish things which aren't really
> common to all CPUs). But I'm not sure how you'd handle the genuinely
> implementation specific cp15 registers (eg cp15 crn=15). We could
> have a property which says "is this a 926/1026/1176/A8/A9/..." (or
> equivalently, key off the relevant fields of the main ID register) but
> it doesn't seem very OO-like to have one class whose behaviour changes
> based on an integer that's basically defining an ad-hoc sub-type...

Rather than key off an ID you could also just break
implementation-specific functionality out into a set of interfaces you
implement/override as part of the objects' initialization. Same ends, and
still fits the model pretty nicely, assuming the functionality is all
derivable from the common base-class.

> 
> Regardless, it seems to me that we ought to be doing this the
> same way for all our target CPUs. I don't know whether mips/x86/
> etc have any preference one way or the other.
> 
> -- PMM
> 

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Peter Maydell]

On 20 March 2012 16:31, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Rather than key off an ID you could also just break
> implementation-specific functionality out into a set of interfaces you
> implement/override as part of the objects' initialization. Same ends, and
> still fits the model pretty nicely, assuming the functionality is all
> derivable from the common base-class.

I'm not sure what you have in mind here -- could you explain
in a little more detail, please?

thanks
-- PMM

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Anthony Liguori]

On 03/20/2012 09:08 AM, Paul Brook wrote:
>> Option two looks kind of nice, but I'm not sure whether it would
>> actually work. I think you could do 95% of what you need for a
>> different CPU that way (lots of properties for "value of ID_MMFR1",
>> "value of "ID_MMFR2", "reset value of SCTLR", etc etc, plus properties
>> for all our existing ARM_FEATURE_*, and some new ones where we're
>> currently keying off "which CPU is this?" rather than using a feature
>> bit, or just failing to distinguish things which aren't really
>> common to all CPUs). But I'm not sure how you'd handle the genuinely
>> implementation specific cp15 registers (eg cp15 crn=15). We could
>> have a property which says "is this a 926/1026/1176/A8/A9/..." (or
>> equivalently, key off the relevant fields of the main ID register) but
>> it doesn't seem very OO-like to have one class whose behaviour changes
>> based on an integer that's basically defining an ad-hoc sub-type...
>
> IIUC the "proper OO" solution to this requires multiple inheritance, which we
> don't have.

There is no such thing as a "proper OO" solution.

When ever you model, you make trade-offs.  Eventually, you'll find that the 
trade-offs that were okay at one point don't meet future expectations and you'll 
need to refactor.

The best way to avoid having to refactor often is to keep hierarchies simple and 
relatively flat.

I think the main issue in this discussion is the assumption that you have to be 
able to define arbitrary ARM CPUs via a configuration file.  I think that's a 
bad assumption for ARM.  There's too many variants and the differences in those 
variants are too complex.

What I'd recommend is:

Make an ARMCPUClass that maps to the existing ARM support.  Do *not* expose all 
of the different features as properties.  Make ARMCPUClass abstract.

Subclass ARMCPUClass for specific models, set default flags to implement the 
necessary logic.  Expose tunables on a case-by-case basis (if there needs to be 
a 'neon' flag for cortex-a9, then make one, but don't make everything a flag 
just for the hell of it).

*If* there is a serious need to have a more configurable processor, make a 
CustomARMCPUClass and expose the tunables that make sense.  But don't start out 
trying to boil the ocean.  Expose tunables that are driven by real use-cases.

The main thing is to try and keep things simple.

Regards,

Anthony Liguori

Regards,

Anthony Liguori

> The problem with subtyping is we can use it for at most one
> characteristic.  Everything else ends up being pushed into a common base class
> and controlled by feature bits (or equivalent).
>
> If we're going to use the class hierachy to implement functionality then there
> are other candidates.  Given the primary purpose of QOM is [IMO] to handle
> interaction between devices, the external interface exposed by the core seems
> like a better candidate for subclassing.  i.e. conventional ARM cores with IRQ
> and FIQ inputs[1] v.s. M profile devices where the core exception model is
> intimately tied to the interrupt controller.
>
> Paul
>
> [1] This still applies to things like the Cortex-A9.  In practice ARM may sell
> you an SMP "cluster", but logically it's still a couple of normal cores and an
> interrupt controller.

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Paul Brook]

> Rather than key off an ID you could also just break
> implementation-specific functionality out into a set of interfaces you
> implement/override as part of the objects' initialization. Same ends, and
> still fits the model pretty nicely, assuming the functionality is all
> derivable from the common base-class.

I don't think that helps a great deal, and the implementation ends up much the 
same.

You're just replacing an explicit enumeration with an implicit set of 
subclasses.  The only reason I can see for that is if you want to build a 
deliberately crippled version of qemu, which seems a bit of a stretch. Of if 
you're trying to maintain something out of tree, in which case you deserve all 
the pain we can give you.

Note that you can't make the interface part of the CPUState object, as single 
inheritance prevents those having any state.  It has to be a separate object 
that the CPUState links to.

Paul

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Paul Brook]

> Make an ARMCPUClass that maps to the existing ARM support.  Do *not* expose
> all of the different features as properties.  Make ARMCPUClass abstract.
> 
> Subclass ARMCPUClass for specific models, set default flags to implement
> the necessary logic.  Expose tunables on a case-by-case basis (if there
> needs to be a 'neon' flag for cortex-a9, then make one, but don't make
> everything a flag just for the hell of it).

As long as we can avoid the sort of duplication and redundant implementation 
that the initial .feature patch introduced.  If only having a neon knob on 
some cores means we have to duplicate a whole bunch of boilerplate between 
those cores then we're doing it wrong.

Paul

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Paul Brook]

> >> Yes, I think I'd agree there. So should we just have an init function
> >> that provides the implementation-specific cp15 registers based on the
> >> value provided in the QOM property for the main ID register?
> > 
> > Something like that, yes.  I'm not convinced the main ID register is the
> > right property to use, but for actual implementation specific bits
> > (rather than bits where an implementation picks one of a few common
> > options) I guess we don't have any alternative but enumerating the
> > implementations we support.
> 
> Mmm, the disgusting thing the TI925T has where it can programmatically
> change the value of its main ID register does somewhat argue against using
> it for this.

I was thinking more for when we have multiple revisions of a chip that are 
(for these purposes) the same. Currently we only have this for pxa, but in 
principle we probably want it for others.

As I mentioned on IRC, this isn't particularly interesting for Linux, which 
will happily run on pretty much anything.  However there are other systems 
that care[1] whether the core reports itself as e.g. arm1136-r0p1 v.s. 
arm1136-r0p2.

Paul

[1] The reason for this is usually not relevant to qemu. e.g. They've got a 
hardcoded table of known CPUID values, and arbitrarily refuse to run on 
anything else.  Or you're checking that a workaround for a particular silicon 
errata doesn't make anything else explode.

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Peter Maydell]

On 20 March 2012 17:14, Paul Brook <paul@codesourcery.com> wrote:
>> >> Yes, I think I'd agree there. So should we just have an init function
>> >> that provides the implementation-specific cp15 registers based on the
>> >> value provided in the QOM property for the main ID register?
>> >
>> > Something like that, yes.  I'm not convinced the main ID register is the
>> > right property to use, but for actual implementation specific bits
>> > (rather than bits where an implementation picks one of a few common
>> > options) I guess we don't have any alternative but enumerating the
>> > implementations we support.
>>
>> Mmm, the disgusting thing the TI925T has where it can programmatically
>> change the value of its main ID register does somewhat argue against using
>> it for this.
>
> I was thinking more for when we have multiple revisions of a chip that are
> (for these purposes) the same. Currently we only have this for pxa, but in
> principle we probably want it for others.

I had in mind that we'd do it based on the main ID with the revision/variant
fields masked out, which makes all the PXA chips the same. On the other hand
that also makes 1136 and 1136_R2 the same, which is a pain because they're
really pretty different.

> As I mentioned on IRC, this isn't particularly interesting for Linux, which
> will happily run on pretty much anything.  However there are other systems
> that care[1] whether the core reports itself as e.g. arm1136-r0p1 v.s.
> arm1136-r0p2.

Yeah, I've encountered this before too.

-- PMM

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Andreas Färber]

Am 20.03.2012 18:14, schrieb Paul Brook:
>> Make an ARMCPUClass that maps to the existing ARM support.  Do *not* expose
>> all of the different features as properties.  Make ARMCPUClass abstract.
>>
>> Subclass ARMCPUClass for specific models, set default flags to implement
>> the necessary logic.  Expose tunables on a case-by-case basis (if there
>> needs to be a 'neon' flag for cortex-a9, then make one, but don't make
>> everything a flag just for the hell of it).
> 
> As long as we can avoid the sort of duplication and redundant implementation 
> that the initial .feature patch introduced.  If only having a neon knob on 
> some cores means we have to duplicate a whole bunch of boilerplate between 
> those cores then we're doing it wrong.

Allowing to parse cpu,+/-feature is what ARMCPU::features is for.

object_new() creates an ARMCPU instance,
initfn copies ARMCPUClass::features into ARMCPU::features,
TBD sets/unsets feature flags.

That's orthogonal to imperative vs. declarative and/or inheritence.

Andreas

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

Re: [Qemu-devel] ARM QOM conversion / class hierarchy

[Michael Roth]

On Tue, Mar 20, 2012 at 04:32:31PM +0000, Peter Maydell wrote:
> On 20 March 2012 16:31, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > Rather than key off an ID you could also just break
> > implementation-specific functionality out into a set of interfaces you
> > implement/override as part of the objects' initialization. Same ends, and
> > still fits the model pretty nicely, assuming the functionality is all
> > derivable from the common base-class.
> 
> I'm not sure what you have in mind here -- could you explain
> in a little more detail, please?

Essentially what Paul suggested, minimal subclassing to encapsulate the
major architectural characteristics of a cpu, but also making use of the fact
that we *do* have multiple inheritance for QOM interfaces to do finer-grained
modeling of specific features. SIMD/NEON/DSP, for instance, could be proper
QOM interfaces that models from multiple families/architectures implement.

Ultimately, as Paul said, it just boils down to implicit subclasses in place of
enumerating features for use by common routines. So..it doesn't buy you
much really, except maybe maintaining devices out of tree. But it's a way to
do what you were suggesting with option #2 while avoiding not being "OO-like",
since it's a more formal way to modify a class' behavior. But maybe that's
a non-issue to begin with.

> 
> thanks
> -- PMM
>