| Message ID | 1508227341-15651-1-git-send-email-gengdongjiu@huawei.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On 10/17/2017 4:02 AM, Dongjiu Geng wrote: > For the SEA notification, the two functions ghes_sea_add() and > ghes_sea_remove() are only called when CONFIG_ACPI_APEI_SEA > is defined. If not, it will return errors in the ghes_probe() > and not continue. If the probe is failed, the ghes_sea_remove() > also has no chance to be called. Hence, remove the unnecessary > handling when CONFIG_ACPI_APEI_SEA is not defined. > > For the NMI notification, it has the same issue as SEA notification, > so also remove the unused dead-code for it. > > Cc: Tyler Baicar <tbaicar@codeaurora.org> > Cc: James Morse <james.morse@arm.com> > Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> Tested-by: Tyler Baicar <tbaicar@codeaurora.org> I've verified v5 of this series testing the SEA path. Thanks, Tyler -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 17, 2017 at 04:02:20PM +0800, Dongjiu Geng wrote: > @@ -1061,23 +1052,9 @@ static void ghes_nmi_init_cxt(void) > init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq); > } > #else /* CONFIG_HAVE_ACPI_APEI_NMI */ > -static inline void ghes_nmi_add(struct ghes *ghes) > -{ > - pr_err(GHES_PFX "ID: %d, trying to add NMI notification which is not supported!\n", > - ghes->generic->header.source_id); > - BUG(); > -} > - > -static inline void ghes_nmi_remove(struct ghes *ghes) > -{ > - pr_err(GHES_PFX "ID: %d, trying to remove NMI notification which is not supported!\n", > - ghes->generic->header.source_id); > - BUG(); > -} > - So GHES NMI notification method is x86-only AFAIK and HAVE_ACPI_APEI_NMI is selected only on x86. Why are you removing those guards? Does ARM have ACPI_HEST_NOTIFY_NMI notification type now too?
Hi,Borislav On 2017/10/18 0:43, Borislav Petkov wrote: >> -} >> - > So GHES NMI notification method is x86-only AFAIK and HAVE_ACPI_APEI_NMI > is selected only on x86. Why are you removing those guards? Does ARM > have ACPI_HEST_NOTIFY_NMI notification type now too? ARM does not have ACPI_HEST_NOTIFY_NMI notification, which should only used by x86. In the code, I see those guards are never used. As you see, if the 'CONFIG_HAVE_ACPI_APEI_NMI' does not defined in [1], it will print error info and goto [2], in the [2], it will return error, then the probe for GHES NMI is failed. so those guards( ghes_nmi_add() and ghes_nmi_remove()) have no chance to execute. so I redefine them to NULL for compiling[3]. static int ghes_probe(struct platform_device *ghes_dev) { struct acpi_hest_generic *generic; struct ghes *ghes = NULL; int rc = -EINVAL; switch (generic->notify.type) { ................... case ACPI_HEST_NOTIFY_NMI: [1] if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) { pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n", generic->header.source_id); goto err; } .............. } switch (generic->notify.type) { ............... case ACPI_HEST_NOTIFY_NMI: ghes_nmi_add(ghes); break; } .......... err: [2] if (ghes) { ghes_fini(ghes); kfree(ghes); } return rc; } [3]: -static inline void ghes_nmi_add(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to add NMI notification which is not supported!\n", - ghes->generic->header.source_id); - BUG(); -} - -static inline void ghes_nmi_remove(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to remove NMI notification which is not supported!\n", - ghes->generic->header.source_id); - BUG(); -} - -static inline void ghes_nmi_init_cxt(void) -{ -} +static inline void ghes_nmi_add(struct ghes *ghes) { } +static inline void ghes_nmi_remove(struct ghes *ghes) { } +static inline void ghes_nmi_init_cxt(void) { } -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 18, 2017 at 11:04:28AM +0800, gengdongjiu wrote: > ARM does not have ACPI_HEST_NOTIFY_NMI notification, which should only > used by x86. In the code, I see those guards are never used. Yeah, I see it now. Thx.
On 2017/10/18 18:17, Borislav Petkov wrote: > On Wed, Oct 18, 2017 at 11:04:28AM +0800, gengdongjiu wrote: >> ARM does not have ACPI_HEST_NOTIFY_NMI notification, which should only >> used by x86. In the code, I see those guards are never used. > Yeah, I see it now. Borislav/Rafael, For this patch(the first one), whether it can be firstly applied? This patch is code clean and not related with the second one. For the second, I may discuss more with James. Thanks so much in advance. > > Thx. > > -- -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 18, 2017 at 08:27:00PM +0800, gengdongjiu wrote: > For this patch(the first one), whether it can be firstly applied? Sure: Reviewed-by: Borislav Petkov <bp@suse.de>
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index d661d45..3eee30a 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -849,17 +849,8 @@ static void ghes_sea_remove(struct ghes *ghes) synchronize_rcu(); } #else /* CONFIG_ACPI_APEI_SEA */ -static inline void ghes_sea_add(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n", - ghes->generic->header.source_id); -} - -static inline void ghes_sea_remove(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n", - ghes->generic->header.source_id); -} +static inline void ghes_sea_add(struct ghes *ghes) { } +static inline void ghes_sea_remove(struct ghes *ghes) { } #endif /* CONFIG_ACPI_APEI_SEA */ #ifdef CONFIG_HAVE_ACPI_APEI_NMI @@ -1061,23 +1052,9 @@ static void ghes_nmi_init_cxt(void) init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq); } #else /* CONFIG_HAVE_ACPI_APEI_NMI */ -static inline void ghes_nmi_add(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to add NMI notification which is not supported!\n", - ghes->generic->header.source_id); - BUG(); -} - -static inline void ghes_nmi_remove(struct ghes *ghes) -{ - pr_err(GHES_PFX "ID: %d, trying to remove NMI notification which is not supported!\n", - ghes->generic->header.source_id); - BUG(); -} - -static inline void ghes_nmi_init_cxt(void) -{ -} +static inline void ghes_nmi_add(struct ghes *ghes) { } +static inline void ghes_nmi_remove(struct ghes *ghes) { } +static inline void ghes_nmi_init_cxt(void) { } #endif /* CONFIG_HAVE_ACPI_APEI_NMI */ static int ghes_probe(struct platform_device *ghes_dev)
For the SEA notification, the two functions ghes_sea_add() and ghes_sea_remove() are only called when CONFIG_ACPI_APEI_SEA is defined. If not, it will return errors in the ghes_probe() and not continue. If the probe is failed, the ghes_sea_remove() also has no chance to be called. Hence, remove the unnecessary handling when CONFIG_ACPI_APEI_SEA is not defined. For the NMI notification, it has the same issue as SEA notification, so also remove the unused dead-code for it. Cc: Tyler Baicar <tbaicar@codeaurora.org> Cc: James Morse <james.morse@arm.com> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com> --- drivers/acpi/apei/ghes.c | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-)