target/cortex_m: check core implementor field

Presently, we only look at the Part Number field of the CPUID, and
completely ignore the Implmentor field, simply assuming it to be ARM.

Parts have since been found, with different implementors, that use
overlapping part numbers, causing detection to fail.

Expand the "part number" field to be a full implementor+part number,
excluding the revision/patch fields, to make checking more reliable.

Change-Id: Id81774f829104f57a0c105320d0d2e479fa01522
Signed-off-by: Karl Palsson <karlp@tweak.au>
Reviewed-on: https://review.openocd.org/c/openocd/+/7845
Tested-by: jenkins
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>

7 files changed, 52 insertions, 38 deletions

diff --git a/src/flash/nor/stm32f1x.c b/src/flash/nor/stm32f1x.c
index ab1ef2a..b3bb843 100644
--- a/src/flash/nor/stm32f1x.c
+++ b/src/flash/nor/stm32f1x.c
@@ -743,7 +743,7 @@ static int stm32x_get_property_addr(struct target *target, struct stm32x_propert
 		return ERROR_TARGET_NOT_EXAMINED;
 	}
 
-	switch (cortex_m_get_partno_safe(target)) {
+	switch (cortex_m_get_impl_part(target)) {
 	case CORTEX_M0_PARTNO: /* STM32F0x devices */
 		addr->device_id = 0x40015800;
 		addr->flash_size = 0x1FFFF7CC;
diff --git a/src/flash/nor/stm32f2x.c b/src/flash/nor/stm32f2x.c
index dcaf260..2e0d158 100644
--- a/src/flash/nor/stm32f2x.c
+++ b/src/flash/nor/stm32f2x.c
@@ -961,7 +961,7 @@ static int stm32x_get_device_id(struct flash_bank *bank, uint32_t *device_id)
 		return retval;
 
 	if ((*device_id & 0xfff) == 0x411
-			&& cortex_m_get_partno_safe(target) == CORTEX_M4_PARTNO) {
+			&& cortex_m_get_impl_part(target) == CORTEX_M4_PARTNO) {
 		*device_id &= ~((0xFFFF << 16) | 0xfff);
 		*device_id |= (0x1000 << 16) | 0x413;
 		LOG_INFO("stm32f4x errata detected - fixing incorrect MCU_IDCODE");
diff --git a/src/flash/nor/stm32h7x.c b/src/flash/nor/stm32h7x.c
index 8be8037..21618b3 100644
--- a/src/flash/nor/stm32h7x.c
+++ b/src/flash/nor/stm32h7x.c
@@ -795,7 +795,7 @@ static int stm32x_probe(struct flash_bank *bank)
 	/* STM32H74x/H75x, the second core (Cortex-M4) cannot read the flash size */
 	retval = ERROR_FAIL;
 	if (device_id == DEVID_STM32H74_H75XX
-			&& cortex_m_get_partno_safe(target) == CORTEX_M4_PARTNO)
+			&& cortex_m_get_impl_part(target) == CORTEX_M4_PARTNO)
 		LOG_WARNING("%s cannot read the flash size register", target_name(target));
 	else
 		retval = target_read_u16(target, stm32x_info->part_info->fsize_addr, &flash_size_in_kb);
diff --git a/src/flash/nor/stm32l4x.c b/src/flash/nor/stm32l4x.c
index 77a89f5..4414cf5 100644
--- a/src/flash/nor/stm32l4x.c
+++ b/src/flash/nor/stm32l4x.c
@@ -1682,7 +1682,7 @@ static int stm32l4_read_idcode(struct flash_bank *bank, uint32_t *id)
 
 	/* CPU2 (Cortex-M0+) is supported only with non-hla adapters because it is on AP1.
 	 * Using HLA adapters armv7m.debug_ap is null, and checking ap_num triggers a segfault */
-	if (cortex_m_get_partno_safe(target) == CORTEX_M0P_PARTNO &&
+	if (cortex_m_get_impl_part(target) == CORTEX_M0P_PARTNO &&
 			armv7m->debug_ap && armv7m->debug_ap->ap_num == 1) {
 		uint32_t uid64_ids;
 
diff --git a/src/target/arm.h b/src/target/arm.h
index fd61d5f..f3abd6c 100644
--- a/src/target/arm.h
+++ b/src/target/arm.h
@@ -58,6 +58,11 @@ enum arm_arch {
 	ARM_ARCH_V8M,
 };
 
+/** Known ARM implementor IDs */
+enum arm_implementor {
+	ARM_IMPLEMENTOR_ARM = 0x41,
+};
+
 /**
  * Represent state of an ARM core.
  *
diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c
index 987dc9b..87a8845 100644
--- a/src/target/cortex_m.c
+++ b/src/target/cortex_m.c
@@ -50,63 +50,63 @@
 /* Supported Cortex-M Cores */
 static const struct cortex_m_part_info cortex_m_parts[] = {
 	{
-		.partno = CORTEX_M0_PARTNO,
+		.impl_part = CORTEX_M0_PARTNO,
 		.name = "Cortex-M0",
 		.arch = ARM_ARCH_V6M,
 	},
 	{
-		.partno = CORTEX_M0P_PARTNO,
+		.impl_part = CORTEX_M0P_PARTNO,
 		.name = "Cortex-M0+",
 		.arch = ARM_ARCH_V6M,
 	},
 	{
-		.partno = CORTEX_M1_PARTNO,
+		.impl_part = CORTEX_M1_PARTNO,
 		.name = "Cortex-M1",
 		.arch = ARM_ARCH_V6M,
 	},
 	{
-		.partno = CORTEX_M3_PARTNO,
+		.impl_part = CORTEX_M3_PARTNO,
 		.name = "Cortex-M3",
 		.arch = ARM_ARCH_V7M,
 		.flags = CORTEX_M_F_TAR_AUTOINCR_BLOCK_4K,
 	},
 	{
-		.partno = CORTEX_M4_PARTNO,
+		.impl_part = CORTEX_M4_PARTNO,
 		.name = "Cortex-M4",
 		.arch = ARM_ARCH_V7M,
 		.flags = CORTEX_M_F_HAS_FPV4 | CORTEX_M_F_TAR_AUTOINCR_BLOCK_4K,
 	},
 	{
-		.partno = CORTEX_M7_PARTNO,
+		.impl_part = CORTEX_M7_PARTNO,
 		.name = "Cortex-M7",
 		.arch = ARM_ARCH_V7M,
 		.flags = CORTEX_M_F_HAS_FPV5,
 	},
 	{
-		.partno = CORTEX_M23_PARTNO,
+		.impl_part = CORTEX_M23_PARTNO,
 		.name = "Cortex-M23",
 		.arch = ARM_ARCH_V8M,
 	},
 	{
-		.partno = CORTEX_M33_PARTNO,
+		.impl_part = CORTEX_M33_PARTNO,
 		.name = "Cortex-M33",
 		.arch = ARM_ARCH_V8M,
 		.flags = CORTEX_M_F_HAS_FPV5,
 	},
 	{
-		.partno = CORTEX_M35P_PARTNO,
+		.impl_part = CORTEX_M35P_PARTNO,
 		.name = "Cortex-M35P",
 		.arch = ARM_ARCH_V8M,
 		.flags = CORTEX_M_F_HAS_FPV5,
 	},
 	{
-		.partno = CORTEX_M55_PARTNO,
+		.impl_part = CORTEX_M55_PARTNO,
 		.name = "Cortex-M55",
 		.arch = ARM_ARCH_V8M,
 		.flags = CORTEX_M_F_HAS_FPV5,
 	},
 	{
-		.partno = STAR_MC1_PARTNO,
+		.impl_part = STAR_MC1_PARTNO,
 		.name = "STAR-MC1",
 		.arch = ARM_ARCH_V8M,
 		.flags = CORTEX_M_F_HAS_FPV5,
@@ -2526,18 +2526,18 @@ int cortex_m_examine(struct target *target)
 		if (retval != ERROR_OK)
 			return retval;
 
-		/* Get ARCH and CPU types */
-		const enum cortex_m_partno core_partno = (cpuid & ARM_CPUID_PARTNO_MASK) >> ARM_CPUID_PARTNO_POS;
+		/* Inspect implementor/part to look for recognized cores  */
+		unsigned int impl_part = cpuid & (ARM_CPUID_IMPLEMENTOR_MASK | ARM_CPUID_PARTNO_MASK);
 
 		for (unsigned int n = 0; n < ARRAY_SIZE(cortex_m_parts); n++) {
-			if (core_partno == cortex_m_parts[n].partno) {
+			if (impl_part == cortex_m_parts[n].impl_part) {
 				cortex_m->core_info = &cortex_m_parts[n];
 				break;
 			}
 		}
 
 		if (!cortex_m->core_info) {
-			LOG_TARGET_ERROR(target, "Cortex-M PARTNO 0x%x is unrecognized", core_partno);
+			LOG_TARGET_ERROR(target, "Cortex-M CPUID: 0x%x is unrecognized", cpuid);
 			return ERROR_FAIL;
 		}
 
@@ -2549,7 +2549,7 @@ int cortex_m_examine(struct target *target)
 				(uint8_t)((cpuid >> 0) & 0xf));
 
 		cortex_m->maskints_erratum = false;
-		if (core_partno == CORTEX_M7_PARTNO) {
+		if (impl_part == CORTEX_M7_PARTNO) {
 			uint8_t rev, patch;
 			rev = (cpuid >> 20) & 0xf;
 			patch = (cpuid >> 0) & 0xf;
diff --git a/src/target/cortex_m.h b/src/target/cortex_m.h
index a1c43b5..806ff59 100644
--- a/src/target/cortex_m.h
+++ b/src/target/cortex_m.h
@@ -31,22 +31,31 @@
 
 #define CPUID		0xE000ED00
 
-#define ARM_CPUID_PARTNO_POS    4
-#define ARM_CPUID_PARTNO_MASK	(0xFFF << ARM_CPUID_PARTNO_POS)
+#define ARM_CPUID_IMPLEMENTOR_POS	24
+#define ARM_CPUID_IMPLEMENTOR_MASK	(0xFF << ARM_CPUID_IMPLEMENTOR_POS)
+#define ARM_CPUID_PARTNO_POS		4
+#define ARM_CPUID_PARTNO_MASK		(0xFFF << ARM_CPUID_PARTNO_POS)
 
-enum cortex_m_partno {
+#define ARM_MAKE_CPUID(impl, partno)	((((impl) << ARM_CPUID_IMPLEMENTOR_POS) & ARM_CPUID_IMPLEMENTOR_MASK) | \
+	(((partno) << ARM_CPUID_PARTNO_POS)  & ARM_CPUID_PARTNO_MASK))
+
+/** Known Arm Cortex masked CPU Ids
+ * This includes the implementor and part number, but _not_ the revision or
+ * patch fields.
+ */
+enum cortex_m_impl_part {
 	CORTEX_M_PARTNO_INVALID,
-	STAR_MC1_PARTNO    = 0x132,
-	CORTEX_M0_PARTNO   = 0xC20,
-	CORTEX_M1_PARTNO   = 0xC21,
-	CORTEX_M3_PARTNO   = 0xC23,
-	CORTEX_M4_PARTNO   = 0xC24,
-	CORTEX_M7_PARTNO   = 0xC27,
-	CORTEX_M0P_PARTNO  = 0xC60,
-	CORTEX_M23_PARTNO  = 0xD20,
-	CORTEX_M33_PARTNO  = 0xD21,
-	CORTEX_M35P_PARTNO = 0xD31,
-	CORTEX_M55_PARTNO  = 0xD22,
+	STAR_MC1_PARTNO    = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0x132), /* FIXME - confirm implementor! */
+	CORTEX_M0_PARTNO   = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC20),
+	CORTEX_M1_PARTNO   = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC21),
+	CORTEX_M3_PARTNO   = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC23),
+	CORTEX_M4_PARTNO   = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC24),
+	CORTEX_M7_PARTNO   = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC27),
+	CORTEX_M0P_PARTNO  = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xC60),
+	CORTEX_M23_PARTNO  = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xD20),
+	CORTEX_M33_PARTNO  = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xD21),
+	CORTEX_M35P_PARTNO = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xD31),
+	CORTEX_M55_PARTNO  = ARM_MAKE_CPUID(ARM_IMPLEMENTOR_ARM, 0xD22),
 };
 
 /* Relevant Cortex-M flags, used in struct cortex_m_part_info.flags */
