ACPICA: Events: Introduce acpi_block_gpe()/acpi_unblock_gpe()/acpi_control_gpe_handling() to allow administrative GPE enabling/disabling
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/3 ACPICA: Events: Introduce acpi_block_gpe()/acpi_unblock_gpe()/acpi_control_gpe_handling() to allow administrative GPE enabling/disabling
There is a facility in Linux, developers can manage GPE enabling/disabling through /sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes. Many users expect to use this facility to implement quirks to disable specific GPEs when there is a gap in Linux causing GPE flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE management purposes.
This patch introduces acpi_block_gpe()/acpi_unblock_gpe() to be used in such situation instead of acpi_enable_gpe()/acpi_disable_gpe().
The idea to implement this is to replace the GPE register EN bit with the managed value, block EN set/clear operations but record the operation results in blocked_enabled, so that after the managed state is removed, restore the saved blocked_enabled back to the GPE register EN bit.
Now OSPMs should be able to use this facility to generate quirks. ACPICA BZ 1102.
This facility can also be used by the administrator to control the GPE handling mode during the runtime when the driver is capable of handling the GPE in both the interrupt mode and the polling mode (for example, the Linux EC driver). acpi_control_gpe_handling() is offered for this purpose. Lv Zheng.
--- drivers/acpi/acpica/acevents.h | 3 + drivers/acpi/acpica/aclocal.h | 1 + drivers/acpi/acpica/evgpe.c | 84 ++++++++++++++++++++++++ drivers/acpi/acpica/evxfgpe.c | 142 ++++++++++++++++++++++++++++++++++++++++ drivers/acpi/acpica/hwgpe.c | 24 ++++++- include/acpi/acpixf.h | 23 +++++-- include/acpi/actypes.h | 85 +++++++++++++++++++----- 7 files changed, 336 insertions(+), 26 deletions(-)
drivers/acpi/acpica/acevents.h +3 -0
@@ -86,6 +86,9 @@ acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info);acpi_status acpi_ev_enable_gpe (struct acpi_gpe_event_info *gpe_event_info);acpi_status +acpi_ev_manage_gpe (struct acpi_gpe_event_info *gpe_event_info,u8 action); + +acpi_status acpi_ev_add_gpe_reference (struct acpi_gpe_event_info *gpe_event_info); acpi_status
drivers/acpi/acpica/aclocal.h +1 -0
@@ -484,6 +484,7 @@ struct acpi_gpe_event_info {u8 flags;/* Misc info about this GPE */ u8 gpe_number;/* This GPE */ u8 runtime_count;/* References to a run GPE */ +u8 blocked_enabled;/* GPE should be enabled but blocked */ };/* Information about a GPE register pair, one per each status/enable pair in an array */
drivers/acpi/acpica/evgpe.c +84 -0
90 lines not on this page · raw message
drivers/acpi/acpica/evxfgpe.c +142 -0
148 lines not on this page · raw message
drivers/acpi/acpica/hwgpe.c +21 -3
@@ -98,7 +98,7 @@ acpi_status acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action) {struct acpi_gpe_register_info *gpe_register_info; -acpi_status status; +acpi_status status =AE_OK ;u32 enable_mask;u32 register_bit;
@@ -148,9 +148,21 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)return (AE_BAD_PARAMETER ); } -/* Write the updated enable mask */ +/* Check if there is an administrative GPE enabling/disabling */ - status =acpi_hw_write (enable_mask, &gpe_register_info->enable_address); +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { +if (enable_mask & register_bit) { + gpe_event_info->blocked_enabled = TRUE; + }else { + gpe_event_info->blocked_enabled = FALSE; + } + }else { +/* Write the updated enable mask */ + + status = +acpi_hw_write (enable_mask, + &gpe_register_info->enable_address); + }return (status); }
@@ -228,6 +240,12 @@ acpi_hw_get_gpe_status(struct acpi_gpe_event_info *gpe_event_info, local_event_status |=ACPI_EVENT_FLAG_HAS_HANDLER ; } +/* GPE currently managed? (enabled/disabled by the system admin?) */ + +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { + local_event_status |=ACPI_EVENT_FLAG_MANAGED ; + } +/* Get the info block for the entire GPE register */ gpe_register_info = gpe_event_info->register_info;
include/acpi/acpixf.h +18 -5
@@ -732,15 +732,28 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_mark_gpe_for_wake(acpi_handle gpe_device, - u32 gpe_number)) + acpi_block_gpe(acpi_handle gpe_device, + u32 gpe_number)) + +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_unblock_gpe(acpi_handle gpe_device, + u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_setup_gpe_for_wake(acpi_handle - parent_device, - acpi_handle gpe_device, + acpi_control_gpe_handling(acpi_handle + gpe_device, + u32 gpe_number, + u8 use_interrupt_mode, + u8 use_polling_mode)) +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_setup_gpe_for_wake(acpi_handle + parent_device, + acpi_handle gpe_device, + u32 gpe_number)) +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 action))
include/acpi/actypes.h +67 -18
112 lines not on this page · raw message
Signature · 1 line
1.7.10
Quoted 10 lines · Lv Zheng
There is a facility in Linux, developers can manage GPE enabling/disabling through
/sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes. Many users expect to use this facility to implement quirks to disable specific GPEs when there is a gap in Linux causing GPE flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE management purposes.
Quoted 2 lines
This patch introduces acpi_block_gpe()/acpi_unblock_gpe() to be used in such situation instead of acpi_enable_gpe()/acpi_disable_gpe().
Up to this point, I agree.
Quoted 4 lines
The idea to implement this is to replace the GPE register EN bit with the managed value, block EN set/clear operations but record the operation results in
blocked_enabled, so that after the managed state is removed, restore the savedblocked_enabledback to the GPE register EN bit.
Unfortunately, I don't really follow the above paragraph, so chances are that whoever is not familiar with the code will not be able to follow it either.
Quoted 7 lines
Now OSPMs should be able to use this facility to generate quirks. ACPICA BZ 1102.
This facility can also be used by the administrator to control the GPE handling mode during the runtime when the driver is capable of handling the GPE in both the interrupt mode and the polling mode (for example, the Linux EC driver).
acpi_control_gpe_handling()is offered for this purpose. Lv Zheng.
That is too much. The patch should focus on the block/unblock functionality. Anything beyond that should be added later IMO.
Quoted 31 lines
Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/acpica/acevents.h | 3 + drivers/acpi/acpica/aclocal.h | 1 + drivers/acpi/acpica/evgpe.c | 84 ++++++++++++++++++++++++ drivers/acpi/acpica/evxfgpe.c | 142 ++++++++++++++++++++++++++++++++++++++++ drivers/acpi/acpica/hwgpe.c | 24 ++++++- include/acpi/acpixf.h | 23 +++++-- include/acpi/actypes.h | 85 +++++++++++++++++++----- 7 files changed, 336 insertions(+), 26 deletions(-)drivers/acpi/acpica/acevents.h +3
-0@@ -86,6 +86,9 @@ acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info);acpi_status acpi_ev_enable_gpe (struct acpi_gpe_event_info *gpe_event_info);acpi_status +acpi_ev_manage_gpe (struct acpi_gpe_event_info *gpe_event_info,u8 action); + +acpi_status acpi_ev_add_gpe_reference (struct acpi_gpe_event_info *gpe_event_info); acpi_statusdrivers/acpi/acpica/aclocal.h +0
-0
drivers/acpi/acpica/aclocal.h struct acpi_gpe_event_info { · line 487
Quoted 4 lines
drivers/acpi/acpica/aclocal.h +1
-0@@ -484,6 +484,7 @@ struct acpi_gpe_event_info {u8 flags;/* Misc info about this GPE */ u8 gpe_number;/* This GPE */ u8 runtime_count;/* References to a run GPE */ +u8 blocked_enabled;/* GPE should be enabled but blocked */
I'm not sure I'm following the logic here.
What's needed is disabled and blocked. Why do we need enabled and blocked?
Quoted 75 lines
drivers/acpi/acpica/aclocal.h +0
-0};/* Information about a GPE register pair, one per each status/enable pair in an array */ drivers/acpi/acpica/evgpe.c +64
-0@@ -130,6 +130,90 @@ acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)/******************************************************************************* * +* FUNCTION: acpi_ev_manage_gpe +* +* PARAMETERS: gpe_event_info - GPE to force enabling/disabling +* action - ACPI_GPE_ENABLE, ACPI_GPE_DISABLE or +* ACPI_GPE_UNMANAGE +* +* RETURN: Status +* +* DESCRIPTION: Unconditionally enable or disable an individual GPE for the +* administrative purposes during the runtime. +* +******************************************************************************/ + +acpi_status +acpi_ev_manage_gpe (struct acpi_gpe_event_info *gpe_event_info,u8 action) +{ +acpi_status status; +acpi_event_status event_status; + +ACPI_FUNCTION_TRACE (ev_manage_gpe); + +/* Perform the action */ + +switch (action) { +case ACPI_GPE_ENABLE : +case ACPI_GPE_DISABLE : + +if (!(gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK )) { + status = +acpi_hw_get_gpe_status (gpe_event_info, + &event_status); +if (ACPI_FAILURE (status)) { +return_ACPI_STATUS (status); + } +if (event_status &ACPI_EVENT_FLAG_ENABLE_SET ) { + gpe_event_info->blocked_enabled = TRUE; + }else { + gpe_event_info->blocked_enabled = FALSE; + } + } + +/* Reset flags so that acpi_hw_low_set_gpe() can take effective */ + + gpe_event_info->flags &= ~ACPI_GPE_MANAGED_FLAG_MASK ; +if (action ==ACPI_GPE_ENABLE ) { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_ENABLE ); + gpe_event_info->flags |=ACPI_GPE_MANAGED_ENABLED ; + }else { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_DISABLE ); + gpe_event_info->flags |=ACPI_GPE_MANAGED_DISABLED ; + } +break ; + +case ACPI_GPE_UNMANAGE : + +if (!(gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK )) { +return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + +/* Reset flags so that acpi_hw_low_set_gpe() can take effective */ + + gpe_event_info->flags &= ~ACPI_GPE_MANAGED_FLAG_MASK ;
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info) · line 206
Quoted 10 lines
drivers/acpi/acpica/evgpe.c +10
-0+if (gpe_event_info->blocked_enabled) { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_ENABLE ); + }else { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_DISABLE ); + } + gpe_event_info->blocked_enabled = FALSE; +break ; +
The code above is really hard to follow.
Maybe the description in the comment above the function should be more, well, descriptive?
Quoted 42 lines
drivers/acpi/acpica/evgpe.c +10
-0+default : + +return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + +return_ACPI_STATUS (AE_OK ); +} + +/******************************************************************************* +* * FUNCTION: acpi_ev_add_gpe_reference * * PARAMETERS: gpe_event_info - Add a reference to this GPE drivers/acpi/acpica/evxfgpe.c +21
-0@@ -257,6 +257,148 @@ ACPI_EXPORT_SYMBOL(acpi_set_gpe)/******************************************************************************* * +* FUNCTION: acpi_block_gpe +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 +* gpe_number - GPE level within the GPE block +* +* RETURN: Status +* +* DESCRIPTION: Unconditionally disable an individual GPE. +* This API is typically used by the system administrator to +* block the GPE enabling, ex., prevent the GPE flooding. +* +******************************************************************************/ +acpi_status acpi_block_gpe (acpi_handle gpe_device,u32 gpe_number) +{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; + +ACPI_FUNCTION_TRACE (acpi_block_gpe); + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock);
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 290
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +10
-0+ +/* Ensure that we have a valid GPE number */ + + gpe_event_info =acpi_ev_get_gpe_event_info (gpe_device, gpe_number); +if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + + status =acpi_ev_manage_gpe (gpe_event_info,ACPI_GPE_DISABLE );
So I would prefer the second argument here to be something like ACPI_GPE_DISABLE | ACPI_GPE_BLOCK as then the code would be quite straightforward to understand.
Quoted 30 lines
drivers/acpi/acpica/evxfgpe.c +30
-0+ +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_block_gpe) + +/******************************************************************************* +* +* FUNCTION: acpi_unblock_gpe +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 +* gpe_number - GPE level within the GPE block +* +* RETURN: Status +* +* DESCRIPTION: Stop unconditional GPE disabling. +* This API reverts acpi_block_gpe(). +* +******************************************************************************/ +acpi_status acpi_unblock_gpe(acpi_handle gpe_device, u32 gpe_number) +{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; + +ACPI_FUNCTION_TRACE (acpi_unblock_gpe); + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock);
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 330
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +10
-0+ +/* Ensure that we have a valid GPE number */ + + gpe_event_info =acpi_ev_get_gpe_event_info (gpe_device, gpe_number); +if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + + status =acpi_ev_manage_gpe (gpe_event_info,ACPI_GPE_UNMANAGE );
And that is completely unclear to me.
Quoted 15 lines
drivers/acpi/acpica/evxfgpe.c +15
-0+ +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_unblock_gpe) + +/******************************************************************************* +* +* FUNCTION: acpi_control_gpe_handling +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 +* gpe_number - GPE level within the GPE block +* use_interrupt_mode - Allow the GPE to be dispatched in the
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 355
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +10
-0+* interrupt mode +* use_polling_mode - Allow the GPE to be dispatched in the +* polling mode +* +* RETURN: Status +* +* DESCRIPTION: Switch the GPE handling mode between the interrupt only mode, +* the polling only mode and the interrupt/polling adaptive mode. +* +******************************************************************************/
This is extra functionality that you want to piggy back on a fix for a real issue.
Please separate it from that fix, it doesn't belong here.
Quoted 170 lines
drivers/acpi/acpica/evxfgpe.c +46
-0+acpi_status +acpi_control_gpe_handling (acpi_handle gpe_device, +u32 gpe_number, +u8 use_interrupt_mode,u8 use_polling_mode) +{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; +u8 action; + +ACPI_FUNCTION_TRACE (acpi_control_gpe_handling); + +if (!use_interrupt_mode && !use_polling_mode) { +return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock); + +/* Ensure that we have a valid GPE number */ + + gpe_event_info =acpi_ev_get_gpe_event_info (gpe_device, gpe_number); +if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + +/* Pick up and peform the correct action */ + + action =ACPI_GPE_UNMANAGE ; +if (!use_interrupt_mode) { + action =ACPI_GPE_DISABLE ; + } +if (!use_polling_mode) { + action =ACPI_GPE_ENABLE ; + } + status =acpi_ev_manage_gpe (gpe_event_info, action); + +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_control_gpe_handling) + +/******************************************************************************* +* * FUNCTION: acpi_mark_gpe_for_wake * * PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 drivers/acpi/acpica/hwgpe.c +21
-3@@ -98,7 +98,7 @@ acpi_status acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action) {struct acpi_gpe_register_info *gpe_register_info; -acpi_status status; +acpi_status status =AE_OK ;u32 enable_mask;u32 register_bit;@@ -148,9 +148,21 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)return (AE_BAD_PARAMETER ); } -/* Write the updated enable mask */ +/* Check if there is an administrative GPE enabling/disabling */ - status =acpi_hw_write (enable_mask, &gpe_register_info->enable_address); +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { +if (enable_mask & register_bit) { + gpe_event_info->blocked_enabled = TRUE; + }else { + gpe_event_info->blocked_enabled = FALSE; + } + }else { +/* Write the updated enable mask */ + + status = +acpi_hw_write (enable_mask, + &gpe_register_info->enable_address); + }return (status); }@@ -228,6 +240,12 @@ acpi_hw_get_gpe_status(struct acpi_gpe_event_info *gpe_event_info, local_event_status |=ACPI_EVENT_FLAG_HAS_HANDLER ; } +/* GPE currently managed? (enabled/disabled by the system admin?) */ + +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { + local_event_status |=ACPI_EVENT_FLAG_MANAGED ; + } +/* Get the info block for the entire GPE register */ gpe_register_info = gpe_event_info->register_info;include/acpi/acpixf.h +18
-5@@ -732,15 +732,28 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_mark_gpe_for_wake(acpi_handle gpe_device, - u32 gpe_number)) + acpi_block_gpe(acpi_handle gpe_device, + u32 gpe_number)) + +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_unblock_gpe(acpi_handle gpe_device, + u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_setup_gpe_for_wake(acpi_handle - parent_device, - acpi_handle gpe_device, + acpi_control_gpe_handling(acpi_handle + gpe_device, + u32 gpe_number, + u8 use_interrupt_mode, + u8 use_polling_mode)) +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_setup_gpe_for_wake(acpi_handle + parent_device, + acpi_handle gpe_device, + u32 gpe_number)) +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 action))include/acpi/actypes.h +11
-10@@ -732,16 +732,17 @@ typedef u32 acpi_event_type;* The encoding of acpi_event_status is illustrated below. * Note that a set bit (1) indicates the property is TRUE * (e.g. if bit 0 is set then the event is enabled). -* +-------------+-+-+-+-+-+ -* | Bits 31:5 |4|3|2|1|0| -* +-------------+-+-+-+-+-+ -* | | | | | | -* | | | | | +- Enabled? -* | | | | +--- Enabled for wake? -* | | | +----- Status bit set? -* | | +------- Enable bit set? -* | +--------- Has a handler? -* +--------------- <Reserved> +* +-------------+-+-+-+-+-+-+ +* | Bits 31:6 |5|4|3|2|1|0| +* +-------------+-+-+-+-+-+-+ +* | | | | | | | +* | | | | | | +- Enabled? +* | | | | | +--- Enabled for wake? +* | | | | +----- Status bit set? +* | | | +------- Enable bit set? +* | | +--------- Has a handler? +* | +----------- Enabling/disabling forced? +* +----------------- <Reserved> */ typedef u32 acpi_event_status ;
include/acpi/actypes.h typedef u32 acpi_event_status; · line 755
Quoted 4 lines
include/acpi/actypes.h +1
-0@@ -751,6 +752,7 @@ typedef u32 acpi_event_status;#define ACPI_EVENT_FLAG_STATUS_SET (acpi_event_status) 0x04#define ACPI_EVENT_FLAG_ENABLE_SET (acpi_event_status) 0x08#define ACPI_EVENT_FLAG_HAS_HANDLER (acpi_event_status) 0x10 +#define ACPI_EVENT_FLAG_MANAGED (acpi_event_status) 0x20
The meaning of this new flag is unclear. It probably should be something like ACPI_EVENT_FLAG_BLOCKED or MASKED meaning that if set, the GPE cannot be disabled or enabled by usual means (until that flag is cleared).
Quoted 73 lines
include/acpi/actypes.h +48
-8#define ACPI_EVENT_FLAG_SET ACPI_EVENT_FLAG_STATUS_SET/* Actions for acpi_set_gpe, acpi_gpe_wakeup, acpi_hw_low_set_gpe */ @@ -758,17 +760,20 @@ typedef u32 acpi_event_status;#define ACPI_GPE_ENABLE 0#define ACPI_GPE_DISABLE 1#define ACPI_GPE_CONDITIONAL_ENABLE 2 +#define ACPI_GPE_UNMANAGE 3/* * GPE info flags - Per GPE -* +-------+-+-+---+ -* | 7:5 |4|3|2:0| -* +-------+-+-+---+ -* | | | | -* | | | +-- Type of dispatch:to method, handler, notify, or none -* | | +----- Interrupt type: edge or level triggered -* | +------- Is a Wake GPE -* +------------ <Reserved> +* +-+-+-+-+-+---+ +* |7|6|5|4|3|2:0| +* +-+-+-+-+-+---+ +* | | | | | | +* | | | | | +-- Type of dispatch:to method, handler, notify, or none +* | | | | +----- Interrupt type: edge or level triggered +* | | | +------- Is a Wake GPE +* | | +--------- Force GPE enabling +* | +----------- Force GPE disabling +* +------------- <Reserved> */ #define ACPI_GPE_DISPATCH_NONE (u8) 0x00#define ACPI_GPE_DISPATCH_METHOD (u8) 0x01@@ -785,6 +790,50 @@ typedef u32 acpi_event_status;#define ACPI_GPE_CAN_WAKE (u8) 0x10/* +* Flags used by the GPE management APIs +* +* The GPE management APIs are not implemented for the GPE drivers. It is +* designed for providing the management purposes out of the GPE drivers' +* awareness. The GPE driver APIs should still be working as expected +* during the period the GPE management purposes are applied. There are 2 +* use cases for the flags: +* +* 1. Prevent the GPE flooding +* These flags can be used to control how the GPE is dispatched by the +* event dispatcher. Note that the ACPI_GPE_MANAGED_DISABLED can also be +* used to prevent the GPE flooding that cannot be prevented due to the +* ACPI_GPE_DISPATCH_NONE sanity check. This kind of the GPE flooding +* happens on the GPE where there is a _Lxx/_Exx prepared by the BIOS +* but the OSPM still cannot handle it correctly by executing the +* method. Such kind of incapability is mostly because of the feature +* gaps in the OSPM: +* ACPI_GPE_MANAGED_ENABLED: force the event dispatcher to always +* enable the GPE +* ACPI_GPE_MANAGED_DISABLED: force the event dispatcher to always +* disable the GPE +* prevent the GPE from flooding +* acpi_block_gpe()/acpi_unblock_gpe() are the APIs offering the GPE +* flooding prevention feature. +* +* 2. Control the GPE handling mode +* A GPE driver may be able to handle the GPE both in the interrupt mode +* and in the polling mode. Since the polling mode is a mechanism +* implemented by the driver itself and is not controlled by the event +* dispatcher, this mechanism can be used to switch between the +* interrupt mode (dispatched via the event dispatcher) and the polling +* mode (implemented by the GPE drivers): +* ACPI_GPE_MANAGED_ENABLED: force the driver to use the interrupt mode +* ACPI_GPE_MANAGED_DISABLED: force the driver to use the polling mode +* None: allow the driver to use the +* interrupt/polling adaptive mode +* acpi_control_gpe_handling() is the API offering the GPE handling mode
include/acpi/actypes.h typedef u32 acpi_event_status; · line 839
Quoted 10 lines
include/acpi/actypes.h +7
-0+* switch feature. +*/ +#define ACPI_GPE_MANAGED_ENABLED (u8) 0x20 +#define ACPI_GPE_MANAGED_DISABLED (u8) 0x40 +#define ACPI_GPE_MANAGED_FLAG_MASK (u8) 0x60 + +/* * Flags for GPE and Lock interfaces */ #define ACPI_NOT_ISR 0x1
Thanks,
Rafael
Hi,
Quoted 23 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH 1/3] ACPICA: Events: Introduce acpi_block_gpe()/acpi_unblock_gpe()/acpi_control_gpe_handling() to allow administrative GPE enabling/disablingQuoted 2 lines · Lv Zheng
There is a facility in Linux, developers can manage GPE enabling/disabling through
/sys/firmware/acpi/interrupts/gpexx. This is mainly fordebugging
Quoted 9 lines
purposes. Many users expect to use this facility to implement quirks to disable specific GPEs when there is a gap in Linux causing GPE flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE management purposes.This patch introduces acpi_block_gpe()/acpi_unblock_gpe() to be used in
such
Quoted 1 line
situation instead of acpi_enable_gpe()/acpi_disable_gpe().
Up to this point, I agree.
Quoted 8 lines
Quoted 4 lines
The idea to implement this is to replace the GPE register EN bit with the managed value, block EN set/clear operations but record the operation results in
blocked_enabled, so that after the managed state is removed, restore the savedblocked_enabledback to the GPE register EN bit.Unfortunately, I don't really follow the above paragraph, so chances are that
whoever is not familiar with the code will not be able to follow it either.
[Lv Zheng]
I see.
I should stop talking this using "managed".
It is better to use "masked".
As this facility is actually trying to implement the kind of the facility that can be seen in many other IRQ chips - the IRQ MASK bit.
Quoted 7 lines
Quoted 6 lines
Now OSPMs should be able to use this facility to generate quirks. ACPICA BZ 1102.
This facility can also be used by the administrator to control the GPE handling mode during the runtime when the driver is capable of handling
the
Quoted 8 lines
Quoted 1 line
GPE in both the interrupt mode and the polling mode (for example, the
Linux
Quoted 1 line
EC driver).
acpi_control_gpe_handling()is offered for this purpose. LvZheng.
That is too much. The patch should focus on the block/unblock functionality.
Anything beyond that should be added later IMO.
[Lv Zheng]
OK.
So after examining all of your comments.
I think what I need to improve is eliminating this feature.
That should be able to make the code simpler and thus easier for the others to follow.
Quoted 45 lines
Quoted 8 lines
Signed-off-by Lv Zheng <[email protected]>--- drivers/acpi/acpica/acevents.h | 3 + drivers/acpi/acpica/aclocal.h | 1 + drivers/acpi/acpica/evgpe.c | 84 ++++++++++++++++++++++++ drivers/acpi/acpica/evxfgpe.c | 142++++++++++++++++++++++++++++++++++++++++
Review
Quoted 6 lines
drivers/acpi/acpica/hwgpe.c | 24 ++++++- include/acpi/acpixf.h | 23 +++++-- include/acpi/actypes.h | 85 +++++++++++++++++++----- 7 files changed, 336 insertions(+), 26 deletions(-)(quoted diff) +0
-0
b/drivers/acpi/acpica/acevents.hdrivers/acpi/acpica/acevents.h acpi_ev_update_gpe_enable_mask(struct
Quoted 4 lines
index 77af91c..3a6e4db 100644
drivers/acpi/acpica/acevents.h +0
-0@@ -86,6 +86,9 @@ acpi_ev_update_gpe_enable_mask(struct
acpi_gpe_event_info*gpe_event_info);drivers/acpi/acpica/acevents.h acpi_ev_update_gpe_enable_mask(struct · line 86
Quoted 1 line
drivers/acpi/acpica/acevents.h +0
-0acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info
*gpe_event_info);drivers/acpi/acpica/acevents.h acpi_ev_update_gpe_enable_mask(struct · line 88
Quoted 3 lines
drivers/acpi/acpica/acevents.h +1
-0acpi_status +acpi_ev_manage_gpe(struct acpi_gpe_event_info *gpe_event_info, u8action);
drivers/acpi/acpica/acevents.h acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info);
Quoted 3 lines
drivers/acpi/acpica/acevents.h +2
-0+ +acpi_status acpi_ev_add_gpe_reference(struct acpi_gpe_event_info
*gpe_event_info);drivers/acpi/acpica/aclocal.h struct acpi_gpe_event_info { · line 487
Quoted 11 lines
drivers/acpi/acpica/acevents.h +0
-0acpi_statusdrivers/acpi/acpica/aclocal.h +1
-0@@ -484,6 +484,7 @@ struct acpi_gpe_event_info {u8 flags;/* Misc info about this GPE */ u8 gpe_number;/* This GPE */ u8 runtime_count;/* References to a run GPE */ +u8 blocked_enabled;/* GPE should be enabled but blocked */ I'm not sure I'm following the logic here.
Quoted 2 lines
What's needed is disabled and blocked. Why do we need enabled and blocked?
[Lv Zheng]
I just used this variable to track the "expected GPE hardware register's EN bit value" from driver's point of view. So:
1. If it is TRUE, it means GPE register's EN bit should be flagged from the drivers' point of view.
2. If it is FALSE, it means GPE register's EN bit should be unflagged from the drivers' point view.
When acpi_unblock_gpe() is invoked, I need the value here to be restored to the "GPE hardware register's EN bit". So it looks like the driver expected "EN bit value" was blocked by this facility.
That's why I use the name of "blocked_enabled".
Is it acceptable now with the justification?
Quoted 106 lines
drivers/acpi/acpica/aclocal.h struct acpi_gpe_event_info { · line 490
Quoted 4 lines
drivers/acpi/acpica/aclocal.h +0
-0};/* Information about a GPE register pair, one per each status/enable pair in an array */
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct
Quoted 5 lines
drivers/acpi/acpica/evgpe.c +0
-0@@ -130,6 +130,90 @@ acpi_status acpi_ev_enable_gpe(struct
acpi_gpe_event_info*gpe_event_info)Quoted 2 lines
/*********************************************************
**********************Quoted 10 lines
*
+ * FUNCTION:acpi_ev_manage_gpe
+ *+ * PARAMETERS: gpe_event_info - GPE to force enabling/disabling + * action - ACPI_GPE_ENABLE, ACPI_GPE_DISABLE or+ *
ACPI_GPE_UNMANAGE
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Unconditionally enable or disable an individual GPE forthe
Quoted 3 lines
+ * administrative purposes during the runtime.
+ *
+**********************************************************
********************/drivers/acpi/acpica/evgpe.c
Quoted 3 lines
drivers/acpi/acpica/evgpe.c +3
-0+ +acpi_status +acpi_ev_manage_gpe(struct acpi_gpe_event_info *gpe_event_info, u8action)
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 13 lines
drivers/acpi/acpica/evgpe.c +13
-0+{ +acpi_status status; +acpi_event_status event_status; + +ACPI_FUNCTION_TRACE (ev_manage_gpe); + +/* Perform the action */ + +switch (action) { +case ACPI_GPE_ENABLE : +case ACPI_GPE_DISABLE : + + if (!(gpe_event_info->flags &
ACPI_GPE_MANAGED_FLAG_MASK)) {drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 7 lines
drivers/acpi/acpica/evgpe.c +7
-0+ status = +acpi_hw_get_gpe_status (gpe_event_info, + &event_status); +if (ACPI_FAILURE (status)) { +return_ACPI_STATUS (status); + } +if (event_status &ACPI_EVENT_FLAG_ENABLE_SET ){
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 7 lines
drivers/acpi/acpica/evgpe.c +7
-0+ gpe_event_info->blocked_enabled = TRUE; + }else { + gpe_event_info->blocked_enabled = FALSE; + } + } + +/* Reset flags so that acpi_hw_low_set_gpe() can take effective */
drivers/acpi/acpica/evgpe.c
Quoted 2 lines
drivers/acpi/acpica/evgpe.c +2
-0+ + gpe_event_info->flags &=~ACPI_GPE_MANAGED_FLAG_MASK;
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 4 lines
drivers/acpi/acpica/evgpe.c +4
-0+if (action ==ACPI_GPE_ENABLE ) { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_ENABLE ); + gpe_event_info->flags |=
ACPI_GPE_MANAGED_ENABLED;drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 4 lines
drivers/acpi/acpica/evgpe.c +4
-0+ }else { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_DISABLE ); + gpe_event_info->flags |=
ACPI_GPE_MANAGED_DISABLED;drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 6 lines
drivers/acpi/acpica/evgpe.c +6
-0+ } +break ; + +case ACPI_GPE_UNMANAGE : + + if (!(gpe_event_info->flags &
ACPI_GPE_MANAGED_FLAG_MASK)) {drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
Quoted 4 lines
drivers/acpi/acpica/evgpe.c +4
-0+return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + +/* Reset flags so that acpi_hw_low_set_gpe() can take effective */
drivers/acpi/acpica/evgpe.c
Quoted 2 lines
drivers/acpi/acpica/evgpe.c +2
-0+ + gpe_event_info->flags &=~ACPI_GPE_MANAGED_FLAG_MASK;
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info) · line 206
Quoted 10 lines
drivers/acpi/acpica/evgpe.c +10
-0+if (gpe_event_info->blocked_enabled) { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_ENABLE ); + }else { + (void )acpi_hw_low_set_gpe (gpe_event_info, +ACPI_GPE_DISABLE ); + } + gpe_event_info->blocked_enabled = FALSE; +break ; +The code above is really hard to follow.
Quoted 3 lines
Maybe the description in the comment above the function should be more, well,
descriptive?
[Lv Zheng]
OK, I'll improve it.
Also with acpi_control_gpe_handling() deleted, code here should be much simpler.
Quoted 63 lines
drivers/acpi/acpica/evgpe.c acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info) · line 214
Quoted 10 lines
drivers/acpi/acpica/evgpe.c +8
-0+default : + +return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + +return_ACPI_STATUS (AE_OK ); +} ++/********************************************************
***********************drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 258
Quoted 11 lines
+ *
* FUNCTION:acpi_ev_add_gpe_reference
*
* PARAMETERS:gpe_event_info- Add a reference to this GPEdrivers/acpi/acpica/evxfgpe.c +0
-0@@ -257,6 +257,148 @@ ACPI_EXPORT_SYMBOL(acpi_set_gpe)/*********************************************************
**********************drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 4 lines
drivers/acpi/acpica/evxfgpe.c +3
-0* +* FUNCTION: acpi_block_gpe +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 277
Quoted 9 lines
drivers/acpi/acpica/evxfgpe.c +9
-0+* gpe_number - GPE level within the GPE block +* +* RETURN: Status +* +* DESCRIPTION: Unconditionally disable an individual GPE. +* This API is typically used by the system administrator to +* block the GPE enabling, ex., prevent the GPE flooding. +* +**********************************************************
********************/drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 13 lines
drivers/acpi/acpica/evxfgpe.c +13
-0+acpi_status acpi_block_gpe (acpi_handle gpe_device,u32 gpe_number) +{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; + +ACPI_FUNCTION_TRACE (acpi_block_gpe); + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock); + +/* Ensure that we have a valid GPE number */ + + gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device,
gpe_number);drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 6 lines
drivers/acpi/acpica/evxfgpe.c +6
-0+if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + + status = acpi_ev_manage_gpe(gpe_event_info,
ACPI_GPE_DISABLE);
Quoted 3 lines
So I would prefer the second argument here to be something like
ACPI_GPE_DISABLE|ACPI_GPE_BLOCKas then the code would be quite straightforward to understand.
[Lv Zheng]
OK.
Quoted 50 lines
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +8
-0+ +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_block_gpe) ++/********************************************************
***********************drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 4 lines
drivers/acpi/acpica/evxfgpe.c +4
-0+* +* FUNCTION: acpi_unblock_gpe +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 317
Quoted 8 lines
drivers/acpi/acpica/evxfgpe.c +8
-0+* gpe_number - GPE level within the GPE block +* +* RETURN: Status +* +* DESCRIPTION: Stop unconditional GPE disabling. +* This API reverts acpi_block_gpe(). +* +**********************************************************
********************/drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 312
Quoted 1 line
drivers/acpi/acpica/evxfgpe.c +1
-0+acpi_status acpi_unblock_gpe(acpi_handle gpe_device, u32
gpe_number)drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 12 lines
drivers/acpi/acpica/evxfgpe.c +12
-0+{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; + +ACPI_FUNCTION_TRACE (acpi_unblock_gpe); + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock); + +/* Ensure that we have a valid GPE number */ + + gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device,
gpe_number);drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 6 lines
drivers/acpi/acpica/evxfgpe.c +6
-0+if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + + status = acpi_ev_manage_gpe(gpe_event_info,
ACPI_GPE_UNMANAGE);
Quoted 1 line
And that is completely unclear to me.
[Lv Zheng]
The original code implemented 3 states.
Blocked (GPE register's EN bit is not changeable) but disabled (and it is always 0). Blocked (GPE register's EN bit is not changeable) but enabled (and it is always 1). Unblocked (GPE register's EN bit is changeable).
So the wording "MANAGE" is really a bad idea, causing all kind of misunderstandings. :)
Quoted 31 lines
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +8
-0+ +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_unblock_gpe) ++/********************************************************
***********************drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 4 lines
drivers/acpi/acpica/evxfgpe.c +4
-0+* +* FUNCTION: acpi_control_gpe_handling +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 9 lines
drivers/acpi/acpica/evxfgpe.c +9
-0+* gpe_number - GPE level within the GPE block +* use_interrupt_mode - Allow the GPE to be dispatched in the +* interrupt mode +* use_polling_mode - Allow the GPE to be dispatched in the +* polling mode +* +* RETURN: Status +* +* DESCRIPTION: Switch the GPE handling mode between the interrupt only mode,
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 353
Quoted 1 line
drivers/acpi/acpica/evxfgpe.c +1
-0+* the polling only mode and the interrupt/polling adaptive mode.
drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe) · line 355
Quoted 2 lines
drivers/acpi/acpica/evxfgpe.c +2
-0+* +
Quoted 7 lines
**********************************************************
********************/This is extra functionality that you want to piggy back on a fix for a real issue.
Please separate it from that fix, it doesn't belong here.
[Lv Zheng]
OK.
Quoted 187 lines
Quoted 12 lines
drivers/acpi/acpica/evxfgpe.c +11
-0+acpi_status +acpi_control_gpe_handling (acpi_handle gpe_device, +u32 gpe_number, +u8 use_interrupt_mode,u8 use_polling_mode) +{ +struct acpi_gpe_event_info *gpe_event_info; +acpi_status status; +acpi_cpu_flags flags; +u8 action; + +ACPI_FUNCTION_TRACE (acpi_control_gpe_handling);drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +10
-0+ +if (!use_interrupt_mode && !use_polling_mode) { +return_ACPI_STATUS (AE_BAD_PARAMETER ); + } + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock); + +/* Ensure that we have a valid GPE number */ + + gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device,
gpe_number);Quoted 14 lines
drivers/acpi/acpica/evxfgpe.c +14
-0+if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + +/* Pick up and peform the correct action */ + + action =ACPI_GPE_UNMANAGE ; +if (!use_interrupt_mode) { + action =ACPI_GPE_DISABLE ; + } +if (!use_polling_mode) { + action =ACPI_GPE_ENABLE ; + }drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 10 lines
drivers/acpi/acpica/evxfgpe.c +9
-0+ status =acpi_ev_manage_gpe (gpe_event_info, action); + +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} + +ACPI_EXPORT_SYMBOL (acpi_control_gpe_handling) ++/********************************************************
***********************drivers/acpi/acpica/evxfgpe.c ACPI_EXPORT_SYMBOL(acpi_set_gpe)
Quoted 4 lines
drivers/acpi/acpica/evxfgpe.c +1
-0+* * FUNCTION: acpi_mark_gpe_for_wake * * PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1
drivers/acpi/acpica/hwgpe.c acpi_status · line 98
Quoted 6 lines
drivers/acpi/acpica/hwgpe.c +0
-0@@ -98,7 +98,7 @@ acpi_status acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info,u32 action)
drivers/acpi/acpica/hwgpe.c acpi_hw_low_set_gpe(struct
Quoted 8 lines
drivers/acpi/acpica/hwgpe.c +1
-1{struct acpi_gpe_register_info *gpe_register_info; -acpi_status status; +acpi_status status =AE_OK ;u32 enable_mask;u32 register_bit;@@ -148,9 +148,21 @@ acpi_hw_low_set_gpe(struct
acpi_gpe_event_info*gpe_event_info, u32 action)drivers/acpi/acpica/hwgpe.c acpi_hw_get_gpe_status(struct
Quoted 25 lines
drivers/acpi/acpica/hwgpe.c +14
-2return (AE_BAD_PARAMETER ); } -/* Write the updated enable mask */ +/* Check if there is an administrative GPE enabling/disabling */ - status =acpi_hw_write (enable_mask, &gpe_register_info- enable_address); +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { +if (enable_mask & register_bit) { + gpe_event_info->blocked_enabled = TRUE; + }else { + gpe_event_info->blocked_enabled = FALSE; + } + }else { +/* Write the updated enable mask */ + + status = +acpi_hw_write (enable_mask, + &gpe_register_info->enable_address); + }return (status); }@@ -228,6 +240,12 @@ acpi_hw_get_gpe_status(struct
acpi_gpe_event_info*gpe_event_info,drivers/acpi/acpica/hwgpe.c acpi_hw_get_gpe_status(struct acpi_gpe_event_info *gpe_event_info,
Quoted 4 lines
drivers/acpi/acpica/hwgpe.c +1
-0local_event_status |=ACPI_EVENT_FLAG_HAS_HANDLER ; } +/* GPE currently managed? (enabled/disabled by the system admin?) */
include/acpi/acpixf.h
Quoted 13 lines
drivers/acpi/acpica/hwgpe.c +5
-0+ +if (gpe_event_info->flags &ACPI_GPE_MANAGED_FLAG_MASK ) { + local_event_status |=ACPI_EVENT_FLAG_MANAGED ; + } +/* Get the info block for the entire GPE register */ gpe_register_info = gpe_event_info->register_info;include/acpi/acpixf.h +0
-0@@ -732,15 +732,28 @@ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
include/acpi/acpixf.h ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
Quoted 4 lines
include/acpi/acpixf.h +0
-1u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_mark_gpe_for_wake(acpi_handle
gpe_device,include/acpi/acpixf.h ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
Quoted 12 lines
include/acpi/acpixf.h +6
-4- u32 gpe_number)) + acpi_block_gpe(acpi_handle gpe_device, + u32 gpe_number)) + +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_unblock_gpe(acpi_handle gpe_device, + u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status - acpi_setup_gpe_for_wake(acpi_handle - parent_device, - acpi_handle
gpe_device,include/acpi/acpixf.h ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
Quoted 4 lines
include/acpi/acpixf.h +4
-0+ acpi_control_gpe_handling(acpi_handle + gpe_device, + u32 gpe_number, + u8
use_interrupt_mode,include/acpi/acpixf.h line 747
Quoted 1 line
include/acpi/acpixf.h +1
-0+ u8
use_polling_mode))include/acpi/acpixf.h line 749
Quoted 2 lines
include/acpi/acpixf.h +2
-0+ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_mark_gpe_for_wake(acpi_handle
gpe_device,include/acpi/acpixf.h ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
Quoted 5 lines
include/acpi/acpixf.h +3
-0u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_setup_gpe_for_wake(acpi_handle + parent_device, + acpi_handle
gpe_device,include/acpi/acpixf.h ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
Quoted 3 lines
include/acpi/acpixf.h +2
-0+ u32 gpe_number)) +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_set_gpe_wake_mask(acpi_handle
gpe_device,Quoted 34 lines
include/acpi/acpixf.h +0
-0u32 gpe_number, u8 action))include/acpi/actypes.h +11
-10@@ -732,16 +732,17 @@ typedef u32 acpi_event_type;* The encoding of acpi_event_status is illustrated below. * Note that a set bit (1) indicates the property is TRUE * (e.g. if bit 0 is set then the event is enabled). -* +-------------+-+-+-+-+-+ -* | Bits 31:5 |4|3|2|1|0| -* +-------------+-+-+-+-+-+ -* | | | | | | -* | | | | | +- Enabled? -* | | | | +--- Enabled for wake? -* | | | +----- Status bit set? -* | | +------- Enable bit set? -* | +--------- Has a handler? -* +--------------- <Reserved> +* +-------------+-+-+-+-+-+-+ +* | Bits 31:6 |5|4|3|2|1|0| +* +-------------+-+-+-+-+-+-+ +* | | | | | | | +* | | | | | | +- Enabled? +* | | | | | +--- Enabled for wake? +* | | | | +----- Status bit set? +* | | | +------- Enable bit set? +* | | +--------- Has a handler? +* | +----------- Enabling/disabling forced? +* +----------------- <Reserved> */ typedef u32 acpi_event_status ;
Quoted 11 lines
include/acpi/actypes.h typedef u32 acpi_event_status; · line 755
Quoted 5 lines
include/acpi/actypes.h +1
-0@@ -751,6 +752,7 @@ typedef u32 acpi_event_status;#define ACPI_EVENT_FLAG_STATUS_SET (acpi_event_status) 0x04#define ACPI_EVENT_FLAG_ENABLE_SET (acpi_event_status) 0x08#define ACPI_EVENT_FLAG_HAS_HANDLER (acpi_event_status) 0x10 +#define ACPI_EVENT_FLAG_MANAGED (acpi_event_status) 0x20The meaning of this new flag is unclear. It probably should be something like
ACPI_EVENT_FLAG_BLOCKEDor MASKED meaning that if set, the GPE cannot be
disabled or enabled by usual means (until that flag is cleared).
[Lv Zheng]
OK.
Quoted 89 lines
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 4 lines
include/acpi/actypes.h +0
-0#define ACPI_EVENT_FLAG_SET ACPI_EVENT_FLAG_STATUS_SET/* Actions for acpi_set_gpe, acpi_gpe_wakeup, acpi_hw_low_set_gpe */
Quoted 30 lines
include/acpi/actypes.h +11
-8@@ -758,17 +760,20 @@ typedef u32 acpi_event_status;#define ACPI_GPE_ENABLE 0#define ACPI_GPE_DISABLE 1#define ACPI_GPE_CONDITIONAL_ENABLE 2 +#define ACPI_GPE_UNMANAGE 3/* * GPE info flags - Per GPE -* +-------+-+-+---+ -* | 7:5 |4|3|2:0| -* +-------+-+-+---+ -* | | | | -* | | | +-- Type of dispatch:to method, handler, notify, or none -* | | +----- Interrupt type: edge or level triggered -* | +------- Is a Wake GPE -* +------------ <Reserved> +* +-+-+-+-+-+---+ +* |7|6|5|4|3|2:0| +* +-+-+-+-+-+---+ +* | | | | | | +* | | | | | +-- Type of dispatch:to method, handler, notify, or none +* | | | | +----- Interrupt type: edge or level triggered +* | | | +------- Is a Wake GPE +* | | +--------- Force GPE enabling +* | +----------- Force GPE disabling +* +------------- <Reserved> */ #define ACPI_GPE_DISPATCH_NONE (u8) 0x00#define ACPI_GPE_DISPATCH_METHOD (u8) 0x01include/acpi/actypes.h typedef u32 acpi_event_status; · line 795
Quoted 6 lines
include/acpi/actypes.h +3
-0@@ -785,6 +790,50 @@ typedef u32 acpi_event_status;#define ACPI_GPE_CAN_WAKE (u8) 0x10/* +* Flags used by the GPE management APIs +* +* The GPE management APIs are not implemented for the GPE drivers. It is
include/acpi/actypes.h typedef u32 acpi_event_status; · line 796
Quoted 1 line
include/acpi/actypes.h +1
-0+* designed for providing the management purposes out of the GPE drivers'
include/acpi/actypes.h typedef u32 acpi_event_status; · line 798
Quoted 2 lines
include/acpi/actypes.h +2
-0+* awareness. The GPE driver APIs should still be working as expected +* during the period the GPE management purposes are applied. There are 2
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 5 lines
include/acpi/actypes.h +5
-0+* use cases for the flags: +* +* 1. Prevent the GPE flooding +* These flags can be used to control how the GPE is dispatched by the +* event dispatcher. Note that the ACPI_GPE_MANAGED_DISABLED can also be
include/acpi/actypes.h typedef u32 acpi_event_status; · line 804
Quoted 1 line
include/acpi/actypes.h +1
-0+* used to prevent the GPE flooding that cannot be prevented due to the
include/acpi/actypes.h typedef u32 acpi_event_status; · line 805
Quoted 1 line
include/acpi/actypes.h +1
-0+* ACPI_GPE_DISPATCH_NONE sanity check. This kind of the GPE flooding
include/acpi/actypes.h typedef u32 acpi_event_status; · line 806
Quoted 1 line
include/acpi/actypes.h +1
-0+* happens on the GPE where there is a _Lxx/_Exx prepared by the BIOS
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 4 lines
include/acpi/actypes.h +4
-0+* but the OSPM still cannot handle it correctly by executing the +* method. Such kind of incapability is mostly because of the feature +* gaps in the OSPM: +* ACPI_GPE_MANAGED_ENABLED: force the event dispatcher to always
include/acpi/actypes.h typedef u32 acpi_event_status; · line 812
Quoted 2 lines
include/acpi/actypes.h +2
-0+* enable the GPE +* ACPI_GPE_MANAGED_DISABLED: force the event dispatcher to always
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 7 lines
include/acpi/actypes.h +7
-0+* disable the GPE +* prevent the GPE from flooding +* acpi_block_gpe()/acpi_unblock_gpe() are the APIs offering the GPE +* flooding prevention feature. +* +* 2. Control the GPE handling mode +* A GPE driver may be able to handle the GPE both in the interrupt mode
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 4 lines
include/acpi/actypes.h +4
-0+* and in the polling mode. Since the polling mode is a mechanism +* implemented by the driver itself and is not controlled by the event +* dispatcher, this mechanism can be used to switch between the +* interrupt mode (dispatched via the event dispatcher) and the polling
include/acpi/actypes.h typedef u32 acpi_event_status; · line 825
Quoted 2 lines
include/acpi/actypes.h +2
-0+* mode (implemented by the GPE drivers): +* ACPI_GPE_MANAGED_ENABLED: force the driver to use the interrupt mode
include/acpi/actypes.h typedef u32 acpi_event_status; · line 826
Quoted 1 line
include/acpi/actypes.h +1
-0+* ACPI_GPE_MANAGED_DISABLED: force the driver to use the polling mode
include/acpi/actypes.h typedef u32 acpi_event_status;
Quoted 3 lines
include/acpi/actypes.h +3
-0+* None: allow the driver to use the +* interrupt/polling adaptive mode +* acpi_control_gpe_handling() is the API offering the GPE handling mode
Quoted 13 lines
include/acpi/actypes.h typedef u32 acpi_event_status; · line 839
Quoted 10 lines
include/acpi/actypes.h +7
-0+* switch feature. +*/ +#define ACPI_GPE_MANAGED_ENABLED (u8) 0x20 +#define ACPI_GPE_MANAGED_DISABLED (u8) 0x40 +#define ACPI_GPE_MANAGED_FLAG_MASK (u8) 0x60 + +/* * Flags for GPE and Lock interfaces */ #define ACPI_NOT_ISR 0x1Thanks,
Rafael
[Lv Zheng]
Thanks for the review.
Best regards
-Lv
Rafael J. Wysocki
maintainerQuoted 30 lines · Lv
Hi,
Quoted 28 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH 1/3] ACPICA: Events: Introduce acpi_block_gpe()/acpi_unblock_gpe()/acpi_control_gpe_handling() to allow administrative GPE enabling/disablingQuoted 2 lines · Lv Zheng
There is a facility in Linux, developers can manage GPE enabling/disabling through
/sys/firmware/acpi/interrupts/gpexx. This is mainly fordebugging
Quoted 9 lines
purposes. Many users expect to use this facility to implement quirks to disable specific GPEs when there is a gap in Linux causing GPE flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE management purposes.This patch introduces acpi_block_gpe()/acpi_unblock_gpe() to be used in
such
Quoted 1 line
situation instead of acpi_enable_gpe()/acpi_disable_gpe().
Up to this point, I agree.
Quoted 4 lines
The idea to implement this is to replace the GPE register EN bit with the managed value, block EN set/clear operations but record the operation results in
blocked_enabled, so that after the managed state is removed, restore the savedblocked_enabledback to the GPE register EN bit.
Quoted 8 lines
Quoted 3 lines
Unfortunately, I don't really follow the above paragraph, so chances are that
whoever is not familiar with the code will not be able to follow it either.[Lv Zheng]
I see.
I should stop talking this using "managed".
It is better to use "masked".
As this facility is actually trying to implement the kind of the facility that can be seen in many other IRQ chips - the IRQ MASK bit.
This is a somewhat simplified special case of it, though.
Quoted 11 lines
Quoted 11 lines
Quoted 5 lines
Now OSPMs should be able to use this facility to generate quirks. ACPICA BZ 1102.
This facility can also be used by the administrator to control the GPE handling mode during the runtime when the driver is capable of handling
the
Quoted 1 line
GPE in both the interrupt mode and the polling mode (for example, the
Linux
Quoted 1 line
EC driver).
acpi_control_gpe_handling()is offered for this purpose. LvZheng.
Quoted 8 lines
Quoted 3 lines
That is too much. The patch should focus on the block/unblock functionality.
Anything beyond that should be added later IMO.[Lv Zheng]
OK.
So after examining all of your comments.
I think what I need to improve is eliminating this feature.
That should be able to make the code simpler and thus easier for the others to follow.
Sounds good!
PATCH v2 2/3 ACPI / sys: Update /sys/firmware/acpi/interrupts/gpexx using new GPE forced disabling/enabling mechanism
In reply to a message not in the archiveNow GPE can be forced enabling/disabling through GPE management APIs, this patch modifies /sys/firmware/acpi/interrupts/gpexx to use this new facility.
The "block" command is implemented to invoke acpi_block_gpe() and the "unblock" command is implemented to invoke acpi_unblock_gpe(). The "force-poll"/"force-irq"/"unforce" command is implemented to invoke acpi_control_gpe_handling() to switch GPE handling mode (for EC driver only).
"EN STS" is returned to display the current hardware register status, along with "!" flag to indicate the register bit unset and "*" flag to indicate the managed state.
--- drivers/acpi/sleep.c | 2 +- drivers/acpi/sysfs.c | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
drivers/acpi/sleep.c +1 -1
@@ -555,7 +555,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state)acpi_get_event_status (ACPI_EVENT_POWER_BUTTON , &pwr_btn_status); -if (pwr_btn_status &ACPI_EVENT_FLAG_SET ) { +if (pwr_btn_status &ACPI_EVENT_FLAG_STATUS_SET ) {acpi_clear_event (ACPI_EVENT_POWER_BUTTON );/* Flag for later */ pwr_btn_event_pending = true;
drivers/acpi/sysfs.c +32 -4
@@ -599,6 +599,19 @@ static ssize_t counter_show(struct kobject *kobj,if (result) goto end; +if (status &ACPI_EVENT_FLAG_ENABLE_SET ) + size +=sprintf (buf + size," EN" ); +else + size +=sprintf (buf + size," !EN" ); +if (status &ACPI_EVENT_FLAG_MANAGED ) + size +=sprintf (buf + size,"*" ); +else + size +=sprintf (buf + size," " ); +if (status &ACPI_EVENT_FLAG_STATUS_SET ) + size +=sprintf (buf + size," STS" ); +else + size +=sprintf (buf + size," !STS" ); +if (!(status &ACPI_EVENT_FLAG_HAS_HANDLER )) size +=sprintf (buf + size," invalid" );else if (status &ACPI_EVENT_FLAG_ENABLED )
@@ -656,8 +669,23 @@ static ssize_t counter_set(struct kobject *kobj,else if (!strcmp (buf,"enable\n" ) && !(status &ACPI_EVENT_FLAG_ENABLED )) result =acpi_enable_gpe (handle, index); +else if (!strcmp (buf,"block\n" )) + result =acpi_block_gpe (handle, index); +else if (!strcmp (buf,"unblock\n" ) && + (status &ACPI_EVENT_FLAG_MANAGED )) + result =acpi_unblock_gpe (handle, index); +else if (!strcmp (buf,"force-poll\n" )) + result =acpi_control_gpe_handling (handle, index, + TRUE, FALSE); +else if (!strcmp (buf,"force-irq\n" )) + result =acpi_control_gpe_handling (handle, index, + FALSE, TRUE); +else if (!strcmp (buf,"unforce\n" ) && + (status &ACPI_EVENT_FLAG_MANAGED )) + result =acpi_control_gpe_handling (handle, index, + TRUE, TRUE);else if (!strcmp (buf,"clear\n" ) && - (status &ACPI_EVENT_FLAG_SET )) + (status &ACPI_EVENT_FLAG_STATUS_SET )) result =acpi_clear_gpe (handle, index);else if (!kstrtoul (buf,0 , &tmp)) all_counters[index].count = tmp;
@@ -666,13 +694,13 @@ static ssize_t counter_set(struct kobject *kobj, }else if (index < num_gpes +ACPI_NUM_FIXED_EVENTS ) {int event = index - num_gpes;if (!strcmp (buf,"disable\n" ) && - (status &ACPI_EVENT_FLAG_ENABLED )) + (status &ACPI_EVENT_FLAG_ENABLE_SET )) result =acpi_disable_event (event,ACPI_NOT_ISR );else if (!strcmp (buf,"enable\n" ) && - !(status &ACPI_EVENT_FLAG_ENABLED )) + !(status &ACPI_EVENT_FLAG_ENABLE_SET )) result =acpi_enable_event (event,ACPI_NOT_ISR );else if (!strcmp (buf,"clear\n" ) && - (status &ACPI_EVENT_FLAG_SET )) + (status &ACPI_EVENT_FLAG_STATUS_SET )) result =acpi_clear_event (event);else if (!kstrtoul (buf,0 , &tmp)) all_counters[index].count = tmp;
Signature · 1 line
1.7.10
PATCH v2 3/3 ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
In reply to a message not in the archiveSometimes, the users may require a quirk to be provided from ACPI subsystem core to prevent a GPE from flooding. Normally, if a GPE cannot be dispatched, ACPICA core automatically prevents the GPE from firing. But there are cases the GPE is dispatched by _Lxx/_Exx provided via AML table, and OSPM is lacking of the knowledge to get _Lxx/_Exx correctly executed to handle the GPE, thus the GPE flooding may still occur.
This patch provides a quirk mechanism to stop this kind of GPE flooding.
--- drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sleep.c | 2 ++ drivers/acpi/sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+)
drivers/acpi/internal.h +1 -0
@@ -37,6 +37,7 @@ void acpi_amba_init(void);static inline void acpi_amba_init (void ) {}#endif int acpi_sysfs_init (void ); +void acpi_gpe_apply_blocked_gpes (void );void acpi_container_init (void );void acpi_memory_hotplug_init (void );#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
drivers/acpi/scan.c +1 -0
@@ -1958,6 +1958,7 @@ int __init acpi_scan_init(void) } } +acpi_gpe_apply_blocked_gpes ();acpi_update_all_gpes (); out:
drivers/acpi/sleep.c +2 -0
@@ -414,6 +414,7 @@ static void acpi_pm_finish(void) acpi_state);acpi_disable_wakeup_devices (acpi_state);acpi_leave_sleep_state (acpi_state); +acpi_gpe_apply_blocked_gpes ();/* reset firmware waking vector */ acpi_set_waking_vector (0 );
@@ -774,6 +775,7 @@ static void acpi_pm_thaw(void) {acpi_ec_unblock_transactions ();acpi_enable_all_runtime_gpes (); +acpi_gpe_apply_blocked_gpes (); }static const struct platform_hibernation_ops acpi_hibernation_ops = {
drivers/acpi/sysfs.c +56 -0
62 lines not on this page · raw message
Signature · 1 line
1.7.10
v2 cover ACPI / gpe: Add GPE masking/unmasking mechanism
In reply to a message not in the archiveThis patchset adds a new mechanism that can be used to mask/unmask GPEs.
Lv Zheng (3):
ACPICA: Events: Introduce acpi_mask_gpe()/acpi_unmask_gpe() to
implement GPE masking mechanism
ACPI / sys: Update /sys/firmware/acpi/interrupts/gpexx using new GPE
masking mechanism
ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
Documentation/kernel-parameters.txt | 10 ++++ drivers/acpi/acpica/acevents.h | 3 ++ drivers/acpi/acpica/aclocal.h | 2 + drivers/acpi/acpica/evgpe.c | 57 ++++++++++++++++++++++ drivers/acpi/acpica/evxfgpe.c | 43 +++++++++++++++++ drivers/acpi/acpica/hwgpe.c | 23 +++++++-- drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sleep.c | 2 +- drivers/acpi/sysfs.c | 89 +++++++++++++++++++++++++++++++---- include/acpi/acpixf.h | 4 ++ include/acpi/actypes.h | 39 ++++++++------- 12 files changed, 242 insertions(+), 32 deletions(-)
Signature · 1 line
1.7.10
PATCH v2 1/3 ACPICA: Events: Introduce acpi_mask_gpe()/acpi_unmask_gpe() to implement GPE masking mechanism
There is a facility in Linux, developers can control the enabling/disabling of a GPE via /sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes.
But many users expect to use this facility to implement quirks to mask a specific GPE when there is a gap in Linux causing this GPE to flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE masking purposes.
However, most of the IRQ chip designs allow masking/unmasking IRQs via a masking bit which is different from the enabled bit to achieve the same purpose. But the GPE hardware doesn't contain such a feature, this brings the trouble.
In this patch, we introduce a software mechanism to implement the GPE masking feature, and acpi_mask_gpe()/acpi_unmask_gpe() are provided to the OSPMs to mask/unmask GPEs in the above mentioned situation instead of acpi_enable_gpe()/acpi_disable_gpe(). ACPICA BZ 1102. Lv Zheng.
--- drivers/acpi/acpica/acevents.h | 3 +++ drivers/acpi/acpica/aclocal.h | 2 ++ drivers/acpi/acpica/evgpe.c | 57 ++++++++++++++++++++++++++++++++++++++++ drivers/acpi/acpica/evxfgpe.c | 43 ++++++++++++++++++++++++++++++ drivers/acpi/acpica/hwgpe.c | 23 ++++++++++++---- include/acpi/acpixf.h | 4 +++ include/acpi/actypes.h | 39 ++++++++++++++------------- 7 files changed, 148 insertions(+), 23 deletions(-)
drivers/acpi/acpica/acevents.h +3 -0
@@ -86,6 +86,9 @@ acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info);acpi_status acpi_ev_enable_gpe (struct acpi_gpe_event_info *gpe_event_info);acpi_status +acpi_ev_mask_gpe (struct acpi_gpe_event_info *gpe_event_info,u8 is_masked); + +acpi_status acpi_ev_add_gpe_reference (struct acpi_gpe_event_info *gpe_event_info); acpi_status
drivers/acpi/acpica/aclocal.h +2 -0
@@ -484,6 +484,7 @@ struct acpi_gpe_event_info {u8 flags;/* Misc info about this GPE */ u8 gpe_number;/* This GPE */ u8 runtime_count;/* References to a run GPE */ +u8 disable_for_dispatch;/* Masked during dispatching */ };/* Information about a GPE register pair, one per each status/enable pair in an array */
@@ -494,6 +495,7 @@ struct acpi_gpe_register_info {u16 base_gpe_number;/* Base GPE number for this register */ u8 enable_for_wake;/* GPEs to keep enabled when sleeping */ u8 enable_for_run;/* GPEs to keep enabled when running */ +u8 mask_for_run;/* GPEs to keep masked when running */ u8 enable_mask;/* Current mask of enabled GPEs */ };
drivers/acpi/acpica/evgpe.c +57 -0
75 lines not on this page · raw message
drivers/acpi/acpica/evxfgpe.c +43 -0
60 lines not on this page · raw message
drivers/acpi/acpica/hwgpe.c +18 -5
@@ -98,7 +98,7 @@ acpi_status acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action) {struct acpi_gpe_register_info *gpe_register_info; -acpi_status status; +acpi_status status =AE_OK ;u32 enable_mask;u32 register_bit;
@@ -148,9 +148,14 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)return (AE_BAD_PARAMETER ); } -/* Write the updated enable mask */ +if (!(register_bit & gpe_register_info->mask_for_run)) { - status =acpi_hw_write (enable_mask, &gpe_register_info->enable_address); +/* Write the updated enable mask */ + + status = +acpi_hw_write (enable_mask, + &gpe_register_info->enable_address); + }return (status); }
@@ -242,6 +247,12 @@ acpi_hw_get_gpe_status(struct acpi_gpe_event_info *gpe_event_info, local_event_status |=ACPI_EVENT_FLAG_ENABLED ; } +/* GPE currently masked? (masked for runtime?) */ + +if (register_bit & gpe_register_info->mask_for_run) { + local_event_status |=ACPI_EVENT_FLAG_MASKED ; + } +/* GPE enabled for wake? */ if (register_bit & gpe_register_info->enable_for_wake) {
@@ -397,6 +408,7 @@ acpi_hw_enable_runtime_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,u32 i;acpi_status status;struct acpi_gpe_register_info *gpe_register_info; +u32 enable_mask;/* NOTE: assumes that all GPEs are currently disabled */
@@ -410,9 +422,10 @@ acpi_hw_enable_runtime_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,/* Enable all "runtime" GPEs in this register */ + enable_mask = gpe_register_info->enable_for_run & + ~gpe_register_info->mask_for_run; status = -acpi_hw_gpe_enable_write (gpe_register_info->enable_for_run, - gpe_register_info); +acpi_hw_gpe_enable_write (enable_mask, gpe_register_info);if (ACPI_FAILURE (status)) {return (status); }
include/acpi/acpixf.h +4 -0
@@ -732,6 +732,10 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status u32 gpe_number)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status + acpi_mask_gpe(acpi_handle gpe_device, + u32 gpe_number, u8 is_masked)) + +ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number))
include/acpi/actypes.h +21 -18
@@ -732,16 +732,17 @@ typedef u32 acpi_event_type;* The encoding of acpi_event_status is illustrated below. * Note that a set bit (1) indicates the property is TRUE * (e.g. if bit 0 is set then the event is enabled). -* +-------------+-+-+-+-+-+ -* | Bits 31:5 |4|3|2|1|0| -* +-------------+-+-+-+-+-+ -* | | | | | | -* | | | | | +- Enabled? -* | | | | +--- Enabled for wake? -* | | | +----- Status bit set? -* | | +------- Enable bit set? -* | +--------- Has a handler? -* +--------------- <Reserved> +* +-------------+-+-+-+-+-+-+ +* | Bits 31:6 |5|4|3|2|1|0| +* +-------------+-+-+-+-+-+-+ +* | | | | | | | +* | | | | | | +- Enabled? +* | | | | | +--- Enabled for wake? +* | | | | +----- Status bit set? +* | | | +------- Enable bit set? +* | | +--------- Has a handler? +* | +----------- Masked? +* +----------------- <Reserved> */ typedef u32 acpi_event_status ;
@@ -751,6 +752,7 @@ typedef u32 acpi_event_status;#define ACPI_EVENT_FLAG_STATUS_SET (acpi_event_status) 0x04#define ACPI_EVENT_FLAG_ENABLE_SET (acpi_event_status) 0x08#define ACPI_EVENT_FLAG_HAS_HANDLER (acpi_event_status) 0x10 +#define ACPI_EVENT_FLAG_MASKED (acpi_event_status) 0x20#define ACPI_EVENT_FLAG_SET ACPI_EVENT_FLAG_STATUS_SET/* Actions for acpi_set_gpe, acpi_gpe_wakeup, acpi_hw_low_set_gpe */
@@ -761,14 +763,15 @@ typedef u32 acpi_event_status;/* * GPE info flags - Per GPE -* +-------+-+-+---+ -* | 7:5 |4|3|2:0| -* +-------+-+-+---+ -* | | | | -* | | | +-- Type of dispatch:to method, handler, notify, or none -* | | +----- Interrupt type: edge or level triggered -* | +------- Is a Wake GPE -* +------------ <Reserved> +* +---+-+-+-+---+ +* |7:6|5|4|3|2:0| +* +---+-+-+-+---+ +* | | | | | +* | | | | +-- Type of dispatch:to method, handler, notify, or none +* | | | +----- Interrupt type: edge or level triggered +* | | +------- Is a Wake GPE +* | +--------- Is GPE masked by the software GPE masking machanism +* +------------ <Reserved> */ #define ACPI_GPE_DISPATCH_NONE (u8) 0x00#define ACPI_GPE_DISPATCH_METHOD (u8) 0x01
Signature · 1 line
1.7.10
Re: [UPDATE PATCH v2 1/3] ACPICA: Events: Introduce acpi_mask_gpe() to implement GPE masking mechanism
Quoted 19 lines · Lv Zheng
(remove
acpi_unmask_gpe()from the patch description)There is a facility in Linux, developers can control the enabling/disabling of a GPE via
/sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes.But many users expect to use this facility to implement quirks to mask a specific GPE when there is a gap in Linux causing this GPE to flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE masking purposes.However, most of the IRQ chip designs allow masking/unmasking IRQs via a masking bit which is different from the enabled bit to achieve the same purpose. But the GPE hardware doesn't contain such a feature, this brings the trouble.
Quoted 7 lines
In this patch, we introduce a software mechanism to implement the GPE masking feature, and
acpi_mask_gpe()are provided to the OSPMs to mask/unmask GPEs in the above mentioned situation instead of acpi_enable_gpe()/acpi_disable_gpe(). ACPICA BZ 1102. Lv Zheng.Signed-off-by Lv Zheng <[email protected]>
I've queued up this one and the [2/3] and please see my comments on the [3/3].
Thanks,
Rafael
Re: [UPDATE PATCH v2 1/3] ACPICA: Events: Introduce acpi_mask_gpe() to implement GPE masking mechanism
Quoted 25 lines · Rafael J. Wysocki
Quoted 24 lines · Lv Zheng
(remove
acpi_unmask_gpe()from the patch description)There is a facility in Linux, developers can control the enabling/disabling of a GPE via
/sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes.But many users expect to use this facility to implement quirks to mask a specific GPE when there is a gap in Linux causing this GPE to flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE masking purposes.However, most of the IRQ chip designs allow masking/unmasking IRQs via a masking bit which is different from the enabled bit to achieve the same purpose. But the GPE hardware doesn't contain such a feature, this brings the trouble.
In this patch, we introduce a software mechanism to implement the GPE masking feature, and
acpi_mask_gpe()are provided to the OSPMs to mask/unmask GPEs in the above mentioned situation instead of acpi_enable_gpe()/acpi_disable_gpe(). ACPICA BZ 1102. Lv Zheng.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Lv Zheng <[email protected]>I've queued up this one and the [2/3] and please see my comments on the [3/3].
I've decided that it's better if this goes in via upstream ACPICA, so it's not in the queue any more.
For the time being, I'd like all changes in the ACPICA code to go in via the upstream.
Thanks,
Rafael
RE: [UPDATE PATCH v2 1/3] ACPICA: Events: Introduce acpi_mask_gpe() to implement GPE masking mechanism
Hi, Rafael
Quoted 38 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [UPDATE PATCH v2 1/3] ACPICA: Events: Introduceacpi_mask_gpe()to implement GPE masking mechanismQuoted 4 lines · Rafael J. Wysocki
Quoted 3 lines · Lv Zheng
(remove
acpi_unmask_gpe()from the patch description)There is a facility in Linux, developers can control the
enabling/disabling
Quoted 4 lines
Quoted 4 lines
of a GPE via
/sys/firmware/acpi/interrupts/gpexx. This is mainly for debugging purposes.But many users expect to use this facility to implement quirks to mask
a
Quoted 8 lines
Quoted 8 lines
specific GPE when there is a gap in Linux causing this GPE to flood. This is not working correctly because currently this facility invokes enabling/disabling counting based GPE driver APIs:
acpi_enable_gpe()/acpi_disable_gpe()
and the GPE drivers can still affect the count to mess up the GPE masking purposes.However, most of the IRQ chip designs allow masking/unmasking IRQs
via a
Quoted 2 lines
Quoted 2 lines
masking bit which is different from the enabled bit to achieve the same purpose. But the GPE hardware doesn't contain such a feature, this
brings
Quoted 3 lines
Quoted 3 lines
the trouble.
In this patch, we introduce a software mechanism to implement the
GPE
Quoted 7 lines
Quoted 6 lines
masking feature, and
acpi_mask_gpe()are provided to the OSPMs to mask/unmask GPEs in the above mentioned situation instead of acpi_enable_gpe()/acpi_disable_gpe(). ACPICA BZ 1102. Lv Zheng.Signed-off-by Lv Zheng <[email protected]>
Quoted 8 lines
Quoted 1 line
I've queued up this one and the [2/3] and please see my comments on
the [3/3].
I've decided that it's better if this goes in via upstream ACPICA, so it's not in the queue any more.
For the time being, I'd like all changes in the ACPICA code to go in via the upstream.
[Lv Zheng]
OK.
Unlike several others, this is not an urgent feature or a feature that ACPICA upstream doesn't have environment to test. So we needn't to make in the upstream Linux first.
Thanks and best regards
-Lv
PATCH v2 2/3 ACPI / sys: Update /sys/firmware/acpi/interrupts/gpexx using new GPE masking mechanism
In reply to Lv ZhengNow GPE can be masked via new acpi_mask_gpe() API, this patch modifies /sys/firmware/acpi/interrupts/gpexx to use this new facility.
Writes "mask/unmask" to this file now invoke acpi_mask_gpe().
Reads from this file now returns new "EN/STS" when the corresponding GPE hardware register's EN/STS bits are flagged, and new "masked/unmasked" attribute to indicate the status of the masking mechanism.
--- drivers/acpi/sleep.c | 2 +- drivers/acpi/sysfs.c | 33 +++++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-)
drivers/acpi/sleep.c +1 -1
@@ -555,7 +555,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state)acpi_get_event_status (ACPI_EVENT_POWER_BUTTON , &pwr_btn_status); -if (pwr_btn_status &ACPI_EVENT_FLAG_SET ) { +if (pwr_btn_status &ACPI_EVENT_FLAG_STATUS_SET ) {acpi_clear_event (ACPI_EVENT_POWER_BUTTON );/* Flag for later */ pwr_btn_event_pending = true;
drivers/acpi/sysfs.c +25 -8
@@ -599,14 +599,27 @@ static ssize_t counter_show(struct kobject *kobj,if (result) goto end; +if (status &ACPI_EVENT_FLAG_ENABLE_SET ) + size +=sprintf (buf + size," EN" ); +else + size +=sprintf (buf + size," " ); +if (status &ACPI_EVENT_FLAG_STATUS_SET ) + size +=sprintf (buf + size," STS" ); +else + size +=sprintf (buf + size," " ); +if (!(status &ACPI_EVENT_FLAG_HAS_HANDLER )) - size +=sprintf (buf + size," invalid" ); + size +=sprintf (buf + size," invalid " );else if (status &ACPI_EVENT_FLAG_ENABLED ) - size +=sprintf (buf + size," enabled" ); + size +=sprintf (buf + size," enabled " );else if (status &ACPI_EVENT_FLAG_WAKE_ENABLED ) - size +=sprintf (buf + size," wake_enabled" ); + size +=sprintf (buf + size," wake_enabled" );else - size +=sprintf (buf + size," disabled" ); + size +=sprintf (buf + size," disabled " ); +if (status &ACPI_EVENT_FLAG_MASKED ) + size +=sprintf (buf + size," masked " ); +else + size +=sprintf (buf + size," unmasked" ); end: size +=sprintf (buf + size,"\n" );
@@ -657,8 +670,12 @@ static ssize_t counter_set(struct kobject *kobj, !(status & ACPI_EVENT_FLAG_ENABLED)) result =acpi_enable_gpe (handle, index); else if (!strcmp(buf, "clear\n") && - (status & ACPI_EVENT_FLAG_SET)) + (status & ACPI_EVENT_FLAG_STATUS_SET)) result =acpi_clear_gpe (handle, index); + else if (!strcmp(buf, "mask\n")) + result =acpi_mask_gpe (handle, index, TRUE); + else if (!strcmp(buf, "unmask\n")) + result =acpi_mask_gpe (handle, index, FALSE); else if (!kstrtoul(buf, 0, &tmp)) all_counters[index].count = tmp; else
@@ -666,13 +683,13 @@ static ssize_t counter_set(struct kobject *kobj, }else if (index < num_gpes +ACPI_NUM_FIXED_EVENTS ) {int event = index - num_gpes;if (!strcmp (buf,"disable\n" ) && - (status &ACPI_EVENT_FLAG_ENABLED )) + (status &ACPI_EVENT_FLAG_ENABLE_SET )) result =acpi_disable_event (event,ACPI_NOT_ISR );else if (!strcmp (buf,"enable\n" ) && - !(status &ACPI_EVENT_FLAG_ENABLED )) + !(status &ACPI_EVENT_FLAG_ENABLE_SET )) result =acpi_enable_event (event,ACPI_NOT_ISR );else if (!strcmp (buf,"clear\n" ) && - (status &ACPI_EVENT_FLAG_SET )) + (status &ACPI_EVENT_FLAG_STATUS_SET )) result =acpi_clear_event (event);else if (!kstrtoul (buf,0 , &tmp)) all_counters[index].count = tmp;
Signature · 1 line
1.7.10
PATCH v2 3/3 ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
In reply to Lv ZhengSometimes, the users may require a quirk to be provided from ACPI subsystem core to prevent a GPE from flooding. Normally, if a GPE cannot be dispatched, ACPICA core automatically prevents the GPE from firing. But there are cases the GPE is dispatched by _Lxx/_Exx provided via AML table, and OSPM is lacking of the knowledge to get _Lxx/_Exx correctly executed to handle the GPE, thus the GPE flooding may still occur.
This patch provides a quirk mechanism to stop this kind of GPE flooding.
--- Documentation/kernel-parameters.txt | 10 +++++++ drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sysfs.c | 56 +++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+)
Documentation/kernel-parameters.txt +10 -0
@@ -274,6 +274,16 @@ bytes respectively. Such letter suffixes can also be entirely omitted. use by PCI Format: <irq>,<irq>... + acpi_mask_gpe= [HW,ACPI] + Due to the existence of _Lxx/_Exx, some GPEs triggered + by unsupported hardware/firmware features can result in + GPE floodings that cannot be automatically disabled by + the GPE dispatcher. + This facility can be used to prevent such uncontrolled + GPE floodings. + Format: <int> + Support masking of GPEs numbered from 0x00 to 0x63. + acpi_no_auto_serialize [HW,ACPI] Disable auto-serialization of AML methods AML control methods that contain the opcodes to create
drivers/acpi/internal.h +1 -0
@@ -37,6 +37,7 @@ void acpi_amba_init(void);static inline void acpi_amba_init (void ) {}#endif int acpi_sysfs_init (void ); +void acpi_gpe_apply_masked_gpes (void );void acpi_container_init (void );void acpi_memory_hotplug_init (void );#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
drivers/acpi/scan.c +1 -0
@@ -1958,6 +1958,7 @@ int __init acpi_scan_init(void) } } +acpi_gpe_apply_masked_gpes ();acpi_update_all_gpes (); out:
drivers/acpi/sysfs.c +56 -0
62 lines not on this page · raw message
Signature · 1 line
1.7.10
Re: [PATCH v2 3/3] ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
Quoted 26 lines · Lv Zheng
Sometimes, the users may require a quirk to be provided from ACPI subsystem core to prevent a GPE from flooding. Normally, if a GPE cannot be dispatched, ACPICA core automatically prevents the GPE from firing. But there are cases the GPE is dispatched by _Lxx/_Exx provided via AML table, and OSPM is lacking of the knowledge to get _Lxx/_Exx correctly executed to handle the GPE, thus the GPE flooding may still occur.
This patch provides a quirk mechanism to stop this kind of GPE flooding.
Signed-off-by Lv Zheng <[email protected]>--- Documentation/kernel-parameters.txt | 10 +++++++ drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sysfs.c | 56 +++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+)Documentation/kernel-parameters.txt +0
-0@@ -274,6 +274,16 @@ bytes respectively. Such letter suffixes can also be entirely omitted. use by PCI Format: <irq>,<irq>...
Documentation/kernel-parameters.txt bytes respectively. Such letter suffixes can also be entirely omitted. · line 285
Quoted 10 lines
Documentation/kernel-parameters.txt +9
-0+ acpi_mask_gpe= [HW,ACPI] + Due to the existence of _Lxx/_Exx, some GPEs triggered + by unsupported hardware/firmware features can result in + GPE floodings that cannot be automatically disabled by + the GPE dispatcher. + This facility can be used to prevent such uncontrolled + GPE floodings. + Format: <int> + Support masking of GPEs numbered from 0x00 to 0x63.
So what about if someone needs to mask more than one GPE?
The code indicates that acpi_mask_gpe= needs to be used multiple times then, but it would be good to mention that here too.
Quoted 58 lines
Documentation/kernel-parameters.txt +1
-0+ acpi_no_auto_serialize [HW,ACPI] Disable auto-serialization of AML methods AML control methods that contain the opcodes to createdrivers/acpi/internal.h +1
-0@@ -37,6 +37,7 @@ void acpi_amba_init(void);static inline void acpi_amba_init (void ) {}#endif int acpi_sysfs_init (void ); +void acpi_gpe_apply_masked_gpes (void );void acpi_container_init (void );void acpi_memory_hotplug_init (void );#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC drivers/acpi/scan.c +1
-0@@ -1958,6 +1958,7 @@ int __init acpi_scan_init(void) } } +acpi_gpe_apply_masked_gpes ();acpi_update_all_gpes (); out:drivers/acpi/sysfs.c +22
-0@@ -704,6 +704,62 @@ end:return result ? result : size; } +/* +* A Quirk Mechanism for GPE Flooding Prevention: +* +* Quirks may be needed to prevent GPE flooding on a specific GPE. The +* flooding typically cannot be detected and automatically prevented by +* ACPI_GPE_DISPATCH_NONE check because there is a _Lxx/_Exx prepared in +* the AML tables. This normally indicates a feature gap in Linux, thus +* instead of providing endless quirk tables, we provide a boot parameter +* for those who want this quirk. For example, if the users want to prevent +* the GPE flooding for GPE 00, they need to specify the following boot +* parameter: +* acpi_mask_gpe=0x00 +* The masking status can be modified by the following runtime controlling +* interface: +* echo unmask > /sys/firmware/acpi/interrupts/gpe00 +*/ + +/* +* Currently, the GPE flooding prevention only supports to mask the GPEs +* numbered from 00 to 63. +*/ +#define ACPI_MASKABLE_GPE_MAX 64
drivers/acpi/sysfs.c end: · line 738
Quoted 10 lines
drivers/acpi/sysfs.c +10
-0+ +static u64 acpi_masked_gpes; + +static int __init acpi_gpe_set_masked_gpes (char *val) +{ +u8 gpe; + +if (kstrtou8 (val,0 , &gpe) || gpe >ACPI_MASKABLE_GPE_MAX ) +return -EINVAL ; + acpi_masked_gpes |= ((u64 )1 <<gpe);
This looks sort of hackish.
In principle you can create a list here and then free it when you do the actual masking.
Also, why can't the masking be applied right here?
Quoted 18 lines
drivers/acpi/sysfs.c +17
-0+ +return 1 ; +} +__setup ("acpi_mask_gpe=" , acpi_gpe_set_masked_gpes); + +void acpi_gpe_apply_masked_gpes (void ) +{ +acpi_handle handle; +acpi_status status; +u8 gpe; + +for (gpe =0 ; + gpe <min_t (u8,ACPI_MASKABLE_GPE_MAX , acpi_current_gpe_count); + gpe++) { +if (acpi_masked_gpes & ((u64 )1 <<gpe)) { + status =acpi_get_gpe_device (gpe, &handle); +if (ACPI_SUCCESS (status)) {
drivers/acpi/sysfs.c end: · line 765
Quoted 10 lines
drivers/acpi/sysfs.c +7
-0+pr_info ("Masking GPE 0x%x.\n" , gpe); + (void )acpi_mask_gpe (handle, gpe, TRUE); + } + } + } +} +void acpi_irq_stats_init (void ) {acpi_status status;
Thanks,
Rafael
PATCH v3 1/1 ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
In reply to a message not in the archiveSometimes, the users may require a quirk to be provided from ACPI subsystem core to prevent a GPE from flooding. Normally, if a GPE cannot be dispatched, ACPICA core automatically prevents the GPE from firing. But there are cases the GPE is dispatched by _Lxx/_Exx provided via AML table, and OSPM is lacking of the knowledge to get _Lxx/_Exx correctly executed to handle the GPE, thus the GPE flooding may still occur.
This patch provides a quirk mechanism to stop this kind of GPE flooding.
--- Documentation/kernel-parameters.txt | 10 +++++++ drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sysfs.c | 56 +++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+)
Documentation/kernel-parameters.txt +10 -0
@@ -305,6 +305,16 @@ bytes respectively. Such letter suffixes can also be entirely omitted. use by PCI Format: <irq>,<irq>... + acpi_mask_gpe= [HW,ACPI] + Due to the existence of _Lxx/_Exx, some GPEs triggered + by unsupported hardware/firmware features can result in + GPE floodings that cannot be automatically disabled by + the GPE dispatcher. + This facility can be used to prevent such uncontrolled + GPE floodings. + Format: <int> + Support masking of GPEs numbered from 0x00 to 0x7f. + acpi_no_auto_serialize [HW,ACPI] Disable auto-serialization of AML methods AML control methods that contain the opcodes to create
drivers/acpi/internal.h +1 -0
@@ -37,6 +37,7 @@static inline void acpi_amba_init (void ) {}#endif int acpi_sysfs_init (void ); +void acpi_gpe_apply_masked_gpes (void );void acpi_container_init (void );void acpi_memory_hotplug_init (void );#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
drivers/acpi/scan.c +1 -0
@@ -2044,6 +2044,7 @@ int __init acpi_scan_init(void) } } +acpi_gpe_apply_masked_gpes ();acpi_update_all_gpes ();acpi_ec_ecdt_start ();
drivers/acpi/sysfs.c +56 -0
62 lines not on this page · raw message
Signature · 1 line
1.7.10
PATCH v4 1/1 ACPI / sysfs: Provide quirk mechanism to prevent GPE flooding
In reply to a message not in the archiveSometimes, the users may require a quirk to be provided from ACPI subsystem core to prevent a GPE from flooding.
Normally, if a GPE cannot be dispatched, ACPICA core automatically prevents the GPE from firing. But there are cases the GPE is dispatched by _Lxx/_Exx provided via AML table, and OSPM is lacking of the knowledge to get _Lxx/_Exx correctly executed to handle the GPE, thus the GPE flooding may still occur.
The existing quirk mechanism can be enabled/disabled using the following commands to prevent such kind of GPE flooding during runtime:
# echo mask > /sys/firmware/acpi/interrupts/gpe00 # echo unmask > /sys/firmware/acpi/interrupts/gpe00 To avoid GPE flooding during boot, we need a boot stage mechanism.
This patch provides such a boot stage quirk mechanism to stop this kind of GPE flooding. This patch doesn't fix any feature gap but since the new feature gaps could be found in the future endlessly, and can disappear if the feature gaps are filled, providing a boot parameter rather than a DMI table should suffice.
--- Documentation/admin-guide/kernel-parameters.txt | 10 ++++ drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/acpi/sysfs.c | 56 +++++++++++++++++++++++ 4 files changed, 68 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +10 -0
@@ -106,6 +106,16 @@ use by PCI Format: <irq>,<irq>... + acpi_mask_gpe= [HW,ACPI] + Due to the existence of _Lxx/_Exx, some GPEs triggered + by unsupported hardware/firmware features can result in + GPE floodings that cannot be automatically disabled by + the GPE dispatcher. + This facility can be used to prevent such uncontrolled + GPE floodings. + Format: <int> + Support masking of GPEs numbered from 0x00 to 0x7f. + acpi_no_auto_serialize [HW,ACPI] Disable auto-serialization of AML methods AML control methods that contain the opcodes to create
drivers/acpi/internal.h +1 -0
@@ -37,6 +37,7 @@static inline void acpi_amba_init (void ) {}#endif int acpi_sysfs_init (void ); +void acpi_gpe_apply_masked_gpes (void );void acpi_container_init (void );void acpi_memory_hotplug_init (void );#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
drivers/acpi/scan.c +1 -0
@@ -2044,6 +2044,7 @@ int __init acpi_scan_init(void) } } +acpi_gpe_apply_masked_gpes ();acpi_update_all_gpes ();acpi_ec_ecdt_start ();
drivers/acpi/sysfs.c +56 -0
62 lines not on this page · raw message
Signature · 1 line
1.7.10