[PATCH 0/3] Refactor GHES to better support non-APEI systems

[PATCH 0/3] Refactor GHES to better support non-APEI systems

[Punit Agrawal]

Hi,

The small series re-factors the GHES driver initialisation to move
memory allocation and checks from driver init to device probe. The net
effect is to improve the situation for systems that do not support
APEI.

The patches are organised as -

* Unify estatus pool creation and expansion into ghes_estatus_pool_init()
* Move memory initialisation and firmware first support checks from
  driver init to device probe
* Do not set hest_disable if HEST not found

The set of patches initially started out as a single patch[0]. The
current series is the re-worked approach based on ensuing
discussion. They patches are based on v4.13-rc3 and have been tested
on a system that does not support APEI. Further testing on systems
that support APEI to ensure that the changes do not break expected
behaviour will be greatly appreciated.

Thanks,
Punit

[0] https://lkml.org/lkml/2017/7/20/430

Punit Agrawal (3):
  GHES: Expand the estatus pool in ghes_estatus_pool_init()
  GHES: Move memory initialisation to ghes_probe()
  ACPI / APEI: HEST: Don't set hest_disable if table not found

 drivers/acpi/apei/ghes.c | 132 ++++++++++++++++++++++++++---------------------
 drivers/acpi/apei/hest.c |   2 +-
 2 files changed, 73 insertions(+), 61 deletions(-)

-- 
2.11.0

[PATCH 1/3] GHES: Expand the estatus pool in ghes_estatus_pool_init()

[Punit Agrawal]

During the GHES driver initialisation, a pool of memory is created by
calling ghes_estatus_pool_init() which is then immediately expanded by
making a call to ghes_estatus_pool_expand().

Re-factor the code so that on successful creation of the pool,
ghes_estatus_pool_init() expands the initialised pool by calling
ghes_estatus_pool_expand().

The change is in preparation for moving the pool creation out of driver
initialisation to when a platform device is being probed.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: James Morse <james.morse@arm.com>
---
 drivers/acpi/apei/ghes.c | 55 +++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d661d452b238..007b38abcb34 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -207,11 +207,25 @@ static void ghes_iounmap_irq(void __iomem *vaddr_ptr)
 	arch_apei_flush_tlb_one(vaddr);
 }
 
-static int ghes_estatus_pool_init(void)
+static int ghes_estatus_pool_expand(unsigned long len)
 {
-	ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
-	if (!ghes_estatus_pool)
-		return -ENOMEM;
+	unsigned long i, pages, size, addr;
+	int ret;
+
+	ghes_estatus_pool_size_request += PAGE_ALIGN(len);
+	size = gen_pool_size(ghes_estatus_pool);
+	if (size >= ghes_estatus_pool_size_request)
+		return 0;
+	pages = (ghes_estatus_pool_size_request - size) / PAGE_SIZE;
+	for (i = 0; i < pages; i++) {
+		addr = __get_free_page(GFP_KERNEL);
+		if (!addr)
+			return -ENOMEM;
+		ret = gen_pool_add(ghes_estatus_pool, addr, PAGE_SIZE, -1);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -229,26 +243,20 @@ static void ghes_estatus_pool_exit(void)
 	gen_pool_destroy(ghes_estatus_pool);
 }
 
-static int ghes_estatus_pool_expand(unsigned long len)
+static int ghes_estatus_pool_init(void)
 {
-	unsigned long i, pages, size, addr;
-	int ret;
+	int rc;
 
-	ghes_estatus_pool_size_request += PAGE_ALIGN(len);
-	size = gen_pool_size(ghes_estatus_pool);
-	if (size >= ghes_estatus_pool_size_request)
-		return 0;
-	pages = (ghes_estatus_pool_size_request - size) / PAGE_SIZE;
-	for (i = 0; i < pages; i++) {
-		addr = __get_free_page(GFP_KERNEL);
-		if (!addr)
-			return -ENOMEM;
-		ret = gen_pool_add(ghes_estatus_pool, addr, PAGE_SIZE, -1);
-		if (ret)
-			return ret;
-	}
+	ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
+	if (!ghes_estatus_pool)
+		return -ENOMEM;
 
-	return 0;
+	rc = ghes_estatus_pool_expand(GHES_ESTATUS_CACHE_AVG_SIZE *
+				      GHES_ESTATUS_CACHE_ALLOCED_MAX);
+	if (rc)
+		ghes_estatus_pool_exit();
+
+	return rc;
 }
 
 static int map_gen_v2(struct ghes *ghes)
@@ -1285,11 +1293,6 @@ static int __init ghes_init(void)
 	if (rc)
 		goto err_ioremap_exit;
 
-	rc = ghes_estatus_pool_expand(GHES_ESTATUS_CACHE_AVG_SIZE *
-				      GHES_ESTATUS_CACHE_ALLOCED_MAX);
-	if (rc)
-		goto err_pool_exit;
-
 	rc = platform_driver_register(&ghes_platform_driver);
 	if (rc)
 		goto err_pool_exit;
-- 
2.11.0

[PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Punit Agrawal]

During the driver init, the ghes driver initialises some data structures
which are only used if a GHES platform device is probed. Similarly, the
init function also checks for support for firmware first mode.

Create a function, ghes_common_init(), that performs the initialisations
and checks that are currently performed on driver init. The function is
called when the GHES device is probed.

Delaying initialisation and checks until probe has the added benefit of
reducing driver prints on systems that do not support ACPI APEI.

Signed-off-by: Punit Agrawal <punit.agrwal@arm.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: James Morse <james.morse@arm.com>
---
 drivers/acpi/apei/ghes.c | 77 +++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 007b38abcb34..befb18338acb 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1088,12 +1088,53 @@ static inline void ghes_nmi_init_cxt(void)
 }
 #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
 
