[APIC0] warning: misaligned APIC access

[mutex]

Hi,

I have been staring blind into a small bug. Cannot seem to really understand what is wrong..

Bochs says:

00105619653i[APIC0] warning: misaligned APIC access. addr=0x00000000fee00301

The function that causes this message is my apicWaitReady that checks for ICR_BUSY in the ICR1 register.. Basically a while loop.

Code: Select all

void apicWaitReady() {
    kprintf(": Waiting for APIC ready\n");

    kprintf("%.8x\n",apicaddr);
    kprintf("%.8x\n",APIC_REG_ICR1);

    memReadFrom(apicaddr + APIC_REG_ICR1);

    while(memReadFrom(apicaddr + APIC_REG_ICR1) & APIC_ICR_BUSY) {
    	// Wait for apic to become ready
    }

    kprintf("%.8x\n",apicaddr);
    kprintf("%.8x\n",APIC_REG_ICR1);
}

I have added some statements to do some debugging. All kprintf statements show 0xfee00000 and 0x300, the single read statement works fine, so its the while block that by some reason manages to get this wrong.

apicaddr is defined globaly and the helper function memReadFrom looks like this; I also made apicaddr volatile..

GCC optimizations are -O0 so they are disabled. Same issue goes for -O1 to -O3 so basically its a common problem.
^- This was not correct. I had defined -02.. When setting to -O0 it works without issue.. Hmm..

Code: Select all

volatile dword apicaddr;

dword memReadFrom(dword addr) {
    return *((dword *) addr);
}

Anyone sees something very wrong here??

Next step now is singlestepping the while-loop and checking what is going on..

regards
Thomas

[mutex]

Here is what the assembly listing of apicWaitReady looks like with -O1 - O3

Code: Select all

c00041b0 <_apicWaitReady>:
c00041b0:       83 ec 1c                sub    $0x1c,%esp
c00041b3:       c7 04 24 4b 22 01 c0    movl   $0xc001224b,(%esp)
c00041ba:       e8 f1 ee ff ff          call   c00030b0 <_kprintf>
c00041bf:       90                      nop
c00041c0:       a1 20 a3 01 c0          mov    0xc001a320,%eax
c00041c5:       f6 80 01 03 00 00 10    testb  $0x10,0x301(%eax)
c00041cc:       75 f2                   jne    c00041c0 <_apicWaitReady+0x10>
c00041ce:       83 c4 1c                add    $0x1c,%esp
c00041d1:       c3                      ret

and without any optimizing

Code: Select all

c0004231 <_apicWaitReady>:
c0004231:       55                      push   %ebp
c0004232:       89 e5                   mov    %esp,%ebp
c0004234:       83 ec 18                sub    $0x18,%esp
c0004237:       c7 04 24 4b f2 00 c0    movl   $0xc000f24b,(%esp)
c000423e:       e8 4d ee ff ff          call   c0003090 <_kprintf>
c0004243:       90                      nop
c0004244:       a1 20 73 01 c0          mov    0xc0017320,%eax
c0004249:       05 00 03 00 00          add    $0x300,%eax
c000424e:       89 04 24                mov    %eax,(%esp)
c0004251:       e8 d7 fa ff ff          call   c0003d2d <_memReadFrom>
c0004256:       25 00 10 00 00          and    $0x1000,%eax
c000425b:       85 c0                   test   %eax,%eax
c000425d:       75 e5                   jne    c0004244 <_apicWaitReady+0x13>
c000425f:       c9                      leave
c0004260:       c3                      ret

Looks like some over-optimizing and/or inlining is going on.. Ends up breaking the code..

[xenos]

You declare "apicaddr" as volatile - however, it's not the variable apicaddr that should be volatile, but rather the memory location you are reading from. So your code should rather look like this (even though it looks a bit cumbersome to me to declare a function just to read from some memory location, instead of just using a pointer):

Code: Select all

dword apicaddr;

dword memReadFrom(dword addr) {
    return *((volatile dword *) addr);
}

