linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Len Brown <lenb@kernel.org>
To: linux-acpi@vger.kernel.org, linux-pm@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org, Gary Hade <garyhade@us.ibm.com>,
	Len Brown <len.brown@intel.com>
Subject: [PATCH 64/76] ACPI, APEI: Fix incorrect APEI register bit width check and usage
Date: Fri, 30 Mar 2012 06:14:07 -0400	[thread overview]
Message-ID: <15afae604651d4e17652d2ffb56f5e36f991cfef.1333101989.git.len.brown@intel.com> (raw)
In-Reply-To: <1333102459-23750-1-git-send-email-lenb@kernel.org>
In-Reply-To: <09f98a825a821f7a3f1b162f9ed023f37213a63b.1333101989.git.len.brown@intel.com>

From: Gary Hade <garyhade@us.ibm.com>

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 <garyhade@us.ibm.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 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 e5d53b7..1d3656f 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, &reg->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;
 	}
-- 
1.7.10.rc2.19.gfae9d


  parent reply	other threads:[~2012-03-30 11:19 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-30 10:13 ACPI & Power Management patches for Linux-3.4 Len Brown
2012-03-30 10:13 ` [PATCH 01/76] x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling tboot_sleep Len Brown
2012-03-30 10:13   ` [PATCH 02/76] tboot: Add return values for tboot_sleep Len Brown
2012-03-30 10:13   ` [PATCH 03/76] ACPI: ignore FADT reset-reg-sup flag Len Brown
2012-03-30 10:13   ` [PATCH 04/76] ACPICA: Fix regression in FADT revision checks Len Brown
2012-03-30 13:14     ` Josh Boyer
2012-04-03 19:58       ` [3.0.y, 3.2.y, 3.3.y] " Jonathan Nieder
2012-04-03 20:15         ` Josh Boyer
2012-04-04 18:58           ` Greg Kroah-Hartman
2012-03-30 10:13   ` [PATCH 05/76] cpuidle: Add common time keeping and irq enabling Len Brown
2012-03-30 10:13   ` [PATCH 06/76] ARM: at91: Consolidate time keeping and irq enable Len Brown
2012-03-30 10:13   ` [PATCH 07/76] ARM: kirkwood: " Len Brown
2012-03-30 10:13   ` [PATCH 08/76] ARM: davinci: " Len Brown
2012-03-30 10:13   ` [PATCH 09/76] ARM: omap: Consolidate OMAP3 " Len Brown
2012-03-30 10:13   ` [PATCH 10/76] ARM: omap: Consolidate OMAP4 " Len Brown
2012-03-30 10:13   ` [PATCH 11/76] ARM: shmobile: Consolidate " Len Brown
2012-03-30 10:13   ` [PATCH 12/76] SH: " Len Brown
2012-03-30 10:13   ` [PATCH 13/76] drivers/thermal/thermal_sys.c: fix build warning Len Brown
2012-03-30 10:13   ` [PATCH 14/76] thermal_sys: remove unnecessary line continuations Len Brown
2012-03-30 10:13   ` [PATCH 15/76] thermal_sys: remove obfuscating used-once macros Len Brown
2012-03-30 10:13   ` [PATCH 16/76] thermal_sys: kernel style cleanups Len Brown
2012-03-30 10:13   ` [PATCH 17/76] thermal_sys: convert printks to pr_<level> Len Brown
2012-03-30 13:41     ` [linux-pm] " Eduardo Valentin
2012-03-30 19:08       ` Joe Perches
2012-04-01 19:13         ` Eduardo Valentin
2012-03-30 10:13   ` [PATCH 18/76] thermal: add support for thermal sensor present on SPEAr13xx machines Len Brown
2012-03-30 10:13   ` [PATCH 19/76] thermal/spear_thermal: replace readl/writel with lighter _relaxed variants Len Brown
2012-03-30 10:13   ` [PATCH 20/76] thermal: spear13xx: checking for NULL instead of IS_ERR() Len Brown
2012-03-30 10:13   ` [PATCH 21/76] thermal: Fix for setting the thermal zone mode to enable/disable Len Brown
2012-03-30 10:13   ` [PATCH 22/76] ARM: davinci: Fix for cpuidle consolidation changes Len Brown
2012-03-30 10:13   ` [PATCH 23/76] ACPICA: Update _REV return value to 5 Len Brown
2012-03-30 10:13   ` [PATCH 24/76] ACPICA: ACPI 5: Support for new FADT SleepStatus, SleepControl registers Len Brown
2012-03-30 10:13   ` [PATCH 25/76] ACPICA: Move ACPI timer prototypes to public acpixf file Len Brown
2012-03-30 10:13   ` [PATCH 26/76] ACPICA: Support for custom ACPICA build for ACPI 5 reduced hardware Len Brown
2012-03-30 10:13   ` [PATCH 27/76] ACPICA: Expand OSL memory read/write interfaces to 64 bits Len Brown
2012-03-30 10:13   ` [PATCH 28/76] ACPICA: ACPI 5: Update debug output for new notify values Len Brown
2012-03-30 10:13   ` [PATCH 29/76] ACPICA: Add acpi_os_physical_table_override interface Len Brown
2012-03-30 10:13   ` [PATCH 30/76] ACPICA: Distill multiple sleep method functions to a single function Len Brown
2012-03-30 10:13   ` [PATCH 31/76] ACPICA: Split sleep/wake functions into two files Len Brown
2012-03-30 10:13   ` [PATCH 32/76] ACPICA: Add table-driven dispatch for sleep/wake functions Len Brown
2012-03-30 10:13   ` [PATCH 33/76] ACPICA: Update to version 20120215 Len Brown
2012-03-30 10:13   ` [PATCH 34/76] ACPICA: Clarify METHOD_NAME* defines for full-pathname cases Len Brown
2012-03-30 10:13   ` [PATCH 35/76] ACPICA: Change exception code for invalid pathname in acpi_evaluate_object Len Brown
2012-03-30 10:13   ` [PATCH 36/76] ACPICA: Debugger: Add missing object info to namespace dump Len Brown
2012-03-30 10:13   ` [PATCH 37/76] ACPICA: Sleep/Wake interfaces: optionally execute _GTS and _BFS Len Brown
2012-03-30 10:13   ` [PATCH 38/76] ACPI: Move module parameter gts and bfs to sleep.c Len Brown
2012-03-30 10:13   ` [PATCH 39/76] tools turbostat: add summary option Len Brown
2012-03-30 10:13   ` [PATCH 40/76] tools turbostat: reduce measurement overhead due to IPIs Len Brown
2012-03-30 10:13   ` [PATCH 41/76] tools turbostat: harden against cpu online/offline Len Brown
2012-03-30 10:13   ` [PATCH 42/76] ACPI: ec: Do request_region outside WARN() Len Brown
2012-03-30 10:13   ` [PATCH 43/76] ACPI: Make ACPI interrupt threaded Len Brown
2012-03-30 10:13   ` [PATCH 44/76] ACPICA: Object repair code: Support to add Package wrappers Len Brown
2012-03-30 10:13   ` [PATCH 45/76] ACPICA: Update to version 20120320 Len Brown
2012-03-30 10:13   ` [PATCH 46/76] ACPI: Introduce ACPI D3_COLD state support Len Brown
2012-04-01  6:53     ` [linux-pm] " Rafael J. Wysocki
2012-03-30 10:13   ` [PATCH 47/76] ACPI: Add interface to register/unregister device to/from power resources Len Brown
2012-03-30 10:13   ` [PATCH 48/76] cpuidle: add a sysfs entry to disable specific C state for debug purpose Len Brown
2012-03-30 10:13   ` [PATCH 49/76] cpuidle: use the driver's state_count as default Len Brown
2012-03-30 10:13   ` [PATCH 50/76] cpuidle: remove useless array definition in cpuidle_structure Len Brown
2012-03-30 10:13   ` [PATCH 51/76] cpuidle: remove unused 'governor_data' field Len Brown
2012-03-30 10:13   ` [PATCH 52/76] ACPI, PCI: Move acpi_dev_run_wake() to ACPI core Len Brown
2012-03-30 10:13   ` [PATCH 53/76] ACPI: Evaluate thermal trip points before reading temperature Len Brown
2012-03-30 10:13   ` [PATCH 54/76] ACPI: Ensure thermal limits match CPU frequencies Len Brown
2012-03-30 10:13   ` [PATCH 55/76] ACPI / PM: print physical addresses consistently with other parts of kernel Len Brown
2012-03-30 10:13   ` [PATCH 56/76] ACPI: Add CPU hotplug support for processor device objects Len Brown
2012-03-30 10:14   ` [PATCH 57/76] ACPI / Video: blacklist some samsung laptops Len Brown
2012-03-30 12:07     ` Corentin Chary
2012-03-30 12:16       ` Len Brown
2012-03-30 10:14   ` [PATCH 58/76] idle, x86: Allow off-lined CPU to enter deeper C states Len Brown
2012-04-02 16:13     ` Tony Luck
2012-04-02 17:25       ` Tony Luck
2012-04-02 17:45         ` Konrad Rzeszutek Wilk
2012-04-02 17:56         ` Boris Ostrovsky
2012-04-02 18:02           ` Tony Luck
2012-04-02 18:10             ` Boris Ostrovsky
2012-03-30 10:14   ` [PATCH 59/76] cpuidle: power_usage should be declared signed integer Len Brown
2012-03-30 10:14   ` [PATCH 60/76] ACPI, APEI, Fix ERST header length check Len Brown
2012-03-30 10:14   ` [PATCH 61/76] ACPI, APEI, EINJ, limit the range of einj_param Len Brown
2012-03-30 10:14   ` [PATCH 62/76] ACPI, APEI, EINJ, new parameter to control trigger action Len Brown
2012-03-30 10:14   ` [PATCH 63/76] Update documentation for parameter *notrigger* in einj.txt Len Brown
2012-03-30 10:14   ` Len Brown [this message]
2012-03-30 10:14   ` [PATCH 65/76] ACPI: processor_driver: add missing kfree Len Brown
2012-03-30 10:14   ` [PATCH 66/76] ACPI: Fix use-after-free in acpi_map_lsapic Len Brown
2012-03-30 10:14   ` [PATCH 67/76] PNPACPI: Fix device ref leaking in acpi_pnp_match Len Brown
2012-03-30 10:14   ` [PATCH 68/76] ACPI: consistently use should_use_kmap() Len Brown
2012-03-30 10:14   ` [PATCH 69/76] ACPI: Fix unprotected smp_processor_id() in acpi_processor_cst_has_changed() Len Brown
2012-03-30 10:14   ` [PATCH 70/76] ACPI: Clean redundant codes in scan.c Len Brown
2012-03-30 10:14   ` [PATCH 71/76] CPER failed to handle generic error records with multiple sections Len Brown
2012-03-30 10:14   ` [PATCH 72/76] ACPI: Fix logic for removing mappings in 'acpi_unmap' Len Brown
2012-03-30 10:14   ` [PATCH 73/76] ACPI: export acpi_kobj Len Brown
2012-03-30 10:14   ` [PATCH 74/76] ACPI: Add support for exposing BGRT data Len Brown
2012-03-30 10:14   ` [PATCH 75/76] Disable MCP limit exceeded messages from Intel IPS driver Len Brown
2012-03-30 10:14   ` [PATCH 76/76] ACPI throttling: fix endian bug in acpi_read_throttling_status() Len Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15afae604651d4e17652d2ffb56f5e36f991cfef.1333101989.git.len.brown@intel.com \
    --to=lenb@kernel.org \
    --cc=garyhade@us.ibm.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).