@@ -55,7 +64,7 @@ enum cortex_m_partno {
 #define CORTEX_M_F_TAR_AUTOINCR_BLOCK_4K  BIT(2)
 
 struct cortex_m_part_info {
-	enum cortex_m_partno partno;
+	enum cortex_m_impl_part impl_part;
 	const char *name;
 	enum arm_arch arch;
 	uint32_t flags;
@@ -292,11 +301,11 @@ target_to_cortex_m_safe(struct target *target)
 }
 
 /**
- * @returns cached value of Cortex-M part number
+ * @returns cached value of the cpuid, masked for implementation and part.
  * or CORTEX_M_PARTNO_INVALID if the magic number does not match
  * or core_info is not initialised.
  */
-static inline enum cortex_m_partno cortex_m_get_partno_safe(struct target *target)
+static inline enum cortex_m_impl_part cortex_m_get_impl_part(struct target *target)
 {
 	struct cortex_m_common *cortex_m = target_to_cortex_m_safe(target);
 	if (!cortex_m)
@@ -305,7 +314,7 @@ static inline enum cortex_m_partno cortex_m_get_partno_safe(struct target *targe
 	if (!cortex_m->core_info)
 		return CORTEX_M_PARTNO_INVALID;
 
-	return cortex_m->core_info->partno;
+	return cortex_m->core_info->impl_part;
 }
 
 int cortex_m_examine(struct target *target);