Skip to contentSkip to conversation

KVM: arm64: vgic-v3: Restrict SEIS workaround to known broken systems

PATCH In review Marc Zyngier 2 participants 3 messages Jan 22, 2022

Subsystems (3)
KVM arm64 KVM arm64
lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

PATCH 1/1 KVM: arm64: vgic-v3: Restrict SEIS workaround to known broken systems

Contrary to what df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors") was asserting, there is at least one other system out there (Cavium ThunderX2) implementing SEIS, and not in an obviously broken way.

So instead of imposing the M1 workaround on an innocent bystander, let's limit it to the two known broken Apple implementations.

Fixes df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors")
Reported-by Ard Biesheuvel <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c |  3 +++
 arch/arm64/kvm/vgic/vgic-v3.c   | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +3 -0
index 20db2f281cf2..4fb419f7b8b6 100644
@@ -983,6 +983,9 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
 	val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
 	/* IDbits */
 	val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
+	/* SEIS */
+	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK)
+		val |= BIT(ICC_CTLR_EL1_SEIS_SHIFT);
 	/* A3V */
 	val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
 	/* EOImode */
arch/arm64/kvm/vgic/vgic-v3.c +15 -2
index 78cf674c1230..d34a795f730c 100644
@@ -609,6 +609,18 @@ static int __init early_gicv4_enable(char *buf)
 }
 early_param("kvm-arm.vgic_v4_enable", early_gicv4_enable);
 
+static struct midr_range broken_seis[] = {
+	MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM),
+	MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM),
+	{},
+};
+
+static bool vgic_v3_broken_seis(void)
+{
+	return ((kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) &&
+		is_midr_in_range_list(read_cpuid_id(), broken_seis));
+}
+
 /**
  * vgic_v3_probe - probe for a VGICv3 compatible interrupt controller
  * @info:	pointer to the GIC description

@@ -676,9 +688,10 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
 		group1_trap = true;
 	}
 
-	if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
-		kvm_info("GICv3 with locally generated SEI\n");
+	if (vgic_v3_broken_seis()) {
+		kvm_info("GICv3 with broken locally generated SEI\n");
 
+		kvm_vgic_global_state.ich_vtr_el2 &= ~ICH_VTR_SEIS_MASK;
 		group0_trap = true;
 		group1_trap = true;
 		if (ich_vtr_el2 & ICH_VTR_TDS_MASK)
Signature · 1 line
2.34.1
Tested-by Acked-by on v1 1/1
#1 lore raw
Quoted 6 lines · Marc Zyngier

Contrary to what df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors") was asserting, there is at least one other system out there (Cavium ThunderX2) implementing SEIS, and not in an obviously broken way.

Quoted 6 lines

So instead of imposing the M1 workaround on an innocent bystander, let's limit it to the two known broken Apple implementations.

Fixes df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors")
Reported-by Ard Biesheuvel <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

Thanks for the fix.

Tested-by Ard Biesheuvel <[email protected]>
Acked-by Ard Biesheuvel <[email protected]>

One nit below.

Quoted 24 lines
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c |  3 +++
 arch/arm64/kvm/vgic/vgic-v3.c   | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +3 -0
index 20db2f281cf2..4fb419f7b8b6 100644
@@ -983,6 +983,9 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu *vcpu, u32 vmcr, int rt)
        val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
        /* IDbits */
        val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
+       /* SEIS */
+       if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK)
+               val |= BIT(ICC_CTLR_EL1_SEIS_SHIFT);
        /* A3V */
        val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;
        /* EOImode */
arch/arm64/kvm/vgic/vgic-v3.c +0 -0
index 78cf674c1230..d34a795f730c 100644

arch/arm64/kvm/vgic/vgic-v3.c static int __init early_gicv4_enable(char *buf) · line 612

Quoted 4 lines
arch/arm64/kvm/vgic/vgic-v3.c +1 -0
@@ -609,6 +609,18 @@ static int __init early_gicv4_enable(char *buf)
 }
 early_param("kvm-arm.vgic_v4_enable", early_gicv4_enable);
 
