[2/7] acpi, apei, ghes: Introduce more generic mechanism to init/deinit GHES error notifications.

Message ID 1397056476-9183-3-git-send-email-tomasz.nowicki@linaro.org
State New
Headers show

Commit Message

Tomasz Nowicki April 9, 2014, 3:14 p.m.
Init/deinit of GHES error notifications are moved to corresponding
functions e.g. for SCI ghes_notify_init_sci{sci} and ghes_notify_remove_{sci}
which in turn are gathered to ghes_notify_tab table.
ghes_probe and ghes_remove check notification support based on
function reference pointer in ghes_notify_tab and call it with ghes argument.

This approach allows us to change address of a given error notification
init/deinit function call in ghes_notify_tab in runtime. In turn,
we can avoid #ifdef usage in common code e.g. for NMI which is currently
supported by x86.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 drivers/acpi/apei/ghes.c |  242 ++++++++++++++++++++++++++++------------------
 1 file changed, 149 insertions(+), 93 deletions(-)

Comments

Borislav Petkov May 13, 2014, 6:13 p.m. | #1
On Wed, Apr 09, 2014 at 05:14:30PM +0200, Tomasz Nowicki wrote:
> Init/deinit of GHES error notifications are moved to corresponding
> functions e.g. for SCI ghes_notify_init_sci{sci} and ghes_notify_remove_{sci}
> which in turn are gathered to ghes_notify_tab table.
> ghes_probe and ghes_remove check notification support based on
> function reference pointer in ghes_notify_tab and call it with ghes argument.
> 
> This approach allows us to change address of a given error notification
> init/deinit function call in ghes_notify_tab in runtime. In turn,
> we can avoid #ifdef usage in common code e.g. for NMI which is currently
> supported by x86.
> 
> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> ---
>  drivers/acpi/apei/ghes.c |  242 ++++++++++++++++++++++++++++------------------
>  1 file changed, 149 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index f7edffc..ca8387e 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -148,6 +148,14 @@ static struct irq_work ghes_proc_irq_work;
>  struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
>  static atomic_t ghes_estatus_cache_alloced;
>  
> +struct ghes_notify_setup {
> +	const char *notif_name;
> +	int 	   (*init_call)(struct ghes *ghes);
> +	void 	   (*remove_call)(struct ghes *ghes);
> +};

Please run through checkpatch before submitting:

WARNING: please, no space before tabs
#44: FILE: drivers/acpi/apei/ghes.c:153:
+^Iint ^I   (*init_call)(struct ghes *ghes);$

WARNING: please, no space before tabs
#45: FILE: drivers/acpi/apei/ghes.c:154:
+^Ivoid ^I   (*remove_call)(struct ghes *ghes);$

