Re: CVS commit: src/sys/arch/x86/x86

[christos%astron.com@localhost (Christos Zoulas)]

In article <20180708092413.GB8981%mail.duskware.de@localhost>,
Martin Husemann  <martin%duskware.de@localhost> wrote:
>On Sun, Jul 08, 2018 at 10:49:53AM +0200, Jaromír Dole?ek wrote:
>> > Module Name:    src
>> > Committed By:   kamil
>> > Date:           Sat Jul  7 23:05:50 UTC 2018
>> >
>> > Modified Files:
>> >         src/sys/arch/x86/x86: mpbios.c
>> >
>> > Log Message:
>> > Remove unaligned access to mpbios_page[]
>> >
>> > Replace unaligned pointer dereference with a more portable construct that
>> > is free from Undefined Behavior semantics.
>> >
>> > sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address 0xffff800031c7a413 for type 'const
>__uint16_t' which requires 2 byte alignment
>
>
>Can we please do NOT do such stupid changes?
>
>This is a bogus error message, please restore the original code!

These changes are pointless; how much code will we need to change
to silence mis-aligned warnings? These changes are also dangerous
when it comes to reading from devices (where multiple reads can
behave differently). If you want to silence the warnings, use
__attribute__, but even that is of questionable use. I'd venture
to say, misaligned warnings on cpus where aligned access (to do
the aligning) is costlier than direct misaligned accesses (like
x86) are useless. In addition, it is not like we would ever turn
on the force alignment bit on an x86 cpu and have things work!

So my preference would be to revert the change and take this up
to tech-kern first (how misaligned accesses should be treated),
because the situation is not that simple (when it comes to things
like SSE operations etc.)
http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html


Best,

christos