On Wed, Aug 08, 2018 at 09:54:22AM +0200, Jean Delvare wrote: > Hi Neil, > > On Tue, 7 Aug 2018 12:00:13 -0400, Neil Horman wrote: > > On Tue, Aug 07, 2018 at 03:14:22PM +0200, Jean Delvare wrote: > > > Is it a "recent" addition? I am certain that it used to crash, back in > > > 2003 - I did not implement ALIGNMENT_WORKAROUND just for fun. The git > > > log also has evidence that gcc used to complain about unaligned > > > structure member copies. If you can point me to the kernel piece of > > > code which deals with that, I'm interested. > > Look to arch/ia64/kernel/unaligned.c > > > > Git history has it as far back as 2005, but thats the import to the current > > tree, so I'm guessing it goes back a bit farther than that. > > > > > That being said, I am > > > afraid that it must be slower to do things wrong and let the kernel > > > cope with the mess, than do the right thing directly? > > > > It is slower of course, because you have to take a trap and fix up the > > access > > (and then the kernel logs it). Its not something you want to happen, I'm > > just > > saying its no longer fatal. That said, the right thing to use is the > > get_unaligned macro to avoid the misaligned access in the first place. > > OK. Then maybe I remember wrong and the workaround in dmidecode was not > to prevent an actual crash but to prevent flooding the kernel log with > warnings. Either way, the workaround is there to stay, in one form or > another, for the reason you just explained. > Thats fine, get_aligned does the same thing that your macro does regardless.
> > > (...) > > > get_unaligned() doesn't seem to be a standard user-space function. > > > > Its standard for linux, but no, its not part of any specification. > > Where is it? I can't find it. > > $ gcc -Wall -W -Wno-unused test-unaligned.c -o test-unaligned > test-unaligned.c: In function 'main': > test-unaligned.c:13:2: warning: implicit declaration of function > 'get_unaligned' [-Wimplicit-function-declaration] > j = get_unaligned(i); > ^ > /tmp/ccJIuhvD.o: In function `main': > test-unaligned.c:(.text+0x69): undefined reference to `get_unaligned' > collect2: error: ld returned 1 exit status > $ grep -r get_unaligned /usr/include > $ > Hmm, thats odd. It was in linux/unaligned.h as part of the kernel headers, and I thought that got exported as part of the user space set, but it doesn't seem to be, so perhaps I'm the one misremembering. > > (...) > > I keep looking at the protocol count field, which the spec lists as being at > > 0x6+n bytes, where n is defined as the interface specific data length from > > earlier in the structure, but I keep finding it on this system at 0x7+n, so > > either they put the count 1 too many bytes in, or the spec if vague as to > > weather the interface specific data length includes the length byte itself. > > Your dump says it complies with SMBIOS 3.0.0, not 3.2.0. The type 42 > structure definition was different in 3.0.0, specifically there was no > length byte at offset 0x05, instead the interface type specific data > was immediately at 0x05. The length of that data had to be inferred from > the data itself, which was pretty fragile (and thus was later changed.) > > I was thinking that maybe that could explain that off-by-one you are > seeing. Unfortunately my theory doesn't really fly. If your data was > actually following the old definition, then it means that the device > type would have value 0x06, which is not specified by DSP0270. If the > type is at offset 0x07, value is 0x02, which is USB, which is specified > and presumably what you expect. > > Unfortunately (again), that means that 0x06 (at offset 0x05) is the > variable length, which is not sufficient to hold the device type (1 > byte, USB) and the device type specific data (at least 6 bytes for a > USB device, more if you include the bString). Very fishy. Maybe the > manufacturer started with SMBIOS 3.0.0, then noticed the change in > 3.2.0 and tried to adjust, but messed up. > That could well be, the system is an engineering sample. Regardless, do you think we need to add some sanity check for SMBIOS version 3 where that length isn't specified, and refuse to decode it for the ambiguity it provides? > To make things worse, it seems that the vendor added another length > byte which is not in the specification: > > Handle 0x2E30, DMI type 42, 170 bytes > Header and Data: > 2A AA 30 2E 40 06 02 00 10 40 B3 04 01 9C 04 9A > ^^ ^^ ^^ ^^ > 16 76 7C B6 46 1F 11 E7 BA 4F C7 78 8C 80 94 22 > 01 01 A9 FE 5F 78 00 00 00 00 00 00 00 00 00 00 > 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 01 01 A9 FE 5F 76 00 00 00 00 00 00 00 00 > 00 00 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 BB 01 00 00 00 00 09 58 43 43 2D 37 > 58 31 32 2D 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 > > Assuming that 01 is the number of protocol records, and 04 is the type > of the first (and only) protocol (Redfish), 9A would be the length of > that protocol (which matches the specification and is consistent with > the overall DMI structure length). 9C would apparently be the total > length of ALL the protocols. It is consistent with the length of the DMI > structure, the problem is that it doesn't follow the SMBIOS > specification, neither version 3.2.0 nor previous versions. > Yeah, its just broken. > So no matter how you look at it, your data is invalid. Is it a > production system? You'll have to talk to the manufacturer and/or look > for a BIOS update. They must follow the SMBIOS specification they claim > to implement. And if they are serious about that Redfish thing then > they really should implement SMBIOS 3.2.0. > No, its pre-production, and pretty old. I'm getting a newer system this week, and should be spec compliant. If not, i'm bringing it up with the manufacturer. Best Neil > -- > Jean Delvare > SUSE L3 Support > _______________________________________________ https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
