qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Paul Brook <paul@codesourcery.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] ARM QOM conversion / class hierarchy
Date: Tue, 20 Mar 2012 11:56:10 -0500	[thread overview]
Message-ID: <4F68B6AA.7060803@codemonkey.ws> (raw)
In-Reply-To: <201203201408.56275.paul@codesourcery.com>

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.

  parent reply	other threads:[~2012-03-20 16:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-20 12:12 [Qemu-devel] ARM QOM conversion / class hierarchy Peter Maydell
2012-03-20 14:08 ` Paul Brook
2012-03-20 14:59   ` Peter Maydell
2012-03-20 16:06     ` Paul Brook
2012-03-20 16:20       ` Peter Maydell
2012-03-20 17:14         ` Paul Brook
2012-03-20 17:19           ` Peter Maydell
2012-03-20 16:56   ` Anthony Liguori [this message]
2012-03-20 17:14     ` Paul Brook
2012-03-20 17:20       ` Andreas Färber
2012-03-20 16:31 ` Michael Roth
2012-03-20 16:32   ` Peter Maydell
2012-03-20 19:04     ` Michael Roth
2012-03-20 17:01   ` Paul Brook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F68B6AA.7060803@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=afaerber@suse.de \
    --cc=paul@codesourcery.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).