* Re: [PATCH] platform/chrome: cros_ec: honor acpi=off
2022-02-16 6:26 [PATCH] platform/chrome: cros_ec: honor acpi=off Tong Zhang
@ 2022-02-16 11:24 ` kernel test robot
2022-02-16 12:35 ` kernel test robot
2022-02-16 16:12 ` Guenter Roeck
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-16 11:24 UTC (permalink / raw)
To: Tong Zhang, Benson Leung, Guenter Roeck, linux-kernel
Cc: kbuild-all, Tong Zhang
Hi Tong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on v5.17-rc4 next-20220216]
[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]
url: https://github.com/0day-ci/linux/commits/Tong-Zhang/platform-chrome-cros_ec-honor-acpi-off/20220216-142709
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220216/202202161903.6YHDLR8F-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/66345a4aecd6e4acba257476c6e44559fccca143
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tong-Zhang/platform-chrome-cros_ec-honor-acpi-off/20220216-142709
git checkout 66345a4aecd6e4acba257476c6e44559fccca143
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/platform/chrome/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/platform/chrome/cros_ec_lpc.c: In function 'cros_ec_lpc_init':
>> drivers/platform/chrome/cros_ec_lpc.c:553:6: warning: the address of 'acpi_disable' will always evaluate as 'true' [-Waddress]
553 | if (acpi_disable)
| ^~~~~~~~~~~~
vim +553 drivers/platform/chrome/cros_ec_lpc.c
547
548 static int __init cros_ec_lpc_init(void)
549 {
550 int ret;
551 acpi_status status;
552
> 553 if (acpi_disable)
554 return -ENODEV;
555
556 status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
557 &cros_ec_lpc_acpi_device_found, NULL);
558 if (ACPI_FAILURE(status))
559 pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
560
561 if (!cros_ec_lpc_acpi_device_found &&
562 !dmi_check_system(cros_ec_lpc_dmi_table)) {
563 pr_err(DRV_NAME ": unsupported system.\n");
564 return -ENODEV;
565 }
566
567 cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0,
568 EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE);
569
570 /* Register the driver */
571 ret = platform_driver_register(&cros_ec_lpc_driver);
572 if (ret) {
573 pr_err(DRV_NAME ": can't register driver: %d\n", ret);
574 cros_ec_lpc_mec_destroy();
575 return ret;
576 }
577
578 if (!cros_ec_lpc_acpi_device_found) {
579 /* Register the device, and it'll get hooked up automatically */
580 ret = platform_device_register(&cros_ec_lpc_device);
581 if (ret) {
582 pr_err(DRV_NAME ": can't register device: %d\n", ret);
583 platform_driver_unregister(&cros_ec_lpc_driver);
584 cros_ec_lpc_mec_destroy();
585 }
586 }
587
588 return ret;
589 }
590
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] platform/chrome: cros_ec: honor acpi=off
2022-02-16 6:26 [PATCH] platform/chrome: cros_ec: honor acpi=off Tong Zhang
2022-02-16 11:24 ` kernel test robot
@ 2022-02-16 12:35 ` kernel test robot
2022-02-16 16:12 ` Guenter Roeck
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-16 12:35 UTC (permalink / raw)
To: Tong Zhang, Benson Leung, Guenter Roeck, linux-kernel
Cc: llvm, kbuild-all, Tong Zhang
Hi Tong,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on v5.17-rc4 next-20220216]
[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]
url: https://github.com/0day-ci/linux/commits/Tong-Zhang/platform-chrome-cros_ec-honor-acpi-off/20220216-142709
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220216/202202162047.eCFMqb2x-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/66345a4aecd6e4acba257476c6e44559fccca143
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tong-Zhang/platform-chrome-cros_ec-honor-acpi-off/20220216-142709
git checkout 66345a4aecd6e4acba257476c6e44559fccca143
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/platform/chrome/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/platform/chrome/cros_ec_lpc.c:553:6: warning: address of function 'acpi_disable' will always evaluate to 'true' [-Wpointer-bool-conversion]
if (acpi_disable)
~~ ^~~~~~~~~~~~
drivers/platform/chrome/cros_ec_lpc.c:553:6: note: prefix with the address-of operator to silence this warning
if (acpi_disable)
^
&
1 warning generated.
vim +553 drivers/platform/chrome/cros_ec_lpc.c
547
548 static int __init cros_ec_lpc_init(void)
549 {
550 int ret;
551 acpi_status status;
552
> 553 if (acpi_disable)
554 return -ENODEV;
555
556 status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
557 &cros_ec_lpc_acpi_device_found, NULL);
558 if (ACPI_FAILURE(status))
559 pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
560
561 if (!cros_ec_lpc_acpi_device_found &&
562 !dmi_check_system(cros_ec_lpc_dmi_table)) {
563 pr_err(DRV_NAME ": unsupported system.\n");
564 return -ENODEV;
565 }
566
567 cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0,
568 EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE);
569
570 /* Register the driver */
571 ret = platform_driver_register(&cros_ec_lpc_driver);
572 if (ret) {
573 pr_err(DRV_NAME ": can't register driver: %d\n", ret);
574 cros_ec_lpc_mec_destroy();
575 return ret;
576 }
577
578 if (!cros_ec_lpc_acpi_device_found) {
579 /* Register the device, and it'll get hooked up automatically */
580 ret = platform_device_register(&cros_ec_lpc_device);
581 if (ret) {
582 pr_err(DRV_NAME ": can't register device: %d\n", ret);
583 platform_driver_unregister(&cros_ec_lpc_driver);
584 cros_ec_lpc_mec_destroy();
585 }
586 }
587
588 return ret;
589 }
590
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] platform/chrome: cros_ec: honor acpi=off
2022-02-16 6:26 [PATCH] platform/chrome: cros_ec: honor acpi=off Tong Zhang
2022-02-16 11:24 ` kernel test robot
2022-02-16 12:35 ` kernel test robot
@ 2022-02-16 16:12 ` Guenter Roeck
2022-02-16 16:43 ` Tong Zhang
2 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2022-02-16 16:12 UTC (permalink / raw)
To: Tong Zhang; +Cc: Benson Leung, Guenter Roeck, linux-kernel
On Tue, Feb 15, 2022 at 10:26 PM Tong Zhang <ztong0001@gmail.com> wrote:
>
> when acpi=off is provided in bootarg, kernel crash with
>
> BUG: kernel NULL pointer dereference, address: 0000000000000018
> RIP: 0010:acpi_ns_walk_namespace+0x57/0x280
> <TASK>
> ? acpi_get_devices+0x140/0x140
> cros_ec_lpc_init+0x25/0x100
>
> Driver should check if ACPI is disabled before calling acpi_get_devices(),
> otherwise acpi_walk_namespace() will dereference null pointer since the
> acpi_gbl_root_node is not initialized.
> This is a common pattern and should be fixed in ACPI framework to prevent
> such crash in the future, but since many drivers are already doing explicit
> check(acpi_disable) we do the same thing here.
>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
> drivers/platform/chrome/cros_ec_lpc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index d6306d2a096f..95412a55ed8d 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -550,6 +550,9 @@ static int __init cros_ec_lpc_init(void)
> int ret;
> acpi_status status;
>
> + if (acpi_disable)
acpi_disabled ?
One does wonder why anyone would disable ACPI on any of the supported
systems, but in either case this is wrong. The driver should not abort
if acpi is disabled but just not call acpi_get_devices().
Guenter
> + return -ENODEV;
> +
> status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
> &cros_ec_lpc_acpi_device_found, NULL);
> if (ACPI_FAILURE(status))
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec: honor acpi=off
2022-02-16 16:12 ` Guenter Roeck
@ 2022-02-16 16:43 ` Tong Zhang
0 siblings, 0 replies; 5+ messages in thread
From: Tong Zhang @ 2022-02-16 16:43 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Benson Leung, Guenter Roeck, linux-kernel
On Wed, Feb 16, 2022 at 8:12 AM Guenter Roeck <groeck@google.com> wrote:
>
> On Tue, Feb 15, 2022 at 10:26 PM Tong Zhang <ztong0001@gmail.com> wrote:
> >
> > when acpi=off is provided in bootarg, kernel crash with
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000018
> > RIP: 0010:acpi_ns_walk_namespace+0x57/0x280
> > <TASK>
> > ? acpi_get_devices+0x140/0x140
> > cros_ec_lpc_init+0x25/0x100
> >
> > Driver should check if ACPI is disabled before calling acpi_get_devices(),
> > otherwise acpi_walk_namespace() will dereference null pointer since the
> > acpi_gbl_root_node is not initialized.
> > This is a common pattern and should be fixed in ACPI framework to prevent
> > such crash in the future, but since many drivers are already doing explicit
> > check(acpi_disable) we do the same thing here.
> >
Thanks for the comment. This is no longer an issue since the ACPI
patch I mentioned is staged in ACPICA tree.
https://github.com/acpica/acpica/commit/b1c3656ef4950098e530be68d4b589584f06cddc
^ permalink raw reply [flat|nested] 5+ messages in thread