| Message ID | 20240827152517.3909653-6-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Handle the lack of GICv3 exposed to a guest | expand |
On Tue, Aug 27, 2024 at 04:25:11PM +0100, Marc Zyngier wrote: > In order to be consistent, we shouldn't advertise a GICv3 when none > is actually usable by the guest. > > Wipe the feature when these conditions apply, and allow the field > to be written from userspace. > > This now allows us to rewrite the kvm_has_gicv3 helper() in terms > of kvm_has_feat(), given that it is always evaluated at runtime. This patch, which is in -next, is causing the set_id_regs tests to fail on a variety of platforms including synquacer (it looks to be everything with GICv3 which wouldn't be surprising but I didn't confirm): # selftests: kvm: set_id_regs # Random seed: 0x6b8b4567 # TAP version 13 # 1..81 # ok 1 ID_AA64DFR0_EL1_PMUVer # ok 2 ID_AA64DFR0_EL1_DebugVer ... # ok 79 ID_AA64ZFR0_EL1_SVEver # ok 80 test_vcpu_ftr_id_regs # ==== Test Assertion Failure ==== # aarch64/set_id_regs.c:449: test_reg_vals[encoding_to_range_idx(uc.args[2])] == uc.args[3] # pid=1716 tid=1716 errno=4 - Interrupted system call # 1 0x000000000040249f: test_guest_reg_read at set_id_regs.c:449 # 2 0x0000000000401cc7: main at set_id_regs.c:580 # 3 0x0000ffff9a737543: ?? ??:0 # 4 0x0000ffff9a737617: ?? ??:0 # 5 0x0000000000401e2f: _start at ??:? # 0x1001111 != 0x1111 (test_reg_vals[encoding_to_range_idx(uc.args[2])] != uc.args[3]) not ok 6 selftests: kvm: set_id_regs # exit=254 That's running test_reset_preserves_id_regs. Full log at: https://lava.sirena.org.uk/scheduler/job/661438 The bisection converges fairly smoothly: git bisect start # status: waiting for both good and bad commits # bad: [195a402a75791e6e0d96d9da27ca77671bc656a8] Add linux-next specific files for 20240828 git bisect bad 195a402a75791e6e0d96d9da27ca77671bc656a8 # status: waiting for good commit(s), bad commit known # good: [0c78c247d6e9dddf53ea0ac009ccc3399f9203ae] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git git bisect good 0c78c247d6e9dddf53ea0ac009ccc3399f9203ae # good: [319121b3e57ddefccb36ca4af417ae602c9f97bc] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git git bisect good 319121b3e57ddefccb36ca4af417ae602c9f97bc # good: [ed51c1e6d8adb0fc6ec023d7473627e03c6b0a2e] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-dt.git git bisect good ed51c1e6d8adb0fc6ec023d7473627e03c6b0a2e # bad: [125bc49fbc0e1333b9602c1200153f2763cb0a3c] Merge branch 'char-misc-next' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git git bisect bad 125bc49fbc0e1333b9602c1200153f2763cb0a3c # good: [c55ba156ad53e2b1f56b3cc53174b0860aef1b10] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/rcu/linux.git git bisect good c55ba156ad53e2b1f56b3cc53174b0860aef1b10 # bad: [8765b2fe7b05721f45e4320c2290c6a7e4f2ccd3] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git git bisect bad 8765b2fe7b05721f45e4320c2290c6a7e4f2ccd3 # bad: [bf8600945fa1536cea05731775d5f7d62e8632c8] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git git bisect bad bf8600945fa1536cea05731775d5f7d62e8632c8 # bad: [be43d82250a5d125e578065615ca805359dc58fe] Merge branch 'topic/ppc-kvm' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git git bisect bad be43d82250a5d125e578065615ca805359dc58fe # bad: [1093a3758142ce8613341ae39fb4591383d5df0a] Merge branch kvm-arm64/vgic-sre-traps into kvmarm-master/next git bisect bad 1093a3758142ce8613341ae39fb4591383d5df0a # good: [d17317a13fd0498cfa2b4bf9c4eebbe5adf929ab] Merge branch kvm-arm64/fpmr into kvmarm-master/next git bisect good d17317a13fd0498cfa2b4bf9c4eebbe5adf929ab # bad: [9f5deace58da737d67ec9c2d23534a475be68481] KVM: arm64: Add ICH_HCR_EL2 to the vcpu state git bisect bad 9f5deace58da737d67ec9c2d23534a475be68481 # good: [8d917e0a8651377321c06513f42e2ab9a86161f4] KVM: arm64: Force GICv3 trap activation when no irqchip is configured on VHE git bisect good 8d917e0a8651377321c06513f42e2ab9a86161f4 # bad: [5cb57a1aff7551bcb3b800d33141b06ef0ac178b] KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest git bisect bad 5cb57a1aff7551bcb3b800d33141b06ef0ac178b # good: [795a0bbaeee2aa993338166bc063fe3c89373d2a] KVM: arm64: Add helper for last ditch idreg adjustments git bisect good 795a0bbaeee2aa993338166bc063fe3c89373d2a # first bad commit: [5cb57a1aff7551bcb3b800d33141b06ef0ac178b] KVM: arm64: Zero ID_AA64PFR0_EL1.GIC when no GICv3 is presented to the guest
On Thu, Aug 29, 2024 at 12:22:52AM +0100, Mark Brown wrote: > On Tue, Aug 27, 2024 at 04:25:11PM +0100, Marc Zyngier wrote: > > In order to be consistent, we shouldn't advertise a GICv3 when none > > is actually usable by the guest. > > > > Wipe the feature when these conditions apply, and allow the field > > to be written from userspace. > > > > This now allows us to rewrite the kvm_has_gicv3 helper() in terms > > of kvm_has_feat(), given that it is always evaluated at runtime. > > This patch, which is in -next, is causing the set_id_regs tests to fail > on a variety of platforms including synquacer (it looks to be everything > with GICv3 which wouldn't be surprising but I didn't confirm): Mind taking this for a spin? Seems to do the trick on my end. https://lore.kernel.org/kvmarm/20240829004622.3058639-1-oliver.upton@linux.dev/
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index bc2d54da3827..e9d8e916e3af 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2365,7 +2365,6 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_AA64PFR0_EL1_MPAM | ID_AA64PFR0_EL1_SVE | ID_AA64PFR0_EL1_RAS | - ID_AA64PFR0_EL1_GIC | ID_AA64PFR0_EL1_AdvSIMD | ID_AA64PFR0_EL1_FP), }, ID_SANITISED(ID_AA64PFR1_EL1), @@ -4634,6 +4633,13 @@ int kvm_finalize_sys_regs(struct kvm_vcpu *vcpu) guard(mutex)(&kvm->arch.config_lock); + if (!(static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) && + irqchip_in_kernel(kvm) && + kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)) { + kvm->arch.id_regs[IDREG_IDX(SYS_ID_AA64PFR0_EL1)] &= ~ID_AA64PFR0_EL1_GIC_MASK; + kvm->arch.id_regs[IDREG_IDX(SYS_ID_PFR1_EL1)] &= ~ID_PFR1_EL1_GIC_MASK; + } + if (vcpu_has_nv(vcpu)) { int ret = kvm_init_nv_sysregs(kvm); if (ret) diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h index c72c38b44234..f2486b4d9f95 100644 --- a/arch/arm64/kvm/vgic/vgic.h +++ b/arch/arm64/kvm/vgic/vgic.h @@ -350,9 +350,7 @@ void vcpu_set_ich_hcr(struct kvm_vcpu *vcpu); static inline bool kvm_has_gicv3(struct kvm *kvm) { - return (static_branch_unlikely(&kvm_vgic_global_state.gicv3_cpuif) && - irqchip_in_kernel(kvm) && - kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3); + return kvm_has_feat(kvm, ID_AA64PFR0_EL1, GIC, IMP); } #endif
In order to be consistent, we shouldn't advertise a GICv3 when none is actually usable by the guest. Wipe the feature when these conditions apply, and allow the field to be written from userspace. This now allows us to rewrite the kvm_has_gicv3 helper() in terms of kvm_has_feat(), given that it is always evaluated at runtime. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/sys_regs.c | 8 +++++++- arch/arm64/kvm/vgic/vgic.h | 4 +--- 2 files changed, 8 insertions(+), 4 deletions(-)