On Mon, Feb 21, 2022 at 7:18 PM Nathan Chancellor [off-list ref] wrote:

Hi Shuai,

On Sat, Jan 22, 2022 at 01:26:17PM +0800, Shuai Xue wrote:

quoted

From commit e147133a42cb ("ACPI / APEI: Make hest.c manage the estatus
memory pool") was merged, ghes_init() relies on acpi_hest_init() to manage
the estatus memory pool. On the other hand, ghes_init() relies on
sdei_init() to detect the SDEI version and (un)register events. The
dependencies are as follows:

    ghes_init() => acpi_hest_init() => acpi_bus_init() => acpi_init()
    ghes_init() => sdei_init()

HEST is not PCI-specific and initcall ordering is implicit and not
well-defined within a level.

Based on above, remove acpi_hest_init() from acpi_pci_root_init() and
convert ghes_init() and sdei_init() from initcalls to explicit calls in the
following order:

    acpi_hest_init()
    ghes_init()
        sdei_init()

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c    | 19 ++++++++-----------
 drivers/acpi/bus.c          |  2 ++
 drivers/acpi/pci_root.c     |  3 ---
 drivers/firmware/Kconfig    |  1 +
 drivers/firmware/arm_sdei.c | 13 ++-----------
 include/acpi/apei.h         |  4 +++-
 include/linux/arm_sdei.h    |  2 ++
 7 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0c5c9acc6254..aadc0a972f18 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c

@@ -1457,33 +1457,35 @@ static struct platform_driver ghes_platform_driver = {
      .remove         = ghes_remove,
 };

-static int __init ghes_init(void)
+void __init ghes_init(void)
 {
      int rc;

+     sdei_init();
+
      if (acpi_disabled)
-             return -ENODEV;
+             return;

      switch (hest_disable) {
      case HEST_NOT_FOUND:
-             return -ENODEV;
+             return;
      case HEST_DISABLED:
              pr_info(GHES_PFX "HEST is not enabled!\n");
-             return -EINVAL;
+             return;
      default:
              break;
      }

      if (ghes_disable) {
              pr_info(GHES_PFX "GHES is not enabled!\n");
-             return -EINVAL;
+             return;
      }

      ghes_nmi_init_cxt();

      rc = platform_driver_register(&ghes_platform_driver);
      if (rc)
-             goto err;
+             return;

      rc = apei_osc_setup();
      if (rc == 0 && osc_sb_apei_support_acked)

@@ -1494,9 +1496,4 @@ static int __init ghes_init(void)
              pr_info(GHES_PFX "APEI firmware first mode is enabled by APEI bit.\n");
      else
              pr_info(GHES_PFX "Failed to enable APEI firmware first mode.\n");
-
-     return 0;
-err:
-     return rc;
 }
-device_initcall(ghes_init);

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 07f604832fd6..3f403db20f69 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c

@@ -1331,6 +1331,8 @@ static int __init acpi_init(void)

      pci_mmcfg_late_init();
      acpi_iort_init();
+     acpi_hest_init();
+     ghes_init();
      acpi_scan_init();
      acpi_ec_init();
      acpi_debugfs_init();

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index b76db99cced3..6f9e75d14808 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c

@@ -22,8 +22,6 @@
 #include <linux/slab.h>
 #include <linux/dmi.h>
 #include <linux/platform_data/x86/apple.h>
-#include <acpi/apei.h>       /* for acpi_hest_init() */
-
 #include "internal.h"

 #define ACPI_PCI_ROOT_CLASS          "pci_bridge"

@@ -943,7 +941,6 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,

 void __init acpi_pci_root_init(void)
 {
-     acpi_hest_init();
      if (acpi_pci_disabled)
              return;

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 75cb91055c17..ad114d9cdf8e 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig

@@ -40,6 +40,7 @@ config ARM_SCPI_POWER_DOMAIN
 config ARM_SDE_INTERFACE
      bool "ARM Software Delegated Exception Interface (SDEI)"
      depends on ARM64
+     select ACPI_APEI_GHES

As the kernel test robot pointed out [1], you cannot do this.
CONFIG_ACPI_APEI_GHES is a user selectable symbol that has dependencies,
which 'select' completely overrides, resulting in build failures when
CONFIG_ACPI_APEI is not enabled.

If CONFIG_ARM_SDE_INTERFACE truly requires CONFIG_ACPI_APEI_GHES, you
should have "depends on ACPI_APEI_GHES".

If CONFIG_ARM_SDE_INTERFACE soft depends on CONFIG_ACPI_APEI_GHES for
functionality but can work without it, you could use
"imply ACPI_APEI_GHES", which will enable CONFIG_ACPI_APEI_GHES if its
dependencies are met.

I noticed the same error with Alpine Linux's aarch64 configuration [2]
if you wanted a quick configuration to test with.

[1]: https://lore.kernel.org/r/202202151504.jWpZGPaH-lkp@intel.com/ (local)
[2]: https://git.alpinelinux.org/aports/plain/community/linux-edge/config-edge.aarch64

Note that I've dropped these patches from my linux-next branch now.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel