From patchwork Wed Jun 15 19:56:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 581981 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 577C9C43334 for ; Wed, 15 Jun 2022 19:57:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351546AbiFOT47 (ORCPT ); Wed, 15 Jun 2022 15:56:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350481AbiFOT44 (ORCPT ); Wed, 15 Jun 2022 15:56:56 -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 377CF31353 for ; Wed, 15 Jun 2022 12:56:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655323014; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yynx7uBPgU1LaWpk2SJ9B5CmNvk7cX/7wHodT57VRik=; b=QYYlLMQPwps9t+yXbf6Aycb16qqcWEtdVObohWGxFZcvoVPYZ541u16TtJZktbts3YY4RQ PNsrgtpYfMd+CiCsRA913MWG8qIFnP1h7J/6hqYwWdE/5egYhsxJBTRjwKK9Uxj9GKt0bO 5lmo1LzPpscv2V851sjia/ZriXlrNnc= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-18-DVBYUW6xNNuOFev0GvRJbg-1; Wed, 15 Jun 2022 15:56:53 -0400 X-MC-Unique: DVBYUW6xNNuOFev0GvRJbg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 78334185A7A4; Wed, 15 Jun 2022 19:56:52 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id D3E6A2026D64; Wed, 15 Jun 2022 19:56:50 +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 1/4] ACPICA: Add an acpi_early_initialize_objects() helper Date: Wed, 15 Jun 2022 21:56:40 +0200 Message-Id: <20220615195643.12608-2-hdegoede@redhat.com> In-Reply-To: <20220615195643.12608-1-hdegoede@redhat.com> References: <20220615195643.12608-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org acpi_initialize_objects() calls \_INI and \_SB.INI before executing _REG OpRegion methods, because the _REG methods may rely on initialization done by these _INI methods. ACPI-2.0 says that the EC OpRegion handler must be available immediately (like the standard default OpRegion handlers). So the OS must probe the ECDT described EC early on and ends up calling the EC's _REG method before calling acpi_initialize_objects(). This may lead to the EC _REG method not working properly e.g. on a "Lenovo IdeaPad 5 15ITL05" \_SB.PC00.LPCB.EC0._REG gets evaluated before \_SB.PC00._INI and that _REG contains: If ((OSYS == 0x07DF)) { Local0 = 0x06 } If ((Acquire (LFCM, 0xA000) == Zero)) { OSTY = Local0 ... With OSTY being a SystemMemory OpRegion field, due to the _INI running too late, Local0 stays at 0. Causing OSTY to be set to 0 instead of 6, which causes the brightness up/down keys to not work, see: https://bugzilla.kernel.org/show_bug.cgi?id=214899 Factor out the early (root) object _INI calls into a new acpi_early_initialize_objects() helper. The OS code can then call this before executing the EC's _REG method so that it will work properly. Note that if the OS code uses acpi_early_initialize_objects(), then it must also pass the new ACPI_NO_EARLY_DEVICE_INIT flag to acpi_initialize_objects() to avoid the _INI methods getting evaluated twice. Without this flag acpi_initialize_objects() will still do the early object _INI calls as before. So by itself this patch does not cause any functional changes. Signed-off-by: Hans de Goede --- drivers/acpi/acpica/acnamesp.h | 2 + drivers/acpi/acpica/nsinit.c | 166 ++++++++++++++++++++------------- drivers/acpi/acpica/utxfinit.c | 21 +++++ include/acpi/acpixf.h | 2 + include/acpi/actypes.h | 3 +- 5 files changed, 127 insertions(+), 67 deletions(-) diff --git a/drivers/acpi/acpica/acnamesp.h b/drivers/acpi/acpica/acnamesp.h index 7b27b9cc5916..4c8196adb98e 100644 --- a/drivers/acpi/acpica/acnamesp.h +++ b/drivers/acpi/acpica/acnamesp.h @@ -56,6 +56,8 @@ */ acpi_status acpi_ns_initialize_objects(void); +acpi_status acpi_ns_early_initialize_devices(void); + acpi_status acpi_ns_initialize_devices(u32 flags); acpi_status diff --git a/drivers/acpi/acpica/nsinit.c b/drivers/acpi/acpica/nsinit.c index 3e6207ad18d8..f5364d44fdb8 100644 --- a/drivers/acpi/acpica/nsinit.c +++ b/drivers/acpi/acpica/nsinit.c @@ -86,6 +86,59 @@ acpi_status acpi_ns_initialize_objects(void) return_ACPI_STATUS(AE_OK); } +/******************************************************************************* + * + * FUNCTION: acpi_ns_early_initialize_devices + * + * PARAMETERS: None + * + * RETURN: acpi_status + * + * DESCRIPTION: Call _INI on various root devices, this must be done + * before any other evaluations as those might depend on this. + * + ******************************************************************************/ +static acpi_string const acpi_ns_early_init_paths[] = { + /* Global \_INI method. Provided for Windows compatibility (Vista+), + * not part of the ACPI specification. */ + ACPI_NS_ROOT_PATH, + /* There appears to be a strict order requirement for \_SB._INI, + * which should be evaluated before any _REG evaluations. */ + "\\_SB", +}; + +acpi_status acpi_ns_early_initialize_devices(void) +{ + struct acpi_evaluate_info *info; + acpi_status status; + acpi_handle handle; + int i; + + ACPI_DEBUG_PRINT((ACPI_DB_EXEC, + "[Init] Initializing Early ACPI Devices\n")); + + info = ACPI_ALLOCATE(sizeof(struct acpi_evaluate_info)); + if (!info) + return AE_NO_MEMORY; + + for (i = 0; i < ARRAY_SIZE(acpi_ns_early_init_paths); i++) { + status = acpi_get_handle(NULL, acpi_ns_early_init_paths[i], &handle); + if (!ACPI_SUCCESS(status)) + continue; + + memset(info, 0, sizeof(struct acpi_evaluate_info)); + info->prefix_node = handle; + info->relative_pathname = METHOD_NAME__INI; + info->parameters = NULL; + info->flags = ACPI_IGNORE_RETURN_VALUE; + + acpi_ns_evaluate(info); + } + + ACPI_FREE(info); + return AE_OK; +} + /******************************************************************************* * * FUNCTION: acpi_ns_initialize_devices @@ -105,12 +158,41 @@ acpi_status acpi_ns_initialize_objects(void) acpi_status acpi_ns_initialize_devices(u32 flags) { acpi_status status = AE_OK; - struct acpi_device_walk_info info; - acpi_handle handle; ACPI_FUNCTION_TRACE(ns_initialize_devices); + if (!(flags & (ACPI_NO_DEVICE_INIT | ACPI_NO_EARLY_DEVICE_INIT))) { + status = acpi_ns_early_initialize_devices(); + if (ACPI_FAILURE(status)) + goto error_exit; + } + + /* + * Run all _REG methods + * + * Note: Any objects accessed by the _REG methods will be automatically + * initialized, even if they contain executable AML (see the call to + * acpi_ns_initialize_objects below). + * + * Note: According to the ACPI specification, we actually needn't execute + * _REG for system_memory/system_io operation regions, but for PCI_Config + * operation regions, it is required to evaluate _REG for those on a PCI + * root bus that doesn't contain _BBN object. So this code is kept here + * in order not to break things. + */ + if (!(flags & ACPI_NO_ADDRESS_SPACE_INIT)) { + ACPI_DEBUG_PRINT((ACPI_DB_EXEC, + "[Init] Executing _REG OpRegion methods\n")); + + status = acpi_ev_initialize_op_regions(); + if (ACPI_FAILURE(status)) { + goto error_exit; + } + } + if (!(flags & ACPI_NO_DEVICE_INIT)) { + struct acpi_device_walk_info info; + ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "[Init] Initializing ACPI Devices\n")); @@ -143,68 +225,6 @@ acpi_status acpi_ns_initialize_devices(u32 flags) goto error_exit; } - /* - * Execute the "global" _INI method that may appear at the root. - * This support is provided for Windows compatibility (Vista+) and - * is not part of the ACPI specification. - */ - info.evaluate_info->prefix_node = acpi_gbl_root_node; - info.evaluate_info->relative_pathname = METHOD_NAME__INI; - info.evaluate_info->parameters = NULL; - info.evaluate_info->flags = ACPI_IGNORE_RETURN_VALUE; - - status = acpi_ns_evaluate(info.evaluate_info); - if (ACPI_SUCCESS(status)) { - info.num_INI++; - } - - /* - * Execute \_SB._INI. - * There appears to be a strict order requirement for \_SB._INI, - * which should be evaluated before any _REG evaluations. - */ - status = acpi_get_handle(NULL, "\\_SB", &handle); - if (ACPI_SUCCESS(status)) { - memset(info.evaluate_info, 0, - sizeof(struct acpi_evaluate_info)); - info.evaluate_info->prefix_node = handle; - info.evaluate_info->relative_pathname = - METHOD_NAME__INI; - info.evaluate_info->parameters = NULL; - info.evaluate_info->flags = ACPI_IGNORE_RETURN_VALUE; - - status = acpi_ns_evaluate(info.evaluate_info); - if (ACPI_SUCCESS(status)) { - info.num_INI++; - } - } - } - - /* - * Run all _REG methods - * - * Note: Any objects accessed by the _REG methods will be automatically - * initialized, even if they contain executable AML (see the call to - * acpi_ns_initialize_objects below). - * - * Note: According to the ACPI specification, we actually needn't execute - * _REG for system_memory/system_io operation regions, but for PCI_Config - * operation regions, it is required to evaluate _REG for those on a PCI - * root bus that doesn't contain _BBN object. So this code is kept here - * in order not to break things. - */ - if (!(flags & ACPI_NO_ADDRESS_SPACE_INIT)) { - ACPI_DEBUG_PRINT((ACPI_DB_EXEC, - "[Init] Executing _REG OpRegion methods\n")); - - status = acpi_ev_initialize_op_regions(); - if (ACPI_FAILURE(status)) { - goto error_exit; - } - } - - if (!(flags & ACPI_NO_DEVICE_INIT)) { - /* Walk namespace to execute all _INIs on present devices */ status = acpi_ns_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT, @@ -497,6 +517,21 @@ acpi_ns_find_ini_methods(acpi_handle obj_handle, return (AE_OK); } +static u8 acpi_ns_is_early_init_device(struct acpi_namespace_node *device_node) +{ + char path[ACPI_PATH_SEGMENT_LENGTH + 2]; + int i; + + acpi_ns_build_normalized_path(device_node, path, sizeof(path), TRUE); + + for (i = 0; i < ARRAY_SIZE(acpi_ns_early_init_paths); i++) { + if (!strcmp(path, acpi_ns_early_init_paths[i])) + return TRUE; + } + + return FALSE; +} + /******************************************************************************* * * FUNCTION: acpi_ns_init_one_device @@ -630,8 +665,7 @@ acpi_ns_init_one_device(acpi_handle obj_handle, * Note: We know there is an _INI within this subtree, but it may not be * under this particular device, it may be lower in the branch. */ - if (!ACPI_COMPARE_NAMESEG(device_node->name.ascii, "_SB_") || - device_node->parent != acpi_gbl_root_node) { + if (!acpi_ns_is_early_init_device(device_node)) { ACPI_DEBUG_EXEC(acpi_ut_display_init_pathname (ACPI_TYPE_METHOD, device_node, METHOD_NAME__INI)); diff --git a/drivers/acpi/acpica/utxfinit.c b/drivers/acpi/acpica/utxfinit.c index f2acec3a0ee3..4396ec7f1246 100644 --- a/drivers/acpi/acpica/utxfinit.c +++ b/drivers/acpi/acpica/utxfinit.c @@ -193,6 +193,27 @@ acpi_status ACPI_INIT_FUNCTION acpi_enable_subsystem(u32 flags) ACPI_EXPORT_SYMBOL_INIT(acpi_enable_subsystem) +/******************************************************************************* + * + * FUNCTION: acpi_early_initialize_objects + * + * PARAMETERS: None + * + * RETURN: Status + * + * DESCRIPTION: Call _INI on various root objects, this must be done before + * any other AML evaluations as those might depend on this. + * This is automatically called by acpi_initialize_objects(), + * unless ACPI_NO_EARLY_DEVICE_INIT is passed. + * + ******************************************************************************/ +acpi_status ACPI_INIT_FUNCTION acpi_early_initialize_objects(void) +{ + return acpi_ns_early_initialize_devices(); +} + +ACPI_EXPORT_SYMBOL_INIT(acpi_early_initialize_objects) + /******************************************************************************* * * FUNCTION: acpi_initialize_objects diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 67c0b9e734b6..4b8d19f48071 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -410,6 +410,8 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION acpi_initialize_subsystem(void)) ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION acpi_enable_subsystem(u32 flags)) +ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION + acpi_early_initialize_objects(void)) ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION acpi_initialize_objects(u32 flags)) ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 3491e454b2ab..e377b7daec54 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -565,7 +565,8 @@ typedef u64 acpi_integer; #define ACPI_NO_HANDLER_INIT 0x0010 #define ACPI_NO_OBJECT_INIT 0x0020 #define ACPI_NO_DEVICE_INIT 0x0040 -#define ACPI_NO_ADDRESS_SPACE_INIT 0x0080 +#define ACPI_NO_EARLY_DEVICE_INIT 0x0080 +#define ACPI_NO_ADDRESS_SPACE_INIT 0x0100 /* * Initialization state From patchwork Wed Jun 15 19:56:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 582777 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 4D390C433EF for ; Wed, 15 Jun 2022 19:57:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347805AbiFOT5A (ORCPT ); Wed, 15 Jun 2022 15:57:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56158 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1352209AbiFOT47 (ORCPT ); Wed, 15 Jun 2022 15:56:59 -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 B8D8431353 for ; Wed, 15 Jun 2022 12:56:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655323017; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=evqLu0nPxDj0y/d6MJFVRDPNvEdjCenckvtl/TbFspY=; b=NBkvGFX8iRHQdEnF9Pw7GcI6mVkQFKw7mAN1nCmiderhsKqFLY5E51lDVyRMzv6jPpjv59 gJbBBDEESK7MZNROo4Ai2qP5kfFbF/CtaLeGT0K6+84Jxz09/cMxuvr5y8Zw+rO+Q9wRhT Zp6/AstXHxDhn641Try12r7qmEKDt+M= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-166-mK2tEywCOYO0QOvX8aJNoA-1; Wed, 15 Jun 2022 15:56:54 -0400 X-MC-Unique: mK2tEywCOYO0QOvX8aJNoA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 30FA7185A7A4; Wed, 15 Jun 2022 19:56:54 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id AD6092026D07; Wed, 15 Jun 2022 19:56:52 +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 2/4] ACPICA: Add \_SB.PC00, \SB.PCI0 to acpi_ns_early_initialize_devices() Date: Wed, 15 Jun 2022 21:56:41 +0200 Message-Id: <20220615195643.12608-3-hdegoede@redhat.com> In-Reply-To: <20220615195643.12608-1-hdegoede@redhat.com> References: <20220615195643.12608-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org Since ACPICA commit f005ee6b90d1 / Linux commit 2d3349de8072 ("ACPICA: Namespace: Reorder \_SB._INI to make sure it is evaluated before _REG evaluations") acpi_initialize_objects() calls \_SB._INI before executing _REG OpRegion methods, because the _REG methods may rely on initialization done by this _INI method. In many DSDTs the \_SB.PC00._INI / \_SB.PCI0._INI methods set an OSYS global variable based on _OSI evaluations. In some cases there are _REG methods which depend on the OSYS value and before this change ACPICA would run these _REG methods before running _SB.PC00._INI / \_SB.PCI0._INI causing issues. 2 examples of problems caused by running _REG methods before these _INI methods are: 1. on a "Lenovo IdeaPad 5 15ITL05" \_SB.PC00.LPCB.EC0._REG gets evaluated before \_SB.PC00._INI and that _REG contains: If ((OSYS == 0x07DF)) { Local0 = 0x06 } If ((Acquire (LFCM, 0xA000) == Zero)) { OSTY = Local0 ... With OSTY being a SystemMemory OpRegion field, due to the _INI running too late, Local0 stays at 0. Causing OSTY to be set to 0 instead of 6, which causes the brightness up/down keys to not work: https://bugzilla.kernel.org/show_bug.cgi?id=214899 2. On a "Lenovo Thinkbook 14-ILL" \\_SB_.PCI0.I2C0._REG gets evaluated before \_SB.PCI0._INI and that _REG contains: If ((OSYS == 0x07DF)) { ... LNUX = Zero TPID = 0x4E } else { LNUX = One TPID = 0xBB } And then later on the TPID value gets used to decide for which of multiple devices describing the touchpad _STA should return 0xF and the one which gets enabled by TPID=0xBB is broken, causing to the touchpad to not work: https://bugzilla.redhat.com/show_bug.cgi?id=1842039 Fix these issues by adding \_SB.PC00._INI / \_SB.PCI0._INI to the list of _INI methods to run early (before executing _REG methods). Signed-off-by: Hans de Goede --- drivers/acpi/acpica/nsinit.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/acpica/nsinit.c b/drivers/acpi/acpica/nsinit.c index f5364d44fdb8..db66df28e4fc 100644 --- a/drivers/acpi/acpica/nsinit.c +++ b/drivers/acpi/acpica/nsinit.c @@ -105,6 +105,10 @@ static acpi_string const acpi_ns_early_init_paths[] = { /* There appears to be a strict order requirement for \_SB._INI, * which should be evaluated before any _REG evaluations. */ "\\_SB", + /* \_SB.PC00._INI or \_SB.PCI0.INI may use \_OSI to set an OSYS global + * variable used in _REG methods. */ + "\\_SB.PC00", + "\\_SB.PCI0", }; acpi_status acpi_ns_early_initialize_devices(void) @@ -519,7 +523,7 @@ acpi_ns_find_ini_methods(acpi_handle obj_handle, static u8 acpi_ns_is_early_init_device(struct acpi_namespace_node *device_node) { - char path[ACPI_PATH_SEGMENT_LENGTH + 2]; + char path[ACPI_PATH_SEGMENT_LENGTH * 2 + 2]; int i; acpi_ns_build_normalized_path(device_node, path, sizeof(path), TRUE); From patchwork Wed Jun 15 19:56:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 582776 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 5C162CCA473 for ; Wed, 15 Jun 2022 19:57:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350481AbiFOT5L (ORCPT ); Wed, 15 Jun 2022 15:57:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56332 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354552AbiFOT5H (ORCPT ); Wed, 15 Jun 2022 15:57:07 -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 AD47633EA6 for ; Wed, 15 Jun 2022 12:57:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655323021; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EllYvUf+BqeGIFNzhqo9UeQvV9ROTX24PLrW916RQIQ=; b=NTrKB7DnOeczyPZmgXgcSmDGdRJSzOxWY+eyJoD7QcI0XyIkas3Q2sNHEPCpusFbXwHxIW /JbLo/HGwrLchGsUfAWBCDDo+psYQBcMwwqyVxyQ9zU4GuJwsPfNVY//U9XVsXzZbBLwFM uNAxNYFKJf3qnBfgULkgZfw/ysBMvps= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-378-sutnA9lUPBinVqcT2idJKw-1; Wed, 15 Jun 2022 15:56:56 -0400 X-MC-Unique: sutnA9lUPBinVqcT2idJKw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DB20183397C; Wed, 15 Jun 2022 19:56:55 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6607D2026D64; Wed, 15 Jun 2022 19:56:54 +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 3/4] ACPICA: Make address-space-handler install and _REG execution 2 separate steps Date: Wed, 15 Jun 2022 21:56:42 +0200 Message-Id: <20220615195643.12608-4-hdegoede@redhat.com> In-Reply-To: <20220615195643.12608-1-hdegoede@redhat.com> References: <20220615195643.12608-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 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). So the OS must probe the ECDT described EC and install the OpRegion 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 OpRegion 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. This _REG depends on _INI problem can be fixed by calling the new acpi_early_initialize_objects() function before probing the EC. But on some boards (e.g. Lenovo X1C8) the root devices _INI method relies on the EC OpRegion so executing the _INI methods before registering the EC OpRegion handler leads to errors there. For the default OpRegion handlers the ACPICA code solves these ordering issues by splitting the Opregion handler install and the _REG execution 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 similar EC OpRegion issues, add a new flags parameter to acpi_install_address_space_handler() to allow doing things in 2 steps for the EC OpRegion handler too. This will allow using the following initialization order to fix things: 1. acpi_load_tables() 2. probe EC, call acpi_install_address_space_handler(ACPI_NO_EXEC__REG) 3. acpi_enable_subsystem() 4. acpi_early_initialize_objects() 5. call acpi_install_address_space_handler(ACPI_NO_INSTALL_SPACE_HANDLER) to run the EC's _REG method 6. acpi_initialize_objects(ACPI_NO_EARLY_DEVICE_INIT) 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. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214899 Signed-off-by: Hans de Goede --- drivers/acpi/acpica/evxfregn.c | 32 +++++++++++++++++++------------- include/acpi/acpixf.h | 33 ++++++++++++++++++++++++--------- include/acpi/actypes.h | 2 ++ 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/drivers/acpi/acpica/evxfregn.c b/drivers/acpi/acpica/evxfregn.c index 0a8372bf6a77..ed79615fb844 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,18 @@ 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; @@ -70,24 +76,24 @@ 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 4b8d19f48071..7116fc2d42a7 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -645,15 +645,30 @@ 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 e377b7daec54..6042c50ad1ee 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -567,6 +567,8 @@ typedef u64 acpi_integer; #define ACPI_NO_DEVICE_INIT 0x0040 #define ACPI_NO_EARLY_DEVICE_INIT 0x0080 #define ACPI_NO_ADDRESS_SPACE_INIT 0x0100 +#define ACPI_NO_INSTALL_SPACE_HANDLER 0x0200 +#define ACPI_NO_EXEC__REG 0x0400 /* * Initialization state From patchwork Wed Jun 15 19:56:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 581980 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 303B7C43334 for ; Wed, 15 Jun 2022 19:57:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345218AbiFOT5K (ORCPT ); Wed, 15 Jun 2022 15:57:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56338 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354405AbiFOT5H (ORCPT ); Wed, 15 Jun 2022 15:57:07 -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 32C6831DEA for ; Wed, 15 Jun 2022 12:57:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655323021; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4IRAOOrn3E/U28/R5lp+r2POD0+f5WZIETyGHTaPKlg=; b=dOwOlvO5YlHhZ85wjGn/qF8jJ6D1ytMS6sx9etNQOlGRs0mMzi2GcwXE1DT//bxkfTYVUv PQ3EE1+IZaSkaSrVfbsLXRw2sSeWYRtt88mB2Z0Q1161qd4RHj+SSy5KrqqBsInwEQhTXs DcOMXnCOzWVBxvFoN//SP8CqtlwqQRQ= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-502-XPRckmE2NLObuB-3g_nw9Q-1; Wed, 15 Jun 2022 15:56:58 -0400 X-MC-Unique: XPRckmE2NLObuB-3g_nw9Q-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 991EF185A79C; Wed, 15 Jun 2022 19:56:57 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.192.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1BFA32026D64; Wed, 15 Jun 2022 19:56:56 +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 4/4] ACPI: fix ECDT EC probe ordering issues Date: Wed, 15 Jun 2022 21:56:43 +0200 Message-Id: <20220615195643.12608-5-hdegoede@redhat.com> In-Reply-To: <20220615195643.12608-1-hdegoede@redhat.com> References: <20220615195643.12608-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 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). 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 . This _REG depends on _INI problem can be fixed by calling the new ACPICA acpi_early_initialize_objects() function before acpi_ec_ecdt_probe(). But on some boards (e.g. Lenovo X1C8) the root devices _INI method relies on the EC OpRegion so executing the _INI methods before registering the EC OpRegion handler leads to errors there. To allow fixing this the ACPICA code now allows to do the OpRegion handler installation early on (without calling _REG) and to do the EC's _REG execution later on as a separate step. This commit uses this new ACPICA functions to fix the EC probe ordering by changing the acpi_bus_init() initialization order to this: 1. acpi_load_tables() 2. acpi_ec_ecdt_probe() This now calls acpi_install_address_space_handler(ACPI_NO_EXEC__REG) which installs the OpRegion handler without executing _REG 3. acpi_enable_subsystem() 4. acpi_early_initialize_objects() This calls the _INI method of the PCI and _SB root devices 5. acpi_ec_ecdt_exec_reg(); This executes the EC's _REG now that the root devices _INI has run 6. acpi_initialize_objects(ACPI_NO_EARLY_DEVICE_INIT) This allows the EC's _REG method to depend on e.g. the \OSYS global/GVNS variable often set by a root-device's _INI method, while at the same time allowing these _INI methods to access EmbeddedController OpRegions. Signed-off-by: Hans de Goede --- drivers/acpi/bus.c | 19 ++++++++++++++++++- drivers/acpi/ec.c | 38 ++++++++++++++++++++++++++++---------- drivers/acpi/internal.h | 1 + 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 86fa61a21826..fe5c46da5265 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -1303,7 +1303,24 @@ static int __init acpi_bus_init(void) goto error1; } - status = acpi_initialize_objects(ACPI_FULL_INITIALIZATION); + /* + * This usually sets an OSYS global variable based on _OSI checks, + * and the EC's _REG method may rely on this OSYS variable, so do + * this before acpi_ec_ecdt_exec_reg(). + */ + status = acpi_early_initialize_objects(); + if (ACPI_FAILURE(status)) { + pr_err("Unable to do early ACPI objects initialization\n"); + goto error1; + } + + acpi_ec_ecdt_exec_reg(); + + /* + * ACPI_NO_EARLY_DEVICE_INIT to avoid ACPICA calling + * acpi_early_initialize_objects() a second time. + */ + status = acpi_initialize_objects(ACPI_NO_EARLY_DEVICE_INIT); if (ACPI_FAILURE(status)) { pr_err("Unable to initialize ACPI objects\n"); goto error1; diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index a1b871a418f8..cd86e68d6b98 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1459,6 +1459,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. + * @flags: Flags to pass to acpi_install_address_space_handler() * * 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 @@ -1471,7 +1472,7 @@ 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, u32 flags) { acpi_status status; @@ -1479,10 +1480,10 @@ 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, flags); if (ACPI_FAILURE(status)) { acpi_ec_stop(ec, false); return -ENODEV; @@ -1575,11 +1576,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, u32 flags) { int ret; - ret = ec_install_handlers(ec, device); + ret = ec_install_handlers(ec, device, flags); if (ret) return ret; @@ -1641,7 +1642,7 @@ static int acpi_ec_add(struct acpi_device *device) } } - ret = acpi_ec_setup(ec, device); + ret = acpi_ec_setup(ec, device, 0); if (ret) goto err; @@ -1761,7 +1762,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, 0); if (ret) { acpi_ec_free(ec); return; @@ -1973,7 +1974,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, ACPI_NO_EXEC__REG); if (ret) { acpi_ec_free(ec); goto out; @@ -1988,6 +1989,23 @@ void __init acpi_ec_ecdt_probe(void) acpi_put_table((struct acpi_table_header *)ecdt_ptr); } +void __init acpi_ec_ecdt_exec_reg(void) +{ + if (!boot_ec || !boot_ec_is_ecdt) + return; + + /* + * Second call, first call is done in acpi_ec_ecdt_probe(), pass + * ACPI_NO_INSTALL_SPACE_HANDLER so as to only exec _REG now that + * the namespace has been setup. + */ + acpi_install_address_space_handler_flags(boot_ec->handle, + ACPI_ADR_SPACE_EC, + &acpi_ec_space_handler, NULL, + boot_ec, + ACPI_NO_INSTALL_SPACE_HANDLER); +} + #ifdef CONFIG_PM_SLEEP static int acpi_ec_suspend(struct device *dev) { diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 628bf8f18130..14dce6830e01 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -203,6 +203,7 @@ typedef int (*acpi_ec_query_func) (void *data); void acpi_ec_init(void); void acpi_ec_ecdt_probe(void); +void acpi_ec_ecdt_exec_reg(void); void acpi_ec_dsdt_probe(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void);