Although I'm not quite sure whether it solves the problem that shows up in your disassembly...

[mutex]

Update;

Preventin inlining of the memReadFrom does the trick.

Code: Select all

__attribute__ ((noinline)) dword memReadFrom(dword addr) {
    return *((dword *) addr);
}

Probably the compiler manages to think that the return statement can be over-simplified and that causes my bug..

Anyone with a god theory why its optimized to this?

[xenos]

No idea where that extra 0x1 comes from yet, though.

I guess GCC replaces the

Code: Select all

c0004256:       25 00 10 00 00          and    $0x1000,%eax
c000425b:       85 c0                   test   %eax,%eax

with a

Code: Select all

c00041c5:       f6 80 01 03 00 00 10    testb  $0x10,0x301(%eax)

because it assumes that checking the corresponding bit can be done easier with a simple testb of bit 5 at apicaddr + 0x301 than first loading the dword at apicaddr + 0x300 into EAX and testing bit 13. I'm not quite sure how to resolve this problem and to force dword access even if optimization is enabled... It apparently works in my APIC code, but actually I never really thought about it:

http://xenos.svn.sourceforge.net/viewvc ... iew=markup
http://xenos.svn.sourceforge.net/viewvc ... iew=markup

Here's the disassembly from one of my functions that does a similar job (compiled with -O2):

Code: Select all

c0106830 <Kernel::Apic::ErrorPending() const>:
c0106830:	55                   	push   %ebp
c0106831:	89 e5                	mov    %esp,%ebp
c0106833:	8b 45 08             	mov    0x8(%ebp),%eax
c0106836:	5d                   	pop    %ebp
c0106837:	8b 80 70 03 00 00    	mov    0x370(%eax),%eax
c010683d:	f6 c4 10             	test   $0x10,%ah
c0106840:	0f 95 c0             	setne  %al
c0106843:	c3                   	ret

[mutex]

Interresting thoughts guys.

My optimized memReadFrom looks like this;

Code: Select all

c0003d50 <_memReadFrom>:
c0003d50:       8b 44 24 04             mov    0x4(%esp),%eax
c0003d54:       8b 00                   mov    (%eax),%eax
c0003d56:       c3                      ret
c0003d57:       89 f6                   mov    %esi,%esi
c0003d59:       8d bc 27 00 00 00 00    lea    0x0(%edi,%eiz,1),%edi

I use this function in the APIC code to simplify and make it more readable, but technically i could do without it.

It seems that the error checking code above does inline the code in a simplified manner. Other code uses it i can see from the listing.

[Brendan]

Hi,

Preventin inlining of the memReadFrom does the trick.

Unfortunately that has the disadvantage of preventing inlining.

Making it volatile properly should fix the problem without preventing the compiler from doing other optimisations (e.g. inlining, register selection, etc). For the while loop, you should end up with something like this:

Code: Select all

.l1:
    testl  $0x00000010,0x301(%eax)
    je .l1

If the compiler won't do what it's told (e.g. won't do 32-bit reads and insists on doing an 8-bit read even though it's volatile), use an inline assembly macro and consider sending a bug report to the compiler developers.


Cheers,

Brendan

[mutex]

Hi,

I guess making it a dword pointer instead of a dword typecasted in the memReadFrom() could probably be more "understandable" for GCC during compilation/optimizing.

Volatile on the dword only "volatiles" the value. A pointer might fix this.

I will do some testing. But if the pointer way does not work, i think it should be considered a flaw/bug in GCC optimizing.

Say tuned!

regards
Thomas

[mutex]

No luck on that part...

But what does seem to work is making the function signature like this "dword apicRead(volatile dword *addr, dword reg)"

Im not sure how happy im with GCC's ability to (over)optimize this part of my code.. No-inline or volatile does the trick though..

[xenos]

The important part is to have a pointer of type volatile dword* - I think this prevents GCC from breaking the dword value (which this pointer points to) into bytes and trying to perform bytewise operations on it.