From 15afae604651d4e17652d2ffb56f5e36f991cfef Mon Sep 17 00:00:00 2001 From: Gary Hade Date: Wed, 21 Mar 2012 15:28:50 -0700 Subject: [PATCH] ACPI, APEI: Fix incorrect APEI register bit width check and usage The current code incorrectly assumes that (1) the APEI register bit width is always 8, 16, 32, or 64 and (2) the APEI register bit width is always equal to the APEI register access width. ERST serialization instructions entries such as: [030h 0048 1] Action : 00 [Begin Write Operation] [031h 0049 1] Instruction : 03 [Write Register Value] [032h 0050 1] Flags (decoded below) : 01 Preserve Register Bits : 1 [033h 0051 1] Reserved : 00 [034h 0052 12] Register Region : [Generic Address Structure] [034h 0052 1] Space ID : 00 [SystemMemory] [035h 0053 1] Bit Width : 03 [036h 0054 1] Bit Offset : 00 [037h 0055 1] Encoded Access Width : 03 [DWord Access:32] [038h 0056 8] Address : 000000007F2D7038 [040h 0064 8] Value : 0000000000000001 [048h 0072 8] Mask : 0000000000000007 break this assumption by yielding: [Firmware Bug]: APEI: Invalid bit width in GAR [0x7f2d7038/3/0] I have found no ACPI specification requirements corresponding with the above assumptions. There is even a good example in the Serialization Instruction Entries section (ACPI 4.0 section 17.4,1.2, ACPI 4.0a section 2.5.1.2, ACPI 5.0 section 18.5.1.2) that mentions a serialization instruction with a bit range of [6:2] which is 5 bits wide, _not_ 8, 16, 32, or 64 bits wide. Compile and boot tested with 3.3.0-rc7 on a IBM HX5. Signed-off-by: Gary Hade Signed-off-by: Len Brown --- drivers/acpi/apei/apei-base.c | 61 +++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c index e5d53b7ddc7e..1d3656f6919f 100644 --- a/drivers/acpi/apei/apei-base.c +++ b/drivers/acpi/apei/apei-base.c @@ -558,33 +558,48 @@ void apei_resources_release(struct apei_resources *resources) } EXPORT_SYMBOL_GPL(apei_resources_release); -static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr) +static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr, + u32 *access_bit_width) { - u32 width, space_id; + u32 bit_width, bit_offset, access_size_code, space_id; - width = reg->bit_width; + bit_width = reg->bit_width; + bit_offset = reg->bit_offset; + access_size_code = reg->access_width; space_id = reg->space_id; /* Handle possible alignment issues */ memcpy(paddr, ®->address, sizeof(*paddr)); if (!*paddr) { pr_warning(FW_BUG APEI_PFX - "Invalid physical address in GAR [0x%llx/%u/%u]\n", - *paddr, width, space_id); + "Invalid physical address in GAR [0x%llx/%u/%u/%u/%u]\n", + *paddr, bit_width, bit_offset, access_size_code, + space_id); return -EINVAL; } - if ((width != 8) && (width != 16) && (width != 32) && (width != 64)) { + if (access_size_code < 1 || access_size_code > 4) { pr_warning(FW_BUG APEI_PFX - "Invalid bit width in GAR [0x%llx/%u/%u]\n", - *paddr, width, space_id); + "Invalid access size code in GAR [0x%llx/%u/%u/%u/%u]\n", + *paddr, bit_width, bit_offset, access_size_code, + space_id); + return -EINVAL; + } + *access_bit_width = 1UL << (access_size_code + 2); + + if ((bit_width + bit_offset) > *access_bit_width) { + pr_warning(FW_BUG APEI_PFX + "Invalid bit width + offset in GAR [0x%llx/%u/%u/%u/%u]\n", + *paddr, bit_width, bit_offset, access_size_code, + space_id); return -EINVAL; } if (space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY && space_id != ACPI_ADR_SPACE_SYSTEM_IO) { pr_warning(FW_BUG APEI_PFX - "Invalid address space type in GAR [0x%llx/%u/%u]\n", - *paddr, width, space_id); + "Invalid address space type in GAR [0x%llx/%u/%u/%u/%u]\n", + *paddr, bit_width, bit_offset, access_size_code, + space_id); return -EINVAL; } @@ -595,23 +610,25 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr) int apei_read(u64 *val, struct acpi_generic_address *reg) { int rc; + u32 access_bit_width; u64 address; acpi_status status; - rc = apei_check_gar(reg, &address); + rc = apei_check_gar(reg, &address, &access_bit_width); if (rc) return rc; *val = 0; switch(reg->space_id) { case ACPI_ADR_SPACE_SYSTEM_MEMORY: - status = acpi_os_read_memory64((acpi_physical_address) - address, val, reg->bit_width); + status = acpi_os_read_memory64((acpi_physical_address) address, + val, access_bit_width); if (ACPI_FAILURE(status)) return -EIO; break; case ACPI_ADR_SPACE_SYSTEM_IO: - status = acpi_os_read_port(address, (u32 *)val, reg->bit_width); + status = acpi_os_read_port(address, (u32 *)val, + access_bit_width); if (ACPI_FAILURE(status)) return -EIO; break; @@ -627,22 +644,23 @@ EXPORT_SYMBOL_GPL(apei_read); int apei_write(u64 val, struct acpi_generic_address *reg) { int rc; + u32 access_bit_width; u64 address; acpi_status status; - rc = apei_check_gar(reg, &address); + rc = apei_check_gar(reg, &address, &access_bit_width); if (rc) return rc; switch (reg->space_id) { case ACPI_ADR_SPACE_SYSTEM_MEMORY: - status = acpi_os_write_memory64((acpi_physical_address) - address, val, reg->bit_width); + status = acpi_os_write_memory64((acpi_physical_address) address, + val, access_bit_width); if (ACPI_FAILURE(status)) return -EIO; break; case ACPI_ADR_SPACE_SYSTEM_IO: - status = acpi_os_write_port(address, val, reg->bit_width); + status = acpi_os_write_port(address, val, access_bit_width); if (ACPI_FAILURE(status)) return -EIO; break; @@ -661,23 +679,24 @@ static int collect_res_callback(struct apei_exec_context *ctx, struct apei_resources *resources = data; struct acpi_generic_address *reg = &entry->register_region; u8 ins = entry->instruction; + u32 access_bit_width; u64 paddr; int rc; if (!(ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)) return 0; - rc = apei_check_gar(reg, &paddr); + rc = apei_check_gar(reg, &paddr, &access_bit_width); if (rc) return rc; switch (reg->space_id) { case ACPI_ADR_SPACE_SYSTEM_MEMORY: return apei_res_add(&resources->iomem, paddr, - reg->bit_width / 8); + access_bit_width / 8); case ACPI_ADR_SPACE_SYSTEM_IO: return apei_res_add(&resources->ioport, paddr, - reg->bit_width / 8); + access_bit_width / 8); default: return -EINVAL; } -- 2.20.1