ACPI: disable stray GPE, prevent ACPI interrupt storm

PATCH Len Brown 3 participants 5 messages Nov 20, 2007 – Nov 30, 2007 ACPI

lore

Share & tools

lore

Len Brown

author reviewer

2007-11-20 18:43 UTC #0 lore raw

Bob,
Rui's Linux patch below modifies an ACPICA file.

thanks,
-Len

commit a7f9b1f24974da287771e2d70b30d9ca7bd66684

Author: Zhang Rui <[email protected]>
Date:   Tue Nov 20 13:38:59 2007 -0500

ACPI: disable stray GPE, prevent ACPI interrupt storm

GPEs are disabled depending on their type -- WAKE, WAKE_RUN, and RUNTIME. An error is returned if we are asked to disable a GPE that has no type.

But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.

In this case, when an external CRT is plugged in, the GPE fires, we attempt to disable the GPE,
but instead just return an error.
So the GPE stays asserted and an ACPI interrupt storm follows.

The fix is to disable a firing GPE,
even if it comes from outer space.

http://bugzilla.kernel.org/show_bug.cgi?id=6217

Signed-off-by Zhang Rui <[email protected]>

Acked-by Alexey Starikovskiy <[email protected]>

Signed-off-by Len Brown <[email protected]>

drivers/acpi/events/evgpe.c +4 -4

index e22f4a9..056b788 100644
@@ -270,18 +270,18 @@ acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info)
 	case ACPI_GPE_TYPE_WAKE_RUN:
 		ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED);
 
-		/*lint -fallthrough */
+		/* fallthrough */
 
 	case ACPI_GPE_TYPE_RUNTIME:
 
 		/* Disable the requested runtime GPE */
 
 		ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED);
-		status = acpi_hw_write_gpe_enable_reg(gpe_event_info);
-		break;
+
+		/* fallthrough */
 
 	default:
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	}
 
 	return_ACPI_STATUS(AE_OK);

Moore, Robert

2007-11-20 20:24 UTC #1 lore raw

/*lint -fallthrough */

Our lint needs this

Quoted 46 lines

-----Original Message-----
From: Len Brown [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 10:43 AM
To: Moore, Robert
Cc: [email protected]; Zhang, Rui; Alexey Starikovskiy
Subject: ACPI: disable stray GPE, prevent ACPI interrupt storm
Bob,
Rui's Linux patch below modifies an ACPICA file.
thanks,
-Len
commit a7f9b1f24974da287771e2d70b30d9ca7bd66684
Author: Zhang Rui <[email protected]>
Date:   Tue Nov 20 13:38:59 2007 -0500
ACPI: disable stray GPE, prevent ACPI interrupt storm
GPEs are disabled depending on their type -- WAKE, WAKE_RUN, and RUNTIME. An error is returned if we are asked to disable a GPE that has no type.
But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.
In this case, when an external CRT is plugged in, the GPE fires, we attempt to disable the GPE,
but instead just return an error.
So the GPE stays asserted and an ACPI interrupt storm follows.
The fix is to disable a firing GPE,
even if it comes from outer space.
http://bugzilla.kernel.org/show_bug.cgi?id=6217
Signed-off-by Zhang Rui <[email protected]>
Acked-by Alexey Starikovskiy <[email protected]>
Signed-off-by Len Brown <[email protected]>
drivers/acpi/events/evgpe.c +0 -0
index e22f4a9..056b788 100644

drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct · line 272

Quoted 3 lines

drivers/acpi/events/evgpe.c +0 -0

@@ -270,18 +270,18 @@ acpi_status acpi_ev_disable_gpe(struct
 acpi_gpe_event_info *gpe_event_info)
 	case ACPI_GPE_TYPE_WAKE_RUN:
 		ACPI_CLEAR_BIT(gpe_event_info->flags,

ACPI_GPE_WAKE_ENABLED);

drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info)

Quoted 9 lines

drivers/acpi/events/evgpe.c +1 -1

-		/*lint -fallthrough */
+		/* fallthrough */
 
 	case ACPI_GPE_TYPE_RUNTIME:
 
 		/* Disable the requested runtime GPE */
 
 		ACPI_CLEAR_BIT(gpe_event_info->flags,

ACPI_GPE_RUN_ENABLED);

Quoted 11 lines

drivers/acpi/events/evgpe.c +3 -3

-		status = acpi_hw_write_gpe_enable_reg(gpe_event_info);
-		break;
+
+		/* fallthrough */
 
 	default:
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	}
 
 	return_ACPI_STATUS(AE_OK);

Moore, Robert

2007-11-20 23:12 UTC #2 lore raw

In reply to Len Brown

BTW, this kind of change begs for a comment, how about:

default:
/*
* Even if we don't know the GPE type, make sure that we always
* disable it. This can prevent a certain type of GPE flood,
where

         * the GPE has no _Lxx/_Exx method, and it cannot be determined
         * whether the GPE is wake, run, or wake/run.
         */
        Status = AcpiHwWriteGpeEnableReg (GpeEventInfo);
        break;

