Message ID | f520f2529bb27d452a2dee762b6968939df42f45.1720436039.git.mchehab+huawei@kernel.org |
---|---|
State | New |
Headers | show |
Series | Fix issues with ARM Processor CPER records | expand |
Em Mon, 8 Jul 2024 13:32:34 +0200 Borislav Petkov <bp@alien8.de> escreveu: > On Mon, Jul 08, 2024 at 01:18:10PM +0200, Mauro Carvalho Chehab wrote: > > From: Daniel Ferguson <danielf@os.amperecomputing.com> > > > > This prevents the unnecessary inclusion of ARM specific RAS error > > s/This prevents/Prevent/ > > Avoid having "This patch" or "This commit" or "This does <bla>" in the commit > message. It is tautologically useless. > > "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" > instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy > to do frotz", as if you are giving orders to the codebase to change > its behaviour." > > From Documentation/process/submitting-patches.rst > > > handling routines in non-ARM platforms. > > Ok, this does "something". Why does it do it? > > Otherwise it won't build on other architectures or is it going to cause code > bloat or why are we doing this? Probably a better description would be: RAS: ACPI: APEI: add conditional compilation to ARM error report functions Don't include ARM Processor specific error handling routines in non-ARM platforms, preparing it to the next patch, as arm-specific kAPI symbols will be used, thus avoiding build breakages when ARM is not selected. [mchehab: avoid unneeded ifdefs and fix coding style issues] Signed-off-by: Daniel Ferguson <danielf@os.amperecomputing.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> This patch itself just add conditionals to optimize out code on non-ARM architectures. The next one will add some ARM-specific bits inside ARM processor CPER trace, thus causing compilation breakages on non-ARM, due to arm-specific kAPI bits that will be used then. Thanks, Mauro
On Mon, Jul 08, 2024 at 02:10:25PM +0200, Mauro Carvalho Chehab wrote: > This patch itself just add conditionals to optimize out code on > non-ARM architectures. The next one will add some ARM-specific bits > inside ARM processor CPER trace, thus causing compilation breakages > on non-ARM, due to arm-specific kAPI bits that will be used then. Are you sure? I have both patches applied and then practically reverting the second one builds an allmodconfig just fine. diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 90efca025d27..524fea3f4f76 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -532,7 +532,6 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata, int sev, bool sync) { bool queued = false; -#if defined(CONFIG_ARM) || defined (CONFIG_ARM64) struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata); int flags = sync ? MF_ACTION_REQUIRED : 0; int sec_sev, i; @@ -570,7 +569,6 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata, error_type); p += err_info->length; } -#endif return queued; } diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c index 75acc09bc96a..359bb163aee0 100644 --- a/drivers/ras/ras.c +++ b/drivers/ras/ras.c @@ -54,7 +54,6 @@ void log_non_standard_event(const guid_t *sec_type, const guid_t *fru_id, void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev) { -#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) struct cper_arm_err_info *err_info; struct cper_arm_ctx_info *ctx_info; u8 *ven_err_data; @@ -97,7 +96,6 @@ void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev) trace_arm_event(err, pei_err, pei_len, ctx_err, ctx_len, ven_err_data, (u32)vsei_len, sev, cpu); -#endif } static int __init ras_init(void)
Em Mon, 8 Jul 2024 16:43:12 +0200 Borislav Petkov <bp@alien8.de> escreveu: > On Mon, Jul 08, 2024 at 02:10:25PM +0200, Mauro Carvalho Chehab wrote: > > This patch itself just add conditionals to optimize out code on > > non-ARM architectures. The next one will add some ARM-specific bits > > inside ARM processor CPER trace, thus causing compilation breakages > > on non-ARM, due to arm-specific kAPI bits that will be used then. > > Are you sure? That is what reviews to past attempts to merge patch 2 implied. > I have both patches applied and then practically reverting the second one > builds an allmodconfig just fine. I double-checked the logic: I noticed just one kABI symbol that it is arm-specific (CPU midr), and there is has already a wrapper for it. I also did a cross-compilation for both x86_64 and s390 to verify, and indeed it is building fine without the ifdefs. So, I'll drop patch 1. Thanks, Mauro
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 623cc0cb4a65..2589a3536d91 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -529,11 +529,12 @@ static bool ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, } static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata, - int sev, bool sync) + int sev, bool sync) { + bool queued = false; +#if defined(CONFIG_ARM) || defined (CONFIG_ARM64) struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata); int flags = sync ? MF_ACTION_REQUIRED : 0; - bool queued = false; int sec_sev, i; char *p; @@ -570,7 +571,7 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata, error_type); p += err_info->length; } - +#endif return queued; } @@ -773,11 +774,9 @@ static bool ghes_do_proc(struct ghes *ghes, arch_apei_report_mem_error(sev, mem_err); queued = ghes_handle_memory_failure(gdata, sev, sync); - } - else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { + } else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { ghes_handle_aer(gdata); - } - else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) { + } else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) { queued = ghes_handle_arm_hw_error(gdata, sev, sync); } else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) { struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata); diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c index a6e4792a1b2e..5d94ab79c8c3 100644 --- a/drivers/ras/ras.c +++ b/drivers/ras/ras.c @@ -54,7 +54,9 @@ void log_non_standard_event(const guid_t *sec_type, const guid_t *fru_id, void log_arm_hw_error(struct cper_sec_proc_arm *err) { +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) trace_arm_event(err); +#endif } static int __init ras_init(void)