+static int ghes_common_init(void)
+{
+	int rc;
+	static bool initialised;
+
+	if (initialised)
+		return 0;
+
+	ghes_nmi_init_cxt();
+
+	rc = ghes_ioremap_init();
+	if (rc)
+		goto err;
+
+	rc = ghes_estatus_pool_init();
+	if (rc)
+		goto err_ioremap_exit;
+
+	rc = apei_osc_setup();
+	if (rc == 0 && osc_sb_apei_support_acked)
+		pr_info(GHES_PFX "APEI firmware first mode is enabled by APEI bit and WHEA _OSC.\n");
+	else if (rc == 0 && !osc_sb_apei_support_acked)
+		pr_info(GHES_PFX "APEI firmware first mode is enabled by WHEA _OSC.\n");
+	else if (rc && osc_sb_apei_support_acked)
+		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");
+
+	initialised = true;
+	return 0;
+
+err_ioremap_exit:
+	ghes_ioremap_exit();
+err:
+	return rc;
+}
+
 static int ghes_probe(struct platform_device *ghes_dev)
 {
 	struct acpi_hest_generic *generic;
 	struct ghes *ghes = NULL;
 
-	int rc = -EINVAL;
+	int rc;
+
+	rc = ghes_common_init();
+	if (rc)
+		return rc;
 
 	generic = *(struct acpi_hest_generic **)ghes_dev->dev.platform_data;
 	if (!generic->enabled)
@@ -1268,8 +1309,6 @@ static struct platform_driver ghes_platform_driver = {
 
 static int __init ghes_init(void)
 {
-	int rc;
-
 	if (acpi_disabled)
 		return -ENODEV;
 
@@ -1283,36 +1322,6 @@ static int __init ghes_init(void)
 		return -EINVAL;
 	}
 
-	ghes_nmi_init_cxt();
-
-	rc = ghes_ioremap_init();
-	if (rc)
-		goto err;
-
-	rc = ghes_estatus_pool_init();
-	if (rc)
-		goto err_ioremap_exit;
-
-	rc = platform_driver_register(&ghes_platform_driver);
-	if (rc)
-		goto err_pool_exit;
-
-	rc = apei_osc_setup();
-	if (rc == 0 && osc_sb_apei_support_acked)
-		pr_info(GHES_PFX "APEI firmware first mode is enabled by APEI bit and WHEA _OSC.\n");
-	else if (rc == 0 && !osc_sb_apei_support_acked)
-		pr_info(GHES_PFX "APEI firmware first mode is enabled by WHEA _OSC.\n");
-	else if (rc && osc_sb_apei_support_acked)
-		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_pool_exit:
-	ghes_estatus_pool_exit();
-err_ioremap_exit:
-	ghes_ioremap_exit();
-err:
-	return rc;
+	return platform_driver_register(&ghes_platform_driver);
 }
 device_initcall(ghes_init);
-- 
2.11.0

[PATCH 3/3] ACPI / APEI: HEST: Don't set hest_disable if table not found

[Punit Agrawal]

During initialisation, the driver sets hest_disable to 1 if the HEST
table is not found. This value is checked during the GHES driver
initialisation and in turn leads to a message such as -

[    3.460067] GHES: HEST is not enabled!

being output in the bootlog. The message appears even on systems that do
not have the HEST table or APEI support.

Update the initialisation to skip setting hest_disable when the HEST
table is not found.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Borislav Petkov <bp@suse.de>
---
 drivers/acpi/apei/hest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 456b488eb1df..3aa1b34459b6 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -233,7 +233,7 @@ void __init acpi_hest_init(void)
 	status = acpi_get_table(ACPI_SIG_HEST, 0,
 				(struct acpi_table_header **)&hest_tab);
 	if (status == AE_NOT_FOUND)
-		goto err;
+		return;
 	else if (ACPI_FAILURE(status)) {
 		const char *msg = acpi_format_exception(status);
 		pr_err(HEST_PFX "Failed to get table, %s\n", msg);
-- 
2.11.0

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Punit Agrawal]

Punit Agrawal <punit.agrawal@arm.com> writes:

> During the driver init, the ghes driver initialises some data structures
> which are only used if a GHES platform device is probed. Similarly, the
> init function also checks for support for firmware first mode.
>
> Create a function, ghes_common_init(), that performs the initialisations
> and checks that are currently performed on driver init. The function is
> called when the GHES device is probed.
>
> Delaying initialisation and checks until probe has the added benefit of
> reducing driver prints on systems that do not support ACPI APEI.
>
> Signed-off-by: Punit Agrawal <punit.agrwal@arm.com>

I managed to get my email wrong in the signed-off-by. And git send-email
usage propagated it to the senders.

Corrected it locally but will wait for comments on the code before
re-sending.

> Cc: Borislav Petkov <bp@suse.de>
> Cc: James Morse <james.morse@arm.com>
> ---
>  drivers/acpi/apei/ghes.c | 77 +++++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 007b38abcb34..befb18338acb 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1088,12 +1088,53 @@ static inline void ghes_nmi_init_cxt(void)
>  }
>  #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
>  
> +static int ghes_common_init(void)
> +{
> +	int rc;
> +	static bool initialised;
> +
> +	if (initialised)
> +		return 0;
> +
> +	ghes_nmi_init_cxt();
> +
> +	rc = ghes_ioremap_init();
> +	if (rc)
> +		goto err;
> +
> +	rc = ghes_estatus_pool_init();
> +	if (rc)
> +		goto err_ioremap_exit;
> +
> +	rc = apei_osc_setup();
> +	if (rc == 0 && osc_sb_apei_support_acked)
> +		pr_info(GHES_PFX "APEI firmware first mode is enabled by APEI bit and WHEA _OSC.\n");
> +	else if (rc == 0 && !osc_sb_apei_support_acked)
> +		pr_info(GHES_PFX "APEI firmware first mode is enabled by WHEA _OSC.\n");
> +	else if (rc && osc_sb_apei_support_acked)
> +		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");
> +
> +	initialised = true;
> +	return 0;
> +
> +err_ioremap_exit:
> +	ghes_ioremap_exit();
> +err:
> +	return rc;
> +}
> +
>  static int ghes_probe(struct platform_device *ghes_dev)
>  {
>  	struct acpi_hest_generic *generic;
>  	struct ghes *ghes = NULL;
>  
> -	int rc = -EINVAL;
> +	int rc;
> +
> +	rc = ghes_common_init();
> +	if (rc)
> +		return rc;
>  
>  	generic = *(struct acpi_hest_generic **)ghes_dev->dev.platform_data;
>  	if (!generic->enabled)
> @@ -1268,8 +1309,6 @@ static struct platform_driver ghes_platform_driver = {
>  
>  static int __init ghes_init(void)
>  {
> -	int rc;
> -
>  	if (acpi_disabled)
>  		return -ENODEV;
>  
> @@ -1283,36 +1322,6 @@ static int __init ghes_init(void)
>  		return -EINVAL;
>  	}
>  
> -	ghes_nmi_init_cxt();
> -
> -	rc = ghes_ioremap_init();
> -	if (rc)
> -		goto err;
> -
> -	rc = ghes_estatus_pool_init();
> -	if (rc)
> -		goto err_ioremap_exit;
> -
> -	rc = platform_driver_register(&ghes_platform_driver);
> -	if (rc)
> -		goto err_pool_exit;
> -
> -	rc = apei_osc_setup();
> -	if (rc == 0 && osc_sb_apei_support_acked)
> -		pr_info(GHES_PFX "APEI firmware first mode is enabled by APEI bit and WHEA _OSC.\n");
> -	else if (rc == 0 && !osc_sb_apei_support_acked)
> -		pr_info(GHES_PFX "APEI firmware first mode is enabled by WHEA _OSC.\n");
> -	else if (rc && osc_sb_apei_support_acked)
> -		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_pool_exit:
> -	ghes_estatus_pool_exit();
> -err_ioremap_exit:
> -	ghes_ioremap_exit();
> -err:
> -	return rc;
> +	return platform_driver_register(&ghes_platform_driver);
>  }
>  device_initcall(ghes_init);

Re: [PATCH 0/3] Refactor GHES to better support non-APEI systems

[Punit Agrawal]

Punit Agrawal <punit.agrawal@arm.com> writes:

> Hi,
>
> The small series re-factors the GHES driver initialisation to move
> memory allocation and checks from driver init to device probe. The net
> effect is to improve the situation for systems that do not support
> APEI.
>
> The patches are organised as -
>
> * Unify estatus pool creation and expansion into ghes_estatus_pool_init()
> * Move memory initialisation and firmware first support checks from
>   driver init to device probe
> * Do not set hest_disable if HEST not found
>
> The set of patches initially started out as a single patch[0]. The
> current series is the re-worked approach based on ensuing
> discussion. They patches are based on v4.13-rc3 and have been tested
> on a system that does not support APEI. Further testing on systems
> that support APEI to ensure that the changes do not break expected
> behaviour will be greatly appreciated.

Ping!

Boris, have you had a chance to look at the re-factoring of the GHES init?

>
> Thanks,
> Punit
>
> [0] https://lkml.org/lkml/2017/7/20/430
>
> Punit Agrawal (3):
>   GHES: Expand the estatus pool in ghes_estatus_pool_init()
>   GHES: Move memory initialisation to ghes_probe()
>   ACPI / APEI: HEST: Don't set hest_disable if table not found
>
>  drivers/acpi/apei/ghes.c | 132 ++++++++++++++++++++++++++---------------------
>  drivers/acpi/apei/hest.c |   2 +-
>  2 files changed, 73 insertions(+), 61 deletions(-)

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Borislav Petkov]

