Re: [PATCH] efi/bgrt: Accept BGRT tables with a version of 0 on Lenovo laptops

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 1/28/20 8:48 AM, Ard Biesheuvel wrote:
> On Sun, 26 Jan 2020 at 16:02, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Some (somewhat older) Lenovo laptops have a correct BGRT table, except
>> that the version field is 0 instead of 1.
>>
>> Quickly after finding this out, even before submitting a first version of
>> this patch upstream, the list of DMI matches for affected models grew to
>> 3 models (all Ivy Bridge based).
>>
>> So rather then maintaining an ever growing list with DMI matches for
>> affected Lenovo models, this commit simply checks if the vendor is Lenovo
>> when the version is 0 and in that case accepts the out of spec version
>> working around the Lenovo firmware bug.
>>
>> Cc: stable@vger.kernel.org
>> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1791273
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/firmware/efi/efi-bgrt.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
>> index b07c17643210..3a2d6d3a590b 100644
>> --- a/drivers/firmware/efi/efi-bgrt.c
>> +++ b/drivers/firmware/efi/efi-bgrt.c
>> @@ -15,6 +15,7 @@
>>   #include <linux/acpi.h>
>>   #include <linux/efi.h>
>>   #include <linux/efi-bgrt.h>
>> +#include <linux/dmi.h>
>>
>>   struct acpi_table_bgrt bgrt_tab;
>>   size_t bgrt_image_size;
>> @@ -42,7 +43,12 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
>>                  return;
>>          }
>>          *bgrt = *(struct acpi_table_bgrt *)table;
>> -       if (bgrt->version != 1) {
>> +       /*
>> +        * Some older Lenovo laptops have a correct BGRT table, except that
>> +        * the version field is 0 instead of 1.
>> +        */
>> +       if (bgrt->version != 1 &&
>> +           !(bgrt->version == 0 && dmi_name_in_vendors("LENOVO"))) {
>>                  pr_notice("Ignoring BGRT: invalid version %u (expected 1)\n",
>>                         bgrt->version);
>>                  goto out;
> 
> Hi Hans,
> 
> Given that the ACPI spec only defines a single version for this table,
> which is version #1, wouldn't it be simpler to just assume that
> version #0 means version #1 in all cases, rather than using DMI
> matches for that? There is no risk of misidentifying another table
> version, since none exist ...

Ok, I will prepare a new version doing this.

Regards,

Hans