This is inside the AtomBIOS parser we got from AMD. AFAIR there is an alternative parser implementation (that is even endian safe), but nobody had time to validate it. Unless we have done that we probably won't fix this issue, because the code would be exchanged completely.

Most, if not all, of these unaligned accesses are in the AtomBIOS. This is just initialization code that is not performance critical. So if you can put up with the polluted kernel logs, I don't think it is much of a problem.

I am running into the same problem on the Itanium processor but I also have endianness problem with the same accesses. I started wrapping them all in a macro.

Ales Fiala



bugzilla-daemon@freedesktop.org wrote:
> http://bugs.freedesktop.org/show_bug.cgi?id=20593
>
>            Summary: radeonhd driver generates unaligned traps on Alpha
>            Product: xorg
>            Version: git
>           Platform: Alpha
>         OS/Version: Linux (All)
>             Status: NEW
>           Severity: minor
>           Priority: low
>          Component: Driver/radeonhd
>         AssignedTo: lverhaegen@suse.de
>         ReportedBy: mcree@orcon.net.nz
>          QAContact: xorg-team@lists.x.org
>                 CC: mcree@orcon.net.nz
>
>
> The radeonhd driver generates unaligned traps on Alpha architecture.  These are
> not a show stopper, but do incur very inefficient memory accesses via kernel
> traps, and pollute the kernel logs with annoying messages.
>
> Using radeonhd at git  df71658688e47614a3f660c28ac1dc77970eb491 
>
> But the version is irrelevant as the misaligned memory accesses have been there
> for a long time - I have been too lazy to report them until now.
>
> >From dmesg:
>
> X(2015): unaligned trap at 0000020000903088: 000002000098c5d7 28 1
> X(2015): unaligned trap at 0000020000903088: 000002000098c5df 28 1
> X(2015): unaligned trap at 0000020000903088: 000002000098c5e7 28 1
> X(2015): unaligned trap at 0000020000903088: 0000020000989771 28 1
>
> Connecting gdb to the running X process reveals the culprit:
>
> (gdb) list *0x0000020000903088
> 0x20000903088 is in GetParametersDirect32 (AtomBios/CD_Operations.c:441).
> 436     }
> 437     
> 438     UINT32 GetParametersDirect32(PARSER_TEMP_DATA STACK_BASED *    
> pParserTempData)
> 439     {
> 440         pParserTempData->CD_Mask.SrcAlignment=alignmentDword;
> 441        
> pParserTempData->Index=*(UINT32*)pParserTempData->pWorkingTableData->IP;
> 442         pParserTempData->pWorkingTableData->IP+=sizeof(UINT32);
> 443         return pParserTempData->Index;
> 444     }
> 445     
>
> Line 441 is a 32bit (long word) access.  The address dereferenced is not
> aligned to a 32bit boundary for an unaligned trap to occur.
>
> The ati driver is worse.  It coughs up unaligned traps from
> GetParametersDirect16() and GetParametersRegister() too, but I haven't seen
> these from radeonhd.
>
> Cheers
> Michael.
>
>
>   