On Tue, Aug 01, 2017 at 02:36:07PM +0100, Punit Agrawal wrote:
> During the driver init, the ghes driver initialises some data structures

Let's stick to "GHES" in capital letters everywhere.

> which are only used if a GHES platform device is probed. Similarly, the
> init function also checks for support for firmware first mode.
> 
> Create a function, ghes_common_init(), that performs the initialisations
> and checks that are currently performed on driver init. The function is
> called when the GHES device is probed.
> 
> Delaying initialisation and checks until probe has the added benefit of
> reducing driver prints on systems that do not support ACPI APEI.
> 
> Signed-off-by: Punit Agrawal <punit.agrwal@arm.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: James Morse <james.morse@arm.com>
> ---
>  drivers/acpi/apei/ghes.c | 77 +++++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 007b38abcb34..befb18338acb 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1088,12 +1088,53 @@ static inline void ghes_nmi_init_cxt(void)
>  }
>  #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
>  
> +static int ghes_common_init(void)
> +{
> +	int rc;
> +	static bool initialised;
> +
> +	if (initialised)
> +		return 0;

Can't say that I like it. Especially for this "initialised" thing, which
practically says that this code doesn't conceptually belong here because
we have to explicitly force it to execute it only once. Even though we
have execute-once path in ghes_init().

What would be much better IMHO is if ghes_init() checked for some APEI
table which is missing on your system and exited early. That would save
us all the trouble and solve the situation properly ...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Borislav Petkov]

