| Message ID | 20250415154656.1698522-1-scott@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] KVM: arm64: fix config.hcr used uninitialized in __kvm_at_s1e01_fast | expand |
On Tue, 15 Apr 2025 16:46:55 +0100, D Scott Phillips <scott@os.amperecomputing.com> wrote: > > In the skip_mmu_switch case, config.hcr was used uninitialized. On my > machine that caused garbage to be written to HCR_EL2 and then the CPU > got stuck at the synchronous exception handler. Also, the restore of > HCR_EL2 was missing at the end of the function in the same case. Huh, how embarrassing. Thanks for spotting this one. > > In skip_mmu_switch case, initialize config.hcr with HCR_HOST_VHE_FLAGS. > > Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> > --- > arch/arm64/kvm/at.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c > index f74a66ce3064b..ff4b06ce661af 100644 > --- a/arch/arm64/kvm/at.c > +++ b/arch/arm64/kvm/at.c > @@ -1233,8 +1233,10 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) > * the right one (as we trapped from vEL2). If not, save the > * full MMU context. > */ > - if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) > + if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) { > + config.hcr = read_sysreg(hcr_el2); > goto skip_mmu_switch; > + } > > /* > * Obtaining the S2 MMU for a L2 is horribly racy, and we may not > @@ -1299,7 +1301,9 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) > if (!fail) > par = read_sysreg_par(); > > - if (!(vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))) > + if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) > + write_sysreg(config.hcr, hcr_el2); > + else > __mmu_config_restore(&config); > > return par; I think the diff below should do the trick (and incidently matches your commit message). Thanks, M. diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c index f74a66ce3064b..773e3b4d5c7e5 100644 --- a/arch/arm64/kvm/at.c +++ b/arch/arm64/kvm/at.c @@ -1214,7 +1214,7 @@ static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) */ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) { - struct mmu_config config; + struct mmu_config config = { .hcr = HCR_HOST_VHE_FLAGS, }; struct kvm_s2_mmu *mmu; bool fail; u64 par; @@ -1301,6 +1301,8 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) if (!(vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))) __mmu_config_restore(&config); + else + write_sysreg(config.hcr, hcr_el2); return par; }
diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c index f74a66ce3064b..ff4b06ce661af 100644 --- a/arch/arm64/kvm/at.c +++ b/arch/arm64/kvm/at.c @@ -1233,8 +1233,10 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) * the right one (as we trapped from vEL2). If not, save the * full MMU context. */ - if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) + if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) { + config.hcr = read_sysreg(hcr_el2); goto skip_mmu_switch; + } /* * Obtaining the S2 MMU for a L2 is horribly racy, and we may not @@ -1299,7 +1301,9 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr) if (!fail) par = read_sysreg_par(); - if (!(vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))) + if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu)) + write_sysreg(config.hcr, hcr_el2); + else __mmu_config_restore(&config); return par;
In the skip_mmu_switch case, config.hcr was used uninitialized. On my machine that caused garbage to be written to HCR_EL2 and then the CPU got stuck at the synchronous exception handler. Also, the restore of HCR_EL2 was missing at the end of the function in the same case. In skip_mmu_switch case, initialize config.hcr with HCR_HOST_VHE_FLAGS. Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com> --- arch/arm64/kvm/at.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)