[Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Ivan Khoronzhuk]

The dmi-sysfs should create "End of Table" entry, that is type 127.
But after adding initial SMBIOS v3 support the 127-0 entry is not
handled any more, as result it's not created in sysfs.
This is important because the size of whole DMI table must correspond
to sum of all DMI entry sizes.

So move "end-of-table" check after it's handled by decode.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---

v2..v1:
	Move end of table check after it's handled instead of removing
	Correct commit

 drivers/firmware/dmi_scan.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index c5f7b4e..a44b87c 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -93,12 +93,6 @@ static void dmi_table(u8 *buf, int len, int num,
 		const struct dmi_header *dm = (const struct dmi_header *)data;
 
 		/*
-		 * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
-		 */
-		if (dm->type == DMI_ENTRY_END_OF_TABLE)
-			break;
-
-		/*
 		 *  We want to know the total length (formatted area and
 		 *  strings) before decoding to make sure we won't run off the
 		 *  table in dmi_decode or dmi_string
@@ -108,6 +102,13 @@ static void dmi_table(u8 *buf, int len, int num,
 			data++;
 		if (data - buf < len - 1)
 			decode(dm, private_data);
+
+		/*
+		 * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
+		 */
+		if (dm->type == DMI_ENTRY_END_OF_TABLE)
+			break;
+
 		data += 2;
 		i++;
 	}
-- 
1.9.1

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Ard Biesheuvel]

On 4 February 2015 at 14:22, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> The dmi-sysfs should create "End of Table" entry, that is type 127.
> But after adding initial SMBIOS v3 support the 127-0 entry is not
> handled any more, as result it's not created in sysfs.
> This is important because the size of whole DMI table must correspond
> to sum of all DMI entry sizes.
>
> So move "end-of-table" check after it's handled by decode.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

Reviewed-by: Ard Biesheuvel <ard@linaro.org>

> ---
>
> v2..v1:
>         Move end of table check after it's handled instead of removing
>         Correct commit
>
>  drivers/firmware/dmi_scan.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index c5f7b4e..a44b87c 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -93,12 +93,6 @@ static void dmi_table(u8 *buf, int len, int num,
>                 const struct dmi_header *dm = (const struct dmi_header *)data;
>
>                 /*
> -                * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
> -                */
> -               if (dm->type == DMI_ENTRY_END_OF_TABLE)
> -                       break;
> -
> -               /*
>                  *  We want to know the total length (formatted area and
>                  *  strings) before decoding to make sure we won't run off the
>                  *  table in dmi_decode or dmi_string
> @@ -108,6 +102,13 @@ static void dmi_table(u8 *buf, int len, int num,
>                         data++;
>                 if (data - buf < len - 1)
>                         decode(dm, private_data);
> +
> +               /*
> +                * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
> +                */
> +               if (dm->type == DMI_ENTRY_END_OF_TABLE)
> +                       break;
> +
>                 data += 2;
>                 i++;
>         }
> --
> 1.9.1
>

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Ivan Khoronzhuk]

Hi, Matt

Could you please pick up this patch.

On 02/05/2015 12:36 PM, Ard Biesheuvel wrote:
> On 4 February 2015 at 14:22, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> The dmi-sysfs should create "End of Table" entry, that is type 127.
>> But after adding initial SMBIOS v3 support the 127-0 entry is not
>> handled any more, as result it's not created in sysfs.
>> This is important because the size of whole DMI table must correspond
>> to sum of all DMI entry sizes.
>>
>> So move "end-of-table" check after it's handled by decode.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> Reviewed-by: Ard Biesheuvel <ard@linaro.org>
>
>> ---
>>
>> v2..v1:
>>          Move end of table check after it's handled instead of removing
>>          Correct commit
>>
>>   drivers/firmware/dmi_scan.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
>> index c5f7b4e..a44b87c 100644
>> --- a/drivers/firmware/dmi_scan.c
>> +++ b/drivers/firmware/dmi_scan.c
>> @@ -93,12 +93,6 @@ static void dmi_table(u8 *buf, int len, int num,
>>                  const struct dmi_header *dm = (const struct dmi_header *)data;
>>
>>                  /*
>> -                * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
>> -                */
>> -               if (dm->type == DMI_ENTRY_END_OF_TABLE)
>> -                       break;
>> -
>> -               /*
>>                   *  We want to know the total length (formatted area and
>>                   *  strings) before decoding to make sure we won't run off the
>>                   *  table in dmi_decode or dmi_string
>> @@ -108,6 +102,13 @@ static void dmi_table(u8 *buf, int len, int num,
>>                          data++;
>>                  if (data - buf < len - 1)
>>                          decode(dm, private_data);
>> +
>> +               /*
>> +                * 7.45 End-of-Table (Type 127) [SMBIOS reference spec v3.0.0]
>> +                */
>> +               if (dm->type == DMI_ENTRY_END_OF_TABLE)
>> +                       break;
>> +
>>                  data += 2;
>>                  i++;
>>          }
>> --
>> 1.9.1
>>

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Matt Fleming]