Quoted 46 lines

-----Original Message-----
From: Len Brown [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 10:43 AM
To: Moore, Robert
Cc: [email protected]; Zhang, Rui; Alexey Starikovskiy
Subject: ACPI: disable stray GPE, prevent ACPI interrupt storm
Bob,
Rui's Linux patch below modifies an ACPICA file.
thanks,
-Len
commit a7f9b1f24974da287771e2d70b30d9ca7bd66684
Author: Zhang Rui <[email protected]>
Date:   Tue Nov 20 13:38:59 2007 -0500
ACPI: disable stray GPE, prevent ACPI interrupt storm
GPEs are disabled depending on their type -- WAKE, WAKE_RUN, and RUNTIME. An error is returned if we are asked to disable a GPE that has no type.
But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.
In this case, when an external CRT is plugged in, the GPE fires, we attempt to disable the GPE,
but instead just return an error.
So the GPE stays asserted and an ACPI interrupt storm follows.
The fix is to disable a firing GPE,
even if it comes from outer space.
http://bugzilla.kernel.org/show_bug.cgi?id=6217
Signed-off-by Zhang Rui <[email protected]>
Acked-by Alexey Starikovskiy <[email protected]>
Signed-off-by Len Brown <[email protected]>
drivers/acpi/events/evgpe.c +0 -0
index e22f4a9..056b788 100644

drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct · line 272

Quoted 3 lines

drivers/acpi/events/evgpe.c +0 -0

@@ -270,18 +270,18 @@ acpi_status acpi_ev_disable_gpe(struct
 acpi_gpe_event_info *gpe_event_info)
 	case ACPI_GPE_TYPE_WAKE_RUN:
 		ACPI_CLEAR_BIT(gpe_event_info->flags,

ACPI_GPE_WAKE_ENABLED);

drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info)

Quoted 9 lines

drivers/acpi/events/evgpe.c +1 -1

-		/*lint -fallthrough */
+		/* fallthrough */
 
 	case ACPI_GPE_TYPE_RUNTIME:
 
 		/* Disable the requested runtime GPE */
 
 		ACPI_CLEAR_BIT(gpe_event_info->flags,

ACPI_GPE_RUN_ENABLED);

Quoted 11 lines

drivers/acpi/events/evgpe.c +3 -3

-		status = acpi_hw_write_gpe_enable_reg(gpe_event_info);
-		break;
+
+		/* fallthrough */
 
 	default:
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	}
 
 	return_ACPI_STATUS(AE_OK);

Zhang Rui

2007-11-21 01:23 UTC #3 lore raw

Quoted 2 lines · Robert

BTW, this kind of change begs for a comment

Sounds good. :)

Thanks,
Rui

Quoted 84 lines

default:
/*
* Even if we don't know the GPE type, make sure that we always
* disable it. This can prevent a certain type of GPE flood,
where
         * the GPE has no _Lxx/_Exx method, and it cannot be determined

         * whether the GPE is wake, run, or wake/run.
         */
        Status = AcpiHwWriteGpeEnableReg (GpeEventInfo);
        break;
Quoted 46 lines
-----Original Message-----
From: Len Brown [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 10:43 AM
To: Moore, Robert
Cc: [email protected]; Zhang, Rui; Alexey Starikovskiy
Subject: ACPI: disable stray GPE, prevent ACPI interrupt storm
Bob,
Rui's Linux patch below modifies an ACPICA file.
thanks,
-Len
commit a7f9b1f24974da287771e2d70b30d9ca7bd66684
Author: Zhang Rui <[email protected]>
Date:   Tue Nov 20 13:38:59 2007 -0500
ACPI: disable stray GPE, prevent ACPI interrupt storm
GPEs are disabled depending on their type -- WAKE, WAKE_RUN, and RUNTIME. An error is returned if we are asked to disable a GPE that has no type.
But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.
In this case, when an external CRT is plugged in, the GPE fires, we attempt to disable the GPE,
but instead just return an error.
So the GPE stays asserted and an ACPI interrupt storm follows.
The fix is to disable a firing GPE,
even if it comes from outer space.
http://bugzilla.kernel.org/show_bug.cgi?id=6217
Signed-off-by Zhang Rui <[email protected]>
Acked-by Alexey Starikovskiy <[email protected]>
Signed-off-by Len Brown <[email protected]>
drivers/acpi/events/evgpe.c +0 -0
index e22f4a9..056b788 100644
drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct · line 272
Quoted 3 lines
drivers/acpi/events/evgpe.c +0 -0
@@ -270,18 +270,18 @@ acpi_status acpi_ev_disable_gpe(struct
 acpi_gpe_event_info *gpe_event_info)
 	case ACPI_GPE_TYPE_WAKE_RUN:
 		ACPI_CLEAR_BIT(gpe_event_info->flags,
ACPI_GPE_WAKE_ENABLED);
drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 9 lines
drivers/acpi/events/evgpe.c +1 -1
-		/*lint -fallthrough */
+		/* fallthrough */
 
 	case ACPI_GPE_TYPE_RUNTIME:
 
 		/* Disable the requested runtime GPE */
 
 		ACPI_CLEAR_BIT(gpe_event_info->flags,
ACPI_GPE_RUN_ENABLED);
Quoted 11 lines
drivers/acpi/events/evgpe.c +3 -3
-		status = acpi_hw_write_gpe_enable_reg(gpe_event_info);
-		break;
+
+		/* fallthrough */
 
 	default:
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	}
 
 	return_ACPI_STATUS(AE_OK);

