Re: [PATCH] ACPI, APEI: Fixup common access width firmware bug

[Gary Hade <garyhade@us.ibm.com>]

On Tue, Jun 12, 2012 at 10:43:28AM +0200, Jean Delvare wrote:
> Many firmwares have a common register definition bug where 8-bit
> access width is specified for a 32-bit register. Ideally this should
> be fixed in the BIOS, but earlier versions of the kernel did not
> complain, so fix that up silently.
> 
> This closes kernel bug #43282:
> https://bugzilla.kernel.org/show_bug.cgi?id=43282
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Gary Hade <garyhade@us.ibm.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: stable@vger.kernel.org [3.4+]
> ---
>  drivers/acpi/apei/apei-base.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- linux-3.4.orig/drivers/acpi/apei/apei-base.c	2012-06-08 10:02:06.000000000 +0200
> +++ linux-3.4/drivers/acpi/apei/apei-base.c	2012-06-08 10:04:16.503779775 +0200
> @@ -586,6 +586,11 @@ static int apei_check_gar(struct acpi_ge
>  	}
>  	*access_bit_width = 1UL << (access_size_code + 2);
> 
> +	/* Fixup common BIOS bug */
> +	if (bit_width == 32 && bit_offset == 0 && (*paddr & 0x03) == 0 &&
> +	    *access_bit_width < 32)
> +		*access_bit_width = 32;
> +
>  	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",

This seems reasonable but since the "Access Size < Register Bit Width"
condition has apparently been seen on a number of systems I have been
wondering if this might simply be a BIOS writer's way of telling us
not to touch some of the high bits in the register.  Reducing the
width of the register seems like a better way, but maybe not the
only way?

Jean, What do you (and others that are listening who know more
about this than I do) think about a change to sanity check only
the bit offset and not complain (at least for now) about cases
where an access using the specified access size will not address
every bit in the register?  

Perhaps something like:
--- linux-3.5-rc2/drivers/acpi/apei/apei-base.c.ORIG	2012-06-08 18:40:09.000000000 -0700
+++ linux-3.5-rc2/drivers/acpi/apei/apei-base.c	2012-06-13 16:32:49.000000000 -0700
@@ -586,9 +586,9 @@ static int apei_check_gar(struct acpi_ge
 	}
 	*access_bit_width = 1UL << (access_size_code + 2);
 
-	if ((bit_width + bit_offset) > *access_bit_width) {
+	if (bit_offset >= *access_bit_width) {
 		pr_warning(FW_BUG APEI_PFX
-			   "Invalid bit width + offset in GAR [0x%llx/%u/%u/%u/%u]\n",
+			   "Invalid bit offset in GAR [0x%llx/%u/%u/%u/%u]\n",
 			   *paddr, bit_width, bit_offset, access_size_code,
 			   space_id);
 		return -EINVAL;

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc