Re: suspicious code in ioapic locking

[Andrew Doran <ad%netbsd.org@localhost>]

On Thu, Apr 24, 2008 at 08:03:09PM +0100, Andrew Doran wrote:

> On Wed, Apr 23, 2008 at 01:20:40AM -0400, Stephen Degler wrote:
> 
> > The apic handling routines embedded in the macros in vector.S come from 
> > i82093reg.h, and operate on struct pic (sys/x86/include/pic.h).  The 
> > __cpu_simple_lock_t in the struct pic is now a byte, so the code is 
> > wrong.  This doesn't seem to break anything, probably due to structure 
> > padding.  The following patch corrects the ioapic code.  Another 
> > approach would be to rearrange the register usage so that the simple 
> > lock routines could be called directly, but this would be more 
> > involved.  In the patch below, I used %bl because %rbx is blown away in 
> > the macros anyway.
> > 
> > Let me know if you think I should commit this.
> 
> cmpxchgb %bl,PIC_LOCK(%rdi)                    ;       \ 
> 
> That should be:
> 
>       lock
>       cmpxchgb        ...

Actually, I think it's still not right because cmpxchg depends on %al and I
don't see it being filled. It's probably better to stick with xchgb so there
is no additional register being used - as I mentioned the register usage
there is really tricky and I know that it has tripped me up a few times.

Andrew