+static struct midr_range broken_seis[] = {

Can this be const?

Quoted 28 lines
arch/arm64/kvm/vgic/vgic-v3.c +14 -3
+       MIDR_ALL_VERSIONS(MIDR_APPLE_M1_ICESTORM),
+       MIDR_ALL_VERSIONS(MIDR_APPLE_M1_FIRESTORM),
+       {},
+};
+
+static bool vgic_v3_broken_seis(void)
+{
+       return ((kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) &&
+               is_midr_in_range_list(read_cpuid_id(), broken_seis));
+}
+
 /**
  * vgic_v3_probe - probe for a VGICv3 compatible interrupt controller
  * @info:      pointer to the GIC description

@@ -676,9 +688,10 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
                group1_trap = true;
        }
 
-       if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK) {
-               kvm_info("GICv3 with locally generated SEI\n");
+       if (vgic_v3_broken_seis()) {
+               kvm_info("GICv3 with broken locally generated SEI\n");
 
+               kvm_vgic_global_state.ich_vtr_el2 &= ~ICH_VTR_SEIS_MASK;
                group0_trap = true;
                group1_trap = true;
                if (ich_vtr_el2 & ICH_VTR_TDS_MASK)
--
Quoted 2 lines

2.34.1

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

author maintainer
#2 lore raw
Quoted 10 lines · Ard Biesheuvel
Quoted 9 lines · Marc Zyngier

Contrary to what df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors") was asserting, there is at least one other system out there (Cavium ThunderX2) implementing SEIS, and not in an obviously broken way.

So instead of imposing the M1 workaround on an innocent bystander, let's limit it to the two known broken Apple implementations.

Quoted 8 lines
Quoted 3 lines
Fixes df652bcf1136 ("KVM: arm64: vgic-v3: Work around GICv3 locally generated SErrors")
Reported-by Ard Biesheuvel <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>

Thanks for the fix.

Tested-by Ard Biesheuvel <[email protected]>
Acked-by Ard Biesheuvel <[email protected]>

Thanks for that.

Quoted 31 lines

One nit below.

Quoted 28 lines
---
 arch/arm64/kvm/hyp/vgic-v3-sr.c |  3 +++
 arch/arm64/kvm/vgic/vgic-v3.c   | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vgic-v3-sr.c +3 -0
index 20db2f281cf2..4fb419f7b8b6 100644
@@ -983,6 +983,9 @@ static void __vgic_v3_read_ctlr(struct kvm_vcpu 
 *vcpu, u32 vmcr, int rt)
        val = ((vtr >> 29) & 7) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
        /* IDbits */
        val |= ((vtr >> 23) & 7) << ICC_CTLR_EL1_ID_BITS_SHIFT;
+       /* SEIS */
+       if (kvm_vgic_global_state.ich_vtr_el2 & ICH_VTR_SEIS_MASK)
+               val |= BIT(ICC_CTLR_EL1_SEIS_SHIFT);
        /* A3V */
        val |= ((vtr >> 21) & 1) << ICC_CTLR_EL1_A3V_SHIFT;

        /* EOImode */
arch/arm64/kvm/vgic/vgic-v3.c +0 -0
index 78cf674c1230..d34a795f730c 100644
@@ -609,6 +609,18 @@ static int __init early_gicv4_enable(char *buf)
 }
 early_param("kvm-arm.vgic_v4_enable", early_gicv4_enable);
Quoted 3 lines

arch/arm64/kvm/vgic/vgic-v3.c static int __init early_gicv4_enable(char *buf) · line 612

Quoted 1 line
arch/arm64/kvm/vgic/vgic-v3.c +1 -0
+static struct midr_range broken_seis[] = {

Can this be const?

Absolutely. I'll fold that in.

Thanks,

        M.
Signature · 1 line
Jazz is not dead. It just smells funny...