linux-edac.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis
@ 2019-11-12 12:27 Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 1/9] rasdaemon: fix the wrong declaring of 'sruct ras_events' in ras-record.h Xiaofei Tan
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

Fix some issues reported by static code analysis, and sub module name issue
in the last patch 3/9 and 4/9.

Xiaofei Tan (9):
  rasdaemon: fix the wrong declaring of 'sruct ras_events' in
    ras-record.h
  rasdaemon: fix an warning reported by PC-Lint
  rasdaemon: decode submodule of OEM type1 for hip08
  rasdaemon: fix sub module name of HHA and DDRC for hip08
  rasdaemon: split OEM type1 table decode function to reduce length
  rasdaemon: split OEM type2 table decode function to reduce length
  rasdaemon: split PCIe local table decode function to reduce length
  rasdaemon: fix magic number issues reported by static code analysis
    for hip08
  rasdaemon: replace sprintf with snprintf for hip08

 non-standard-hisi_hip08.c | 686 +++++++++++++++++++++++++++++++---------------
 ras-record.h              |   2 +-
 2 files changed, 462 insertions(+), 226 deletions(-)

-- 
2.8.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/9] rasdaemon: fix the wrong declaring of 'sruct ras_events' in ras-record.h
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 2/9] rasdaemon: fix an warning reported by PC-Lint Xiaofei Tan
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

The following warning can be found by PC-Lint when do static code
analysis to the file non-standard-hisi_hip08.c.
Warning -- Declaration of symbol 'ras' hides symbol 'ras' (line 28, file ras-record.h)

This means that the local variable name 'ras' is same as an global
variable. In fact, there is no global variable named 'ras', but an
wrong declaring in ras-record.h.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 ras-record.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ras-record.h b/ras-record.h
index 5311c67..da72557 100644
--- a/ras-record.h
+++ b/ras-record.h
@@ -25,7 +25,7 @@
 
 extern long user_hz;
 
-struct ras_events *ras;
+struct ras_events;
 
 struct ras_mc_event {
 	char timestamp[64];
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/9] rasdaemon: fix an warning reported by PC-Lint
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 1/9] rasdaemon: fix the wrong declaring of 'sruct ras_events' in ras-record.h Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08 Xiaofei Tan
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

This patch fixes the following warning, and no function change.
Warning -- Storage class specified after a type

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index 1774ec7..5e4b6a0 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -811,8 +811,7 @@ struct ras_ns_dec_tab hip08_ns_oem_tab[] = {
 	{ /* sentinel */ }
 };
 
-__attribute__((constructor))
-static void hip08_init(void)
+static void __attribute__((constructor)) hip08_init(void)
 {
 	register_ns_dec_tab(hip08_ns_oem_tab);
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 1/9] rasdaemon: fix the wrong declaring of 'sruct ras_events' in ras-record.h Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 2/9] rasdaemon: fix an warning reported by PC-Lint Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-26  6:07   ` Mauro Carvalho Chehab
  2019-11-12 12:27 ` [PATCH 4/9] rasdaemon: fix sub module name of HHA and DDRC " Xiaofei Tan
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

Decode submodule of OEM type1 for hip08, and reconstruct the functions
of geting OEM module name and submodule name.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 286 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 221 insertions(+), 65 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index 5e4b6a0..afdd649 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -182,6 +182,13 @@ enum {
 	hip08_pcie_local_field_regs_dump,
 };
 
+struct hisi_module_info {
+	int id;
+	const char *name;
+	const char **sub;
+	int sub_num;
+};
+
 /* helper functions */
 static char *err_severity(uint8_t err_sev)
 {
@@ -194,37 +201,137 @@ static char *err_severity(uint8_t err_sev)
 	return "unknown";
 }
 
-static char *oem_type1_module_name(uint8_t module_id)
-{
-	switch (module_id) {
-	case HISI_OEM_MODULE_ID_MN: return "MN";
-	case HISI_OEM_MODULE_ID_PLL: return "PLL";
-	case HISI_OEM_MODULE_ID_SLLC: return "SLLC";
-	case HISI_OEM_MODULE_ID_AA: return "AA";
-	case HISI_OEM_MODULE_ID_SIOE: return "SIOE";
-	case HISI_OEM_MODULE_ID_POE: return "POE";
-	case HISI_OEM_MODULE_ID_DISP: return "DISP";
-	case HISI_OEM_MODULE_ID_LPC: return "LPC";
-	case HISI_OEM_MODULE_ID_GIC: return "GIC";
-	case HISI_OEM_MODULE_ID_RDE: return "RDE";
-	case HISI_OEM_MODULE_ID_SAS: return "SAS";
-	case HISI_OEM_MODULE_ID_SATA: return "SATA";
-	case HISI_OEM_MODULE_ID_USB: return "USB";
-	}
-	return "unknown";
-}
+static const char *pll_submodule_name[] = {
+	"TB_PLL0",
+	"TB_PLL1",
+	"TB_PLL2",
+	"TB_PLL3",
+	"TA_PLL0",
+	"TA_PLL1",
+	"TA_PLL2",
+	"TA_PLL3",
+	"NIMBUS_PLL0",
+	"NIMBUS_PLL1",
+	"NIMBUS_PLL2",
+	"NIMBUS_PLL3",
+	"NIMBUS_PLL4",
+};
 
