All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toshi Kani <toshi.kani@hpe.com>
To: "bp@alien8.de" <bp@alien8.de>
Cc: "alex_gagniuc@dellteam.com" <alex_gagniuc@dellteam.com>,
	"tony.luck@intel.com" <tony.luck@intel.com>,
	"james.morse@arm.com" <james.morse@arm.com>,
	"austin_bolen@dell.com" <austin_bolen@dell.com>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Subject: ghes, EDAC: Fix ghes_edac registration
Date: Mon, 30 Apr 2018 22:55:02 +0000	[thread overview]
Message-ID: <1525128847.2693.552.camel@hpe.com> (raw)

On Mon, 2018-04-30 at 12:52 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Tony reported seeing
> 
>   "Internal error: Can't find EDAC structure"
> 
> when injecting correctable errors due to the fact that ghes_edac would
> still load even if the whitelist won't hit. Drop the pr_err() in
> ghes_edac_report_mem_error() for now due to the hacky way how ghes_edac
> depends on ghes.c.
> 
> While at it, make ghes_edac_register() return an error if it doesn't hit
> in the whitelist as it is the only sensible thing to do in that
> situation.
> 
> Furthermore, move the call to it to happen last in ghes_probe() so that
> GHES initializing properly does not depend on ghes_edac init at all
> as latter is only reporting errors and not required for GHES's proper
> functioning.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Toshi Kani <toshi.kani@hpe.com>
> Link: https://lkml.kernel.org/r/20180420182015.zao3olss4tvvlxki@agluck-desk
> ---
>  drivers/acpi/apei/ghes.c | 14 ++++++--------
>  drivers/edac/ghes_edac.c |  6 ++----
>  include/acpi/ghes.h      |  2 +-
>  3 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 1efefe919555..88103333ee1b 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1087,10 +1087,6 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  		goto err;
>  	}
>  
> -	rc = ghes_edac_register(ghes, &ghes_dev->dev);
> -	if (rc < 0)
> -		goto err;
> -
>  	switch (generic->notify.type) {
>  	case ACPI_HEST_NOTIFY_POLLED:
>  		timer_setup(&ghes->timer, ghes_poll_func, TIMER_DEFERRABLE);
> @@ -1102,14 +1098,14 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  		if (rc) {
>  			pr_err(GHES_PFX "Failed to map GSI to IRQ for generic hardware error source: %d\n",
>  			       generic->header.source_id);
> -			goto err_edac_unreg;
> +			goto err;
>  		}
>  		rc = request_irq(ghes->irq, ghes_irq_func, IRQF_SHARED,
>  				 "GHES IRQ", ghes);
>  		if (rc) {
>  			pr_err(GHES_PFX "Failed to register IRQ for generic hardware error source: %d\n",
>  			       generic->header.source_id);
> -			goto err_edac_unreg;
> +			goto err;
>  		}
>  		break;
>  
> @@ -1132,14 +1128,16 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  	default:
>  		BUG();
>  	}
> +
>  	platform_set_drvdata(ghes_dev, ghes);
>  
> +	ghes_edac_register(ghes, &ghes_dev->dev);
> +
>  	/* Handle any pending errors right away */
>  	ghes_proc(ghes);
>  
>  	return 0;
> -err_edac_unreg:
> -	ghes_edac_unregister(ghes);
> +
>  err:
>  	if (ghes) {
>  		ghes_fini(ghes);
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index 68b6ee18bea6..7fdbfe871f5c 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -183,10 +183,8 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
>  	char *p;
>  	u8 grain_bits;
>  
> -	if (!pvt) {
> -		pr_err("Internal error: Can't find EDAC structure\n");
> +	if (!pvt)
>  		return;
> -	}
>  

I think we need the same 'if (!ghes_pvt)' check in
ghes_edac_unregister().

Thanks,
-Toshi

             reply	other threads:[~2018-04-30 22:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-30 22:55 Toshi Kani [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-05-01 13:43 ghes, EDAC: Fix ghes_edac registration Toshi Kani
2018-05-01  8:15 Borislav Petkov
2018-04-30 10:52 Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1525128847.2693.552.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=alex_gagniuc@dellteam.com \
    --cc=austin_bolen@dell.com \
    --cc=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.