On Tue, Aug 15, 2017 at 11:10:05AM +0100, Punit Agrawal wrote:
> In the absence of any GHES entries these are wasted.

I know. This whole APEI init code would need a proper cleanup, like
acpi_pci_root_init() calls acpi_hest_init() and it shouldn't have to
communicate through a variable with GHES whether to init or not but it
should initialize GHES itself. And ghes_init() being a device_initcall()
is just yuck.

Something for the todo list I guess...

> The system does not provide the Hardware Error Source Table (HEST) which
> is checked in the hest driver (drivers/acpi/apei/hest.c).
> 
> I think I'll go with your original suggestion to change hest_disable
> from a boolean to something with more states. Re-checking for the HEST
> table again feels like duplication.
> 
> Makes sense?

Right, and then depending on the setting of hest_disable, either issue
the "HEST is not enabled" message or not.

Thanks.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Punit Agrawal]

Hi Boris,

Borislav Petkov <bp@suse.de> writes:

> On Tue, Aug 01, 2017 at 02:36:07PM +0100, Punit Agrawal wrote:
>> During the driver init, the ghes driver initialises some data structures
>
> Let's stick to "GHES" in capital letters everywhere.
>
>> which are only used if a GHES platform device is probed. Similarly, the
>> init function also checks for support for firmware first mode.
>> 
>> Create a function, ghes_common_init(), that performs the initialisations
>> and checks that are currently performed on driver init. The function is
>> called when the GHES device is probed.
>> 
>> Delaying initialisation and checks until probe has the added benefit of
>> reducing driver prints on systems that do not support ACPI APEI.
>> 
>> Signed-off-by: Punit Agrawal <punit.agrwal@arm.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: James Morse <james.morse@arm.com>
>> ---
>>  drivers/acpi/apei/ghes.c | 77 +++++++++++++++++++++++++++---------------------
>>  1 file changed, 43 insertions(+), 34 deletions(-)
>> 
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 007b38abcb34..befb18338acb 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -1088,12 +1088,53 @@ static inline void ghes_nmi_init_cxt(void)
>>  }
>>  #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
>>  
>> +static int ghes_common_init(void)
>> +{
>> +	int rc;
>> +	static bool initialised;
>> +
>> +	if (initialised)
>> +		return 0;
>
> Can't say that I like it. Especially for this "initialised" thing, which
> practically says that this code doesn't conceptually belong here because
> we have to explicitly force it to execute it only once. Even though we
> have execute-once path in ghes_init().

I can see your point. My rationale for the change was that the allocated
resources are only ever useful if there is an actual GHES device in the
system - which we can't know until we hit probe.

In the absence of any GHES entries these are wasted.

>
> What would be much better IMHO is if ghes_init() checked for some APEI
> table which is missing on your system and exited early. That would save
> us all the trouble and solve the situation properly ...

The system does not provide the Hardware Error Source Table (HEST) which
is checked in the hest driver (drivers/acpi/apei/hest.c).

I think I'll go with your original suggestion to change hest_disable
from a boolean to something with more states. Re-checking for the HEST
table again feels like duplication.

Makes sense?

Thanks for taking a look at the patches.

Punit

>
> -- 
> Regards/Gruss,
>     Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Punit Agrawal]