-static char *oem_type2_module_name(uint8_t module_id)
-{
-	switch (module_id) {
-	case HISI_OEM_MODULE_ID_SMMU: return "SMMU";
-	case HISI_OEM_MODULE_ID_HHA: return "HHA";
-	case HISI_OEM_MODULE_ID_HLLC: return "HLLC";
-	case HISI_OEM_MODULE_ID_PA: return "PA";
-	case HISI_OEM_MODULE_ID_DDRC: return "DDRC";
-	}
-	return "unknown module";
-}
+static const char *sllc_submodule_name[] = {
+	"TB_SLLC0",
+	"TB_SLLC1",
+	"TB_SLLC2",
+	"TA_SLLC0",
+	"TA_SLLC1",
+	"TA_SLLC2",
+	"NIMBUS_SLLC0",
+	"NIMBUS_SLLC1",
+};
+
+static const char *sioe_submodule_name[] = {
+	"TB_SIOE0",
+	"TB_SIOE1",
+	"TB_SIOE2",
+	"TB_SIOE3",
+	"TA_SIOE0",
+	"TA_SIOE1",
+	"TA_SIOE2",
+	"TA_SIOE3",
+	"NIMBUS_SIOE0",
+	"NIMBUS_SIOE1",
+};
+
+static const char *poe_submodule_name[] = {
+	"TB_POE",
+	"TA_POE",
+};
+
+static const char *disp_submodule_name[] = {
+	"TB_PERI_DISP",
+	"TB_POE_DISP",
+	"TB_GIC_DISP",
+	"TA_PERI_DISP",
+	"TA_POE_DISP",
+	"TA_GIC_DISP",
+	"HAC_DISP",
+	"PCIE_DISP",
+	"IO_MGMT_DISP",
+	"NETWORK_DISP",
+};
+
+static const char *sas_submodule_name[] = {
+	"SAS0",
+	"SAS1",
+};
+
+static const struct hisi_module_info hisi_oem_type1_module[] = {
+	{
+		.id = HISI_OEM_MODULE_ID_PLL,
+		.name = "PLL",
+		.sub = pll_submodule_name,
+		.sub_num = ARRAY_SIZE(pll_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_SAS,
+		.name = "SAS",
+		.sub = sas_submodule_name,
+		.sub_num = ARRAY_SIZE(sas_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_POE,
+		.name = "POE",
+		.sub = poe_submodule_name,
+		.sub_num = ARRAY_SIZE(poe_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_SLLC,
+		.name = "SLLC",
+		.sub = sllc_submodule_name,
+		.sub_num = ARRAY_SIZE(sllc_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_SIOE,
+		.name = "SIOE",
+		.sub = sioe_submodule_name,
+		.sub_num = ARRAY_SIZE(sioe_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_DISP,
+		.name = "DISP",
+		.sub = disp_submodule_name,
+		.sub_num = ARRAY_SIZE(disp_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_MN,
+		.name = "MN",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_AA,
+		.name = "AA",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_LPC,
+		.name = "LPC",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_GIC,
+		.name = "GIC",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_RDE,
+		.name = "RDE",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_SATA,
+		.name = "SATA",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_USB,
+		.name = "USB",
+	},
+	{
+	}
+};
 
 static const char *smmu_submodule_name[] = {
 	"HAC_SMMU",
@@ -257,27 +364,72 @@ static const char *ddrc_submodule_name[] = {
 	"TB_DDRC3",
 };
 
-static const char *oem_type2_sub_module_name(uint8_t module_id, uint8_t sub_module_id)
+static const struct hisi_module_info hisi_oem_type2_module[] = {
+	{
+		.id = HISI_OEM_MODULE_ID_SMMU,
+		.name = "SMMU",
+		.sub = smmu_submodule_name,
+		.sub_num = ARRAY_SIZE(smmu_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_HHA,
+		.name = "HHA",
+		.sub = hha_submodule_name,
+		.sub_num = ARRAY_SIZE(hha_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_PA,
+		.name = "PA",
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_HLLC,
+		.name = "HLLC",
+		.sub = hllc_submodule_name,
+		.sub_num = ARRAY_SIZE(hllc_submodule_name),
+	},
+	{
+		.id = HISI_OEM_MODULE_ID_DDRC,
+		.name = "DDRC",
+		.sub = ddrc_submodule_name,
+		.sub_num = ARRAY_SIZE(ddrc_submodule_name),
+	},
+	{
+	}
+};
+
+static const char *oem_module_name(const struct hisi_module_info *info,
+				   uint8_t module_id)
 {
-	switch (module_id) {
-	case HISI_OEM_MODULE_ID_SMMU:
-		if (sub_module_id < sizeof(smmu_submodule_name)/sizeof(char *))
-			return smmu_submodule_name[sub_module_id];
-		break;
-	case HISI_OEM_MODULE_ID_HLLC:
-		if (sub_module_id < sizeof(hllc_submodule_name)/sizeof(char *))
-			return hllc_submodule_name[sub_module_id];
-		break;
-	case HISI_OEM_MODULE_ID_PA:
-		return "PA";
-	case HISI_OEM_MODULE_ID_HHA:
-		if (sub_module_id < sizeof(hha_submodule_name)/sizeof(char *))
-			return hha_submodule_name[sub_module_id];
-		break;
-	case HISI_OEM_MODULE_ID_DDRC:
-		if (sub_module_id < sizeof(ddrc_submodule_name)/sizeof(char *))
-			return ddrc_submodule_name[sub_module_id];
-		break;
+	const struct hisi_module_info *module = &info[0];
+
+	for (; module->name; module++) {
+		if (module->id != module_id)
+			continue;
+
+		return module->name;
+	}
+
+	return "unknown";
+}
+
+static const char *oem_submodule_name(const struct hisi_module_info *info,
+				      uint8_t module_id, uint8_t sub_module_id)
+{
+	const struct hisi_module_info *module = &info[0];
+
+	for (; module->name; module++) {
+		const char **submodule = module->sub;
+
+		if (module->id != module_id)
+			continue;
+
+		if (module->sub == NULL)
+			return module->name;
+
+		if (sub_module_id >= module->sub_num)
+			return "unknown";
+
+		return submodule[sub_module_id];
 	}
 
 	return "unknown";
@@ -467,23 +619,24 @@ static int decode_hip08_oem_type1_error(struct ras_events *ras,
 	}
 
 	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
-		p += sprintf(p, "module=%s ",
-			     oem_type1_module_name(err->module_id));
+		const char *str = oem_module_name(hisi_oem_type1_module,
+						  err->module_id);
+
+		p += snprintf(p, end - p, "module=%s ", str);
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type1_field_module_id,
-				   0, oem_type1_module_name(err->module_id));
+				   0, str);
 	}
 
 	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
-		char submodule_name[64];
+		const char *str = oem_submodule_name(hisi_oem_type1_module,
+						     err->module_id,
+						     err->sub_module_id);
 
-		sprintf(submodule_name, "%s%d",
-			oem_type1_module_name(err->module_id),
-			err->sub_module_id);
-		p += sprintf(p, "submodule=%s ", submodule_name);
+		p += snprintf(p, end - p, "submodule=%s ", str);
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type1_field_sub_module_id,
-				   0, submodule_name);
+				   0, str);
 	}
 
 	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY) {
@@ -596,18 +749,21 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
 	}
 
 	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
-		p += sprintf(p, "module=%s ",
-			     oem_type2_module_name(err->module_id));
+		const char *str = oem_module_name(hisi_oem_type2_module,
+						  err->module_id);
+
+		p += snprintf(p, end - p, "module=%s ", str);
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type2_field_module_id,
-				   0, oem_type2_module_name(err->module_id));
+				   0, str);
 	}
 
 	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
-		const char *str = oem_type2_sub_module_name(err->module_id,
-							    err->sub_module_id);
+		const char *str = oem_submodule_name(hisi_oem_type2_module,
+						     err->module_id,
+						     err->sub_module_id);
 
-		p += sprintf(p, "submodule=%s ", str);
+		p += snprintf(p, end - p, "submodule=%s ", str);
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type2_field_sub_module_id,
 				   0, str);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/9] rasdaemon: fix sub module name of HHA and DDRC for hip08
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (2 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08 Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 5/9] rasdaemon: split OEM type1 table decode function to reduce length Xiaofei Tan
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

Fix sub module name of HHA and DDRC for hip08, and add const to the
pointer parameter 'name' of step_vendor_data_tab().

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index afdd649..7e10a0b 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -347,21 +347,21 @@ static const char *hllc_submodule_name[] = {
 };
 
 static const char *hha_submodule_name[] = {
-	"TA_HHA0",
-	"TA_HHA1",
 	"TB_HHA0",
-	"TB_HHA1"
+	"TB_HHA1",
+	"TA_HHA0",
+	"TA_HHA1"
 };
 
 static const char *ddrc_submodule_name[] = {
-	"TA_DDRC0",
-	"TA_DDRC1",
-	"TA_DDRC2",
-	"TA_DDRC3",
 	"TB_DDRC0",
 	"TB_DDRC1",
 	"TB_DDRC2",
 	"TB_DDRC3",
+	"TA_DDRC0",
+	"TA_DDRC1",
+	"TA_DDRC2",
+	"TA_DDRC3",
 };
 
 static const struct hisi_module_info hisi_oem_type2_module[] = {
@@ -526,7 +526,8 @@ static void record_vendor_data(struct ras_ns_dec_tab *dec_tab,
 	}
 }
 
-static int step_vendor_data_tab(struct ras_ns_dec_tab *dec_tab, char *name)
+static int step_vendor_data_tab(struct ras_ns_dec_tab *dec_tab,
+				const char *name)
 {
 	int rc;
 
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/9] rasdaemon: split OEM type1 table decode function to reduce length
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (3 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 4/9] rasdaemon: fix sub module name of HHA and DDRC " Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 6/9] rasdaemon: split OEM type2 " Xiaofei Tan
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

This patch splits function decode_hip08_oem_type1_error() to reduce
length. Move header decoding and register dump to single function
separately.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 76 ++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index 7e10a0b..4e65695 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -561,38 +561,13 @@ static int step_vendor_data_tab(struct ras_ns_dec_tab *dec_tab, char *name)
 }
 #endif
 