On Wed, 04 Feb, at 04:22:05PM, Ivan Khoronzhuk wrote:
> The dmi-sysfs should create "End of Table" entry, that is type 127.
> But after adding initial SMBIOS v3 support the 127-0 entry is not
> handled any more, as result it's not created in sysfs.
> This is important because the size of whole DMI table must correspond
> to sum of all DMI entry sizes.
> 
> So move "end-of-table" check after it's handled by decode.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> 
> v2..v1:
> 	Move end of table check after it's handled instead of removing
> 	Correct commit
> 
>  drivers/firmware/dmi_scan.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

The way that the commit log is written makes this sound like a
regression. If that's the case, then you need to list which commit
introduced the regression, because that will tell me whether it needs
tagging for stable.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Ivan Khoronzhuk]

Ok, I'll correct commit msg like:

But after adding initial SMBIOS v3 support (fc43026278b23b3515cf8f909ec29df94b3ae1a2)
the 127-0 entry is nothandled any more, as result it's not created in sysfs.


On 02/18/2015 03:04 PM, Matt Fleming wrote:
> On Wed, 04 Feb, at 04:22:05PM, Ivan Khoronzhuk wrote:
>> The dmi-sysfs should create "End of Table" entry, that is type 127.
>> But after adding initial SMBIOS v3 support the 127-0 entry is not
>> handled any more, as result it's not created in sysfs.
>> This is important because the size of whole DMI table must correspond
>> to sum of all DMI entry sizes.
>>
>> So move "end-of-table" check after it's handled by decode.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>
>> v2..v1:
>> 	Move end of table check after it's handled instead of removing
>> 	Correct commit
>>
>>   drivers/firmware/dmi_scan.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
> The way that the commit log is written makes this sound like a
> regression. If that's the case, then you need to list which commit
> introduced the regression, because that will tell me whether it needs
> tagging for stable.
>

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Matt Fleming]

On 18 February 2015 at 13:20, Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
> Ok, I'll correct commit msg like:
>
> But after adding initial SMBIOS v3 support
> (fc43026278b23b3515cf8f909ec29df94b3ae1a2)
> the 127-0 entry is nothandled any more, as result it's not created in sysfs.

Sounds good. FYI, the canonical way to reference commits is commit
<12-digit hash> ("<commit title>"), e.g...

But after adding initial SMBIOS v3 support in commit fc43026278b2
("dmi: add support for SMBIOS 3.0 64-bit entry point") the 127-0 entry
is not handled any more, as a result it's not created in sysfs.

Re: [Patch v2] firmware: dmi_scan: fix dmi scan to handle "End of Table" structure

[Ivan Khoronzhuk]

On 02/18/2015 03:38 PM, Matt Fleming wrote:
> On 18 February 2015 at 13:20, Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>> Ok, I'll correct commit msg like:
>>
>> But after adding initial SMBIOS v3 support
>> (fc43026278b23b3515cf8f909ec29df94b3ae1a2)
>> the 127-0 entry is nothandled any more, as result it's not created in sysfs.
> Sounds good. FYI, the canonical way to reference commits is commit
> <12-digit hash> ("<commit title>"), e.g...
>
> But after adding initial SMBIOS v3 support in commit fc43026278b2
> ("dmi: add support for SMBIOS 3.0 64-bit entry point") the 127-0 entry
> is not handled any more, as a result it's not created in sysfs.

Done. v3