(In reply to comment #2)
> I am running into the same problem on the Itanium processor but I also have
> endianness problem with the same accesses. I started wrapping them all in a
> macro.
>

the parser in radeon should be endian safe.

> --- Comment #2 from Ales Fiala <ales.fiala@hp.com>  2009-03-11  
> 12:41:35 PST ---
> Most, if not all, of these unaligned accesses are in the AtomBIOS.  
> This is just
> initialization code that is not performance critical. So if you can  
> put up with
> the polluted kernel logs, I don't think it is much of a problem.

I am aware of one distribution that considers unaligned traps bugs  
that, ideally, should be fixed before release.

For me the misaligned accesses raise the question:

How are data aligned in the AtomBIOS?  Are 16-bit data always aligned  
on 16-bit boundaries, 32-bit data on 32-bit boundaries, etc.?

If so, then the misaligned accesses represent bad accesses to the  
AtomBIOS that should be investigated.

If not, then the the routines should be rewritten to access the data  
on correctly aligned boundaries and reorganise the bits between two  
accesses to give the required datum.  This should be done for at least  
those architectures that do not support misaligned accesses.  (On the  
Alpha I am lucky these can be simulated by kernel traps.)

Michael.

The AtomBIOS data is not aligned at all.  16 and 32 bit words can appear 
on any byte boundary and they do.  AtomBios was written for x86 
architecture which is of course little endian and has no alignment 
issues.  I suspect one of their objectives was to make it as compact as 
possible since it resides in a flash ROM.

I started wrapping all these accesses in a macro that will do the endian 
byte swapping for me. It also solves the alignment problem at the same 
time since to do the byte swapping it is convenient to read the AtomBIOS 
data out one byte at a time anyway.

All this work was done just for my benefit so far and I haven't even 
considered whether to return any of this to X.org and whether they would 
even want it, since most people don't have a problem with it.

Ales Fiala


bugzilla-daemon@freedesktop.org wrote:
> http://bugs.freedesktop.org/show_bug.cgi?id=20593
>
>
>
>
>
> --- Comment #4 from Michael Cree <mcree@orcon.net.nz>  2009-03-11 16:55:26 PST ---
>   
>> --- Comment #2 from Ales Fiala <ales.fiala@hp.com>  2009-03-11  
>> 12:41:35 PST ---
>> Most, if not all, of these unaligned accesses are in the AtomBIOS.  
>> This is just
>> initialization code that is not performance critical. So if you can  
>> put up with
>> the polluted kernel logs, I don't think it is much of a problem.
>>     
>
> I am aware of one distribution that considers unaligned traps bugs  
> that, ideally, should be fixed before release.
>
> For me the misaligned accesses raise the question:
>
> How are data aligned in the AtomBIOS?  Are 16-bit data always aligned  
> on 16-bit boundaries, 32-bit data on 32-bit boundaries, etc.?
>
> If so, then the misaligned accesses represent bad accesses to the  
> AtomBIOS that should be investigated.
>
> If not, then the the routines should be rewritten to access the data  
> on correctly aligned boundaries and reorganise the bits between two  
> accesses to give the required datum.  This should be done for at least  
> those architectures that do not support misaligned accesses.  (On the  
> Alpha I am lucky these can be simulated by kernel traps.)
>
> Michael.
>
>
>   

(In reply to comment #5)
> The AtomBIOS data is not aligned at all.  16 and 32 bit words can appear 
> on any byte boundary and they do.

Yeah, that is what I was expecting.  Coupled with your statement following, it explains the lack of interest in these issues.

>  AtomBios was written for x86 
> architecture which is of course little endian and has no alignment 
> issues.  

x86 does have alignment issues.  I am lead to believe that accesses to a misaligned datum takes extra CPU clock cycles if it requires more than one memory access.  In time critical code that may be important. Admittedly that's not an issue for us.

> I started wrapping all these accesses in a macro that will do the endian 
> byte swapping for me. It also solves the alignment problem at the same 
> time since to do the byte swapping it is convenient to read the AtomBIOS 
> data out one byte at a time anyway.
> 
> All this work was done just for my benefit so far and I haven't even 
> considered whether to return any of this to X.org and whether they would 
> even want it, since most people don't have a problem with it.

Are people not wanting to run radeonhd on PPC?  Isn't that big endian?

Isn't Xorg 'sposed to be a multiplatform project?  If so, then porting access to the AtomBIOS, at some point in the development cycle, will become an issue.  I'm happy to accept that it cannot be priority at the moment, when there are other much more important issues to focus on, nevertheless I think it is good to list these issues at this stage, and get a feel for how many people are impacted by them.  That will determine implementation focus at a later stage.

Michael.

the -ati driver has already ported the atombios parser to PowerPC, with help from IBM and Red Hat.

Don't think radeonhd have shown much interest in taking the changes into their driver.

Now we do have another atom parser used for kernel modesetting, this parser is more likely to be useable to avoid this stuff.

It mostly does byte accesses and or's them together which might help.

It's clear radeonhd is dead and offers nothing over -ati.

Marking as WONTFIX.