Subject: Re: load_unaligned() and "uld" instruction

From: Ralf Baechle <[email-masked]>
Date: Thu, 5 Oct 2000 14:13:54 +0200

On Tue, Sep 26, 2000 at 11:04:12AM -0700, Jun Sun wrote:

> > > If we have to use "-mips2" option, is there a clean way which allows us
> > > to "uld/usw" instructions (instead of manually twicking the compilation
> > > for each file that uses them)?
> >
> 
> Ralf, before the perfect solution is found, the following patch makes
> the gcc complain go away.  It just use ".set mips3" pragma.

It's still perfectly broken.  Uld is a 64-bit instruction meaning you still
could get into problems with register corruption or even reserved instruction
exceptions on 32-bit cpus.  Not too mention that nobody did notice that
the constraints of the inline assembler were broken for all access sizes
plus a cast that would have cut off the upper 32 bit of a 64 bit access in
any case.  That's fixed now.

> I am pretty close to get USB running with the v2.4-test5.  The unaligned
> access is the minor problem.  The bigger problem I am fighting with now
> is bus_to_virt()/virt_to_bus() and USB interrupt.

The unaligned exception handler is fairly expensive.  I suggest you should
try to get proper alignment and where that is not possible go through
the entire code and use get_unaligned.  It's going to make a noticable
difference in performance.

  Ralf

---

From: Jun Sun <[email-masked]>
Date: Thu, 05 Oct 2000 18:11:40 -0700

> any case.  That's fixed now.

With my limited wisdom, I am totally confused by this paragraph.

I think you mentioned a couple of times before where 64-bit instructions
corrupt registers in 32-bit mode.  I think I have done that before with
R5000 R4500.  I did not notice any corruption.  What exactly is the
corruption you are referring to?

With the second half, are you saying the "cut-off-upper-32-bit" bug
actually hides the register corruption problem?  If so, maybe we need
the "cut-off-upper_32-bit" bug for the 32-bit MIPS tree.

Anyway, in short, what is your suggestion for fixing this bug?

Maciej suggested that we use packed struct of gcc (I assume gcc will
generate two loads and get the results with some bit masking and
shifting).  That does not sound too bad, although that does require one
to use the newer gcc.

> difference in performance.

Fortunately, the USB guys have already used get_unaligned() in all the
places - I hope.

Jun

---

From: "Kevin D. Kissell" <[email-masked]>
Date: Thu, 5 Oct 2000 21:41:13 +0200

> > > the gcc complain go away.  It just use ".set mips3" pragma.

Which, as Ralf correctly observes, will generate code that will
crash on 32-bit CPUs, and apparently do entirely the wrong
thing for other reasons on the 64-bit ones.

> corruption you are referring to?

Uld is an unaligned doubleword load macro that should generate
a LDL/LDR sequence if MIPS III, IV, V or MIPS64 is enabled in
the compiler/assembler.  That sequence should either execute
correctly or deliver a reserved instruction exception.  No
MIPS-compatible CPU should silently fail or corrupt registers.

> the "cut-off-upper_32-bit" bug for the 32-bit MIPS tree.

This is a joke, right?

            Kevin K.

---

From: Jun Sun <[email-masked]>
Date: Thu, 05 Oct 2000 21:32:41 -0700

"Kevin D. Kissell" wrote:
> crash on 32-bit CPUs, 

Why will it crash 32-bit CPUs?  On my R5432 CPU, the lwl/lwr sequence
executes just fine.

Or do you mean it will crash SOME 32-bit CPUs?  Do those 32-bit CPUs
support lwl or lwr?  If they don't, they should generate a reserved
instruction exception.  If they do, I don't see any problem. 

> > the "cut-off-upper_32-bit" bug for the 32-bit MIPS tree.

Not entirely.  I was thinking if the unaligned load/store instruction
corrupts the upper 32 bit content on SOME cpus, maybe we do need to cut
the upper 32bit as a workaround.  Well, I hope it is not necessary.

Jun

---

From: "Kevin D. Kissell" <[email-masked]>
Date: Fri, 6 Oct 2000 00:10:18 +0200

