Message ID | 20231227040401.548977-1-kai.heng.feng@canonical.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp1232701dyb; Tue, 26 Dec 2023 20:04:34 -0800 (PST) X-Google-Smtp-Source: AGHT+IEcKZR2HdIUbjh8mnytk6hOXYvylPVpaeG7Z+hMD7UQi+GFihnqozk8uVeXZpgdlJ3Csdcw X-Received: by 2002:a05:6a20:4003:b0:190:12a3:7edb with SMTP id z3-20020a056a20400300b0019012a37edbmr7500889pze.20.1703649874610; Tue, 26 Dec 2023 20:04:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703649874; cv=none; d=google.com; s=arc-20160816; b=dtngVv2do942g98dpNyALusHvQapILBQL7g0+Nv/hH1QgevfbDCcYjrEyXDf8cUk4i tD+Cm1DeVFG65/3OoCQlMvrjiKySBDznz45V65Da1f7qMe2nqIwQ1EgxIyI14ySVh39Q wuEmGnGJ0If5hhoxV3ROqGJ3yXitttdepEzdKHF3SLrBY81nTGW01cpAypjTFm/GilBP 2f/xSjKmW5ucP+VdhMD9LZwC46rth2m1UdOTk7NLK7Gv91ofqYomw0Jg00Xd97cDUn0g YE8hU5GDu1ENYTIO2oF1TQBoAhb5gXHqi1xDrxqYBdDigrdZ+rYzvIhuCD2J50HUVLtm 606Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=m76mF9xbAsx2/TveqoahDVkZj7zH+GFIW9wWNEteV4M=; fh=ZxFAT952z5V4ZNpiRWZkB9iTmh+3TjFME+x81ePjuH4=; b=x28T0zCQdxGIre6GS3nVW5xlYKL/Q9OWtDYt9ShgRZviFfBckE3P99pQxUpBrT9bE7 rvEJPPLvYC9wDururdg0ZGqswGBXyFrtJk1eJ4xWg3Pzkhd3mgRBmOqkEysoQRlpdjRx 3aaZaX+9BUzPZVddMtgUk95jyM5T1vNMd7JDk/DLEoYDrKPFr3iyxOhUJfQV41eQD9n7 PY68bkPy4k4ynma+puBeEk+ms9+3NCuRBj6BlDX8ENFlQT0BET8ObdyjmsGd1KeuLe78 0aasWxEWavbeYg0b0x3Hh1ZtIw+qI274HSP3uNxEfQFoOE72dQb16hlcjPikNiRwQJEP 2R6g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@canonical.com header.s=20210705 header.b=IPDsRAUh; spf=pass (google.com: domain of linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id l10-20020a635b4a000000b005cd90f5bc6dsi5310856pgm.455.2023.12.26.20.04.34 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Dec 2023 20:04:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@canonical.com header.s=20210705 header.b=IPDsRAUh; spf=pass (google.com: domain of linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-11814-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canonical.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 64B4E2834A5 for <ouuuleilei@gmail.com>; Wed, 27 Dec 2023 04:04:34 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 407F04410; Wed, 27 Dec 2023 04:04:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=canonical.com header.i=@canonical.com header.b="IPDsRAUh" X-Original-To: linux-kernel@vger.kernel.org Received: from smtp-relay-canonical-1.canonical.com (smtp-relay-canonical-1.canonical.com [185.125.188.121]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 61981522C; Wed, 27 Dec 2023 04:04:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=canonical.com Received: from HP-EliteBook-x360-830-G8-Notebook-PC.. (unknown [10.101.196.174]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-1.canonical.com (Postfix) with ESMTPSA id ABE2F3F722; Wed, 27 Dec 2023 04:04:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1703649848; bh=m76mF9xbAsx2/TveqoahDVkZj7zH+GFIW9wWNEteV4M=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=IPDsRAUh3WjnCMbM4WW6k5e7qtHi7XoquVsYApwl6XZ5TtlWlqkfEZx20tKvrk562 KRuKIC0MjiPDpFtVv2RinBs1WYcIA8Td8GzepHpYkMbHGbBVN5uVyK1sr39HptFrWF aErdL3vq6R0+kZrraazNcadCg4eLa5TqwXWHZyhuIq7Ol6s/yZP+XJudoGPQMFUdtw MWaUFzW9FgICMoEqQ0OFzzsY3a90Wuu2HhCf8MenJVQWkJiKO50ErKF2N50j/lgUBf 819FCss6DqjBNimwT/KmgC8MuEuovfbHM4OPVh2Zd5yjYXYUXRr5/yAO/ea/mkzKUj 2ZRDvAwqkoAfw== From: Kai-Heng Feng <kai.heng.feng@canonical.com> To: jdelvare@suse.com, linux@roeck-us.net Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>, "Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <lenb@kernel.org>, Robert Moore <robert.moore@intel.com>, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org, acpica-devel@lists.linux.dev Subject: [PATCH v2] hwmon: (acpi_power_meter) Ensure IPMI space handler is ready on Dell systems Date: Wed, 27 Dec 2023 12:04:00 +0800 Message-Id: <20231227040401.548977-1-kai.heng.feng@canonical.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1786406371060832592 X-GMAIL-MSGID: 1786406371060832592 |
Series |
[v2] hwmon: (acpi_power_meter) Ensure IPMI space handler is ready on Dell systems
|
|
Commit Message
Kai-Heng Feng
Dec. 27, 2023, 4:04 a.m. UTC
The following error can be observed at boot:
[ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130)
[ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261)
[ 3.717936] No Local Variables are initialized for Method [_GHL]
[ 3.717938] No Arguments are initialized for method [_GHL]
[ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529)
[ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529)
[ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST
On Dell systems several methods of acpi_power_meter access variables in
IPMI region [0], so wait until IPMI space handler is installed by
acpi_ipmi and also wait until SMI is selected to make the space handler
fully functional.
[0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Use completion instead of request_module().
drivers/acpi/acpi_ipmi.c | 13 ++++++++++++-
drivers/hwmon/acpi_power_meter.c | 4 ++++
include/acpi/acpi_bus.h | 4 ++++
3 files changed, 20 insertions(+), 1 deletion(-)
Comments
Hi Kai-Heng, kernel test robot noticed the following build errors: [auto build test ERROR on groeck-staging/hwmon-next] [also build test ERROR on rafael-pm/linux-next rafael-pm/acpi-bus linus/master v6.7-rc7 next-20231222] [cannot apply to rafael-pm/devprop] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Kai-Heng-Feng/hwmon-acpi_power_meter-Ensure-IPMI-space-handler-is-ready-on-Dell-systems/20231227-120900 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next patch link: https://lore.kernel.org/r/20231227040401.548977-1-kai.heng.feng%40canonical.com patch subject: [PATCH v2] hwmon: (acpi_power_meter) Ensure IPMI space handler is ready on Dell systems config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20231228/202312280116.QtpZzDTy-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231228/202312280116.QtpZzDTy-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312280116.QtpZzDTy-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/acpi/acpi_ipmi.c:585:6: error: redefinition of 'wait_for_acpi_ipmi' 585 | void wait_for_acpi_ipmi(void) | ^~~~~~~~~~~~~~~~~~ In file included from include/linux/acpi.h:35, from drivers/acpi/acpi_ipmi.c:11: include/acpi/acpi_bus.h:676:20: note: previous definition of 'wait_for_acpi_ipmi' with type 'void(void)' 676 | static inline void wait_for_acpi_ipmi(void) | ^~~~~~~~~~~~~~~~~~ vim +/wait_for_acpi_ipmi +585 drivers/acpi/acpi_ipmi.c 584 > 585 void wait_for_acpi_ipmi(void) 586 { 587 wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); 588 } 589 EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); 590
On Wed, Dec 27, 2023 at 12:04:00PM +0800, Kai-Heng Feng wrote: > The following error can be observed at boot: > [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) > [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) > > [ 3.717936] No Local Variables are initialized for Method [_GHL] > > [ 3.717938] No Arguments are initialized for method [_GHL] > > [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST > > On Dell systems several methods of acpi_power_meter access variables in > IPMI region [0], so wait until IPMI space handler is installed by > acpi_ipmi and also wait until SMI is selected to make the space handler > fully functional. > > [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > v2: > - Use completion instead of request_module(). > > drivers/acpi/acpi_ipmi.c | 13 ++++++++++++- > drivers/hwmon/acpi_power_meter.c | 4 ++++ > include/acpi/acpi_bus.h | 4 ++++ This needs to be split into two patches. > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > index 0555f68c2dfd..2ea8b7e6cebf 100644 > --- a/drivers/acpi/acpi_ipmi.c > +++ b/drivers/acpi/acpi_ipmi.c > @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); > #define IPMI_TIMEOUT (5000) > #define ACPI_IPMI_MAX_MSG_LENGTH 64 > > +static struct completion smi_selected; > + > struct acpi_ipmi_device { > /* the device list attached to driver_data.ipmi_devices */ > struct list_head head; > @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) > if (temp->handle == handle) > goto err_lock; > } > - if (!driver_data.selected_smi) > + if (!driver_data.selected_smi) { > driver_data.selected_smi = ipmi_device; > + complete(&smi_selected); > + } > list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); > mutex_unlock(&driver_data.ipmi_lock); > > @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, > return status; > } > > +void wait_for_acpi_ipmi(void) > +{ > + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); wait_for_completion_interruptible_timeout) returns an error code which should be returned to the caller. > +} > +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); > + > static int __init acpi_ipmi_init(void) > { > int result; > acpi_status status; > + init_completion(&smi_selected); > > if (acpi_disabled) > return 0; > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c > index 703666b95bf4..acaf1ae68dc8 100644 > --- a/drivers/hwmon/acpi_power_meter.c > +++ b/drivers/hwmon/acpi_power_meter.c > @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) > strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); > device->driver_data = resource; > > + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && > + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) > + wait_for_acpi_ipmi(); wait_for_acpi_ipmi() should return an error code which should be handled here. > + > res = read_capabilities(resource); > if (res) > goto exit_free; > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 1216d72c650f..ed59fb89721e 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s > bool acpi_quirk_skip_acpi_ac_and_battery(void); > int acpi_install_cmos_rtc_space_handler(acpi_handle handle); > void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); > +void wait_for_acpi_ipmi(void); > #else > static inline bool acpi_device_override_status(struct acpi_device *adev, > unsigned long long *status) > @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) > static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) > { > } > +static inline void wait_for_acpi_ipmi(void) > +{ > +} Something with the conditional is wrong. See 0-day report. Guenter > #endif > > #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS) > -- > 2.34.1 > >
On Thu, Dec 28, 2023 at 5:58 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On Wed, Dec 27, 2023 at 12:04:00PM +0800, Kai-Heng Feng wrote: > > The following error can be observed at boot: > > [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) > > [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) > > > > [ 3.717936] No Local Variables are initialized for Method [_GHL] > > > > [ 3.717938] No Arguments are initialized for method [_GHL] > > > > [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > > [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > > [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST > > > > On Dell systems several methods of acpi_power_meter access variables in > > IPMI region [0], so wait until IPMI space handler is installed by > > acpi_ipmi and also wait until SMI is selected to make the space handler > > fully functional. > > > > [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > v2: > > - Use completion instead of request_module(). > > > > drivers/acpi/acpi_ipmi.c | 13 ++++++++++++- > > drivers/hwmon/acpi_power_meter.c | 4 ++++ > > include/acpi/acpi_bus.h | 4 ++++ > > This needs to be split into two patches. Will do. > > > 3 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > > index 0555f68c2dfd..2ea8b7e6cebf 100644 > > --- a/drivers/acpi/acpi_ipmi.c > > +++ b/drivers/acpi/acpi_ipmi.c > > @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); > > #define IPMI_TIMEOUT (5000) > > #define ACPI_IPMI_MAX_MSG_LENGTH 64 > > > > +static struct completion smi_selected; > > + > > struct acpi_ipmi_device { > > /* the device list attached to driver_data.ipmi_devices */ > > struct list_head head; > > @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) > > if (temp->handle == handle) > > goto err_lock; > > } > > - if (!driver_data.selected_smi) > > + if (!driver_data.selected_smi) { > > driver_data.selected_smi = ipmi_device; > > + complete(&smi_selected); > > + } > > list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); > > mutex_unlock(&driver_data.ipmi_lock); > > > > @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, > > return status; > > } > > > > +void wait_for_acpi_ipmi(void) > > +{ > > + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); > > wait_for_completion_interruptible_timeout) returns an error code which > should be returned to the caller. Will do in next revision. > > > +} > > +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); > > + > > static int __init acpi_ipmi_init(void) > > { > > int result; > > acpi_status status; > > + init_completion(&smi_selected); > > > > if (acpi_disabled) > > return 0; > > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c > > index 703666b95bf4..acaf1ae68dc8 100644 > > --- a/drivers/hwmon/acpi_power_meter.c > > +++ b/drivers/hwmon/acpi_power_meter.c > > @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) > > strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); > > device->driver_data = resource; > > > > + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && > > + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) > > + wait_for_acpi_ipmi(); > > wait_for_acpi_ipmi() should return an error code which > should be handled here. OK, I think print an message should be informative. > > > + > > res = read_capabilities(resource); > > if (res) > > goto exit_free; > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 1216d72c650f..ed59fb89721e 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s > > bool acpi_quirk_skip_acpi_ac_and_battery(void); > > int acpi_install_cmos_rtc_space_handler(acpi_handle handle); > > void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); > > +void wait_for_acpi_ipmi(void); > > #else > > static inline bool acpi_device_override_status(struct acpi_device *adev, > > unsigned long long *status) > > @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) > > static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) > > { > > } > > +static inline void wait_for_acpi_ipmi(void) > > +{ > > +} > > Something with the conditional is wrong. See 0-day report. Will fix that in next revision. Kai-Heng > > Guenter > > #endif > > > > #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS) > > -- > > 2.34.1 > > > >
Am 27.12.23 um 05:04 schrieb Kai-Heng Feng: > The following error can be observed at boot: > [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) > [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) > > [ 3.717936] No Local Variables are initialized for Method [_GHL] > > [ 3.717938] No Arguments are initialized for method [_GHL] > > [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST > > On Dell systems several methods of acpi_power_meter access variables in > IPMI region [0], so wait until IPMI space handler is installed by > acpi_ipmi and also wait until SMI is selected to make the space handler > fully functional. Hi, could it be that the ACPI firmware expects us to support the ACPI SPMI table? The ACPI SPMI table contains a description of the IPMI interface used by the platform, and its purpose seems to be similar to the ACPI ECDT table in that it allows the OS to access IPMI resources before all ACPI namespaces are enumerated. Adding support for this table would solve this problem without stalling the boot process by waiting for the ACPI IPMI device to probe. It would also avoid any issues should the ACPI IPMI device be removed later. Armin Wolf > > [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > v2: > - Use completion instead of request_module(). > > drivers/acpi/acpi_ipmi.c | 13 ++++++++++++- > drivers/hwmon/acpi_power_meter.c | 4 ++++ > include/acpi/acpi_bus.h | 4 ++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > index 0555f68c2dfd..2ea8b7e6cebf 100644 > --- a/drivers/acpi/acpi_ipmi.c > +++ b/drivers/acpi/acpi_ipmi.c > @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); > #define IPMI_TIMEOUT (5000) > #define ACPI_IPMI_MAX_MSG_LENGTH 64 > > +static struct completion smi_selected; > + > struct acpi_ipmi_device { > /* the device list attached to driver_data.ipmi_devices */ > struct list_head head; > @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) > if (temp->handle == handle) > goto err_lock; > } > - if (!driver_data.selected_smi) > + if (!driver_data.selected_smi) { > driver_data.selected_smi = ipmi_device; > + complete(&smi_selected); > + } > list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); > mutex_unlock(&driver_data.ipmi_lock); > > @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, > return status; > } > > +void wait_for_acpi_ipmi(void) > +{ > + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); > +} > +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); > + > static int __init acpi_ipmi_init(void) > { > int result; > acpi_status status; > + init_completion(&smi_selected); > > if (acpi_disabled) > return 0; > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c > index 703666b95bf4..acaf1ae68dc8 100644 > --- a/drivers/hwmon/acpi_power_meter.c > +++ b/drivers/hwmon/acpi_power_meter.c > @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) > strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); > device->driver_data = resource; > > + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && > + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) > + wait_for_acpi_ipmi(); > + > res = read_capabilities(resource); > if (res) > goto exit_free; > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 1216d72c650f..ed59fb89721e 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s > bool acpi_quirk_skip_acpi_ac_and_battery(void); > int acpi_install_cmos_rtc_space_handler(acpi_handle handle); > void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); > +void wait_for_acpi_ipmi(void); > #else > static inline bool acpi_device_override_status(struct acpi_device *adev, > unsigned long long *status) > @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) > static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) > { > } > +static inline void wait_for_acpi_ipmi(void) > +{ > +} > #endif > > #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
On Thu, Jan 4, 2024 at 5:23 AM Armin Wolf <W_Armin@gmx.de> wrote: > > Am 27.12.23 um 05:04 schrieb Kai-Heng Feng: > > > The following error can be observed at boot: > > [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) > > [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) > > > > [ 3.717936] No Local Variables are initialized for Method [_GHL] > > > > [ 3.717938] No Arguments are initialized for method [_GHL] > > > > [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > > [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) > > [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST > > > > On Dell systems several methods of acpi_power_meter access variables in > > IPMI region [0], so wait until IPMI space handler is installed by > > acpi_ipmi and also wait until SMI is selected to make the space handler > > fully functional. > > Hi, > > could it be that the ACPI firmware expects us to support the ACPI SPMI table? > The ACPI SPMI table contains a description of the IPMI interface used by the platform, > and its purpose seems to be similar to the ACPI ECDT table in that it allows the OS > to access IPMI resources before all ACPI namespaces are enumerated. > > Adding support for this table would solve this problem without stalling the boot > process by waiting for the ACPI IPMI device to probe. It would also avoid any issues > should the ACPI IPMI device be removed later. Hi, the ACPI firmware on the system doesn't have SPMI table: $ ls /sys/firmware/acpi/tables/ APIC BERT data DMAR DSDT dynamic EINJ ERST FACP FACS HEST HMAT HPET MCFG MIGT MSCT NBFT OEM4 SLIC SLIT SRAT SSDT1 SSDT2 SSDT3 SSDT4 SSDT5 SSDT6 TPM2 UEFI1 UEFI2 VFCT WSMT Kai-Heng > > Armin Wolf > > > > > [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > v2: > > - Use completion instead of request_module(). > > > > drivers/acpi/acpi_ipmi.c | 13 ++++++++++++- > > drivers/hwmon/acpi_power_meter.c | 4 ++++ > > include/acpi/acpi_bus.h | 4 ++++ > > 3 files changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > > index 0555f68c2dfd..2ea8b7e6cebf 100644 > > --- a/drivers/acpi/acpi_ipmi.c > > +++ b/drivers/acpi/acpi_ipmi.c > > @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); > > #define IPMI_TIMEOUT (5000) > > #define ACPI_IPMI_MAX_MSG_LENGTH 64 > > > > +static struct completion smi_selected; > > + > > struct acpi_ipmi_device { > > /* the device list attached to driver_data.ipmi_devices */ > > struct list_head head; > > @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) > > if (temp->handle == handle) > > goto err_lock; > > } > > - if (!driver_data.selected_smi) > > + if (!driver_data.selected_smi) { > > driver_data.selected_smi = ipmi_device; > > + complete(&smi_selected); > > + } > > list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); > > mutex_unlock(&driver_data.ipmi_lock); > > > > @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, > > return status; > > } > > > > +void wait_for_acpi_ipmi(void) > > +{ > > + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); > > +} > > +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); > > + > > static int __init acpi_ipmi_init(void) > > { > > int result; > > acpi_status status; > > + init_completion(&smi_selected); > > > > if (acpi_disabled) > > return 0; > > diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c > > index 703666b95bf4..acaf1ae68dc8 100644 > > --- a/drivers/hwmon/acpi_power_meter.c > > +++ b/drivers/hwmon/acpi_power_meter.c > > @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) > > strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); > > device->driver_data = resource; > > > > + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && > > + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) > > + wait_for_acpi_ipmi(); > > + > > res = read_capabilities(resource); > > if (res) > > goto exit_free; > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 1216d72c650f..ed59fb89721e 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s > > bool acpi_quirk_skip_acpi_ac_and_battery(void); > > int acpi_install_cmos_rtc_space_handler(acpi_handle handle); > > void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); > > +void wait_for_acpi_ipmi(void); > > #else > > static inline bool acpi_device_override_status(struct acpi_device *adev, > > unsigned long long *status) > > @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) > > static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) > > { > > } > > +static inline void wait_for_acpi_ipmi(void) > > +{ > > +} > > #endif > > > > #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
Am 04.01.24 um 03:14 schrieb Kai-Heng Feng: > On Thu, Jan 4, 2024 at 5:23 AM Armin Wolf <W_Armin@gmx.de> wrote: >> Am 27.12.23 um 05:04 schrieb Kai-Heng Feng: >> >>> The following error can be observed at boot: >>> [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) >>> [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) >>> >>> [ 3.717936] No Local Variables are initialized for Method [_GHL] >>> >>> [ 3.717938] No Arguments are initialized for method [_GHL] >>> >>> [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) >>> [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) >>> [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST >>> >>> On Dell systems several methods of acpi_power_meter access variables in >>> IPMI region [0], so wait until IPMI space handler is installed by >>> acpi_ipmi and also wait until SMI is selected to make the space handler >>> fully functional. >> Hi, >> >> could it be that the ACPI firmware expects us to support the ACPI SPMI table? >> The ACPI SPMI table contains a description of the IPMI interface used by the platform, >> and its purpose seems to be similar to the ACPI ECDT table in that it allows the OS >> to access IPMI resources before all ACPI namespaces are enumerated. >> >> Adding support for this table would solve this problem without stalling the boot >> process by waiting for the ACPI IPMI device to probe. It would also avoid any issues >> should the ACPI IPMI device be removed later. > Hi, the ACPI firmware on the system doesn't have SPMI table: > $ ls /sys/firmware/acpi/tables/ > APIC BERT data DMAR DSDT dynamic EINJ ERST FACP FACS HEST > HMAT HPET MCFG MIGT MSCT NBFT OEM4 SLIC SLIT SRAT SSDT1 > SSDT2 SSDT3 SSDT4 SSDT5 SSDT6 TPM2 UEFI1 UEFI2 VFCT WSMT > > Kai-Heng Thanks for checking this, in this case you might ignore my suggestion above. Armin Wolf >> Armin Wolf >> >>> [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 >>> >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >>> --- >>> v2: >>> - Use completion instead of request_module(). >>> >>> drivers/acpi/acpi_ipmi.c | 13 ++++++++++++- >>> drivers/hwmon/acpi_power_meter.c | 4 ++++ >>> include/acpi/acpi_bus.h | 4 ++++ >>> 3 files changed, 20 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c >>> index 0555f68c2dfd..2ea8b7e6cebf 100644 >>> --- a/drivers/acpi/acpi_ipmi.c >>> +++ b/drivers/acpi/acpi_ipmi.c >>> @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); >>> #define IPMI_TIMEOUT (5000) >>> #define ACPI_IPMI_MAX_MSG_LENGTH 64 >>> >>> +static struct completion smi_selected; >>> + >>> struct acpi_ipmi_device { >>> /* the device list attached to driver_data.ipmi_devices */ >>> struct list_head head; >>> @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) >>> if (temp->handle == handle) >>> goto err_lock; >>> } >>> - if (!driver_data.selected_smi) >>> + if (!driver_data.selected_smi) { >>> driver_data.selected_smi = ipmi_device; >>> + complete(&smi_selected); >>> + } >>> list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); >>> mutex_unlock(&driver_data.ipmi_lock); >>> >>> @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, >>> return status; >>> } >>> >>> +void wait_for_acpi_ipmi(void) >>> +{ >>> + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); >>> +} >>> +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); >>> + >>> static int __init acpi_ipmi_init(void) >>> { >>> int result; >>> acpi_status status; >>> + init_completion(&smi_selected); >>> >>> if (acpi_disabled) >>> return 0; >>> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c >>> index 703666b95bf4..acaf1ae68dc8 100644 >>> --- a/drivers/hwmon/acpi_power_meter.c >>> +++ b/drivers/hwmon/acpi_power_meter.c >>> @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) >>> strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); >>> device->driver_data = resource; >>> >>> + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && >>> + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) >>> + wait_for_acpi_ipmi(); >>> + >>> res = read_capabilities(resource); >>> if (res) >>> goto exit_free; >>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h >>> index 1216d72c650f..ed59fb89721e 100644 >>> --- a/include/acpi/acpi_bus.h >>> +++ b/include/acpi/acpi_bus.h >>> @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s >>> bool acpi_quirk_skip_acpi_ac_and_battery(void); >>> int acpi_install_cmos_rtc_space_handler(acpi_handle handle); >>> void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); >>> +void wait_for_acpi_ipmi(void); >>> #else >>> static inline bool acpi_device_override_status(struct acpi_device *adev, >>> unsigned long long *status) >>> @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) >>> static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) >>> { >>> } >>> +static inline void wait_for_acpi_ipmi(void) >>> +{ >>> +} >>> #endif >>> >>> #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 0555f68c2dfd..2ea8b7e6cebf 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -23,6 +23,8 @@ MODULE_LICENSE("GPL"); #define IPMI_TIMEOUT (5000) #define ACPI_IPMI_MAX_MSG_LENGTH 64 +static struct completion smi_selected; + struct acpi_ipmi_device { /* the device list attached to driver_data.ipmi_devices */ struct list_head head; @@ -463,8 +465,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) if (temp->handle == handle) goto err_lock; } - if (!driver_data.selected_smi) + if (!driver_data.selected_smi) { driver_data.selected_smi = ipmi_device; + complete(&smi_selected); + } list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); mutex_unlock(&driver_data.ipmi_lock); @@ -578,10 +582,17 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, return status; } +void wait_for_acpi_ipmi(void) +{ + wait_for_completion_interruptible_timeout(&smi_selected, 2 * HZ); +} +EXPORT_SYMBOL_GPL(wait_for_acpi_ipmi); + static int __init acpi_ipmi_init(void) { int result; acpi_status status; + init_completion(&smi_selected); if (acpi_disabled) return 0; diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 703666b95bf4..acaf1ae68dc8 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -883,6 +883,10 @@ static int acpi_power_meter_add(struct acpi_device *device) strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); device->driver_data = resource; + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.") && + acpi_dev_get_first_match_dev("IPI0001", NULL, -1)) + wait_for_acpi_ipmi(); + res = read_capabilities(resource); if (res) goto exit_free; diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 1216d72c650f..ed59fb89721e 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -655,6 +655,7 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s bool acpi_quirk_skip_acpi_ac_and_battery(void); int acpi_install_cmos_rtc_space_handler(acpi_handle handle); void acpi_remove_cmos_rtc_space_handler(acpi_handle handle); +void wait_for_acpi_ipmi(void); #else static inline bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *status) @@ -672,6 +673,9 @@ static inline int acpi_install_cmos_rtc_space_handler(acpi_handle handle) static inline void acpi_remove_cmos_rtc_space_handler(acpi_handle handle) { } +static inline void wait_for_acpi_ipmi(void) +{ +} #endif #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)