Subject: Re: cpufunc.h
To: None <Richard.Earnshaw@buzzard.freeserve.co.uk>
From: John Fremlin <vii@users.sourceforge.net>
List: port-arm
Date: 06/01/2001 01:36:32
Richard Earnshaw <rearnsha@buzzard.freeserve.co.uk> writes:

> > set_current_stackptr is a low level primitive. Its semantics mean
> > that it does lowlevel stuff. If you use it wrong, you get
> > screwed. So what. Caveat emptor.
> 
> No; caveat the poor guy who's trying to work out why the kernel
> no-longer runs correctly if built with gcc-8.7!

That sucker has enough problems already, without gratuitously
inflicting these asm issues on him or her, I admit :-)

> > (Ben Harris suggested that the compiler might
> > mess with the stack with inline functions so I've turned it into a
> > macro.)

[...]

> > Please explain in more detail the reason for which it is impossible to
> > use. 
> 
> Try the following:
> 
> extern void *newstack;
> 
> my_func()
> {
>   printf("Running from old stack pointer\n");
>   set_current_stackptr(newstack);
>   printf("Running from new stack pointer: %d %d %d %d\n", 1, 2, 3, 4);
>   ...
> }
> 
> Nothing obviously wrong with this code

Yes there is. It is abusing the set_current_stackptr primitive. The
primitive was not intended to be exported to clueless people, it was
just an artifact of inlining the set_stackptr (as per the original
implementors intention). I first called it __set_stackptr to signal
that it was "private" &c. I could even make it disappear with #undef
if its existance irritates you.

[...]

> However, my objection is as much conceptual as it is based on any
> known problems that make it impossible right now.
> 
> My feeling is that you shouldn't be trying to encode something in
> the middle of a C function that fundamentally breaks the model the
> compiler is using.  A function that switches to another stack (in a
> non-temporary manner, as a subroutine might do) very definitely
> breaks this model.

If used like that yes. All sensible uses involve using it very
temporarily. Caveat emptor. If you use it wrong you get screwed. 

[...]

> I strongly believe that routines that want to mess with the stack in this 
> way (or, for that matter, want to switch processor mode), should kept 
> short and written entirely in assembler. 

I suppose it's a matter of attitude. As soon as you start writing
something in assembler, you tend to put bits that could just as well
be done in C in there. A case in point is the current SetCPSR routine.

(Gotta love the first comment for one thing)

ENTRY_NP(SetCPSR)
        mrs     r3, cpsr_all		/* Set the CPSR */
	bic	r2, r3, r0
        eor     r2, r2, r1
        msr     cpsr_all, r2

        mov	r0, r3			/* Return the old CPSR */

	mov	pc, lr

My inline assem version of SetCPSR is a lot clearer IMHO

static __inline u_int get_cpsr(void)
{
	u_int psr;
	__asm __volatile("mrs %0, CPSR" : "=r" (psr));
	return psr;
}

#define set_cpsr(psr) \
	__asm __volatile("msr CPSR_all, %0" : /* no outputs */ \
	    : "r" (psr) \
	    : "r8","r9","r10","r11","r12","r13","r14","cc" )

static __inline u_int SetCPSR(u_int mask, u_int eor)
{
	u_int psr = get_cpsr();
	psr &= ~mask;
	psr ^= eor;
	set_cpsr(psr);
	return psr;
}

And because the current version screws up the link register the poor
hpcarm blighters do this inline

	__asm("mov r0, sp; mov r1, lr; mrs r2, cpsr_all;");
	/* PSR_MODE, PSR_SVC32_MODE" */
	__asm("bic r2, r2, #31; orr r2, r2, #19;");
	__asm("msr cpsr_all, r2; mov sp, r0; mov lr, r1;");

For another bad example of assembly, this time from locore.S:

ASENTRY_NP(addrexc)
	mrs	r1, cpsr_all
	mrs	r2, spsr_all
	mov	r3, lr
	add	r0, pc, #Laddrexcmsg - . - 8
	bl	_C_LABEL(printf)
	b	data_abort_entry

Laddrexcmsg:
	.asciz	"address exception CPSR=%08x SPSR=%08x lr=%08x\n"
	.align	0

Most of which would be better written in C. The thing is, once you
start on assembly, it's hard to stop. And after all, if all you want
is one insn, then for goodness sake why call a function?

[...]

> Note that this doesn't mean that I think all use of "asm" is bad.
> Disabling (temporarily of course!) interrupts is perfectly OK; 
> as is using the swp instruction for semaphone type operations.  But
> that doesn't mean the feature should be abused by using it for
> things that just aren't likely to be safe -- that way lies all the
> problems that Linux kernel builds seem to have.

O dear. I'd like to point out once again that the NetBSD kernel is not
portable to gcc-2.95, whereas the current linux kernel will build
AFAIK correctly on egcs, gcc 2.95 (except alpha), gcc 2.96 redhat and
even on gcc CVS. 

[OK, enough NetBSD baiting ;-)]

-- 

	http://ape.n3.net