> +
> +static struct ghes_notify_setup	ghes_notify_tab[];
> +
>  static int ghes_ioremap_init(void)
>  {
>  	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
> @@ -879,10 +887,6 @@ out:
>  	return ret;
>  }
>  
> -static struct notifier_block ghes_notifier_sci = {
> -	.notifier_call = ghes_notify_sci,
> -};
> -
>  static unsigned long ghes_esource_prealloc_size(
>  	const struct acpi_hest_generic *generic)
>  {
> @@ -898,33 +902,151 @@ static unsigned long ghes_esource_prealloc_size(
>  	return prealloc_size;
>  }
>  
> +static int ghes_notify_init_nmi(struct ghes *ghes)
> +{
> +	unsigned long len;
> +
> +	len = ghes_esource_prealloc_size(ghes->generic);
> +	ghes_estatus_pool_expand(len);
> +	mutex_lock(&ghes_list_mutex);
> +	if (list_empty(&ghes_nmi))
> +		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
> +	list_add_rcu(&ghes->list, &ghes_nmi);
> +	mutex_unlock(&ghes_list_mutex);
> +
> +	return 0;

Unconditionally returning 0 seems kinda moot. At least
register_nmi_handler() retval could be returned...

...

> +static struct ghes_notify_setup
> +       ghes_notify_tab[ACPI_HEST_NOTIFY_RESERVED] = {
> +               [ACPI_HEST_NOTIFY_POLLED]   = {"POLLED",
> +                                              ghes_notify_init_polled,
> +                                              ghes_notify_remove_polled},
> +               [ACPI_HEST_NOTIFY_EXTERNAL] = {"EXT_IRQ",
> +                                              ghes_notify_init_external,
> +                                              ghes_notify_remove_external},
> +               [ACPI_HEST_NOTIFY_LOCAL]    = {"LOCAL_IRQ", NULL, NULL},
> +               [ACPI_HEST_NOTIFY_SCI]      = {"SCI",
> +                                              ghes_notify_init_sci,
> +                                              ghes_notify_remove_sci},
> +               [ACPI_HEST_NOTIFY_NMI]      = {"NMI",
> +                                              ghes_notify_init_nmi,
> +                                              ghes_notify_remove_nmi},
> +               [ACPI_HEST_NOTIFY_CMCI]     = {"CMCI", NULL, NULL},
> +               [ACPI_HEST_NOTIFY_MCE]      = {"MCE", NULL, NULL},
> +};

I guess you don't need to save the ->notif_name strings but add
a function which maps ACPI_HEST_* to strings => smaller struct
ghes_notify_setup.


> @@ -944,48 +1066,10 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  	if (rc < 0)
>  		goto err;
>  
> -	switch (generic->notify.type) {
> -	case ACPI_HEST_NOTIFY_POLLED:
> -		ghes->timer.function = ghes_poll_func;
> -		ghes->timer.data = (unsigned long)ghes;
> -		init_timer_deferrable(&ghes->timer);
> -		ghes_add_timer(ghes);
> -		break;
> -	case ACPI_HEST_NOTIFY_EXTERNAL:
> -		/* External interrupt vector is GSI */
> -		rc = acpi_gsi_to_irq(generic->notify.vector, &ghes->irq);
> -		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;
> -		}
> -		rc = request_irq(ghes->irq, ghes_irq_func, 0, "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;
> -		}
> -		break;
> -	case ACPI_HEST_NOTIFY_SCI:
> -		mutex_lock(&ghes_list_mutex);
> -		if (list_empty(&ghes_sci))
> -			register_acpi_hed_notifier(&ghes_notifier_sci);
> -		list_add_rcu(&ghes->list, &ghes_sci);
> -		mutex_unlock(&ghes_list_mutex);
> -		break;
> -	case ACPI_HEST_NOTIFY_NMI:
> -		len = ghes_esource_prealloc_size(generic);
> -		ghes_estatus_pool_expand(len);
> -		mutex_lock(&ghes_list_mutex);
> -		if (list_empty(&ghes_nmi))
> -			register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0,
> -						"ghes");
> -		list_add_rcu(&ghes->list, &ghes_nmi);
> -		mutex_unlock(&ghes_list_mutex);
> -		break;
> -	default:
> -		BUG();

What happened to those BUG() catch-all stoppers?

> -	}
> +	rc = (*ghes_init_call)(ghes);
> +	if (rc)
> +		goto err_edac_unreg;
> +
>  	platform_set_drvdata(ghes_dev, ghes);
>  
>  	return 0;
> @@ -1003,44 +1087,16 @@ static int ghes_remove(struct platform_device *ghes_dev)
>  {
>  	struct ghes *ghes;
>  	struct acpi_hest_generic *generic;
> -	unsigned long len;
> +	void (*ghes_remove_call)(struct ghes *ghes);
>  
>  	ghes = platform_get_drvdata(ghes_dev);
> +	ghes->flags |= GHES_EXITING;
> +
>  	generic = ghes->generic;
> +	ghes_remove_call = ghes_notify_tab[generic->notify.type].remove_call;
>  
> -	ghes->flags |= GHES_EXITING;
> -	switch (generic->notify.type) {
> -	case ACPI_HEST_NOTIFY_POLLED:
> -		del_timer_sync(&ghes->timer);
> -		break;
> -	case ACPI_HEST_NOTIFY_EXTERNAL:
> -		free_irq(ghes->irq, ghes);
> -		break;
> -	case ACPI_HEST_NOTIFY_SCI:
> -		mutex_lock(&ghes_list_mutex);
> -		list_del_rcu(&ghes->list);
> -		if (list_empty(&ghes_sci))
> -			unregister_acpi_hed_notifier(&ghes_notifier_sci);
> -		mutex_unlock(&ghes_list_mutex);
> -		break;
> -	case ACPI_HEST_NOTIFY_NMI:
> -		mutex_lock(&ghes_list_mutex);
> -		list_del_rcu(&ghes->list);
> -		if (list_empty(&ghes_nmi))
> -			unregister_nmi_handler(NMI_LOCAL, "ghes");
> -		mutex_unlock(&ghes_list_mutex);
> -		/*
> -		 * To synchronize with NMI handler, ghes can only be
> -		 * freed after NMI handler finishes.
> -		 */
> -		synchronize_rcu();
> -		len = ghes_esource_prealloc_size(generic);
> -		ghes_estatus_pool_shrink(len);
> -		break;
> -	default:
> -		BUG();
> -		break;

This one too.
Tomasz Nowicki May 15, 2014, 2:31 p.m. | #2
On 13.05.2014 20:13, Borislav Petkov wrote:
> On Wed, Apr 09, 2014 at 05:14:30PM +0200, Tomasz Nowicki wrote:
>> Init/deinit of GHES error notifications are moved to corresponding
>> functions e.g. for SCI ghes_notify_init_sci{sci} and ghes_notify_remove_{sci}
>> which in turn are gathered to ghes_notify_tab table.
>> ghes_probe and ghes_remove check notification support based on
>> function reference pointer in ghes_notify_tab and call it with ghes argument.
>>
>> This approach allows us to change address of a given error notification
>> init/deinit function call in ghes_notify_tab in runtime. In turn,
>> we can avoid #ifdef usage in common code e.g. for NMI which is currently
>> supported by x86.
>>
>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>> ---
>>   drivers/acpi/apei/ghes.c |  242 ++++++++++++++++++++++++++++------------------
>>   1 file changed, 149 insertions(+), 93 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index f7edffc..ca8387e 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -148,6 +148,14 @@ static struct irq_work ghes_proc_irq_work;
>>   struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
>>   static atomic_t ghes_estatus_cache_alloced;
>>
>> +struct ghes_notify_setup {
>> +	const char *notif_name;
>> +	int 	   (*init_call)(struct ghes *ghes);
>> +	void 	   (*remove_call)(struct ghes *ghes);
>> +};
>
> Please run through checkpatch before submitting:
>
> WARNING: please, no space before tabs
> #44: FILE: drivers/acpi/apei/ghes.c:153:
> +^Iint ^I   (*init_call)(struct ghes *ghes);$
>
> WARNING: please, no space before tabs
> #45: FILE: drivers/acpi/apei/ghes.c:154:
> +^Ivoid ^I   (*remove_call)(struct ghes *ghes);$

My bad, will run it before next submission.

>
>> +
>> +static struct ghes_notify_setup	ghes_notify_tab[];
>> +
>>   static int ghes_ioremap_init(void)
>>   {
>>   	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
>> @@ -879,10 +887,6 @@ out:
>>   	return ret;
>>   }
>>
>> -static struct notifier_block ghes_notifier_sci = {
>> -	.notifier_call = ghes_notify_sci,
>> -};
>> -
>>   static unsigned long ghes_esource_prealloc_size(
>>   	const struct acpi_hest_generic *generic)
>>   {
>> @@ -898,33 +902,151 @@ static unsigned long ghes_esource_prealloc_size(
>>   	return prealloc_size;
>>   }
>>
>> +static int ghes_notify_init_nmi(struct ghes *ghes)
>> +{
>> +	unsigned long len;
>> +
>> +	len = ghes_esource_prealloc_size(ghes->generic);
>> +	ghes_estatus_pool_expand(len);
>> +	mutex_lock(&ghes_list_mutex);
>> +	if (list_empty(&ghes_nmi))
>> +		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
>> +	list_add_rcu(&ghes->list, &ghes_nmi);
>> +	mutex_unlock(&ghes_list_mutex);
>> +
>> +	return 0;
>
> Unconditionally returning 0 seems kinda moot. At least
> register_nmi_handler() retval could be returned...
>
> ...

Yes, it's worth returning register_nmi_handler() retval. Nice catch.

>
>> +static struct ghes_notify_setup
>> +       ghes_notify_tab[ACPI_HEST_NOTIFY_RESERVED] = {
>> +               [ACPI_HEST_NOTIFY_POLLED]   = {"POLLED",
>> +                                              ghes_notify_init_polled,
>> +                                              ghes_notify_remove_polled},
>> +               [ACPI_HEST_NOTIFY_EXTERNAL] = {"EXT_IRQ",
>> +                                              ghes_notify_init_external,
>> +                                              ghes_notify_remove_external},
>> +               [ACPI_HEST_NOTIFY_LOCAL]    = {"LOCAL_IRQ", NULL, NULL},
>> +               [ACPI_HEST_NOTIFY_SCI]      = {"SCI",
>> +                                              ghes_notify_init_sci,
>> +                                              ghes_notify_remove_sci},
>> +               [ACPI_HEST_NOTIFY_NMI]      = {"NMI",
>> +                                              ghes_notify_init_nmi,
>> +                                              ghes_notify_remove_nmi},
>> +               [ACPI_HEST_NOTIFY_CMCI]     = {"CMCI", NULL, NULL},
>> +               [ACPI_HEST_NOTIFY_MCE]      = {"MCE", NULL, NULL},
>> +};
>
> I guess you don't need to save the ->notif_name strings but add
> a function which maps ACPI_HEST_* to strings => smaller struct
> ghes_notify_setup.

Well, we need to keep these strings somewhere :) IMO, this is simpler 
solution. What would be advantage of additional map function?

>
>
>> @@ -944,48 +1066,10 @@ static int ghes_probe(struct platform_device *ghes_dev)
>>   	if (rc < 0)
>>   		goto err;
>>
>> -	switch (generic->notify.type) {
>> -	case ACPI_HEST_NOTIFY_POLLED:
>> -		ghes->timer.function = ghes_poll_func;
>> -		ghes->timer.data = (unsigned long)ghes;
>> -		init_timer_deferrable(&ghes->timer);
>> -		ghes_add_timer(ghes);
>> -		break;
>> -	case ACPI_HEST_NOTIFY_EXTERNAL:
>> -		/* External interrupt vector is GSI */
>> -		rc = acpi_gsi_to_irq(generic->notify.vector, &ghes->irq);
>> -		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;
>> -		}
>> -		rc = request_irq(ghes->irq, ghes_irq_func, 0, "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;
>> -		}
>> -		break;
>> -	case ACPI_HEST_NOTIFY_SCI:
>> -		mutex_lock(&ghes_list_mutex);
>> -		if (list_empty(&ghes_sci))
>> -			register_acpi_hed_notifier(&ghes_notifier_sci);
>> -		list_add_rcu(&ghes->list, &ghes_sci);
>> -		mutex_unlock(&ghes_list_mutex);
>> -		break;
>> -	case ACPI_HEST_NOTIFY_NMI:
>> -		len = ghes_esource_prealloc_size(generic);
>> -		ghes_estatus_pool_expand(len);
>> -		mutex_lock(&ghes_list_mutex);
>> -		if (list_empty(&ghes_nmi))
>> -			register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0,
>> -						"ghes");
>> -		list_add_rcu(&ghes->list, &ghes_nmi);
>> -		mutex_unlock(&ghes_list_mutex);
>> -		break;
>> -	default:
>> -		BUG();
>
> What happened to those BUG() catch-all stoppers?

I am validating generic->notify.type at the begin of function and return 
error in case of unknown value. I think there is no need to check that 
again.

>
>> -	}
>> +	rc = (*ghes_init_call)(ghes);
>> +	if (rc)
>> +		goto err_edac_unreg;
>> +
>>   	platform_set_drvdata(ghes_dev, ghes);
>>
>>   	return 0;
>> @@ -1003,44 +1087,16 @@ static int ghes_remove(struct platform_device *ghes_dev)
>>   {
>>   	struct ghes *ghes;
>>   	struct acpi_hest_generic *generic;
>> -	unsigned long len;
>> +	void (*ghes_remove_call)(struct ghes *ghes);
>>
>>   	ghes = platform_get_drvdata(ghes_dev);
>> +	ghes->flags |= GHES_EXITING;
>> +
>>   	generic = ghes->generic;
>> +	ghes_remove_call = ghes_notify_tab[generic->notify.type].remove_call;
>>
>> -	ghes->flags |= GHES_EXITING;
>> -	switch (generic->notify.type) {
>> -	case ACPI_HEST_NOTIFY_POLLED:
>> -		del_timer_sync(&ghes->timer);
>> -		break;
>> -	case ACPI_HEST_NOTIFY_EXTERNAL:
>> -		free_irq(ghes->irq, ghes);
>> -		break;
>> -	case ACPI_HEST_NOTIFY_SCI:
>> -		mutex_lock(&ghes_list_mutex);
>> -		list_del_rcu(&ghes->list);
>> -		if (list_empty(&ghes_sci))
>> -			unregister_acpi_hed_notifier(&ghes_notifier_sci);
>> -		mutex_unlock(&ghes_list_mutex);
>> -		break;
>> -	case ACPI_HEST_NOTIFY_NMI:
>> -		mutex_lock(&ghes_list_mutex);
>> -		list_del_rcu(&ghes->list);
>> -		if (list_empty(&ghes_nmi))
>> -			unregister_nmi_handler(NMI_LOCAL, "ghes");
>> -		mutex_unlock(&ghes_list_mutex);
>> -		/*
>> -		 * To synchronize with NMI handler, ghes can only be
>> -		 * freed after NMI handler finishes.
>> -		 */
>> -		synchronize_rcu();
>> -		len = ghes_esource_prealloc_size(generic);
>> -		ghes_estatus_pool_shrink(len);
>> -		break;
>> -	default:
>> -		BUG();
>> -		break;
>
> This one too.
>
For this one I should provide generic->notify.type validation. Will 
rethink and fix it.

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Borislav Petkov May 21, 2014, 6:11 p.m. | #3
On Thu, May 15, 2014 at 04:31:20PM +0200, Tomasz Nowicki wrote:
> Well, we need to keep these strings somewhere :) IMO, this is simpler
> solution. What would be advantage of additional map function?

Nevermind, I got confused here.

> I am validating generic->notify.type at the begin of function and
> return error in case of unknown value. I think there is no need to
> check that again.

Yes, you do :)

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index f7edffc..ca8387e 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -148,6 +148,14 @@  static struct irq_work ghes_proc_irq_work;
 struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
 static atomic_t ghes_estatus_cache_alloced;
 
+struct ghes_notify_setup {
+	const char *notif_name;
+	int 	   (*init_call)(struct ghes *ghes);
+	void 	   (*remove_call)(struct ghes *ghes);
+};
+
+static struct ghes_notify_setup	ghes_notify_tab[];
+
 static int ghes_ioremap_init(void)
 {
 	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
@@ -879,10 +887,6 @@  out:
 	return ret;
 }
 
-static struct notifier_block ghes_notifier_sci = {
-	.notifier_call = ghes_notify_sci,
-};
-
 static unsigned long ghes_esource_prealloc_size(
 	const struct acpi_hest_generic *generic)
 {
@@ -898,33 +902,151 @@  static unsigned long ghes_esource_prealloc_size(
 	return prealloc_size;
 }
 
+static int ghes_notify_init_nmi(struct ghes *ghes)
+{
+	unsigned long len;
+
+	len = ghes_esource_prealloc_size(ghes->generic);
+	ghes_estatus_pool_expand(len);
+	mutex_lock(&ghes_list_mutex);
+	if (list_empty(&ghes_nmi))
+		register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
+	list_add_rcu(&ghes->list, &ghes_nmi);
+	mutex_unlock(&ghes_list_mutex);
+
+	return 0;
+}
+
+static void ghes_notify_remove_nmi(struct ghes *ghes)
+{
+	unsigned long len;
+
+	mutex_lock(&ghes_list_mutex);
+	list_del_rcu(&ghes->list);
+	if (list_empty(&ghes_nmi))
+		unregister_nmi_handler(NMI_LOCAL, "ghes");
+	mutex_unlock(&ghes_list_mutex);
+	/*
+	 * To synchronize with NMI handler, ghes can only be
+	 * freed after NMI handler finishes.
+	 */
+	synchronize_rcu();
+	len = ghes_esource_prealloc_size(ghes->generic);
+	ghes_estatus_pool_shrink(len);
+}
+
+static int ghes_notify_init_polled(struct ghes *ghes)
+{
+	ghes->timer.function = ghes_poll_func;
+	ghes->timer.data = (unsigned long)ghes;
+	init_timer_deferrable(&ghes->timer);
+	ghes_add_timer(ghes);
+
+	return 0;
+}
+
+static void ghes_notify_remove_polled(struct ghes *ghes)
+{
+	del_timer_sync(&ghes->timer);
+}
+
+static int ghes_notify_init_external(struct ghes *ghes)
+{
+	int rc;
+
+	/* External interrupt vector is GSI */
+	rc = acpi_gsi_to_irq(ghes->generic->notify.vector, &ghes->irq);
+	if (rc) {
+		pr_err(GHES_PFX "Failed to map GSI to IRQ for generic hardware "
+		       "error source: %d\n", ghes->generic->header.source_id);
+		goto out;
+	}
+
+	rc = request_irq(ghes->irq, ghes_irq_func, 0, "GHES IRQ", ghes);
+	if (rc)
+		pr_err(GHES_PFX "Failed to register IRQ for generic hardware "
+		       "error source: %d\n", ghes->generic->header.source_id);
+
+out:
+	return rc;
+}
+
+static void ghes_notify_remove_external(struct ghes *ghes)
+{
+	free_irq(ghes->irq, ghes);
+}
+
+static struct notifier_block ghes_notifier_sci = {
+	.notifier_call = ghes_notify_sci,
+};
+
+static int ghes_notify_init_sci(struct ghes *ghes)
+{
+	mutex_lock(&ghes_list_mutex);
+	if (list_empty(&ghes_sci))
+		register_acpi_hed_notifier(&ghes_notifier_sci);
+	list_add_rcu(&ghes->list, &ghes_sci);
+	mutex_unlock(&ghes_list_mutex);
+
+	return 0;
+}
+
+static void ghes_notify_remove_sci(struct ghes *ghes)
+{
+	mutex_lock(&ghes_list_mutex);
+	list_del_rcu(&ghes->list);
+	if (list_empty(&ghes_sci))
+		unregister_acpi_hed_notifier(&ghes_notifier_sci);
+	mutex_unlock(&ghes_list_mutex);
+}
+
+static struct ghes_notify_setup
+	ghes_notify_tab[ACPI_HEST_NOTIFY_RESERVED] = {
+		[ACPI_HEST_NOTIFY_POLLED]   = {"POLLED",
+					       ghes_notify_init_polled,
+					       ghes_notify_remove_polled},
+		[ACPI_HEST_NOTIFY_EXTERNAL] = {"EXT_IRQ",
+					       ghes_notify_init_external,
+					       ghes_notify_remove_external},
+		[ACPI_HEST_NOTIFY_LOCAL]    = {"LOCAL_IRQ", NULL, NULL},
+		[ACPI_HEST_NOTIFY_SCI]      = {"SCI",
+					       ghes_notify_init_sci,
+					       ghes_notify_remove_sci},
+		[ACPI_HEST_NOTIFY_NMI]      = {"NMI",
+					       ghes_notify_init_nmi,
+					       ghes_notify_remove_nmi},
+		[ACPI_HEST_NOTIFY_CMCI]     = {"CMCI", NULL, NULL},
+		[ACPI_HEST_NOTIFY_MCE]      = {"MCE", NULL, NULL},
+};
+
 static int ghes_probe(struct platform_device *ghes_dev)
 {
 	struct acpi_hest_generic *generic;
 	struct ghes *ghes = NULL;
-	unsigned long len;
 	int rc = -EINVAL;
+	int (*ghes_init_call)(struct ghes *ghes);
+	const char *ghes_notif_name;
 
 	generic = *(struct acpi_hest_generic **)ghes_dev->dev.platform_data;
 	if (!generic->enabled)
 		return -ENODEV;
 
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-	case ACPI_HEST_NOTIFY_SCI:
-	case ACPI_HEST_NOTIFY_NMI:
-		break;
-	case ACPI_HEST_NOTIFY_LOCAL:
-		pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
-			   generic->header.source_id);
-		goto err;
-	default:
-		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
+	if (generic->notify.type >= ACPI_HEST_NOTIFY_RESERVED) {
+		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for "
+			   "generic hardware error source: %d\n",
 			   generic->notify.type, generic->header.source_id);
 		goto err;
 	}
 
+	ghes_init_call = ghes_notify_tab[generic->notify.type].init_call;
+	ghes_notif_name = ghes_notify_tab[generic->notify.type].notif_name;
+	if (!ghes_init_call) {
+		pr_warning(GHES_PFX "Generic hardware error source: %d notified"
+			   " via %s is not supported!\n",
+			   generic->header.source_id, ghes_notif_name);
+		goto err;
+	}
+
 	rc = -EIO;
 	if (generic->error_block_length <
 	    sizeof(struct acpi_generic_status)) {
@@ -944,48 +1066,10 @@  static int ghes_probe(struct platform_device *ghes_dev)
 	if (rc < 0)
 		goto err;
 
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-		ghes->timer.function = ghes_poll_func;
-		ghes->timer.data = (unsigned long)ghes;
-		init_timer_deferrable(&ghes->timer);
-		ghes_add_timer(ghes);
-		break;
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-		/* External interrupt vector is GSI */
-		rc = acpi_gsi_to_irq(generic->notify.vector, &ghes->irq);
-		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;
-		}
-		rc = request_irq(ghes->irq, ghes_irq_func, 0, "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;
-		}
-		break;
-	case ACPI_HEST_NOTIFY_SCI:
-		mutex_lock(&ghes_list_mutex);
-		if (list_empty(&ghes_sci))
-			register_acpi_hed_notifier(&ghes_notifier_sci);
-		list_add_rcu(&ghes->list, &ghes_sci);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	case ACPI_HEST_NOTIFY_NMI:
-		len = ghes_esource_prealloc_size(generic);
-		ghes_estatus_pool_expand(len);
-		mutex_lock(&ghes_list_mutex);
-		if (list_empty(&ghes_nmi))
-			register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0,
-						"ghes");
-		list_add_rcu(&ghes->list, &ghes_nmi);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	default:
-		BUG();
-	}
+	rc = (*ghes_init_call)(ghes);
+	if (rc)
+		goto err_edac_unreg;
+
 	platform_set_drvdata(ghes_dev, ghes);
 
 	return 0;
