| Message ID | 1505165777-17121-1-git-send-email-jhugo@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] dmi: dmicheck: Add BMC Interface Type definitions from SMBIOS spec | expand |
On 2017-09-11 02:36 PM, 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@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); > break; > > case 39: /* 7.40 */ > Acked-by: Alex Hung <alex.hung@canonical.com>
-----Original Message----- From: fwts-devel [mailto:fwts-devel-bounces@lists.ubuntu.com] On Behalf Of Alex Hung Sent: Tuesday, September 12, 2017 10:34 AM To: fwts-devel@lists.ubuntu.com Subject: ACK: [PATCH 1/2] dmi: dmicheck: Add BMC Interface Type definitions from SMBIOS spec On 2017-09-11 02:36 PM, 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@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 info. 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); IMO this change re-introduces the bug, that was fixed in : 00445adbcb5a ("dmi: dmicheck: add SBBR compliance tests") > break; > > case 39: /* 7.40 */ > Acked-by: Alex Hung <alex.hung@canonical.com> -- fwts-devel mailing list fwts-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
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@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); > break; > > case 39: /* 7.40 */ > Acked-by: Colin Ian King <colin.king@canonical.com>
Hi, My two cents on this patch. Any comments? Thanks, Sakar -----Original Message----- From: fwts-devel [mailto:fwts-devel-bounces@lists.ubuntu.com] On Behalf Of Colin Ian King Sent: Tuesday, September 12, 2017 1:31 PM To: fwts-devel@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@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") > break; > > case 39: /* 7.40 */ > Acked-by: Colin Ian King <colin.king@canonical.com> -- fwts-devel mailing list fwts-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
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@lists.ubuntu.com] On Behalf Of Colin Ian King > Sent: Tuesday, September 12, 2017 1:31 PM > To: fwts-devel@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@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.
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); break; case 39: /* 7.40 */
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@codeaurora.org> --- src/dmi/dmicheck/dmicheck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)