Re: [Qemu-devel] [PATCH] SH: Add prefi, icbi, synco

[Paul Mundt <lethal@linux-sh.org>]

On Mon, Oct 20, 2008 at 11:54:20AM +0100, Paul Brook wrote:
> On Monday 20 October 2008, Paul Mundt wrote:
> > On Fri, Oct 17, 2008 at 08:56:52PM +0100, Paul Brook wrote:
> > > On Friday 17 October 2008, Vladimir Prus wrote:
> > > > This patch makes qemu recognize (and ignore), three instructions from
> > > > SH4A.
> > >
> > > Shouldn't these generate an illegal instruction exception on SH4 cpus?
> >
> > Note that all of these are SH-5 instructions, which gradually found their
> > way in to the SH-4A ISA. There are also other SH-5 instructions that have
> > found their way in to the SH-2A ISA which are not as of yet reflected in
> > SH-4A. The dependency tracking gets to be pretty ugly, as is evident in
> > binutils. In order to get this right, it would be necessary to throw ISA
> > versioning in to the CPU definition and test to figure out what to
> > support, as per the table in binutils. While this would be a good idea in
> > general, it is something that should be done as a larger follow-up to
> > these patches.
> 
> I disagree. This is something that should be done right from the start. Trying 
> to fix it up later is a real pain.  Doing fine grained features isn't that 
> hard. MIPS, sparc, arm, ppc, m68k and sparc already do this. IIRC binutils is 
> only complicated because it tried to create a strict hieracy of features, 
> rather than using feature bits.
> 
When we do it does not matter, but it is completely unrelated from this
patch, in that there are already plenty of instructions that are specific
to a certain CPU family that we don't perform an illegal instruction
exception for. Trying to force the prefi/icbi/synco cases to rework all
of the existing instructions that aren't universal doesn't make a lot of
sense, as it is a clear incremental change of existing behaviour, rather
than a situation caused purely by the addition of these new instructions.

At a quick glance:

	movca.l - SH-4 and up
	movua.l - SH-4A and up
	ocbi/ocbp/ocbwb - SH-4 and up
	pref - SH-2A or SH-4 and up, excluding other legacy parts.
	shad/shld - SH-2A or SH-3 and up.

etc, etc.

The fact that the qemu target directory is 'target-sh4' instead of
'target-sh' already makes it clear that the original patch was only
concerned with SH-4, and so therefore did not have any ISA versioning or
feature differentiation in mind. Even things like the register layout
will be different for the other CPUs.

Given that, perhaps the best solution is something like:

	- Add synco/icbi/prefi
	- Add ISA versioning based on the binutils inheritance list
	- Rename to target-sh

These are all going to be prerequisites for fitting in other more exotic
CPUs, like the SH-2A and SH-5 also.