[v1,3/4] ACPI: OSL: Allow Notify () handlers to run on all CPUs
Commit Message
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Notify () handlers, like GPE handlers, are only allowed to run on CPU0
now out of the concern that they might trigger an SMM trap and that (in
some cases) the SMM code running as a result of that might corrupt
memory if not run on CPU0.
However, Notify () handlers are registered by kernel code and they
are not likely to evaluate AML that would trigger an SMM trap. In
fact, many of them don't even evaluate any AML at all and even if
they do, that AML may as well be evaluated in other code paths. In
other words, they are not special from the AML evaluation perspective,
so there is no real reason to treat them in any special way.
Accordingly, allow Notify () handlers, unlike GPE handlers, to be
executed by all CPUs in the system.
Also adjust the allocation of the "notify" workqueue to allow multiple
handlers to be executed at the same time, because they need not be
serialized.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/osl.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
Comments
On Wed, Nov 29, 2023 at 02:50:54PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Notify () handlers, like GPE handlers, are only allowed to run on CPU0
> now out of the concern that they might trigger an SMM trap and that (in
> some cases) the SMM code running as a result of that might corrupt
> memory if not run on CPU0.
Pardon my French, but I'm a bit lost in the semantics of all those "that".
Maybe the above can be simplified?
> However, Notify () handlers are registered by kernel code and they
> are not likely to evaluate AML that would trigger an SMM trap. In
> fact, many of them don't even evaluate any AML at all and even if
> they do, that AML may as well be evaluated in other code paths. In
> other words, they are not special from the AML evaluation perspective,
> so there is no real reason to treat them in any special way.
>
> Accordingly, allow Notify () handlers, unlike GPE handlers, to be
> executed by all CPUs in the system.
>
> Also adjust the allocation of the "notify" workqueue to allow multiple
> handlers to be executed at the same time, because they need not be
> serialized.
Code wise LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
On Wed, Nov 29, 2023 at 3:34 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Nov 29, 2023 at 02:50:54PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Notify () handlers, like GPE handlers, are only allowed to run on CPU0
> > now out of the concern that they might trigger an SMM trap and that (in
> > some cases) the SMM code running as a result of that might corrupt
> > memory if not run on CPU0.
>
> Pardon my French, but I'm a bit lost in the semantics of all those "that".
Well, fair enough.
> Maybe the above can be simplified?
It can be rephrased.
===================================================================
@@ -1061,7 +1061,6 @@ acpi_status acpi_os_execute(acpi_execute
acpi_osd_exec_callback function, void *context)
{
struct acpi_os_dpc *dpc;
- struct workqueue_struct *queue;
int ret;
ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
@@ -1101,24 +1100,22 @@ acpi_status acpi_os_execute(acpi_execute
*/
switch (type) {
case OSL_NOTIFY_HANDLER:
- queue = kacpi_notify_wq;
+ ret = queue_work(kacpi_notify_wq, &dpc->work);
break;
case OSL_GPE_HANDLER:
- queue = kacpid_wq;
+ /*
+ * On some machines, a software-initiated SMI causes corruption
+ * unless the SMI runs on CPU 0. An SMI can be initiated by
+ * any AML, but typically it's done in GPE-related methods that
+ * are run via workqueues, so we can avoid the known corruption
+ * cases by always queueing on CPU 0.
+ */
+ ret = queue_work_on(0, kacpid_wq, &dpc->work);
break;
default:
pr_err("Unsupported os_execute type %d.\n", type);
goto err;
}
-
- /*
- * On some machines, a software-initiated SMI causes corruption unless
- * the SMI runs on CPU 0. An SMI can be initiated by any AML, but
- * typically it's done in GPE-related methods that are run via
- * workqueues, so we can avoid the known corruption cases by always
- * queueing on CPU 0.
- */
- ret = queue_work_on(0, queue, &dpc->work);
if (!ret) {
pr_err("Unable to queue work\n");
goto err;
@@ -1668,7 +1665,7 @@ acpi_status __init acpi_os_initialize(vo
acpi_status __init acpi_os_initialize1(void)
{
kacpid_wq = alloc_workqueue("kacpid", 0, 1);
- kacpi_notify_wq = alloc_workqueue("kacpi_notify", 0, 1);
+ kacpi_notify_wq = alloc_workqueue("kacpi_notify", 0, 0);
kacpi_hotplug_wq = alloc_ordered_workqueue("kacpi_hotplug", 0);
BUG_ON(!kacpid_wq);
BUG_ON(!kacpi_notify_wq);