[ thanks for dropping the bad email! ]

Borislav Petkov <bp@suse.de> writes:

> On Tue, Aug 15, 2017 at 11:10:05AM +0100, Punit Agrawal wrote:
>> In the absence of any GHES entries these are wasted.
>
> I know. This whole APEI init code would need a proper cleanup, like
> acpi_pci_root_init() calls acpi_hest_init() and it shouldn't have to
> communicate through a variable with GHES whether to init or not but it
> should initialize GHES itself. And ghes_init() being a device_initcall()
> is just yuck.

I'd seen the link between acpi_pci_root_init() and acpi_hest_init() but
didn't want to open another can of worms... :)

$SUBJECT was my attempt (small) at improving the situation but I guess I
didn't go far enough.

>
> Something for the todo list I guess...
>
>> The system does not provide the Hardware Error Source Table (HEST) which
>> is checked in the hest driver (drivers/acpi/apei/hest.c).
>> 
>> I think I'll go with your original suggestion to change hest_disable
>> from a boolean to something with more states. Re-checking for the HEST
>> table again feels like duplication.
>> 
>> Makes sense?
>
> Right, and then depending on the setting of hest_disable, either issue
> the "HEST is not enabled" message or not.

Ack!

>
> Thanks.
>
> -- 
> Regards/Gruss,
>     Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 2/3] GHES: Move memory initialisation to ghes_probe()

[Borislav Petkov]

On Tue, Aug 15, 2017 at 11:31:57AM +0100, Punit Agrawal wrote:
> I'd seen the link between acpi_pci_root_init() and acpi_hest_init() but
> didn't want to open another can of worms... :)
> 
> $SUBJECT was my attempt (small) at improving the situation but I guess I
> didn't go far enough.

Well, one fine day, if you feel bored and your hands are itching to do
some hacking, I won't be mad atcha if you did a second try...

:-)))

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--