> instruction exception.  If they do, I don't see any problem.

Please re-read my previous message.  I wasn't talking about the
MIPS I lwl/lwr sequence for loading an unaligned 32-bit word, I was
talking about the MIPS III ldl/ldr sequence for loading an unaligned
64-bit doubleword.

            Kevin K.

---

From: Jun Sun <[email-masked]>
Date: Thu, 05 Oct 2000 22:53:34 -0700

> talking about the MIPS III ldl/ldr sequence for loading an unaligned

Ahh, my bad.  

Although the usb does use get_unaligned(u64) (ldl/ldr), it actually does
not run into it - at least in my test so far.  That probably explains
why my fix runs on the R5432 CPU so far.

Ralf, I notice you have fixed it in the CVS tree.  Just did a test, and
it looks good here.

Thanks.

Jun

---

From: "Kevin D. Kissell" <[email-masked]>
Date: Fri, 6 Oct 2000 01:14:40 +0200

> why my fix runs on the R5432 CPU so far.

The 5432 may have a 32-bit external bus, but it's still (as far
as I know) a 64-bit part internally, so as long as you're executing
in kernel mode, the ldl/ldr's should work as designed.

            Kevin K.

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 6 Oct 2000 02:43:37 +0200

On Wed, Sep 27, 2000 at 12:06:31PM +0200, Maciej W. Rozycki wrote:

>  Please don't.  Gcc already has means to generate proper unaligned
> accesses.  See include/asm-alpha/unaligned.h for how to achieve them in a
> portable way (i.e. using packed structs) without the problematic inline
> asm.

That's all very nice and guess what - I tried it when I originally wrote
ualigned.h for Linux.  Try building the mentioed Alpha code with and older
compiler like egcs 1.0.3a and take a look at it [1].  23 instructions for
loading a double world - that's just mindboggling.

  Ralf

[1] free barf bag on request.

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Fri, 6 Oct 2000 11:54:18 +0200 (MET DST)

> loading a double world - that's just mindboggling.

 Have you actually looked at the code?  They fall back to an inline asm
for pre-egcs 1.1.2 for exactly that reason for now.  It's surprising,
OTOH, as I am sure native egcs 1.0.3 did build a proper lwl/lwr sequence
for me on Ultrix a few years ago...  Maybe it's just a MIPS backend
configuration problem for other targets? 

 I vote for dual code for now and then we may remove the egcs 1.0.3
compatibility cruft one day (for 2.6, for example). 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email-masked], PGP key available        +

---

From: "Kevin D. Kissell" <[email-masked]>
Date: Fri, 6 Oct 2000 22:46:34 +0200

Jun Sun wrote:
> Ralf Baechle wrote:
> >
> > On Thu, Oct 05, 2000 at 09:32:41PM -0700, Jun Sun wrote:
> >
> > That's a 64-bit CPU with a 32-bit bus ...
> >
>
> That is what the manual claims.  However I did find something strange.
>
> I run the following code on R5432:
>
> 0x8019dc34 <my_get_unaligned+4>:        ldl     $a2,7($a0)
> 0x8019dc38 <my_get_unaligned+8>:        ldr     $a2,0($a0)
> 0x8019dc3c <my_get_unaligned+12>:       srl     $a2,$a2,0x10
>
> As Kevin has guessed, it actually runs fine.  However, the register
> content in $a2 is not right.  Basically it appears that $a2 is a 32-bit
> register instead of 64-bit register.  I put a srl instruction to make
> sure I was not fooled by gdb.

Please read the instruction manual for srl more closely.
In order to preserve binary compatibility with 32-bit MIPS
CPUs, srl, sll, and sra always work *as if* only a 32-bit register
is implemented.  If you want to shift the full 64 bits, you need
to use explicit 64-bit shifts: dsrl, dsll, dsra, etc.  Use a dsrl
instead of an srl and you *may* see what you are expecting.

But there is also the issue that  Ralf alluded to in earlier
messages on this thread:  If your kernel exception
handler is only saving and restoring register state
using 32-bit loads and stores, the upper 32-bits of
the registers will tend to decay into sign-extensions
of the least significant 32-bits.

            Regards,

            Kevin K.

