Skip to content

Commit

Permalink
Events: Cleanup GPE dispatcher type obtaining code.
Browse files Browse the repository at this point in the history
There is an issue in AcpiInstallGpeHandler() and AcpiRemoveGpeHandler().
The code to obtain the GPE dispatcher type from the Handler->OriginalFlags
is wrong:
    if (((Handler->OriginalFlags & ACPI_GPE_DISPATCH_METHOD) ||
         (Handler->OriginalFlags & ACPI_GPE_DISPATCH_NOTIFY)) &&
ACPI_GPE_DISPATCH_NOTIFY is 0x03 and ACPI_GPE_DISPATCH_METHOD is 0x02, thus
this statement is TRUE for the following dispatcher types:
    0x01 (ACPI_GPE_DISPATCH_HANDLER): not expected
    0x02 (ACPI_GPE_DISPATCH_METHOD): expected
    0x03 (ACPI_GPE_DISPATCH_NOTIFY): expected

There is no functional issue due to this because Handler->OriginalFlags is
only set in AcpiInstallGpeHandler(), and an earlier checker has excluded
the ACPI_GPE_DISPATCH_HANDLER:
    if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
            ACPI_GPE_DISPATCH_HANDLER)
    {
        Status = AE_ALREADY_EXISTS;
        goto FreeAndExit;
    }
    ...
    Handler->OriginalFlags = (UINT8) (GpeEventInfo->Flags &
        (ACPI_GPE_XRUPT_TYPE_MASK | ACPI_GPE_DISPATCH_MASK));

We need to clean this up before modifying the GPE dispatcher type values.

In order to prevent such issue from happening in the future, this patch
introduces ACPI_GPE_DISPATCH_TYPE() macro to be used to obtain the GPE
dispatcher types. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Lv Zheng committed Dec 11, 2014
1 parent 04f25ac commit 7926d5c
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 21 deletions.
7 changes: 4 additions & 3 deletions source/components/debugger/dbdisply.c
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ AcpiDbDisplayGpes (
GpeIndex = (i * ACPI_GPE_REGISTER_WIDTH) + j;
GpeEventInfo = &GpeBlock->EventInfo[GpeIndex];

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_NONE)
{
/* This GPE is not used (no method or handler), ignore it */
Expand Down Expand Up @@ -1005,7 +1005,7 @@ AcpiDbDisplayGpes (
AcpiOsPrintf ("RunOnly, ");
}

switch (GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK)
switch (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags))
{
case ACPI_GPE_DISPATCH_NONE:

Expand All @@ -1016,6 +1016,7 @@ AcpiDbDisplayGpes (

AcpiOsPrintf ("Method");
break;

case ACPI_GPE_DISPATCH_HANDLER:

AcpiOsPrintf ("Handler");
Expand All @@ -1036,7 +1037,7 @@ AcpiDbDisplayGpes (
default:

AcpiOsPrintf ("UNKNOWN: %X",
GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK);
ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags));
break;
}