-/* error data decoding functions */
-static int decode_hip08_oem_type1_error(struct ras_events *ras,
-					struct ras_ns_dec_tab *dec_tab,
-					struct trace_seq *s,
-					struct ras_non_standard_event *event)
+static void decode_oem_type1_err_hdr(struct ras_ns_dec_tab *dec_tab,
+				     struct trace_seq *s,
+				     const struct hisi_oem_type1_err_sec *err)
 {
-	const struct hisi_oem_type1_err_sec *err =
-			(struct hisi_oem_type1_err_sec*)event->error;
 	char buf[1024];
 	char *p = buf;
 
-	if (err->val_bits == 0) {
-		trace_seq_printf(s, "%s: no valid error information\n",
-				 __func__);
-		return -1;
-	}
-
-#ifdef HAVE_SQLITE3
-	if (!dec_tab->stmt_dec_record) {
-		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
-					    &hip08_oem_type1_event_tab)
-			!= SQLITE_OK) {
-			trace_seq_printf(s,
-					"create sql hip08_oem_type1_event_tab fail\n");
-			return -1;
-		}
-	}
-#endif
-	record_vendor_data(dec_tab, hisi_oem_data_type_text,
-			   hip08_oem_type1_field_timestamp,
-			   0, event->timestamp);
-
 	p += sprintf(p, "[ ");
 	p += sprintf(p, "table_version=%d ", err->version);
 	record_vendor_data(dec_tab, hisi_oem_data_type_int,
@@ -649,10 +624,16 @@ static int decode_hip08_oem_type1_error(struct ras_events *ras,
 	}
 
 	p += sprintf(p, "]");
-	trace_seq_printf(s, "\nHISI HIP08: OEM Type-1 Error\n");
 	trace_seq_printf(s, "%s\n", buf);
+}
+
+static void decode_oem_type1_err_regs(struct ras_ns_dec_tab *dec_tab,
+				      struct trace_seq *s,
+				      const struct hisi_oem_type1_err_sec *err)
+{
+	char buf[1024];
+	char *p = buf;
 
-	p = buf;
 	trace_seq_printf(s, "Reg Dump:\n");
 	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_0) {
 		trace_seq_printf(s, "ERR_MISC0=0x%x\n", err->err_misc_0);
@@ -689,6 +670,41 @@ static int decode_hip08_oem_type1_error(struct ras_events *ras,
 			   hip08_oem_type1_field_regs_dump, 0, buf);
 
 	step_vendor_data_tab(dec_tab, "hip08_oem_type1_event_tab");
+}
+
+/* error data decoding functions */
+static int decode_hip08_oem_type1_error(struct ras_events *ras,
+					struct ras_ns_dec_tab *dec_tab,
+					struct trace_seq *s,
+					struct ras_non_standard_event *event)
+{
+	const struct hisi_oem_type1_err_sec *err =
+			(struct hisi_oem_type1_err_sec*)event->error;
+
+	if (err->val_bits == 0) {
+		trace_seq_printf(s, "%s: no valid error information\n",
+				 __func__);
+		return -1;
+	}
+
+#ifdef HAVE_SQLITE3
+	if (!dec_tab->stmt_dec_record) {
+		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
+					    &hip08_oem_type1_event_tab)
+			!= SQLITE_OK) {
+			trace_seq_printf(s,
+					"create sql hip08_oem_type1_event_tab fail\n");
+			return -1;
+		}
+	}
+#endif
+	record_vendor_data(dec_tab, hisi_oem_data_type_text,
+			   hip08_oem_type1_field_timestamp,
+			   0, event->timestamp);
+
+	trace_seq_printf(s, "\nHISI HIP08: OEM Type-1 Error\n");
+	decode_oem_type1_err_hdr(dec_tab, s, err);
+	decode_oem_type1_err_regs(dec_tab, s, err);
 
 	return 0;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/9] rasdaemon: split OEM type2 table decode function to reduce length
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (4 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 5/9] rasdaemon: split OEM type1 table decode function to reduce length Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 7/9] rasdaemon: split PCIe local " Xiaofei Tan
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

This patch splits function decode_hip08_oem_type2_error() to reduce
length. Move header decoding and register dump to single function
separately.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 73 +++++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index 4e65695..ea640a6 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -709,36 +709,13 @@ static int decode_hip08_oem_type1_error(struct ras_events *ras,
 	return 0;
 }
 
-static int decode_hip08_oem_type2_error(struct ras_events *ras,
-					struct ras_ns_dec_tab *dec_tab,
-					struct trace_seq *s,
-					struct ras_non_standard_event *event)
+static void decode_oem_type2_err_hdr(struct ras_ns_dec_tab *dec_tab,
+				     struct trace_seq *s,
+				     const struct hisi_oem_type2_err_sec *err)
 {
-	const struct hisi_oem_type2_err_sec *err =
-			(struct hisi_oem_type2_err_sec *)event->error;
 	char buf[1024];
 	char *p = buf;
 
-	if (err->val_bits == 0) {
-		trace_seq_printf(s, "%s: no valid error information\n",
-				 __func__);
-		return -1;
-	}
-
-#ifdef HAVE_SQLITE3
-	if (!dec_tab->stmt_dec_record) {
-		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
-			&hip08_oem_type2_event_tab) != SQLITE_OK) {
-			trace_seq_printf(s,
-				"create sql hip08_oem_type2_event_tab fail\n");
-			return -1;
-		}
-	}
-#endif
-	record_vendor_data(dec_tab, hisi_oem_data_type_text,
-			   hip08_oem_type2_field_timestamp,
-			   0, event->timestamp);
-
 	p += sprintf(p, "[ ");
 	p += sprintf(p, "table_version=%d ", err->version);
 	record_vendor_data(dec_tab, hisi_oem_data_type_int,
@@ -795,10 +772,16 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
 	}
 
 	p += sprintf(p, "]");
-	trace_seq_printf(s, "\nHISI HIP08: OEM Type-2 Error\n");
 	trace_seq_printf(s, "%s\n", buf);
+}
+
+static void decode_oem_type2_err_regs(struct ras_ns_dec_tab *dec_tab,
+				      struct trace_seq *s,
+				      const struct hisi_oem_type2_err_sec *err)
+{
+	char buf[1024];
+	char *p = buf;
 
-	p = buf;
 	trace_seq_printf(s, "Reg Dump:\n");
 	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_FR) {
 		trace_seq_printf(s, "ERR_FR_0=0x%x\n", err->err_fr_0);
@@ -847,6 +830,40 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
 			   hip08_oem_type2_field_regs_dump, 0, buf);
 
 	step_vendor_data_tab(dec_tab, "hip08_oem_type2_event_tab");
+}
+
+static int decode_hip08_oem_type2_error(struct ras_events *ras,
+					struct ras_ns_dec_tab *dec_tab,
+					struct trace_seq *s,
+					struct ras_non_standard_event *event)
+{
+	const struct hisi_oem_type2_err_sec *err =
+			(struct hisi_oem_type2_err_sec *)event->error;
+
+	if (err->val_bits == 0) {
+		trace_seq_printf(s, "%s: no valid error information\n",
+				 __func__);
+		return -1;
+	}
+
+#ifdef HAVE_SQLITE3
+	if (!dec_tab->stmt_dec_record) {
+		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
+			&hip08_oem_type2_event_tab) != SQLITE_OK) {
+			trace_seq_printf(s,
+				"create sql hip08_oem_type2_event_tab fail\n");
+			return -1;
+		}
+	}
+#endif
+	record_vendor_data(dec_tab, hisi_oem_data_type_text,
+			   hip08_oem_type2_field_timestamp,
+			   0, event->timestamp);
+
+
+	trace_seq_printf(s, "\nHISI HIP08: OEM Type-2 Error\n");
+	decode_oem_type2_err_hdr(dec_tab, s, err);
+	decode_oem_type2_err_regs(dec_tab, s, err);
 
 	return 0;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/9] rasdaemon: split PCIe local table decode function to reduce length
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (5 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 6/9] rasdaemon: split OEM type2 " Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 8/9] rasdaemon: fix magic number issues reported by static code analysis for hip08 Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 9/9] rasdaemon: replace sprintf with snprintf " Xiaofei Tan
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

This patch splits function decode_hip08_pcie_local_error() to reduce
length. Move header decoding and register dump to single function
separately.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 75 ++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index ea640a6..fcc7a0e 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -868,36 +868,12 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
 	return 0;
 }
 
-static int decode_hip08_pcie_local_error(struct ras_events *ras,
-					 struct ras_ns_dec_tab *dec_tab,
-					 struct trace_seq *s,
-					 struct ras_non_standard_event *event)
+static void decode_pcie_local_err_hdr(struct ras_ns_dec_tab *dec_tab,
+				      struct trace_seq *s,
+				      const struct hisi_pcie_local_err_sec *err)
 {
-	const struct hisi_pcie_local_err_sec *err =
-			(struct hisi_pcie_local_err_sec *)event->error;
 	char buf[1024];
 	char *p = buf;
-	uint32_t i;
-
-	if (err->val_bits == 0) {
-		trace_seq_printf(s, "%s: no valid error information\n",
-				 __func__);
-		return -1;
-	}
-
-#ifdef HAVE_SQLITE3
-	if (!dec_tab->stmt_dec_record) {
-		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
-				&hip08_pcie_local_event_tab) != SQLITE_OK) {
-			trace_seq_printf(s,
-				"create sql hip08_pcie_local_event_tab fail\n");
-			return -1;
-		}
-	}
-#endif
-	record_vendor_data(dec_tab, hisi_oem_data_type_text,
-			   hip08_pcie_local_field_timestamp,
-			   0, event->timestamp);
 
 	p += sprintf(p, "[ ");
 	p += sprintf(p, "table_version=%d ", err->version);