---

From: Jun Sun <[email-masked]>
Date: Fri, 06 Oct 2000 18:24:29 -0700

I know R5432 is derived from R5000 FOR 32-bit systems.  I guess there
are probably a lot of short-cuts for 64-bit operations.

Jun

---

From: Jun Sun <[email-masked]>
Date: Fri, 06 Oct 2000 18:35:15 -0700

> Ralf, I notice you have fixed it in the CVS tree.  Just did a test, and

I was too soon to say that ... :-)

While the __ldq_u() did work, I had a couple of syntax problems with
put_unaligned().  See the patch below.

In addition, my usb subsystem now hangs.  It might mean a bug in the new
unaligned.h or the fix to unaligned.h reveals another bug.  I will let
you know.

Jun

--- unaligned.h.ralf    Fri Oct  6 18:32:34 2000
+++ unaligned.h Fri Oct  6 18:01:43 2000
@@ -117,20 +117,20 @@
        __val;                                                         
\
 })

-#define put_unaligned(x,ptr)                                          
\
+#define put_unaligned(val,ptr)                                        
\
 do {                                                                  
\
        switch (sizeof(*(ptr))) {                                      
\
        case 1:                                                        
\
-               *(unsigned char *)ptr = (val);                         
\
+               *(unsigned char *)(ptr) = (val);                       
\
                break;                                                 
\
        case 2:                                                        
\
-               __stw_u(val, (unsigned short *)ptr);                   
\
+               __stw_u(val, (unsigned short *)(ptr));                 
\
                break;                                                 
\
        case 4:                                                        
\
-               __stl_u(val, (unsigned int *)ptr);                     
\
+               __stl_u(val, (unsigned int *)(ptr));                   
\
                break;                                                 
\
        case 8:                                                        
\
-               __stq_u(val, (unsigned long long *)ptr);               
\
+               __stq_u(val, (unsigned long long *)(ptr));             
\
                break;                                                 
\
        default:                                                       
\
                __put_unaligned_bad_length();                          
\

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 6 Oct 2000 18:28:21 +0200

> instruction exception.  If they do, I don't see any problem. 

It will crash all 32-bit CPUs.

> the upper 32bit as a workaround.  Well, I hope it is not necessary.

No, it happens on all CPUs.  Interrupts only restore the lower 32-bit of
the registers.  Partially this happens for the sake of compatibility with
32-bit cpus, partially it's also the because otherwise 8kb kernel stack
wouldn't be sufficient, we'd have to go up to 16kb stacks which again
has potencial influence on the memory managment that can reduce the
reliability of the kernel when low on memory, it increases the overhead.
In short unless a system has serious needs for 64-bit supporting 64-bit
is quite a loss.

  Ralf

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 6 Oct 2000 18:21:22 +0200

> compatibility cruft one day (for 2.6, for example). 

Not much point in that - we end up with performancewise identical code for
both C and assembler variants with current compilers.  So whenever we
finally retire egcs 1.0.3 I think we should switch completly to the
new compiler and the C written unaligned.h.

  Ralf

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 6 Oct 2000 18:32:08 +0200

> why my fix runs on the R5432 CPU so far.

No, you just never hit the window where the your 64-bit reg got corrupted by
an exception.  The old broken macros also had a cast to long in them
which was truncating the loaded 64-bit word so in 100% of cases the upper
32-bit was modified in creative ways.  So I guess you were just lucky and
never hit the case were this actually bits.

  Ralf

---

From: Ralf Baechle <[email-masked]>
Date: Sat, 7 Oct 2000 00:26:35 +0200

> unaligned.h or the fix to unaligned.h reveals another bug.  I will let

I had already a patch for a the x vs. val thing in the CVS, so I just took
the part which adds the additional brackets from your patch to make
sure the semantic is identical with functions calls.

  Ralf

---

From: Jun Sun <[email-masked]>
Date: Sat, 07 Oct 2000 00:16:02 -0700

> instead of an srl and you *may* see what you are expecting.

Just re-did the test with dsrl.  It does show that the higher 32-bit are
loaded correctly by ldl/ldr.  The result still was not completely right,
due to the inline assembler bug noted by Ralf earlier.  That bug casts
off the higher 32-bit upon the function return.

Thanks, Kevin.

Jun

... learn something new each day ...

---

From: Dominic Sweetman <[email-masked]>
Date: Mon, 9 Oct 2000 15:49:10 +0100 (GMT/BST)

It started when I wrote:

> > Hmm.  I wish it was that simple.  But some MIPS CPUs have 
> > instruction set additions which are not related to the mips1, mips2,
> > etc.  For example, a whole collection of parts with a vaguely
> > "embedded" orientation has integer multiply/accumulate instructions.
> > 
> > Algorithmics' version of GCC (and, I'm sure, others) picks up on the
> > -mcpu=xxx flag to do that.  In fact, I don't think there's any other
> > way to allow the compiler to warn you of some bizarre omissions from
> > one or two rogue CPUs.

Ralf Baechle ([email-masked]) replied:

> Ouch.  The gcc documentation says this:
> 
> `-mcpu=CPU TYPE'
>      Assume the defaults for the machine type CPU TYPE when
>      scheduling instructions.  The choices for CPU TYPE are `r2000',
>      `r3000', `r4000', `r4400', `r4600', and `r6000'.  While picking
>      a specific CPU TYPE will schedule things appropriately for that
>      particular chip, the compiler will not generate any code that
>      does not meet level 1 of the MIPS ISA (instruction set
>      architecture) without the `-mips2' or `-mips3' switches being
>      used.
> 
> So in other words I wouldn't expect anything like mmad to be used
> unless -mmad is also being choosen.  -mcpu not influencing the set
> of instructions being used to build a program is a general gcc
> convention, not only for MIPS.  So if the Algorithmics compiler does
> things different I'd consider it to be off the track.

I think we comply with a somewhat weaker reading of the same
paragraph, in that no "MIPS III" instruction will be used unless you
say -mips3 (or greater).

I could also be pedantic and point out that the effect of
"-mcpu=r3900" (for instance) is not defined by that quotation...

-mmad: as you know (but all readers might not) the integer
multiply-accumulate instructions are not in *any* numbered MIPS
instruction set - at least not until MIPS32, which is a different
series.  If they existed as a single, coherent add-on a single "-mmad"
flag would be the best solution - but they don't: no two
manufacturer's implementations are quite the same.

And the Vr41xx is MIPS III, except that it leaves out the "semaphore"
instructions LL and SC.  We want our toolchain to know these
instructions aren't there, and it seems natural to overload the
-mcpu=r4100 flag for this purpose.  Perhaps we'll propose a change to
the manual!

> > Outside SGI circles, I believe, "32-bit kernels" are all that are
> > likely to work...
> 
> Currently.  Some embedded people are actually asking for more than
> the 512mb memory supported by the 32-bit kernel.  So expect the
> 64-bit kernel to become the predominant race in the not to distant
> future.

I can see why that might be sensible.  Most MIPS CPUs except the
lowest-end are now 64-bit, so why try to fix the memory limitation
twice?

I can sketch some reasons, though, why this might not be automatically
and obviously correct outside SGI:

1. Linux on other architectures doesn't depend on being able to
   address the whole of physical memory through an "unmapped" window
   like MIPS' kseg0.  (So this dependency can't extend into
   machine-independent code).