Expand Down
4 changes: 2 additions & 2 deletions source/components/events/evgpe.c
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ AcpiEvAsynchExecuteGpeMethod (

/* Do the correct dispatch - normal method or implicit notify */

switch (GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK)
switch (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags))
{
case ACPI_GPE_DISPATCH_NOTIFY:
/*
Expand Down Expand Up @@ -834,7 +834,7 @@ AcpiEvGpeDispatch (
* If there is neither a handler nor a method, leave the GPE
* disabled.
*/
switch (GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK)
switch (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags))
{
case ACPI_GPE_DISPATCH_HANDLER:

Expand Down
4 changes: 2 additions & 2 deletions source/components/events/evgpeblk.c
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,8 @@ AcpiEvInitializeGpeBlock (
* Ignore GPEs that have no corresponding _Lxx/_Exx method
* and GPEs that are used to wake the system
*/
if (((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) == ACPI_GPE_DISPATCH_NONE) ||
((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) == ACPI_GPE_DISPATCH_HANDLER) ||
if ((ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) == ACPI_GPE_DISPATCH_NONE) ||
(ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) == ACPI_GPE_DISPATCH_HANDLER) ||
(GpeEventInfo->Flags & ACPI_GPE_CAN_WAKE))
{
continue;
Expand Down
4 changes: 2 additions & 2 deletions source/components/events/evgpeinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -491,15 +491,15 @@ AcpiEvMatchGpeMethod (
return_ACPI_STATUS (AE_OK);
}

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_HANDLER)
{
/* If there is already a handler, ignore this GPE method */

return_ACPI_STATUS (AE_OK);
}

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_METHOD)
{
/*
Expand Down
4 changes: 2 additions & 2 deletions source/components/events/evgpeutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ AcpiEvDeleteGpeHandlers (
GpeEventInfo = &GpeBlock->EventInfo[((ACPI_SIZE) i *
ACPI_GPE_REGISTER_WIDTH) + j];

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_HANDLER)
{
/* Delete an installed handler block */
Expand All @@ -443,7 +443,7 @@ AcpiEvDeleteGpeHandlers (
GpeEventInfo->Dispatch.Handler = NULL;
GpeEventInfo->Flags &= ~ACPI_GPE_DISPATCH_MASK;
}
else if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
else if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_NOTIFY)
{
/* Delete the implicit notification device list */
Expand Down
16 changes: 10 additions & 6 deletions source/components/events/evxface.c
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ AcpiInstallGpeHandler (

/* Make sure that there isn't a handler there already */

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_HANDLER)
{
Status = AE_ALREADY_EXISTS;
Expand All @@ -980,8 +980,10 @@ AcpiInstallGpeHandler (
* automatically during initialization, in which case it has to be
* disabled now to avoid spurious execution of the handler.
*/
if (((Handler->OriginalFlags & ACPI_GPE_DISPATCH_METHOD) ||
(Handler->OriginalFlags & ACPI_GPE_DISPATCH_NOTIFY)) &&
if (((ACPI_GPE_DISPATCH_TYPE (Handler->OriginalFlags) ==
ACPI_GPE_DISPATCH_METHOD) ||
(ACPI_GPE_DISPATCH_TYPE (Handler->OriginalFlags) ==
ACPI_GPE_DISPATCH_NOTIFY)) &&
GpeEventInfo->RuntimeCount)
{
Handler->OriginallyEnabled = TRUE;
Expand Down Expand Up @@ -1076,7 +1078,7 @@ AcpiRemoveGpeHandler (

/* Make sure that a handler is indeed installed */

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) !=
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) !=
ACPI_GPE_DISPATCH_HANDLER)
{
Status = AE_NOT_EXIST;
Expand Down Expand Up @@ -1108,8 +1110,10 @@ AcpiRemoveGpeHandler (
* enabled, it should be enabled at this point to restore the
* post-initialization configuration.
*/
if (((Handler->OriginalFlags & ACPI_GPE_DISPATCH_METHOD) ||
(Handler->OriginalFlags & ACPI_GPE_DISPATCH_NOTIFY)) &&
if (((ACPI_GPE_DISPATCH_TYPE (Handler->OriginalFlags) ==
ACPI_GPE_DISPATCH_METHOD) ||
(ACPI_GPE_DISPATCH_TYPE (Handler->OriginalFlags) ==
ACPI_GPE_DISPATCH_NOTIFY)) &&
Handler->OriginallyEnabled)
{
(void) AcpiEvAddGpeReference (GpeEventInfo);
Expand Down
6 changes: 3 additions & 3 deletions source/components/events/evxfgpe.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ AcpiEnableGpe (
GpeEventInfo = AcpiEvGetGpeEventInfo (GpeDevice, GpeNumber);
if (GpeEventInfo)
{
if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) !=
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) !=
ACPI_GPE_DISPATCH_NONE)
{
Status = AcpiEvAddGpeReference (GpeEventInfo);
Expand Down Expand Up @@ -511,7 +511,7 @@ AcpiSetupGpeForWake (
* known as an "implicit notify". Note: The GPE is assumed to be
* level-triggered (for windows compatibility).
*/
if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_NONE)
{
/*
Expand All @@ -526,7 +526,7 @@ AcpiSetupGpeForWake (
* If we already have an implicit notify on this GPE, add
* this device to the notify list.
*/
if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) ==
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) ==
ACPI_GPE_DISPATCH_NOTIFY)
{
/* Ensure that the device is not already in the list */
Expand Down
2 changes: 1 addition & 1 deletion source/components/hardware/hwgpe.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ AcpiHwGetGpeStatus (

/* GPE currently handled? */

if ((GpeEventInfo->Flags & ACPI_GPE_DISPATCH_MASK) !=
if (ACPI_GPE_DISPATCH_TYPE (GpeEventInfo->Flags) !=
ACPI_GPE_DISPATCH_NONE)
{
LocalEventStatus |= ACPI_EVENT_FLAG_HAS_HANDLER;
Expand Down
1 change: 1 addition & 0 deletions source/include/actypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,7 @@ typedef UINT32 ACPI_EVENT_STATUS;
#define ACPI_GPE_DISPATCH_HANDLER (UINT8) 0x02
#define ACPI_GPE_DISPATCH_NOTIFY (UINT8) 0x03
#define ACPI_GPE_DISPATCH_MASK (UINT8) 0x03
#define ACPI_GPE_DISPATCH_TYPE(flags) ((UINT8) ((flags) & ACPI_GPE_DISPATCH_MASK))

#define ACPI_GPE_LEVEL_TRIGGERED (UINT8) 0x04
#define ACPI_GPE_EDGE_TRIGGERED (UINT8) 0x00
Expand Down

0 comments on commit 7926d5c

Please sign in to comment.