@@ -962,11 +938,17 @@ static int decode_hip08_pcie_local_error(struct ras_events *ras,
 				   err->err_type, NULL);
 	}
 	p += sprintf(p, "]");
-
-	trace_seq_printf(s, "\nHISI HIP08: PCIe local error\n");
 	trace_seq_printf(s, "%s\n", buf);
+}
+
+static void decode_pcie_local_err_regs(struct ras_ns_dec_tab *dec_tab,
+				       struct trace_seq *s,
+				       const struct hisi_pcie_local_err_sec *err)
+{
+	char buf[1024];
+	char *p = buf;
+	uint32_t i;
 
-	p = buf;
 	trace_seq_printf(s, "Reg Dump:\n");
 	for (i = 0; i < 33; i++) {
 		if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i)) {
@@ -981,6 +963,39 @@ static int decode_hip08_pcie_local_error(struct ras_events *ras,
 			   hip08_pcie_local_field_regs_dump, 0, buf);
 
 	step_vendor_data_tab(dec_tab, "hip08_pcie_local_event_tab");
+}
+
+static int decode_hip08_pcie_local_error(struct ras_events *ras,
+					 struct ras_ns_dec_tab *dec_tab,
+					 struct trace_seq *s,
+					 struct ras_non_standard_event *event)
+{
+	const struct hisi_pcie_local_err_sec *err =
+			(struct hisi_pcie_local_err_sec *)event->error;
+
+	if (err->val_bits == 0) {
+		trace_seq_printf(s, "%s: no valid error information\n",
+				 __func__);
+		return -1;
+	}
+
+#ifdef HAVE_SQLITE3
+	if (!dec_tab->stmt_dec_record) {
+		if (ras_mc_add_vendor_table(ras, &dec_tab->stmt_dec_record,
+				&hip08_pcie_local_event_tab) != SQLITE_OK) {
+			trace_seq_printf(s,
+				"create sql hip08_pcie_local_event_tab fail\n");
+			return -1;
+		}
+	}
+#endif
+	record_vendor_data(dec_tab, hisi_oem_data_type_text,
+			   hip08_pcie_local_field_timestamp,
+			   0, event->timestamp);
+
+	trace_seq_printf(s, "\nHISI HIP08: PCIe local error\n");
+	decode_pcie_local_err_hdr(dec_tab, s, err);
+	decode_pcie_local_err_regs(dec_tab, s, err);
 
 	return 0;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 8/9] rasdaemon: fix magic number issues reported by static code analysis for hip08
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (6 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 7/9] rasdaemon: split PCIe local " Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  2019-11-12 12:27 ` [PATCH 9/9] rasdaemon: replace sprintf with snprintf " Xiaofei Tan
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

Fix magic number issues reported by static code analysis for hip08.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index fcc7a0e..976345d 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -78,6 +78,14 @@
 #define HISI_PCIE_LOCAL_VALID_ERR_SEVERITY	BIT(8)
 #define HISI_PCIE_LOCAL_VALID_ERR_MISC		9
 