@@ -1003,44 +1087,16 @@  static int ghes_remove(struct platform_device *ghes_dev)
 {
 	struct ghes *ghes;
 	struct acpi_hest_generic *generic;
-	unsigned long len;
+	void (*ghes_remove_call)(struct ghes *ghes);
 
 	ghes = platform_get_drvdata(ghes_dev);
+	ghes->flags |= GHES_EXITING;
+
 	generic = ghes->generic;
+	ghes_remove_call = ghes_notify_tab[generic->notify.type].remove_call;
 
-	ghes->flags |= GHES_EXITING;
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-		del_timer_sync(&ghes->timer);
-		break;
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-		free_irq(ghes->irq, ghes);
-		break;
-	case ACPI_HEST_NOTIFY_SCI:
-		mutex_lock(&ghes_list_mutex);
-		list_del_rcu(&ghes->list);
-		if (list_empty(&ghes_sci))
-			unregister_acpi_hed_notifier(&ghes_notifier_sci);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	case ACPI_HEST_NOTIFY_NMI:
-		mutex_lock(&ghes_list_mutex);
-		list_del_rcu(&ghes->list);
-		if (list_empty(&ghes_nmi))
-			unregister_nmi_handler(NMI_LOCAL, "ghes");
-		mutex_unlock(&ghes_list_mutex);
-		/*
-		 * To synchronize with NMI handler, ghes can only be
-		 * freed after NMI handler finishes.
-		 */
-		synchronize_rcu();
-		len = ghes_esource_prealloc_size(generic);
-		ghes_estatus_pool_shrink(len);
-		break;
-	default:
-		BUG();
-		break;
-	}
+	if (ghes_remove_call)
+		(*ghes_remove_call)(ghes);
 
 	ghes_fini(ghes);