Patchwork [Precise,SRU,1/1] Revert "x86/ioapic: Add register level checks to detect bogus io-apic entries"

login
register
mail settings
Submitter Leann Ogasawara
Date April 10, 2012, 1:35 p.m.
Message ID <836417e379eb2df487d8c63f2626b7c196e8f442.1334063696.git.leann.ogasawara@canonical.com>
Download mbox | patch
Permalink /patch/151553/
State New
Headers show

Comments

Leann Ogasawara - April 10, 2012, 1:35 p.m.
From: Leann Ogasawara <leann.ogasawara@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/974982

This reverts commit 73d63d038ee9f769f5e5b46792d227fe20e442c5.

This patch results in a NULL pointer dereference which renders a system
unbootable if attempting to boot in pvops mode.  We received this patch
via upstream stable v3.2.14.  However, upstream discussion indicates
this patch was not meant to be included in any v3.2.y upstream stable
release.  Revert it.

http://www.mail-archive.com/stable@vger.kernel.org/msg04782.html

Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
---
 arch/x86/kernel/apic/io_apic.c |   40 ++++++++--------------------------------
 1 files changed, 8 insertions(+), 32 deletions(-)
Tim Gardner - April 10, 2012, 1:44 p.m.
Based on test results...
Stefan Bader - April 10, 2012, 1:57 p.m.
On 10.04.2012 15:35, leann.ogasawara@canonical.com wrote:
> From: Leann Ogasawara <leann.ogasawara@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/974982
> 
> This reverts commit 73d63d038ee9f769f5e5b46792d227fe20e442c5.
> 
> This patch results in a NULL pointer dereference which renders a system
> unbootable if attempting to boot in pvops mode.  We received this patch
> via upstream stable v3.2.14.  However, upstream discussion indicates
> this patch was not meant to be included in any v3.2.y upstream stable
> release.  Revert it.
> 
> http://www.mail-archive.com/stable@vger.kernel.org/msg04782.html
> 
> Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
> ---
>  arch/x86/kernel/apic/io_apic.c |   40 ++++++++--------------------------------
>  1 files changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index a25e276..6d939d7 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3963,36 +3963,18 @@ int mp_find_ioapic_pin(int ioapic, u32 gsi)
>  static __init int bad_ioapic(unsigned long address)
>  {
>  	if (nr_ioapics >= MAX_IO_APICS) {
> -		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded (found %d), skipping\n",
> -			MAX_IO_APICS, nr_ioapics);
> +		printk(KERN_WARNING "WARNING: Max # of I/O APICs (%d) exceeded "
> +		       "(found %d), skipping\n", MAX_IO_APICS, nr_ioapics);
>  		return 1;
>  	}
>  	if (!address) {
> -		pr_warn("WARNING: Bogus (zero) I/O APIC address found in table, skipping!\n");
> +		printk(KERN_WARNING "WARNING: Bogus (zero) I/O APIC address"
> +		       " found in table, skipping!\n");
>  		return 1;
>  	}
>  	return 0;
>  }
>  
> -static __init int bad_ioapic_register(int idx)
> -{
> -	union IO_APIC_reg_00 reg_00;
> -	union IO_APIC_reg_01 reg_01;
> -	union IO_APIC_reg_02 reg_02;
> -
> -	reg_00.raw = io_apic_read(idx, 0);
> -	reg_01.raw = io_apic_read(idx, 1);
> -	reg_02.raw = io_apic_read(idx, 2);
> -
> -	if (reg_00.raw == -1 && reg_01.raw == -1 && reg_02.raw == -1) {
> -		pr_warn("I/O APIC 0x%x registers return all ones, skipping!\n",
> -			mpc_ioapic_addr(idx));
> -		return 1;
> -	}
> -
> -	return 0;
> -}
> -
>  void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
>  {
>  	int idx = 0;
> @@ -4009,12 +3991,6 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
>  	ioapics[idx].mp_config.apicaddr = address;
>  
>  	set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
> -
> -	if (bad_ioapic_register(idx)) {
> -		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
> -		return;
> -	}
> -
>  	ioapics[idx].mp_config.apicid = io_apic_unique_id(id);
>  	ioapics[idx].mp_config.apicver = io_apic_get_version(idx);
>  
> @@ -4035,10 +4011,10 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
>  	if (gsi_cfg->gsi_end >= gsi_top)
>  		gsi_top = gsi_cfg->gsi_end + 1;
>  
> -	pr_info("IOAPIC[%d]: apic_id %d, version %d, address 0x%x, GSI %d-%d\n",
> -		idx, mpc_ioapic_id(idx),
> -		mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
> -		gsi_cfg->gsi_base, gsi_cfg->gsi_end);
> +	printk(KERN_INFO "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
> +	       "GSI %d-%d\n", idx, mpc_ioapic_id(idx),
> +	       mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
> +	       gsi_cfg->gsi_base, gsi_cfg->gsi_end);
>  
>  	nr_ioapics++;
>  }

