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>

diff --git a/src/flash/nor/stm32f1x.c b/src/flash/nor/stm32f1x.c
index ab1ef2aefef0642d831a088a8c915f6ea9021f00..b3bb8433584e0a8d3ef746046afe48ab8fb42d5a 100644 (file)
--- 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 dcaf260d97d807c4eefabddd92ce734471d137be..2e0d158975456f28fb1f9c385d1d8d86636972d4 100644 (file)
--- 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 8be8037287f5ef780e04f01549a8db42f01380a6..21618b39b8d2de3a203b48431625af2edf1aefb8 100644 (file)
--- 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 77a89f53c04971f2d4571dbd541c69b8e06ecbcb..4414cf539608574f7f1de8c3897ff4bc9446234b 100644 (file)
--- 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 fd61d5f5146c712d186b339330c7c56b13c05fac..f3abd6cbab93085e38f8e6bca706110ba14246ef 100644 (file)
--- 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 987dc9b245164dd84c5934b7b42b4b868e43854e..87a88455270d836a4701ddf6f4840a3204fba16a 100644 (file)
--- 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 a1c43b56de526c164d02ae4afd9a43daad3fe24e..806ff59106be6004d207417a1f6c21c6d0f9367a 100644 (file)
--- 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);