[PATCH 1/2] dmi: dmicheck: Add BMC Interface Type definitions from SMBIOS spec

Jeffrey Hugo jhugo at codeaurora.org
Tue Sep 12 13:45:37 UTC 2017


On 9/12/2017 5:04 AM, Sakar Arora wrote:
> Hi,
> 
> My two cents on this patch. Any comments?
> 
> Thanks,
> Sakar
> 
> -----Original Message-----
> From: fwts-devel [mailto:fwts-devel-bounces at lists.ubuntu.com] On Behalf Of Colin Ian King
> Sent: Tuesday, September 12, 2017 1:31 PM
> To: fwts-devel at lists.ubuntu.com
> Subject: ACK: [PATCH 1/2] dmi: dmicheck: Add BMC Interface Type definitions from SMBIOS spec
> 
> On 11/09/17 22:36, Jeffrey Hugo wrote:
>> The SMBIOS spec defines BMC Interface Types in section 7.39.1.  Value
>> 0 is defined as "Unknown" (might be a platform specific interface),
>> and value 3 is "BT: Block Transfer".  Update the test to accept these definitions.
>>
>> Fixes: 00445adbcb5a ("dmi: dmicheck: add SBBR compliance tests")
>> Signed-off-by: Jeffrey Hugo <jhugo at codeaurora.org>
>> ---
>>   src/dmi/dmicheck/dmicheck.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
>> index 61c6b81..c0e78f0 100644
>> --- a/src/dmi/dmicheck/dmicheck.c
>> +++ b/src/dmi/dmicheck/dmicheck.c
>> @@ -1772,7 +1772,7 @@ static void dmicheck_entry(fwts_framework *fw,
>>
>>   dmi_reserved_bits_check(fw, table, addr, "Base Addr Modifier/Interrupt Info", hdr, sizeof(uint8_t), 0x10, 2, 2);
>>   dmi_reserved_bits_check(fw, table, addr, "Base Addr Modifier/Interrupt Info", hdr, sizeof(uint8_t), 0x10, 5, 5);
>> -dmi_min_max_mask_uint8_check(fw, table, addr, "Base Addr Modifier/Interrupt Info)", hdr, 0x10, 0x1, 0x2, 6, 0x3);
>> +dmi_min_max_mask_uint8_check(fw, table, addr, "Base Addr
>> +Modifier/Interrupt Info)", hdr, 0x10, 0x0, 0x3, 6, 0x3);
> 
> The above test checks the range of the bits 6:7 of Base Addr Modifier/Interrupt Info at offset 0x10. As per SMBIOS spec that value 0x3 is reserved for these 2 bits. Hence the accepted values should be 0x0 to 0x2. The BMC interface type field is at offset 0x4 which is being correctly tested for range 0x0 to 0x3 in the first line of this switch case block.
> 
> dmi_min_max_uint8_check(fw, table, addr, "Interface Type", hdr, 0x4, 0x0, 0x3);
> 
> This change re-introduces the bug, that was fixed in : 00445adbcb5a ("dmi: dmicheck: add SBBR compliance tests")

You are right, partially.  3 is not valid here, but 0 is, which you 
didn't fix last time (and your commit message wasn't clear exactly about 
what you were fixing).

I'll rework this series.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.



More information about the fwts-devel mailing list