Likely needs to be working sooner than later. So ack for the revert.

Patch

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index a25e276..6d939d7 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3963,36 +3963,18 @@  int mp_find_ioapic_pin(int ioapic, u32 gsi)
 static __init int bad_ioapic(unsigned long address)
 {
 	if (nr_ioapics >= MAX_IO_APICS) {
-		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded (found %d), skipping\n",
-			MAX_IO_APICS, nr_ioapics);
+		printk(KERN_WARNING "WARNING: Max # of I/O APICs (%d) exceeded "
+		       "(found %d), skipping\n", MAX_IO_APICS, nr_ioapics);
 		return 1;
 	}
 	if (!address) {
-		pr_warn("WARNING: Bogus (zero) I/O APIC address found in table, skipping!\n");
+		printk(KERN_WARNING "WARNING: Bogus (zero) I/O APIC address"
+		       " found in table, skipping!\n");
 		return 1;
 	}
 	return 0;
 }
 
-static __init int bad_ioapic_register(int idx)
-{
-	union IO_APIC_reg_00 reg_00;
-	union IO_APIC_reg_01 reg_01;
-	union IO_APIC_reg_02 reg_02;
-
-	reg_00.raw = io_apic_read(idx, 0);
-	reg_01.raw = io_apic_read(idx, 1);
-	reg_02.raw = io_apic_read(idx, 2);
-
-	if (reg_00.raw == -1 && reg_01.raw == -1 && reg_02.raw == -1) {
-		pr_warn("I/O APIC 0x%x registers return all ones, skipping!\n",
-			mpc_ioapic_addr(idx));
-		return 1;
-	}
-
-	return 0;
-}
-
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 {
 	int idx = 0;
@@ -4009,12 +3991,6 @@  void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	ioapics[idx].mp_config.apicaddr = address;
 
 	set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
-
-	if (bad_ioapic_register(idx)) {
-		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
-		return;
-	}
-
 	ioapics[idx].mp_config.apicid = io_apic_unique_id(id);
 	ioapics[idx].mp_config.apicver = io_apic_get_version(idx);
 
@@ -4035,10 +4011,10 @@  void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	if (gsi_cfg->gsi_end >= gsi_top)
 		gsi_top = gsi_cfg->gsi_end + 1;
 
-	pr_info("IOAPIC[%d]: apic_id %d, version %d, address 0x%x, GSI %d-%d\n",
-		idx, mpc_ioapic_id(idx),
-		mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
-		gsi_cfg->gsi_base, gsi_cfg->gsi_end);
+	printk(KERN_INFO "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
+	       "GSI %d-%d\n", idx, mpc_ioapic_id(idx),
+	       mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
+	       gsi_cfg->gsi_base, gsi_cfg->gsi_end);
 
 	nr_ioapics++;
 }