+#define HISI_PCIE_LOCAL_ERR_MISC_MAX	33
+#define HISI_BUF_LEN	1024
+
+#define HISI_ERR_SEVERITY_NFE	0
+#define HISI_ERR_SEVERITY_FE	1
+#define HISI_ERR_SEVERITY_CE	2
+#define HISI_ERR_SEVERITY_NONE	3
+
 struct hisi_oem_type1_err_sec {
 	uint32_t   val_bits;
 	uint8_t    version;
@@ -132,7 +140,7 @@ struct hisi_pcie_local_err_sec {
 	uint8_t    err_severity;
 	uint16_t   err_type;
 	uint8_t    reserv[2];
-	uint32_t   err_misc[33];
+	uint32_t   err_misc[HISI_PCIE_LOCAL_ERR_MISC_MAX];
 };
 
 enum hisi_oem_data_type {
@@ -193,10 +201,10 @@ struct hisi_module_info {
 static char *err_severity(uint8_t err_sev)
 {
 	switch (err_sev) {
-	case 0: return "recoverable";
-	case 1: return "fatal";
-	case 2: return "corrected";
-	case 3: return "none";
+	case HISI_ERR_SEVERITY_NFE: return "recoverable";
+	case HISI_ERR_SEVERITY_FE: return "fatal";
+	case HISI_ERR_SEVERITY_CE: return "corrected";
+	case HISI_ERR_SEVERITY_NONE: return "none";
 	}
 	return "unknown";
 }
@@ -565,7 +573,7 @@ static void decode_oem_type1_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				     struct trace_seq *s,
 				     const struct hisi_oem_type1_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 
 	p += sprintf(p, "[ ");
@@ -631,7 +639,7 @@ static void decode_oem_type1_err_regs(struct ras_ns_dec_tab *dec_tab,
 				      struct trace_seq *s,
 				      const struct hisi_oem_type1_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 
 	trace_seq_printf(s, "Reg Dump:\n");
@@ -713,7 +721,7 @@ static void decode_oem_type2_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				     struct trace_seq *s,
 				     const struct hisi_oem_type2_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 
 	p += sprintf(p, "[ ");
@@ -779,7 +787,7 @@ static void decode_oem_type2_err_regs(struct ras_ns_dec_tab *dec_tab,
 				      struct trace_seq *s,
 				      const struct hisi_oem_type2_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 
 	trace_seq_printf(s, "Reg Dump:\n");
@@ -872,7 +880,7 @@ static void decode_pcie_local_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				      struct trace_seq *s,
 				      const struct hisi_pcie_local_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 
 	p += sprintf(p, "[ ");
@@ -945,12 +953,12 @@ static void decode_pcie_local_err_regs(struct ras_ns_dec_tab *dec_tab,
 				       struct trace_seq *s,
 				       const struct hisi_pcie_local_err_sec *err)
 {
-	char buf[1024];
+	char buf[HISI_BUF_LEN];
 	char *p = buf;
 	uint32_t i;
 
 	trace_seq_printf(s, "Reg Dump:\n");
-	for (i = 0; i < 33; i++) {
+	for (i = 0; i < HISI_PCIE_LOCAL_ERR_MISC_MAX; i++) {
 		if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i)) {
 			trace_seq_printf(s, "ERR_MISC_%d=0x%x\n", i,
 					 err->err_misc[i]);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 9/9] rasdaemon: replace sprintf with snprintf for hip08
  2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
                   ` (7 preceding siblings ...)
  2019-11-12 12:27 ` [PATCH 8/9] rasdaemon: fix magic number issues reported by static code analysis for hip08 Xiaofei Tan
@ 2019-11-12 12:27 ` Xiaofei Tan
  8 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-12 12:27 UTC (permalink / raw)
  To: mchehab, linux-edac; +Cc: Xiaofei Tan, linuxarm, shiju.jose, jonathan.cameron

Replace sprintf with snprintf for hip08 to improve reliability.
Besides, add border check for buffer pointer.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 non-standard-hisi_hip08.c | 196 ++++++++++++++++++++++++++--------------------
 1 file changed, 110 insertions(+), 86 deletions(-)

diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
index 976345d..66fc4c8 100644
--- a/non-standard-hisi_hip08.c
+++ b/non-standard-hisi_hip08.c
@@ -569,40 +569,41 @@ static int step_vendor_data_tab(struct ras_ns_dec_tab *dec_tab, char *name)
 }
 #endif
 
+#define NO_OVERFLOW(p) ((p) >= buf && (p) < end)
 static void decode_oem_type1_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				     struct trace_seq *s,
 				     const struct hisi_oem_type1_err_sec *err)
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 
-	p += sprintf(p, "[ ");
-	p += sprintf(p, "table_version=%d ", err->version);
+	p += snprintf(p, end - p, "[ table_version=%d ", err->version);
 	record_vendor_data(dec_tab, hisi_oem_data_type_int,
 			   hip08_oem_type1_field_version, err->version, NULL);
 
-	if (err->val_bits & HISI_OEM_VALID_SOC_ID) {
-		p += sprintf(p, "SOC_ID=%d ", err->soc_id);
+	if (err->val_bits & HISI_OEM_VALID_SOC_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "SOC_ID=%d ", err->soc_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type1_field_soc_id,
 				   err->soc_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_SOCKET_ID) {
-		p += sprintf(p, "socket_ID=%d ", err->socket_id);
+	if (err->val_bits & HISI_OEM_VALID_SOCKET_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "socket_ID=%d ", err->socket_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type1_field_socket_id,
 				   err->socket_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_NIMBUS_ID) {
-		p += sprintf(p, "nimbus_ID=%d ", err->nimbus_id);
+	if (err->val_bits & HISI_OEM_VALID_NIMBUS_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "nimbus_ID=%d ", err->nimbus_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type1_field_nimbus_id,
 				   err->nimbus_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
+	if (err->val_bits & HISI_OEM_VALID_MODULE_ID && NO_OVERFLOW(p)) {
 		const char *str = oem_module_name(hisi_oem_type1_module,
 						  err->module_id);
 
@@ -612,7 +613,7 @@ static void decode_oem_type1_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				   0, str);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
+	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID && NO_OVERFLOW(p)) {
 		const char *str = oem_submodule_name(hisi_oem_type1_module,
 						     err->module_id,
 						     err->sub_module_id);
@@ -623,15 +624,17 @@ static void decode_oem_type1_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				   0, str);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY) {
-		p += sprintf(p, "error_severity=%s ",
+	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "error_severity=%s ",
 			     err_severity(err->err_severity));
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type1_field_err_sev,
 				   0, err_severity(err->err_severity));
 	}
 
-	p += sprintf(p, "]");
+	if (NO_OVERFLOW(p))
+		p += snprintf(p, end - p, "]");
+
 	trace_seq_printf(s, "%s\n", buf);
 }
 
@@ -641,42 +644,48 @@ static void decode_oem_type1_err_regs(struct ras_ns_dec_tab *dec_tab,
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 
 	trace_seq_printf(s, "Reg Dump:\n");
 	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_0) {
 		trace_seq_printf(s, "ERR_MISC0=0x%x\n", err->err_misc_0);
-		p += sprintf(p, "ERR_MISC0=0x%x ", err->err_misc_0);
+		p += snprintf(p, end - p, "ERR_MISC0=0x%x ", err->err_misc_0);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_1) {
+	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_1 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC1=0x%x\n", err->err_misc_1);
-		p += sprintf(p, "ERR_MISC1=0x%x ", err->err_misc_1);
+		p += snprintf(p, end - p, "ERR_MISC1=0x%x ", err->err_misc_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_2) {
+	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_2 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC2=0x%x\n", err->err_misc_2);
-		p += sprintf(p, "ERR_MISC2=0x%x ", err->err_misc_2);
+		p += snprintf(p, end - p, "ERR_MISC2=0x%x ", err->err_misc_2);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_3) {
+	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_3 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC3=0x%x\n", err->err_misc_3);
-		p += sprintf(p, "ERR_MISC3=0x%x ", err->err_misc_3);
+		p += snprintf(p, end - p, "ERR_MISC3=0x%x ", err->err_misc_3);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_4) {
+	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_MISC_4 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC4=0x%x\n", err->err_misc_4);
-		p += sprintf(p, "ERR_MISC4=0x%x ", err->err_misc_4);
+		p += snprintf(p, end - p, "ERR_MISC4=0x%x ", err->err_misc_4);
+	}
+
+	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_ADDR && NO_OVERFLOW(p)) {
+		trace_seq_printf(s, "ERR_ADDR=0x%llx\n",
+				 (unsigned long long)err->err_addr);
+		p += snprintf(p, end - p, "ERR_ADDR=0x%llx ",
+			     (unsigned long long)err->err_addr);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE1_VALID_ERR_ADDR) {
-		trace_seq_printf(s, "ERR_ADDR=0x%p\n", (void *)err->err_addr);
-		p += sprintf(p, "ERR_ADDR=0x%p ", (void *)err->err_addr);
+	if (p > buf && p < end) {
+		p--;
+		*p = '\0';
 	}
 
-	*(--p) = '\0';
 	record_vendor_data(dec_tab, hisi_oem_data_type_text,
 			   hip08_oem_type1_field_regs_dump, 0, buf);
-
 	step_vendor_data_tab(dec_tab, "hip08_oem_type1_event_tab");
 }
 
@@ -723,34 +732,34 @@ static void decode_oem_type2_err_hdr(struct ras_ns_dec_tab *dec_tab,
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 
-	p += sprintf(p, "[ ");
-	p += sprintf(p, "table_version=%d ", err->version);
+	p += snprintf(p, end - p, "[ table_version=%d ", err->version);
 	record_vendor_data(dec_tab, hisi_oem_data_type_int,
-			   hip08_oem_type2_field_version,
-			   err->version, NULL);
-	if (err->val_bits & HISI_OEM_VALID_SOC_ID) {
-		p += sprintf(p, "SOC_ID=%d ", err->soc_id);
+			   hip08_oem_type2_field_version, err->version, NULL);
+
+	if (err->val_bits & HISI_OEM_VALID_SOC_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "SOC_ID=%d ", err->soc_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type2_field_soc_id,
 				   err->soc_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_SOCKET_ID) {
-		p += sprintf(p, "socket_ID=%d ", err->socket_id);
+	if (err->val_bits & HISI_OEM_VALID_SOCKET_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "socket_ID=%d ", err->socket_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type2_field_socket_id,
 				   err->socket_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_NIMBUS_ID) {
-		p += sprintf(p, "nimbus_ID=%d ", err->nimbus_id);
+	if (err->val_bits & HISI_OEM_VALID_NIMBUS_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "nimbus_ID=%d ", err->nimbus_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_oem_type2_field_nimbus_id,
 				   err->nimbus_id, NULL);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
+	if (err->val_bits & HISI_OEM_VALID_MODULE_ID && NO_OVERFLOW(p)) {
 		const char *str = oem_module_name(hisi_oem_type2_module,
 						  err->module_id);
 
@@ -760,7 +769,7 @@ static void decode_oem_type2_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				   0, str);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
+	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID && NO_OVERFLOW(p)) {
 		const char *str = oem_submodule_name(hisi_oem_type2_module,
 						     err->module_id,
 						     err->sub_module_id);
@@ -771,15 +780,17 @@ static void decode_oem_type2_err_hdr(struct ras_ns_dec_tab *dec_tab,
 				   0, str);
 	}
 
-	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY) {
-		p += sprintf(p, "error_severity=%s ",
+	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "error_severity=%s ",
 			     err_severity(err->err_severity));
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_oem_type2_field_err_sev,
 				   0, err_severity(err->err_severity));
 	}
 
-	p += sprintf(p, "]");
+	if (NO_OVERFLOW(p))
+		p += snprintf(p, end - p, "]");
+
 	trace_seq_printf(s, "%s\n", buf);
 }
 
@@ -789,54 +800,58 @@ static void decode_oem_type2_err_regs(struct ras_ns_dec_tab *dec_tab,
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 
 	trace_seq_printf(s, "Reg Dump:\n");
 	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_FR) {
 		trace_seq_printf(s, "ERR_FR_0=0x%x\n", err->err_fr_0);
 		trace_seq_printf(s, "ERR_FR_1=0x%x\n", err->err_fr_1);
-		p += sprintf(p, "ERR_FR_0=0x%x ERR_FR_1=0x%x ",
+		p += snprintf(p, end - p, "ERR_FR_0=0x%x ERR_FR_1=0x%x ",
 			     err->err_fr_0, err->err_fr_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_CTRL) {
+	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_CTRL && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_CTRL_0=0x%x\n", err->err_ctrl_0);
 		trace_seq_printf(s, "ERR_CTRL_1=0x%x\n", err->err_ctrl_1);
-		p += sprintf(p, "ERR_CTRL_0=0x%x ERR_CTRL_1=0x%x ",
-				err->err_ctrl_0, err->err_ctrl_1);
+		p += snprintf(p, end - p, "ERR_CTRL_0=0x%x ERR_CTRL_1=0x%x ",
+			      err->err_ctrl_0, err->err_ctrl_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_STATUS) {
+	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_STATUS && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_STATUS_0=0x%x\n", err->err_status_0);
 		trace_seq_printf(s, "ERR_STATUS_1=0x%x\n", err->err_status_1);
-		p += sprintf(p, "ERR_STATUS_0=0x%x ERR_STATUS_1=0x%x ",
-			     err->err_status_0, err->err_status_1);
+		p += snprintf(p, end - p, "ERR_STATUS_0=0x%x ERR_STATUS_1=0x%x ",
+			      err->err_status_0, err->err_status_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_ADDR) {
+	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_ADDR && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_ADDR_0=0x%x\n", err->err_addr_0);
 		trace_seq_printf(s, "ERR_ADDR_1=0x%x\n", err->err_addr_1);
-		p += sprintf(p, "ERR_ADDR_0=0x%x ERR_ADDR_1=0x%x ",
-			     err->err_addr_0, err->err_addr_1);
+		p += snprintf(p, end - p, "ERR_ADDR_0=0x%x ERR_ADDR_1=0x%x ",
+			      err->err_addr_0, err->err_addr_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_MISC_0) {
+	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_MISC_0 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC0_0=0x%x\n", err->err_misc0_0);
 		trace_seq_printf(s, "ERR_MISC0_1=0x%x\n", err->err_misc0_1);
-		p += sprintf(p, "ERR_MISC0_0=0x%x ERR_MISC0_1=0x%x ",
-			     err->err_misc0_0, err->err_misc0_1);
+		p += snprintf(p, end - p, "ERR_MISC0_0=0x%x ERR_MISC0_1=0x%x ",
+			      err->err_misc0_0, err->err_misc0_1);
 	}
 
-	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_MISC_1) {
+	if (err->val_bits & HISI_OEM_TYPE2_VALID_ERR_MISC_1 && NO_OVERFLOW(p)) {
 		trace_seq_printf(s, "ERR_MISC1_0=0x%x\n", err->err_misc1_0);
 		trace_seq_printf(s, "ERR_MISC1_1=0x%x\n", err->err_misc1_1);
-		p += sprintf(p, "ERR_MISC1_0=0x%x ERR_MISC1_1=0x%x ",
-			     err->err_misc1_0, err->err_misc1_1);
+		p += snprintf(p, end - p, "ERR_MISC1_0=0x%x ERR_MISC1_1=0x%x ",
+			      err->err_misc1_0, err->err_misc1_1);
+	}
+
+	if (p > buf && p < end) {
+		p--;
+		*p = '\0';
 	}
 
-	*(--p) = '\0';
 	record_vendor_data(dec_tab, hisi_oem_data_type_text,
 			   hip08_oem_type2_field_regs_dump, 0, buf);
-
 	step_vendor_data_tab(dec_tab, "hip08_oem_type2_event_tab");
 }
 
@@ -868,7 +883,6 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
 			   hip08_oem_type2_field_timestamp,
 			   0, event->timestamp);
 
-
 	trace_seq_printf(s, "\nHISI HIP08: OEM Type-2 Error\n");
 	decode_oem_type2_err_hdr(dec_tab, s, err);
 	decode_oem_type2_err_regs(dec_tab, s, err);
@@ -882,70 +896,74 @@ static void decode_pcie_local_err_hdr(struct ras_ns_dec_tab *dec_tab,
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 
-	p += sprintf(p, "[ ");
-	p += sprintf(p, "table_version=%d ", err->version);
+	p += snprintf(p, end - p, "[ table_version=%d ", err->version);
 	record_vendor_data(dec_tab, hisi_oem_data_type_int,
 			   hip08_pcie_local_field_version,
 			   err->version, NULL);
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SOC_ID) {
-		p += sprintf(p, "SOC_ID=%d ", err->soc_id);
+
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SOC_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "SOC_ID=%d ", err->soc_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_soc_id,
 				   err->soc_id, NULL);
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SOCKET_ID) {
-		p += sprintf(p, "socket_ID=%d ", err->socket_id);
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SOCKET_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "socket_ID=%d ", err->socket_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_socket_id,
 				   err->socket_id, NULL);
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_NIMBUS_ID) {
-		p += sprintf(p, "nimbus_ID=%d ", err->nimbus_id);
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_NIMBUS_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "nimbus_ID=%d ", err->nimbus_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_nimbus_id,
 				   err->nimbus_id, NULL);
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SUB_MODULE_ID) {
-		p += sprintf(p, "submodule=%s ",
-			     pcie_local_sub_module_name(err->sub_module_id));
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_SUB_MODULE_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "submodule=%s ",
+			      pcie_local_sub_module_name(err->sub_module_id));
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_pcie_local_field_sub_module_id,
 				   0, pcie_local_sub_module_name(err->sub_module_id));
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_CORE_ID) {
-		p += sprintf(p, "core_ID=core%d ", err->core_id);
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_CORE_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "core_ID=core%d ", err->core_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_core_id,
 				   err->core_id, NULL);
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_PORT_ID) {
-		p += sprintf(p, "port_ID=port%d ", err->port_id);
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_PORT_ID && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "port_ID=port%d ", err->port_id);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_port_id,
 				   err->port_id, NULL);
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_ERR_SEVERITY) {
-		p += sprintf(p, "error_severity=%s ",
-			     err_severity(err->err_severity));
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_ERR_SEVERITY && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "error_severity=%s ",
+			      err_severity(err->err_severity));
 		record_vendor_data(dec_tab, hisi_oem_data_type_text,
 				   hip08_pcie_local_field_err_sev,
 				   0, err_severity(err->err_severity));
 	}
 
-	if (err->val_bits & HISI_PCIE_LOCAL_VALID_ERR_TYPE) {
-		p += sprintf(p, "error_type=0x%x ", err->err_type);
+	if (err->val_bits & HISI_PCIE_LOCAL_VALID_ERR_TYPE && NO_OVERFLOW(p)) {
+		p += snprintf(p, end - p, "error_type=0x%x ", err->err_type);
 		record_vendor_data(dec_tab, hisi_oem_data_type_int,
 				   hip08_pcie_local_field_err_type,
 				   err->err_type, NULL);
 	}
-	p += sprintf(p, "]");
+
+	if (NO_OVERFLOW(p))
+		p += snprintf(p, end - p, "]");
+
 	trace_seq_printf(s, "%s\n", buf);
 }
 
@@ -955,21 +973,27 @@ static void decode_pcie_local_err_regs(struct ras_ns_dec_tab *dec_tab,
 {
 	char buf[HISI_BUF_LEN];
 	char *p = buf;
+	char *end = buf + HISI_BUF_LEN;
 	uint32_t i;
 
 	trace_seq_printf(s, "Reg Dump:\n");
 	for (i = 0; i < HISI_PCIE_LOCAL_ERR_MISC_MAX; i++) {
-		if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i)) {
+		if (err->val_bits & BIT(HISI_PCIE_LOCAL_VALID_ERR_MISC + i) &&
+		    NO_OVERFLOW(p)) {
 			trace_seq_printf(s, "ERR_MISC_%d=0x%x\n", i,
 					 err->err_misc[i]);
-			p += sprintf(p, "ERR_MISC_%d=0x%x ", i, err->err_misc[i]);
+			p += snprintf(p, end - p, "ERR_MISC_%d=0x%x ",
+				      i, err->err_misc[i]);
 		}
 	}
 
-	*(--p) = '\0';
+	if (p > buf && p < end) {
+		p--;
+		*p = '\0';
+	}
+
 	record_vendor_data(dec_tab, hisi_oem_data_type_text,
 			   hip08_pcie_local_field_regs_dump, 0, buf);
-
 	step_vendor_data_tab(dec_tab, "hip08_pcie_local_event_tab");
 }
 
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08
  2019-11-12 12:27 ` [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08 Xiaofei Tan
@ 2019-11-26  6:07   ` Mauro Carvalho Chehab
  2019-11-26  8:28     ` Xiaofei Tan
  0 siblings, 1 reply; 12+ messages in thread
From: Mauro Carvalho Chehab @ 2019-11-26  6:07 UTC (permalink / raw)
  To: Xiaofei Tan; +Cc: mchehab, linux-edac, linuxarm, shiju.jose, jonathan.cameron

Em Tue, 12 Nov 2019 20:27:08 +0800
Xiaofei Tan <tanxiaofei@huawei.com> escreveu:

> Decode submodule of OEM type1 for hip08, and reconstruct the functions
> of geting OEM module name and submodule name.
> 
> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> ---
>  non-standard-hisi_hip08.c | 286 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 221 insertions(+), 65 deletions(-)
> 
> diff --git a/non-standard-hisi_hip08.c b/non-standard-hisi_hip08.c
> index 5e4b6a0..afdd649 100644
> --- a/non-standard-hisi_hip08.c
> +++ b/non-standard-hisi_hip08.c
> @@ -182,6 +182,13 @@ enum {
>  	hip08_pcie_local_field_regs_dump,
>  };
>  
> +struct hisi_module_info {
> +	int id;
> +	const char *name;
> +	const char **sub;
> +	int sub_num;
> +};
> +
>  /* helper functions */
>  static char *err_severity(uint8_t err_sev)
>  {
> @@ -194,37 +201,137 @@ static char *err_severity(uint8_t err_sev)
>  	return "unknown";
>  }
>  
> -static char *oem_type1_module_name(uint8_t module_id)
> -{
> -	switch (module_id) {
> -	case HISI_OEM_MODULE_ID_MN: return "MN";
> -	case HISI_OEM_MODULE_ID_PLL: return "PLL";
> -	case HISI_OEM_MODULE_ID_SLLC: return "SLLC";
> -	case HISI_OEM_MODULE_ID_AA: return "AA";
> -	case HISI_OEM_MODULE_ID_SIOE: return "SIOE";
> -	case HISI_OEM_MODULE_ID_POE: return "POE";
> -	case HISI_OEM_MODULE_ID_DISP: return "DISP";
> -	case HISI_OEM_MODULE_ID_LPC: return "LPC";
> -	case HISI_OEM_MODULE_ID_GIC: return "GIC";
> -	case HISI_OEM_MODULE_ID_RDE: return "RDE";
> -	case HISI_OEM_MODULE_ID_SAS: return "SAS";
> -	case HISI_OEM_MODULE_ID_SATA: return "SATA";
> -	case HISI_OEM_MODULE_ID_USB: return "USB";
> -	}
> -	return "unknown";
> -}
> +static const char *pll_submodule_name[] = {
> +	"TB_PLL0",
> +	"TB_PLL1",
> +	"TB_PLL2",
> +	"TB_PLL3",
> +	"TA_PLL0",
> +	"TA_PLL1",
> +	"TA_PLL2",
> +	"TA_PLL3",
> +	"NIMBUS_PLL0",
> +	"NIMBUS_PLL1",
> +	"NIMBUS_PLL2",
> +	"NIMBUS_PLL3",
> +	"NIMBUS_PLL4",
> +};
>  
> -static char *oem_type2_module_name(uint8_t module_id)
> -{
> -	switch (module_id) {
> -	case HISI_OEM_MODULE_ID_SMMU: return "SMMU";
> -	case HISI_OEM_MODULE_ID_HHA: return "HHA";
> -	case HISI_OEM_MODULE_ID_HLLC: return "HLLC";
> -	case HISI_OEM_MODULE_ID_PA: return "PA";
> -	case HISI_OEM_MODULE_ID_DDRC: return "DDRC";
> -	}
> -	return "unknown module";
> -}
> +static const char *sllc_submodule_name[] = {
> +	"TB_SLLC0",
> +	"TB_SLLC1",
> +	"TB_SLLC2",
> +	"TA_SLLC0",
> +	"TA_SLLC1",
> +	"TA_SLLC2",
> +	"NIMBUS_SLLC0",
> +	"NIMBUS_SLLC1",
> +};
> +
> +static const char *sioe_submodule_name[] = {
> +	"TB_SIOE0",
> +	"TB_SIOE1",
> +	"TB_SIOE2",
> +	"TB_SIOE3",
> +	"TA_SIOE0",
> +	"TA_SIOE1",
> +	"TA_SIOE2",
> +	"TA_SIOE3",
> +	"NIMBUS_SIOE0",
> +	"NIMBUS_SIOE1",
> +};
> +
> +static const char *poe_submodule_name[] = {
> +	"TB_POE",
> +	"TA_POE",
> +};
> +
> +static const char *disp_submodule_name[] = {
> +	"TB_PERI_DISP",
> +	"TB_POE_DISP",
> +	"TB_GIC_DISP",
> +	"TA_PERI_DISP",
> +	"TA_POE_DISP",
> +	"TA_GIC_DISP",
> +	"HAC_DISP",
> +	"PCIE_DISP",
> +	"IO_MGMT_DISP",
> +	"NETWORK_DISP",
> +};
> +
> +static const char *sas_submodule_name[] = {
> +	"SAS0",
> +	"SAS1",
> +};
> +
> +static const struct hisi_module_info hisi_oem_type1_module[] = {
> +	{
> +		.id = HISI_OEM_MODULE_ID_PLL,
> +		.name = "PLL",
> +		.sub = pll_submodule_name,
> +		.sub_num = ARRAY_SIZE(pll_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_SAS,
> +		.name = "SAS",
> +		.sub = sas_submodule_name,
> +		.sub_num = ARRAY_SIZE(sas_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_POE,
> +		.name = "POE",
> +		.sub = poe_submodule_name,
> +		.sub_num = ARRAY_SIZE(poe_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_SLLC,
> +		.name = "SLLC",
> +		.sub = sllc_submodule_name,
> +		.sub_num = ARRAY_SIZE(sllc_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_SIOE,
> +		.name = "SIOE",
> +		.sub = sioe_submodule_name,
> +		.sub_num = ARRAY_SIZE(sioe_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_DISP,
> +		.name = "DISP",
> +		.sub = disp_submodule_name,
> +		.sub_num = ARRAY_SIZE(disp_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_MN,
> +		.name = "MN",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_AA,
> +		.name = "AA",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_LPC,
> +		.name = "LPC",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_GIC,
> +		.name = "GIC",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_RDE,
> +		.name = "RDE",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_SATA,
> +		.name = "SATA",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_USB,
> +		.name = "USB",
> +	},
> +	{
> +	}
> +};
>  
>  static const char *smmu_submodule_name[] = {
>  	"HAC_SMMU",
> @@ -257,27 +364,72 @@ static const char *ddrc_submodule_name[] = {
>  	"TB_DDRC3",
>  };
>  
> -static const char *oem_type2_sub_module_name(uint8_t module_id, uint8_t sub_module_id)
> +static const struct hisi_module_info hisi_oem_type2_module[] = {
> +	{
> +		.id = HISI_OEM_MODULE_ID_SMMU,
> +		.name = "SMMU",
> +		.sub = smmu_submodule_name,
> +		.sub_num = ARRAY_SIZE(smmu_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_HHA,
> +		.name = "HHA",
> +		.sub = hha_submodule_name,
> +		.sub_num = ARRAY_SIZE(hha_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_PA,
> +		.name = "PA",
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_HLLC,
> +		.name = "HLLC",
> +		.sub = hllc_submodule_name,
> +		.sub_num = ARRAY_SIZE(hllc_submodule_name),
> +	},
> +	{
> +		.id = HISI_OEM_MODULE_ID_DDRC,
> +		.name = "DDRC",
> +		.sub = ddrc_submodule_name,
> +		.sub_num = ARRAY_SIZE(ddrc_submodule_name),
> +	},
> +	{
> +	}
> +};
> +
> +static const char *oem_module_name(const struct hisi_module_info *info,
> +				   uint8_t module_id)
>  {
> -	switch (module_id) {
> -	case HISI_OEM_MODULE_ID_SMMU:
> -		if (sub_module_id < sizeof(smmu_submodule_name)/sizeof(char *))
> -			return smmu_submodule_name[sub_module_id];
> -		break;
> -	case HISI_OEM_MODULE_ID_HLLC:
> -		if (sub_module_id < sizeof(hllc_submodule_name)/sizeof(char *))
> -			return hllc_submodule_name[sub_module_id];
> -		break;
> -	case HISI_OEM_MODULE_ID_PA:
> -		return "PA";
> -	case HISI_OEM_MODULE_ID_HHA:
> -		if (sub_module_id < sizeof(hha_submodule_name)/sizeof(char *))
> -			return hha_submodule_name[sub_module_id];
> -		break;
> -	case HISI_OEM_MODULE_ID_DDRC:
> -		if (sub_module_id < sizeof(ddrc_submodule_name)/sizeof(char *))
> -			return ddrc_submodule_name[sub_module_id];
> -		break;
> +	const struct hisi_module_info *module = &info[0];
> +
> +	for (; module->name; module++) {
> +		if (module->id != module_id)
> +			continue;
> +
> +		return module->name;
> +	}
> +
> +	return "unknown";
> +}
> +
> +static const char *oem_submodule_name(const struct hisi_module_info *info,
> +				      uint8_t module_id, uint8_t sub_module_id)
> +{
> +	const struct hisi_module_info *module = &info[0];
> +
> +	for (; module->name; module++) {
> +		const char **submodule = module->sub;
> +
> +		if (module->id != module_id)
> +			continue;
> +
> +		if (module->sub == NULL)
> +			return module->name;
> +
> +		if (sub_module_id >= module->sub_num)
> +			return "unknown";
> +
> +		return submodule[sub_module_id];
>  	}
>  
>  	return "unknown";
> @@ -467,23 +619,24 @@ static int decode_hip08_oem_type1_error(struct ras_events *ras,
>  	}
>  
>  	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
> -		p += sprintf(p, "module=%s ",
> -			     oem_type1_module_name(err->module_id));
> +		const char *str = oem_module_name(hisi_oem_type1_module,
> +						  err->module_id);
> +
> +		p += snprintf(p, end - p, "module=%s ", str);



Rasdaemon doesn't build after applying this patch, as "end" doesn't exist
yet.

I suspect that a latter patch in this series could be adding it, but
the better is to not break rasdaemon build on some random patch, as, if
we ever need to do a git bisect, this would make it harder for it to
work.


>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>  				   hip08_oem_type1_field_module_id,
> -				   0, oem_type1_module_name(err->module_id));
> +				   0, str);
>  	}
>  
>  	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
> -		char submodule_name[64];
> +		const char *str = oem_submodule_name(hisi_oem_type1_module,
> +						     err->module_id,
> +						     err->sub_module_id);
>  
> -		sprintf(submodule_name, "%s%d",
> -			oem_type1_module_name(err->module_id),
> -			err->sub_module_id);
> -		p += sprintf(p, "submodule=%s ", submodule_name);
> +		p += snprintf(p, end - p, "submodule=%s ", str);
>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>  				   hip08_oem_type1_field_sub_module_id,
> -				   0, submodule_name);
> +				   0, str);
>  	}
>  
>  	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY) {
> @@ -596,18 +749,21 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
>  	}
>  
>  	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
> -		p += sprintf(p, "module=%s ",
> -			     oem_type2_module_name(err->module_id));
> +		const char *str = oem_module_name(hisi_oem_type2_module,
> +						  err->module_id);
> +
> +		p += snprintf(p, end - p, "module=%s ", str);
>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>  				   hip08_oem_type2_field_module_id,
> -				   0, oem_type2_module_name(err->module_id));
> +				   0, str);
>  	}
>  
>  	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
> -		const char *str = oem_type2_sub_module_name(err->module_id,
> -							    err->sub_module_id);
> +		const char *str = oem_submodule_name(hisi_oem_type2_module,
> +						     err->module_id,
> +						     err->sub_module_id);
>  
> -		p += sprintf(p, "submodule=%s ", str);
> +		p += snprintf(p, end - p, "submodule=%s ", str);
>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>  				   hip08_oem_type2_field_sub_module_id,
>  				   0, str);




Cheers,
Mauro

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08
  2019-11-26  6:07   ` Mauro Carvalho Chehab
@ 2019-11-26  8:28     ` Xiaofei Tan
  0 siblings, 0 replies; 12+ messages in thread
From: Xiaofei Tan @ 2019-11-26  8:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: mchehab, linux-edac, linuxarm, shiju.jose, jonathan.cameron

Hi Mauro,

On 2019/11/26 14:07, Mauro Carvalho Chehab wrote:
> Em Tue, 12 Nov 2019 20:27:08 +0800
> Xiaofei Tan <tanxiaofei@huawei.com> escreveu:
> 
>> Decode submodule of OEM type1 for hip08, and reconstruct the functions
>> of geting OEM module name and submodule name.
>>
>> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>

...

>> +		const char *str = oem_module_name(hisi_oem_type1_module,
>> +						  err->module_id);
>> +
>> +		p += snprintf(p, end - p, "module=%s ", str);
> 
> 
> 
> Rasdaemon doesn't build after applying this patch, as "end" doesn't exist
> yet.
> 
> I suspect that a latter patch in this series could be adding it, but
> the better is to not break rasdaemon build on some random patch, as, if
> we ever need to do a git bisect, this would make it harder for it to
> work.
> 

Oh, Yes, this is my fault. I will fix this issue and send v2 patch set after removing two applied. thanks.

> 
>>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>>  				   hip08_oem_type1_field_module_id,
>> -				   0, oem_type1_module_name(err->module_id));
>> +				   0, str);
>>  	}
>>  
>>  	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
>> -		char submodule_name[64];
>> +		const char *str = oem_submodule_name(hisi_oem_type1_module,
>> +						     err->module_id,
>> +						     err->sub_module_id);
>>  
>> -		sprintf(submodule_name, "%s%d",
>> -			oem_type1_module_name(err->module_id),
>> -			err->sub_module_id);
>> -		p += sprintf(p, "submodule=%s ", submodule_name);
>> +		p += snprintf(p, end - p, "submodule=%s ", str);
>>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>>  				   hip08_oem_type1_field_sub_module_id,
>> -				   0, submodule_name);
>> +				   0, str);
>>  	}
>>  
>>  	if (err->val_bits & HISI_OEM_VALID_ERR_SEVERITY) {
>> @@ -596,18 +749,21 @@ static int decode_hip08_oem_type2_error(struct ras_events *ras,
>>  	}
>>  
>>  	if (err->val_bits & HISI_OEM_VALID_MODULE_ID) {
>> -		p += sprintf(p, "module=%s ",
>> -			     oem_type2_module_name(err->module_id));
>> +		const char *str = oem_module_name(hisi_oem_type2_module,
>> +						  err->module_id);
>> +
>> +		p += snprintf(p, end - p, "module=%s ", str);
>>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>>  				   hip08_oem_type2_field_module_id,
>> -				   0, oem_type2_module_name(err->module_id));
>> +				   0, str);
>>  	}
>>  
>>  	if (err->val_bits & HISI_OEM_VALID_SUB_MODULE_ID) {
>> -		const char *str = oem_type2_sub_module_name(err->module_id,
>> -							    err->sub_module_id);
>> +		const char *str = oem_submodule_name(hisi_oem_type2_module,
>> +						     err->module_id,
>> +						     err->sub_module_id);
>>  
>> -		p += sprintf(p, "submodule=%s ", str);
>> +		p += snprintf(p, end - p, "submodule=%s ", str);
>>  		record_vendor_data(dec_tab, hisi_oem_data_type_text,
>>  				   hip08_oem_type2_field_sub_module_id,
>>  				   0, str);
> 
> 
> 
> 
> Cheers,
> Mauro
> 
> .
> 

-- 
 thanks
tanxiaofei


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-11-26  8:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 12:27 [PATCH 0/9] rasdaemon: fix some issues reported by static code analysis Xiaofei Tan
2019-11-12 12:27 ` [PATCH 1/9] rasdaemon: fix the wrong declaring of 'sruct ras_events' in ras-record.h Xiaofei Tan
2019-11-12 12:27 ` [PATCH 2/9] rasdaemon: fix an warning reported by PC-Lint Xiaofei Tan
2019-11-12 12:27 ` [PATCH 3/9] rasdaemon: decode submodule of OEM type1 for hip08 Xiaofei Tan
2019-11-26  6:07   ` Mauro Carvalho Chehab
2019-11-26  8:28     ` Xiaofei Tan
2019-11-12 12:27 ` [PATCH 4/9] rasdaemon: fix sub module name of HHA and DDRC " Xiaofei Tan
2019-11-12 12:27 ` [PATCH 5/9] rasdaemon: split OEM type1 table decode function to reduce length Xiaofei Tan
2019-11-12 12:27 ` [PATCH 6/9] rasdaemon: split OEM type2 " Xiaofei Tan
2019-11-12 12:27 ` [PATCH 7/9] rasdaemon: split PCIe local " Xiaofei Tan
2019-11-12 12:27 ` [PATCH 8/9] rasdaemon: fix magic number issues reported by static code analysis for hip08 Xiaofei Tan
2019-11-12 12:27 ` [PATCH 9/9] rasdaemon: replace sprintf with snprintf " Xiaofei Tan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).