[coreboot-gerrit] Patch merged into coreboot/master: Revert "arch/x86/smbios: Correct manufacturer ID"

gerrit at coreboot.org gerrit at coreboot.org
Sat Dec 17 17:53:46 CET 2016


the following patch was just integrated into master:
commit a01695bf9ac5ad401950af96682b494127ad8a8d
Author: Nico Huber <nico.h at gmx.de>
Date:   Wed Dec 14 23:48:09 2016 +0100

    Revert "arch/x86/smbios: Correct manufacturer ID"
    
    This reverts commit c86da67436827c25919a2f5966049485a58fc984.
    
    Alas, I have to disagree with this in every single line. The comment
    added to the top of the file only applies to a single function therein
    which sits over a hundred lines below. That's not much helpful. More-
    over, the link in the comment is already down ofc.
    
    The comment is also irritating as it doesn't state in which way (enco-
    ding!) it applies to the code, which presumably led to the wrong in-
    terpretation of the IDs.
    
    At last, if anything should have changed it is the strings, the IDs
    are resolved to. `smbios_fill_dimm_manufacturer_from_id()` has to
    resolve the IDs it gets actually fed and not a random selection from
    any spec.
    
    Since I digged into it, here's why the numbers are correct: The func-
    tion started with the SPD encoding of DDR3 in mind. There, the lower
    byte is the number of a "bank" of IDs with an odd-parity in the upper
    most bit. The upper byte is the ID within the bank. The "correction"
    was to clear the parity bit for naught. The function was later exten-
    ded with IDs in the DDR2-SPD encoding (which is actually 64-bit not
    16). There, a byte, starting from the lowest, is either an ID below
    127 plus odd-parity, or 127 which means look in the next byte/bank.
    Unused bytes seem to be filled with 0xff, I guess from the 0xff2c.
    
    Change-Id: Icdb48e4f2c102f619fbdca856e938e85135cfb18
    Reviewed-on: https://review.coreboot.org/17873
    Tested-by: build bot (Jenkins)
    Reviewed-by: Matt DeVillier <matt.devillier at gmail.com>
    Reviewed-by: HAOUAS Elyes <ehaouas at noos.fr>
    Reviewed-by: Paul Menzel <paulepanter at users.sourceforge.net>
    Reviewed-by: Martin Roth <martinroth at google.com>


See https://review.coreboot.org/17873 for details.

-gerrit



More information about the coreboot-gerrit mailing list