Moore, Robert

2007-11-30 22:15 UTC #4 lore raw

I don't understand why the GPE was enabled in the first place.

GPEs start out all disabled. They only get enabled if an _Lxx/_Exx method is found.

I'm all for defensive programming (if all else fails...), but I'm worried that there is a bug somewhere else that is accidentally enabling this GPE.

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.

Yes, it should be "initialized". All GPEs are disabled by default.

Please check this out.
Thanks,
Bob

Quoted 8 lines

-----Original Message-----
From: Zhang, Rui
Sent: Tuesday, November 20, 2007 5:33 PM
To: Moore, Robert
Cc: Len Brown; [email protected]; Alexey Starikovskiy
Subject: RE: ACPI: disable stray GPE, prevent ACPI interrupt storm

Quoted 10 lines

Quoted 2 lines · Robert
BTW, this kind of change begs for a comment
Sounds good. :)
Thanks,
Rui
Quoted 3 lines
default:
/*
* Even if we don't know the GPE type, make sure that we

always

Quoted 3 lines

Quoted 3 lines
* disable it. This can prevent a certain type of GPE flood,
where
* the GPE has no _Lxx/_Exx method, and it cannot be

determined

Quoted 42 lines

Quoted 42 lines
         * whether the GPE is wake, run, or wake/run.
         */
        Status = AcpiHwWriteGpeEnableReg (GpeEventInfo);
        break;
Quoted 35 lines
-----Original Message-----
From: Len Brown [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 10:43 AM
To: Moore, Robert
Cc: [email protected]; Zhang, Rui; Alexey Starikovskiy
Subject: ACPI: disable stray GPE, prevent ACPI interrupt storm
Bob,
Rui's Linux patch below modifies an ACPICA file.
thanks,
-Len
commit a7f9b1f24974da287771e2d70b30d9ca7bd66684
Author: Zhang Rui <[email protected]>
Date:   Tue Nov 20 13:38:59 2007 -0500
ACPI: disable stray GPE, prevent ACPI interrupt storm
GPEs are disabled depending on their type -- WAKE, WAKE_RUN, and RUNTIME. An error is returned if we are asked to disable a GPE that has no type.
But at least one system exists that enables a GPE from AML that is not the EC GPE, and has no _Lxx/_Exx AML handler, and is thus never initialized.
In this case, when an external CRT is plugged in, the GPE fires, we attempt to disable the GPE,
but instead just return an error.
So the GPE stays asserted and an ACPI interrupt storm follows.
The fix is to disable a firing GPE,
even if it comes from outer space.

Quoted 7 lines

Quoted 7 lines
Review
Quoted 7 lines
http://bugzilla.kernel.org/show_bug.cgi?id=6217
Signed-off-by Zhang Rui <[email protected]>
Acked-by Alexey Starikovskiy <[email protected]>
Signed-off-by Len Brown <[email protected]>
(quoted diff) +0 -0

b/drivers/acpi/events/evgpe.c

Quoted 29 lines

Quoted 29 lines
drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct · line 272
Quoted 7 lines
index e22f4a9..056b788 100644
drivers/acpi/events/evgpe.c +0 -0
@@ -270,18 +270,18 @@ acpi_status acpi_ev_disable_gpe(struct
 acpi_gpe_event_info *gpe_event_info)
 	case ACPI_GPE_TYPE_WAKE_RUN:
 		ACPI_CLEAR_BIT(gpe_event_info->flags,
ACPI_GPE_WAKE_ENABLED);
drivers/acpi/events/evgpe.c acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 9 lines
drivers/acpi/events/evgpe.c +1 -1
-		/*lint -fallthrough */
+		/* fallthrough */
 
 	case ACPI_GPE_TYPE_RUNTIME:
 
 		/* Disable the requested runtime GPE */
 
 		ACPI_CLEAR_BIT(gpe_event_info->flags,
ACPI_GPE_RUN_ENABLED);
Quoted 11 lines
drivers/acpi/events/evgpe.c +3 -3
-		status = acpi_hw_write_gpe_enable_reg(gpe_event_info);
-		break;
+
+		/* fallthrough */
 
 	default:
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	}
 
 	return_ACPI_STATUS(AE_OK);