2. One effect of making the kernel 64-bit will be the memory
   swallowed by all those double-size pointers.

3. You're missing the advantage of a neat trick in the MIPS
   architecture, where 32-bit code running on a mips3+ CPU
   automatically "sign-extends" 32-bit pointers to generate valid
   64-bit addresses.
   
   So it's not obvious why you shouldn't go the other way, and use
   32-bit pointers inside a kernel which supports 64-bit-pointer
   applications.  

> Also expect embedded SMP kernels in the not to far future.

That's orthogonal to the pointer size.

> > Even calling a subroutine compiled 32-bit may corrupt one of the
> > registers which are supposed to be preserved.
> 
> Sure, but that's kind of expected and obvious when following the
> instruction sequence as it gets executed while the corruption by an
> exception was pretty unobvious when I first ran into it ...

(With me it was the other way around... the interrupt problem was
obvious, but I found it harder to see how the C compiler puns data
between compiler-world types and "register" data types.)

I guess anyone interested needs to be very careful to make the
distinction (familiar to old hands) between:

1. Using a "64-bit capable" CPU (MIPS III or higher), which has 64-bit
   registers, data path and so on...

2. Compiling in an environment where some C variables are implemented
   with 64-bit mips3+ instructions or rely on 64-bit registers.

3. Compiling in an environment where C pointers become 64-bit objects.

