Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
From: Nicolai Stange <hidden>
Date: 2017-01-12 11:54:47
Also in:
lkml
Possibly related (same subject, not in this thread)
- 2017-01-17 · Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code · Ard Biesheuvel <hidden>
- 2017-01-16 · Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code · Bhupesh Sharma <hidden>
- 2017-01-16 · Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code · Dave Young <hidden>
- 2017-01-13 · Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code · Nicolai Stange <hidden>
- 2017-01-13 · Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code · Dave Young <hidden>
On Thu, Jan 12 2017, Dave Young wrote:
-void __init efi_bgrt_init(void)
+void __init efi_bgrt_init(struct acpi_table_header *table)
{
- acpi_status status;
void *image;
struct bmp_header bmp_header;
if (acpi_disabled)
return;
- status = acpi_get_table("BGRT", 0,
- (struct acpi_table_header **)&bgrt_tab);
- if (ACPI_FAILURE(status))
- return;Not sure, but wouldn't it be safer to reverse the order of this assignment
+ bgrt_tab = *(struct acpi_table_bgrt *)table;
and this length check
- if (bgrt_tab->header.length < sizeof(*bgrt_tab)) {
+ if (bgrt_tab.header.length < sizeof(bgrt_tab)) {
pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
- bgrt_tab->header.length, sizeof(*bgrt_tab));
+ bgrt_tab.header.length, sizeof(bgrt_tab));
return;
}? Also, from here on, all error paths should zero out bgrt_tab.image_address (or so) to signal failure to bgrt_init(): bgrt_init() now checks for !bgrt_tab.image_address whereas before it had tested bgrt_image and the latter used to be set at the very end of efi_bgrt_init().
quoted hunk
- if (bgrt_tab->version != 1) { + if (bgrt_tab.version != 1) { pr_notice("Ignoring BGRT: invalid version %u (expected 1)\n", - bgrt_tab->version); + bgrt_tab.version); return; } - if (bgrt_tab->status & 0xfe) { + if (bgrt_tab.status & 0xfe) { pr_notice("Ignoring BGRT: reserved status bits are non-zero %u\n", - bgrt_tab->status); + bgrt_tab.status); return; } - if (bgrt_tab->image_type != 0) { + if (bgrt_tab.image_type != 0) { pr_notice("Ignoring BGRT: invalid image type %u (expected 0)\n", - bgrt_tab->image_type); + bgrt_tab.image_type); return; } - if (!bgrt_tab->image_address) { + if (!bgrt_tab.image_address) { pr_notice("Ignoring BGRT: null image address\n"); return; } - image = memremap(bgrt_tab->image_address, sizeof(bmp_header), MEMREMAP_WB); + image = early_memremap(bgrt_tab.image_address, sizeof(bmp_header)); if (!image) { pr_notice("Ignoring BGRT: failed to map image header memory\n"); return; } memcpy(&bmp_header, image, sizeof(bmp_header)); - memunmap(image); + early_memunmap(image, sizeof(bmp_header)); if (bmp_header.id != 0x4d42) { pr_notice("Ignoring BGRT: Incorrect BMP magic number 0x%x (expected 0x4d42)\n", bmp_header.id);@@ -82,12 +77,5 @@ void __init efi_bgrt_init(void) } bgrt_image_size = bmp_header.size; - bgrt_image = memremap(bgrt_tab->image_address, bmp_header.size, MEMREMAP_WB); - if (!bgrt_image) { - pr_notice("Ignoring BGRT: failed to map image memory\n"); - bgrt_image = NULL; - return; - } - - efi_mem_reserve(bgrt_tab->image_address, bgrt_image_size); + efi_mem_reserve(bgrt_tab.image_address, bgrt_image_size); } --- linux-x86.orig/drivers/acpi/bgrt.c +++ linux-x86/drivers/acpi/bgrt.c@@ -15,40 +15,41 @@ #include <linux/sysfs.h> #include <linux/efi-bgrt.h> +static void *bgrt_image;
[...]
quoted hunk
@@ -84,9 +85,17 @@ static int __init bgrt_init(void) { int ret; - if (!bgrt_image) + if (!bgrt_tab.image_address) return -ENODEV; + bgrt_image = memremap(bgrt_tab.image_address, bgrt_image_size, + MEMREMAP_WB); + if (!bgrt_image) { + pr_notice("Ignoring BGRT: failed to map image memory\n"); + bgrt_image = NULL; + return -ENOMEM; + } + bin_attr_image.private = bgrt_image; bin_attr_image.size = bgrt_image_size;
Thanks, Nicolai