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.
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.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.