From patchwork Wed Jul 6 20:14:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 12908559 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 243D6C43334 for ; Wed, 6 Jul 2022 20:14:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233908AbiGFUOV (ORCPT ); Wed, 6 Jul 2022 16:14:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57298 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231724AbiGFUOU (ORCPT ); Wed, 6 Jul 2022 16:14:20 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C330C21B1 for ; Wed, 6 Jul 2022 13:14:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1657138458; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8xfDMOGch/IV+fHWlNEOhMkFLCnf5tqI8lB1lNGVAg0=; b=FYUjDLmVSa5aRxDOnsHy0P8CeLuZybX5bcvi12ZAXD7QJWSB5Eq62hrbtyetzBk8c3Wpzi Y5L7ALYzCAU3xhoWYupYKiINMr4dJfj/Hi1yKA1231KOTmHvbvzaO9jvgtTu/YAfO8AmvG Q8HptdurG0i85fP5XliRCUoiEQc0Z0o= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-52-a_mFrUyXM0mygSP06ue3iw-1; Wed, 06 Jul 2022 16:14:15 -0400 X-MC-Unique: a_mFrUyXM0mygSP06ue3iw-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5FE7C3C0D862; Wed, 6 Jul 2022 20:14:14 +0000 (UTC) Received: from shalem.redhat.com (unknown [10.39.192.108]) by smtp.corp.redhat.com (Postfix) with ESMTP id DD97A1415117; Wed, 6 Jul 2022 20:14:12 +0000 (UTC) From: Hans de Goede To: "Rafael J . Wysocki" , Len Brown , Robert Moore , Mika Westerberg Cc: Hans de Goede , Zhang Rui , Andy Shevchenko , kai.heng.feng@canonical.com, =?utf-8?q?J?= =?utf-8?q?ohannes_Pen=C3=9Fel?= , linux-acpi@vger.kernel.org, devel@acpica.org Subject: [RFC v2 1/2] ACPICA: Make address_space_handler Install and _REG execution 2 separate steps Date: Wed, 6 Jul 2022 22:14:09 +0200 Message-Id: <20220706201410.88244-2-hdegoede@redhat.com> In-Reply-To: <20220706201410.88244-1-hdegoede@redhat.com> References: <20220706201410.88244-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org ACPICA pull-req: https://github.com/acpica/acpica/pull/786 ACPI-2.0 says that the EC op_region handler must be available immediately (like the standard default op_region handlers): Quoting from the ACPI spec version 6.3: "6.5.4 _REG (Region) ... 2. OSPM must make Embedded Controller operation regions, accessed via the Embedded Controllers described in ECDT, available before executing any control method. These operation regions may become inaccessible after OSPM runs _REG(EmbeddedControl, 0)." So the OS must probe the ECDT described EC and install the op_region handler before calling acpi_enable_subsystem() and acpi_initialize_objects(). This is a problem because calling acpi_install_address_space_handler() does not just install the op_region handler, it also runs the EC's _REG method. This _REG method may rely on initialization done by the _INI methods of one of the PCI / _SB root devices. For the other early/default op_region handlers the op_region handler install and the _REG execution is split into 2 separate steps: 1. acpi_ev_install_region_handlers(), called early from acpi_load_tables() 2. acpi_ev_initialize_op_regions(), called from acpi_initialize_objects() To fix the EC op_region issue, add a new flags parameter to acpi_install_address_space_handler() to allow doing things in 2 steps for other op_region handlers, like the EC handler, too. To avoid having to modify all acpi_install_address_space_handler() callers, the function is renamed to acpi_install_address_space_handler_flags() and a static inline acpi_install_address_space_handler() is provided. bug_link: https://bugzilla.kernel.org/show_bug.cgi?id=214899 Link: https://github.com/acpica/acpica/commit/a44f29d0 Reported-and-tested-by: Johannes Penßel Signed-off-by: Hans de Goede Signed-off-by: Bob Moore Signed-off-by: --- drivers/acpi/acpica/evxfregn.c | 32 +++++++++++++++++++------------ include/acpi/acpixf.h | 35 +++++++++++++++++++++++++--------- include/acpi/actypes.h | 2 ++ 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/drivers/acpi/acpica/evxfregn.c b/drivers/acpi/acpica/evxfregn.c index 0a8372bf6a77..da6ab9ce1dc8 100644 --- a/drivers/acpi/acpica/evxfregn.c +++ b/drivers/acpi/acpica/evxfregn.c @@ -27,6 +27,7 @@ ACPI_MODULE_NAME("evxfregn") * handler - Address of the handler * setup - Address of the setup function * context - Value passed to the handler on each access + * flags - Flags * * RETURN: Status * @@ -37,13 +38,17 @@ ACPI_MODULE_NAME("evxfregn") * are executed here, and these methods can only be safely executed after * the default handlers have been installed and the hardware has been * initialized (via acpi_enable_subsystem.) + * To avoid this problem pass the ACPI_NO_EXEC__REG flag and + * later call this function again with ACPI_NO_INSTALL_SPACE_HANDLER to + * execute _REG. * ******************************************************************************/ acpi_status -acpi_install_address_space_handler(acpi_handle device, - acpi_adr_space_type space_id, - acpi_adr_space_handler handler, - acpi_adr_space_setup setup, void *context) +acpi_install_address_space_handler_flags(acpi_handle device, + acpi_adr_space_type space_id, + acpi_adr_space_handler handler, + acpi_adr_space_setup setup, + void *context, u32 flags) { struct acpi_namespace_node *node; acpi_status status; @@ -71,23 +76,26 @@ acpi_install_address_space_handler(acpi_handle device, /* Install the handler for all Regions for this Space ID */ - status = - acpi_ev_install_space_handler(node, space_id, handler, setup, - context); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; + if (!(flags & ACPI_NO_INSTALL_SPACE_HANDLER)) { + status = + acpi_ev_install_space_handler(node, space_id, handler, + setup, context); + if (ACPI_FAILURE(status)) { + goto unlock_and_exit; + } } /* Run all _REG methods for this address space */ - - acpi_ev_execute_reg_methods(node, space_id, ACPI_REG_CONNECT); + if (!(flags & ACPI_NO_EXEC__REG)) { + acpi_ev_execute_reg_methods(node, space_id, ACPI_REG_CONNECT); + } unlock_and_exit: (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); return_ACPI_STATUS(status); } -ACPI_EXPORT_SYMBOL(acpi_install_address_space_handler) +ACPI_EXPORT_SYMBOL(acpi_install_address_space_handler_flags) /******************************************************************************* * diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 67c0b9e734b6..08e8cfbed1a0 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -643,15 +643,32 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_notify_handler handler)) ACPI_EXTERNAL_RETURN_STATUS(acpi_status - acpi_install_address_space_handler(acpi_handle - device, - acpi_adr_space_type - space_id, - acpi_adr_space_handler - handler, - acpi_adr_space_setup - setup, - void *context)) + acpi_install_address_space_handler_flags(acpi_handle + device, + acpi_adr_space_type + space_id, + acpi_adr_space_handler + handler, + acpi_adr_space_setup + setup, + void + *context, + u32 flags)) +static ACPI_INLINE acpi_status acpi_install_address_space_handler(acpi_handle + device, + acpi_adr_space_type + space_id, + acpi_adr_space_handler + handler, + acpi_adr_space_setup + setup, + void *context) +{ + return acpi_install_address_space_handler_flags(device, space_id, + handler, setup, context, + ACPI_FULL_INITIALIZATION); +} + ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_remove_address_space_handler(acpi_handle device, diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 3491e454b2ab..aba0a86fcf1e 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -566,6 +566,8 @@ typedef u64 acpi_integer; #define ACPI_NO_OBJECT_INIT 0x0020 #define ACPI_NO_DEVICE_INIT 0x0040 #define ACPI_NO_ADDRESS_SPACE_INIT 0x0080 +#define ACPI_NO_INSTALL_SPACE_HANDLER 0x0100 +#define ACPI_NO_EXEC__REG 0x0200 /* * Initialization state From patchwork Wed Jul 6 20:14:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 12908561 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C816C43334 for ; Wed, 6 Jul 2022 20:14:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231724AbiGFUOX (ORCPT ); Wed, 6 Jul 2022 16:14:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57314 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233952AbiGFUOW (ORCPT ); Wed, 6 Jul 2022 16:14:22 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id CAEBA1018 for ; Wed, 6 Jul 2022 13:14:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1657138460; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ti+nZsEOP1Aj3/9iJ9rsuq04U1ZpFkFYqRwK0yE0Yks=; b=d03UMcrHGH9d3eMsEZCns63vT5cIfhtf62Hy+ZNKEAgakvatWek0gXxIdlWKjaMwpjBXy6 jmcKZEMqpAW9p+htXH4iBhcvC0JeC62j/AQfN4JLm+YFEm8LUnwgRD9NUEU+IEQGaFwuEL 3f8FQn/gmzq3wN7Q226sDKImCHSvAXI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-447-6nrN2FaEN9i37Rh1XeSaaQ-1; Wed, 06 Jul 2022 16:14:16 -0400 X-MC-Unique: 6nrN2FaEN9i37Rh1XeSaaQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 129CC29AA2FB; Wed, 6 Jul 2022 20:14:16 +0000 (UTC) Received: from shalem.redhat.com (unknown [10.39.192.108]) by smtp.corp.redhat.com (Postfix) with ESMTP id 94866140EBE3; Wed, 6 Jul 2022 20:14:14 +0000 (UTC) From: Hans de Goede To: "Rafael J . Wysocki" , Len Brown , Robert Moore , Mika Westerberg Cc: Hans de Goede , Zhang Rui , Andy Shevchenko , kai.heng.feng@canonical.com, =?utf-8?q?J?= =?utf-8?q?ohannes_Pen=C3=9Fel?= , linux-acpi@vger.kernel.org, devel@acpica.org Subject: [RFC v2 2/2] ACPI: EC: fix ECDT probe ordering issues Date: Wed, 6 Jul 2022 22:14:10 +0200 Message-Id: <20220706201410.88244-3-hdegoede@redhat.com> In-Reply-To: <20220706201410.88244-1-hdegoede@redhat.com> References: <20220706201410.88244-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org ACPI-2.0 says that the EC OpRegion handler must be available immediately (like the standard default OpRegion handlers): Quoting from the ACPI spec version 6.3: "6.5.4 _REG (Region) ... 2. OSPM must make Embedded Controller operation regions, accessed via the Embedded Controllers described in ECDT, available before executing any control method. These operation regions may become inaccessible after OSPM runs _REG(EmbeddedControl, 0)." So acpi_bus_init() calls acpi_ec_ecdt_probe(), which calls acpi_install_address_space_handler() to install the EC's OpRegion handler, early on. This not only installs the OpRegion handler, but also calls the EC's _REG method. The _REG method call is a problem because it may rely on initialization done by the _INI methods of one of the PCI / _SB root devs, see for example: https://bugzilla.kernel.org/show_bug.cgi?id=214899 . Generally speaking _REG methods are executed when the ACPI-device they are part of has a driver bound to it. Where as _INI methods must be executed at table load time (according to the spec). The problem here is that the early acpi_install_address_space_handler() call causes the _REG handler to run too early. To allow fixing this the ACPICA code now allows to split the OpRegion handler installation and the executing of _REG into 2 separate steps. This commit uses this ACPICA functionality to fix the EC probe ordering by delaying the executing of _REG for ECDT described ECs till the matching EC device in the DSDT gets parsed and acpi_ec_add() for it gets called. This moves the calling of _REG for the EC on devices with an ECDT to the same point in time where it is called on devices without an ECDT table. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214899 Reported-and-tested-by: Johannes Penßel Signed-off-by: Hans de Goede --- drivers/acpi/ec.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index c95e535035a0..1e93677e4b82 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -94,6 +94,7 @@ enum { EC_FLAGS_QUERY_ENABLED, /* Query is enabled */ EC_FLAGS_EVENT_HANDLER_INSTALLED, /* Event handler installed */ EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */ + EC_FLAGS_EC_REG_CALLED, /* OpReg ACPI _REG method called */ EC_FLAGS_QUERY_METHODS_INSTALLED, /* _Qxx handlers installed */ EC_FLAGS_STARTED, /* Driver is started */ EC_FLAGS_STOPPED, /* Driver is stopped */ @@ -1450,6 +1451,7 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec) * ec_install_handlers - Install service callbacks and register query methods. * @ec: Target EC. * @device: ACPI device object corresponding to @ec. + * @call_reg: If _REG should be called to notify OpRegion availability * * Install a handler for the EC address space type unless it has been installed * already. If @device is not NULL, also look for EC query methods in the @@ -1462,7 +1464,8 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec) * -EPROBE_DEFER if GPIO IRQ acquisition needs to be deferred, * or 0 (success) otherwise. */ -static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) +static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device, + bool call_reg) { acpi_status status; @@ -1470,10 +1473,11 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) if (!test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) { acpi_ec_enter_noirq(ec); - status = acpi_install_address_space_handler(ec->handle, - ACPI_ADR_SPACE_EC, - &acpi_ec_space_handler, - NULL, ec); + status = acpi_install_address_space_handler_flags(ec->handle, + ACPI_ADR_SPACE_EC, + &acpi_ec_space_handler, + NULL, ec, + ACPI_NO_EXEC__REG); if (ACPI_FAILURE(status)) { acpi_ec_stop(ec, false); return -ENODEV; @@ -1481,6 +1485,15 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device) set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags); } + if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) { + acpi_install_address_space_handler_flags(ec->handle, + ACPI_ADR_SPACE_EC, + &acpi_ec_space_handler, + NULL, ec, + ACPI_NO_INSTALL_SPACE_HANDLER); + set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags); + } + if (!device) return 0; @@ -1566,11 +1579,11 @@ static void ec_remove_handlers(struct acpi_ec *ec) } } -static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device) +static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device, bool call_reg) { int ret; - ret = ec_install_handlers(ec, device); + ret = ec_install_handlers(ec, device, call_reg); if (ret) return ret; @@ -1635,7 +1648,7 @@ static int acpi_ec_add(struct acpi_device *device) } } - ret = acpi_ec_setup(ec, device); + ret = acpi_ec_setup(ec, device, true); if (ret) goto err; @@ -1755,7 +1768,7 @@ void __init acpi_ec_dsdt_probe(void) * At this point, the GPE is not fully initialized, so do not to * handle the events. */ - ret = acpi_ec_setup(ec, NULL); + ret = acpi_ec_setup(ec, NULL, true); if (ret) { acpi_ec_free(ec); return; @@ -1939,7 +1952,7 @@ void __init acpi_ec_ecdt_probe(void) * At this point, the namespace is not initialized, so do not find * the namespace objects, or handle the events. */ - ret = acpi_ec_setup(ec, NULL); + ret = acpi_ec_setup(ec, NULL, false); if (ret) { acpi_ec_free(ec); goto out;