It's easy to slip into saying "64-bit" to mean "whichever of
these I'm currently thinking of."

You mentioned Kevin's suggested virtues for a kernel:

> > 1. build for either 32-bit ("MIPS32" and near-miss) and 64-bit
> >   (MIPS3, MIPS4 and MIPS64) CPUs.

Kevin works for MIPS, who have invented MIPS32 to try to stem
incompatible proliferation of the instructino set of MIPS CPUs with
only 32-bit registers and data paths.  This is still new - few, if
any, MIPS32 CPUs have shipped in systems yet.
   
Linux kernels to run on 32-bit CPUs should perhaps rely on just the
MIPS I instruction set plus a usable TLB (MIPS MMU hardware).  It's
true there are two major branches of the CPU-control instructions, but
it's not that hard to cover up, and surely not a good use of scarce
resources to assume compliance to MIPS32 just now.

> > 2. Allow 64-bit operations on 64-bit CPUs, without insisting that
> >    [standard integer/pointer] C data types grow.  Need to save the
> >    whole of registers and compile "long long" and "double" data
> >    types...

Algorithmics thought that was a good idea, and it's a door we've kept
open to our "embedded" customers, where 64-bit pointers are not
much wanted.  It does create a lot of unexpected side-effects in
return for rather intangible benefits, so I sympathise with Ralf on
that one.

> I was thinking about moving all the 64-bit CPUs over to the mips64
> kernel and leave the `mips' kernel to the true 32-bit stuff.

I think by "64-bit CPUs" you mean all of my (1-3) above, and by "true
32-bit stuff" you mean... I'm really not sure what.

Somewhere buried under this is the problem of maintaining a Linux/MIPS
kernel and providing any kind of confidence that it will (at any
particular version) build and run correctly on "any reasonable MIPS
CPU".

To provide stability on variant platforms means identifying the
interfaces between variant-dependent and -independent code, freezing
those interfaces and treating them with great respect.  I think that's
still foreign to most of the Linux community, because they've grown up
with PCs.

It may simply be the best decision to allow the MIPS kernel landscape
to fragment into islands, with the "compatibility" layer at the
kernel/application interface (and some informal conventions to ease
device driver porting).

> Most users will currently still not want to use a 64-bit address
> space for apps.  That's ok, we can add support for 2-level page
> tables to `mips64'.

I can't see, offhand, why a kernel which can map a large user space
for applications with 64-bit pointers should require different page
tables for applications which use 32-bit pointers.  32-bit pointers
generate perfectly good 64-bit addresses.  The userspace layout of
32-bit-pointer applications needs to feature stack space (for example)
within reach of the 32-bit pointers - but does it really need such
large changes to the VM code?

> The calling conventions used by -mips3 are slight confusing, if not
> even dangerous.  Older gccs use a non-standard calling convention
> which essentially is a blind extension of the 32-bit ABI to
> 64-bit...
>
> Newer gccs support the N32 and 64 ABIs.  Unfortunately currently gcc
> does not support building a single compiler that supports all three
> 32, N32 and 64 ABIs.

While it would be nice to fix it, a single compiler which does all
three is perhaps not so critical... Using o32 puts you in such a
different universe that having a separate compiler is not such a big
deal. 

-- 
Dominic Sweetman
Algorithmics Ltd
The Fruit Farm, Ely Road, Chittering, CAMBS CB5 9PH, ENGLAND
phone: +44 1223 706200 / fax: +44 1223 706250 / http://www.algor.co.uk