arm64: fix midr range for Cortex-A57 erratum 832075

PATCH New Bo Yan 5 participants 6 messages Mar 31, 2015 – Apr 1, 2015 arm64

Share & tools

Bo Yan

author

2015-03-31 20:30 UTC #0 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

Register MIDR_EL1 is masked to get variant and revision fields, then compared against midr_range_min and midr_range_max when checking whether CPU is affected by any particular erratum. However, variant and revision fields in MIDR_EL1 are separated by 16 bits, so the min and max of midr range should be constructed accordingly, otherwise the patch will not be applied when variant field is non-0.

Signed-off-by Bo Yan <[email protected]>

---
 arch/arm64/kernel/cpu_errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/kernel/cpu_errata.c +1 -1

index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{

Signature · 1 line

2.1.4

Paul Walmsley

2015-03-31 21:17 UTC #1 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

Quoted 27 lines · Bo Yan

Register MIDR_EL1 is masked to get variant and revision fields, then compared against midr_range_min and midr_range_max when checking whether CPU is affected by any particular erratum. However, variant and revision fields in MIDR_EL1 are separated by 16 bits, so the min and max of midr range should be constructed accordingly, otherwise the patch will not be applied when variant field is non-0.
Signed-off-by Bo Yan <[email protected]>
---
 arch/arm64/kernel/cpu_errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpu_errata.c +1 -1
index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{
Signature · 1 line
2.1.4

Reviewed-by Paul Walmsley <[email protected]>

Reviewed against DDI0488G section 4.3.1 "Main ID Register, EL1". Looks like a cut-and-paste error from the A53 workarounds.

- Paul

Will Deacon

2015-04-01 09:22 UTC #2 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

Quoted 32 lines · Paul Walmsley

Quoted 27 lines · Bo Yan
Register MIDR_EL1 is masked to get variant and revision fields, then compared against midr_range_min and midr_range_max when checking whether CPU is affected by any particular erratum. However, variant and revision fields in MIDR_EL1 are separated by 16 bits, so the min and max of midr range should be constructed accordingly, otherwise the patch will not be applied when variant field is non-0.
Signed-off-by Bo Yan <[email protected]>
---
 arch/arm64/kernel/cpu_errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpu_errata.c +1 -1
index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{
Signature · 1 line
2.1.4
Reviewed-by Paul Walmsley <[email protected]>

Quoted 2 lines

Reviewed against DDI0488G section 4.3.1 "Main ID Register, EL1". Looks like a cut-and-paste error from the A53 workarounds.

Thanks guys, I agree that this is a bug. I'll apply this, but using an explicit shift to set the variant (tweaked version below).

Will

--->8

arch/arm64/kernel/cpu_errata.c +1 -1

index a66f4fa4d541..c998345a052f 100644
@@ -70,7 +70,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
        /* Cortex-A57 r0p0 - r1p2 */
                .desc = "ARM erratum 832075",
                .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, (1 << MIDR_VARIANT_SHIFT) | 2,
        },
 #endif
        {

Andre Przywara

2015-04-01 09:27 UTC #3 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

Quoted 9 lines · Will Deacon

Quoted 8 lines · Paul Walmsley
Quoted 6 lines · Bo Yan
Register MIDR_EL1 is masked to get variant and revision fields, then compared against midr_range_min and midr_range_max when checking whether CPU is affected by any particular erratum. However, variant and revision fields in MIDR_EL1 are separated by 16 bits, so the min and max of midr range should be constructed accordingly, otherwise the patch will not be applied when variant field is non-0.

Ah yes, good catch, thank you!

Quoted 40 lines

Quoted 24 lines
Quoted 21 lines
Signed-off-by Bo Yan <[email protected]>
---
 arch/arm64/kernel/cpu_errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpu_errata.c +1 -1
index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{
Signature · 1 line
2.1.4
Reviewed-by Paul Walmsley <[email protected]>
Quoted 2 lines
Reviewed against DDI0488G section 4.3.1 "Main ID Register, EL1". Looks like a cut-and-paste error from the A53 workarounds.
Thanks guys, I agree that this is a bug. I'll apply this, but using an explicit shift to set the variant (tweaked version below).
Will
--->8
arch/arm64/kernel/cpu_errata.c +0 -0
index a66f4fa4d541..c998345a052f 100644

arch/arm64/kernel/cpu_errata.c const struct arm64_cpu_capabilities arm64_errata[] = { · line 76

Quoted 8 lines

arch/arm64/kernel/cpu_errata.c +1 -1

@@ -70,7 +70,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
        /* Cortex-A57 r0p0 - r1p2 */
                .desc = "ARM erratum 832075",
                .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, (1 << MIDR_VARIANT_SHIFT) | 2,
        },
 #endif
        {

I like this version more:

Acked-by Andre Przywara <[email protected]>

Cheers,
Andre.

Will Deacon

2015-04-01 09:29 UTC #4 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

In reply to Will Deacon

Quoted 35 lines · Will Deacon

Quoted 19 lines · Paul Walmsley
Quoted 15 lines · Bo Yan
arch/arm64/kernel/cpu_errata.c +1 -1
index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{
Signature · 1 line
2.1.4
Reviewed-by Paul Walmsley <[email protected]>
Quoted 2 lines
Reviewed against DDI0488G section 4.3.1 "Main ID Register, EL1". Looks like a cut-and-paste error from the A53 workarounds.
Thanks guys, I agree that this is a bug. I'll apply this, but using an explicit shift to set the variant (tweaked version below).
Will
--->8
arch/arm64/kernel/cpu_errata.c +0 -0
index a66f4fa4d541..c998345a052f 100644

arch/arm64/kernel/cpu_errata.c const struct arm64_cpu_capabilities arm64_errata[] = { · line 73

Quoted 5 lines

arch/arm64/kernel/cpu_errata.c +1 -1

@@ -70,7 +70,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
        /* Cortex-A57 r0p0 - r1p2 */
                .desc = "ARM erratum 832075",
                .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, (1 << MIDR_VARIANT_SHIFT) | 2,

With the missing close bracket, of course :)

I'll push out once I've done some testing.

Will

Mark Rutland

reviewer

2015-04-01 10:55 UTC #5 lore raw

PATCH 1/1 arm64: fix midr range for Cortex-A57 erratum 832075

In reply to Will Deacon

Quoted 48 lines · Will Deacon

Quoted 32 lines · Paul Walmsley
Quoted 27 lines · Bo Yan
Register MIDR_EL1 is masked to get variant and revision fields, then compared against midr_range_min and midr_range_max when checking whether CPU is affected by any particular erratum. However, variant and revision fields in MIDR_EL1 are separated by 16 bits, so the min and max of midr range should be constructed accordingly, otherwise the patch will not be applied when variant field is non-0.
Signed-off-by Bo Yan <[email protected]>
---
 arch/arm64/kernel/cpu_errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/cpu_errata.c +1 -1
index fa62637e63a8..7838f1578019 100644
@@ -88,7 +88,7 @@ struct arm64_cpu_capabilities arm64_errata[] = {
 	/* Cortex-A57 r0p0 - r1p2 */
 		.desc = "ARM erratum 832075",
 		.capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+		MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x100002),
 	},
 #endif
 	{
Signature · 1 line
2.1.4
Reviewed-by Paul Walmsley <[email protected]>
Quoted 2 lines
Reviewed against DDI0488G section 4.3.1 "Main ID Register, EL1". Looks like a cut-and-paste error from the A53 workarounds.
Thanks guys, I agree that this is a bug. I'll apply this, but using an explicit shift to set the variant (tweaked version below).
Will
--->8
arch/arm64/kernel/cpu_errata.c +0 -0
index a66f4fa4d541..c998345a052f 100644

arch/arm64/kernel/cpu_errata.c const struct arm64_cpu_capabilities arm64_errata[] = { · line 74

Quoted 6 lines

arch/arm64/kernel/cpu_errata.c +1 -1

@@ -70,7 +70,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
        /* Cortex-A57 r0p0 - r1p2 */
                .desc = "ARM erratum 832075",
                .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
-               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, 0x12),
+               MIDR_RANGE(MIDR_CORTEX_A57, 0x00, (1 << MIDR_VARIANT_SHIFT) | 2,
        },

That works, though it's going to make this really painful to read. Perhaps we should update MIDR_RANGE to take separate variant and revision parameters?

Then we could have MIDR_RANGE(MIDR_CORTEX_A57, 0x0, 0x0, 0x1, 0x2) for Cortex-A57 r0p0 to r1p2.

Mark.