Message ID | 20240315174939.2530483-1-pbonzini@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] KVM changes for Linux 6.9 merge window | expand |
The pull request you sent on Fri, 15 Mar 2024 13:49:39 -0400:
> https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4f712ee0cbbd5c777d270427092bb301fc31044f
Thank you!
On Fri, 15 Mar 2024 at 10:49, Paolo Bonzini <pbonzini@redhat.com> wrote: > > https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus Argh. This causes my arm64 build to fail, but since I don't do that between every pull, I didn't notice until after I had already pushed things out. I get a failure on arch/arm64/kvm/check-res-bits.h (line 60): BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))); and at least in my build, the generated sysreg-defs.h file has #define ID_AA64DFR1_EL1_RES0 (UL(0)) so yeah, it most definitely doesn't match that GENMASK_ULL(63, 0). I did *not* go delve into how arch/arm64/tools/gen-sysreg.awk works. I don't really do awk any more. The immediate cause of the failure is commit b80b701d5a67 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking") but I hope it worked at *some* point. I can't see how. I would guess / assume that commit cfc680bb04c5 ("arm64: sysreg: Add layout for ID_AA64MMFR4_EL1") is also involved, but having recoiled in horror from the awk script, I really can't even begin to guess at what is going on. Bringing in other people who hopefully can sort this out. Linus
On Fri, Mar 15, 2024 at 03:28:29PM -0700, Linus Torvalds wrote: > The immediate cause of the failure is commit b80b701d5a67 ("KVM: > arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later > checking") but I hope it worked at *some* point. I can't see how. Looks like commit fdd867fe9b32 ("arm64/sysreg: Add register fields for ID_AA64DFR1_EL1") changed the register definition that tripped the BUILD_BUG_ON(). But it'd be *wildly* unfair to blame that, the KVM assertions are added out of fear of new register definitions breaking our sysreg emulation. > I would guess / assume that commit cfc680bb04c5 ("arm64: sysreg: Add > layout for ID_AA64MMFR4_EL1") is also involved, but having recoiled in > horror from the awk script, I really can't even begin to guess at what > is going on. > > Bringing in other people who hopefully can sort this out. At this point I'm heavily biased towards just dropping the KVM checks for now than attempt a fix-forward. We can work things out better with arm64 folks next release. So unless anyone screams, I say we revert: 99101dda29e3 ("KVM: arm64: Make build-time check of RES0/RES1 bits optional") 891766581dea ("KVM: arm64: Add debugfs file for guest's ID registers") and do so atomically to avoid any further breakage of bisection.
On Fri, Mar 15, 2024 at 04:32:10PM -0700, Oliver Upton wrote: > On Fri, Mar 15, 2024 at 03:28:29PM -0700, Linus Torvalds wrote: > > The immediate cause of the failure is commit b80b701d5a67 ("KVM: > > arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later > > checking") but I hope it worked at *some* point. I can't see how. > > Looks like commit fdd867fe9b32 ("arm64/sysreg: Add register fields for > ID_AA64DFR1_EL1") changed the register definition that tripped the > BUILD_BUG_ON(). > > But it'd be *wildly* unfair to blame that, the KVM assertions are added > out of fear of new register definitions breaking our sysreg emulation. > > > I would guess / assume that commit cfc680bb04c5 ("arm64: sysreg: Add > > layout for ID_AA64MMFR4_EL1") is also involved, but having recoiled in > > horror from the awk script, I really can't even begin to guess at what > > is going on. > > > > Bringing in other people who hopefully can sort this out. > > At this point I'm heavily biased towards just dropping the KVM checks > for now than attempt a fix-forward. We can work things out better with > arm64 folks next release. > > So unless anyone screams, I say we revert: > > 99101dda29e3 ("KVM: arm64: Make build-time check of RES0/RES1 bits optional") > 891766581dea ("KVM: arm64: Add debugfs file for guest's ID registers") Duh, that second one should actually be: b80b701d5a67 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
Hi Linus, On Fri, 15 Mar 2024 15:28:29 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Fri, 15 Mar 2024 at 10:49, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > https://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus > > Argh. > > This causes my arm64 build to fail, but since I don't do that between > every pull, I didn't notice until after I had already pushed things > out. > > I get a failure on arch/arm64/kvm/check-res-bits.h (line 60): > > BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))); https://lore.kernel.org/linux-next/20240222220349.1889c728@canb.auug.org.au/
On Sat, Mar 16, 2024 at 12:50 AM Oliver Upton <oliver.upton@linux.dev> wrote: > > On Fri, Mar 15, 2024 at 04:32:10PM -0700, Oliver Upton wrote: > > On Fri, Mar 15, 2024 at 03:28:29PM -0700, Linus Torvalds wrote: > > > The immediate cause of the failure is commit b80b701d5a67 ("KVM: > > > arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later > > > checking") but I hope it worked at *some* point. I can't see how. > > > > Looks like commit fdd867fe9b32 ("arm64/sysreg: Add register fields for > > ID_AA64DFR1_EL1") changed the register definition that tripped the > > BUILD_BUG_ON(). > > > > But it'd be *wildly* unfair to blame that, the KVM assertions are added > > out of fear of new register definitions breaking our sysreg emulation. > > > > > I would guess / assume that commit cfc680bb04c5 ("arm64: sysreg: Add > > > layout for ID_AA64MMFR4_EL1") is also involved, but having recoiled in > > > horror from the awk script, I really can't even begin to guess at what > > > is going on. Linus, were you compiling with allyesconfig so that you got CONFIG_KVM_ARM64_RES_BITS_PARANOIA on? > > So unless anyone screams, I say we revert: > > > > 99101dda29e3 ("KVM: arm64: Make build-time check of RES0/RES1 bits optional") Yes, in retrospect it's kinda obvious that, even if it cures default config, allyesconfig still fails with this change. > b80b701d5a67 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking") You can also make CONFIG_KVM_ARM64_RES_BITS_PARANOIA depend on !COMPILE_TEST. Paolo
On Sat, 16 Mar 2024 at 01:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Linus, were you compiling with allyesconfig so that you got > CONFIG_KVM_ARM64_RES_BITS_PARANOIA on? Regular allmodconfig. > You can also make CONFIG_KVM_ARM64_RES_BITS_PARANOIA depend on !COMPILE_TEST. No. WTF is wrong with you? You're saying "let's turn off this compile-time sanity check when we're doing compile testing". That's insane. The sanity check was WRONG. People hadn't tested it. Stephen points out that it was reported to you almost a month ago in https://lore.kernel.org/linux-next/20240222220349.1889c728@canb.auug.org.au/ and you're still trying to just *HIDE* this garbage? Stop it. Linus
On Sat, 16 Mar 2024 16:01:47 +0000, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sat, 16 Mar 2024 at 01:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > Linus, were you compiling with allyesconfig so that you got > > CONFIG_KVM_ARM64_RES_BITS_PARANOIA on? > > Regular allmodconfig. > > > You can also make CONFIG_KVM_ARM64_RES_BITS_PARANOIA depend on !COMPILE_TEST. > > No. > > WTF is wrong with you? > > You're saying "let's turn off this compile-time sanity check when > we're doing compile testing". > > That's insane. > > The sanity check was WRONG. People hadn't tested it. Stephen points > out that it was reported to you almost a month ago in > > https://lore.kernel.org/linux-next/20240222220349.1889c728@canb.auug.org.au/ > > and you're still trying to just *HIDE* this garbage? > > Stop it. Well, if you really need to shout at someone, it should be me, as I was the one who didn't get Stephen's hint last time. I'll try to resurrect it as a selftest, or maybe just keep it out of tree for my own use. M.
Retrying without HTML. Paolo Il 17 marzo 2024 14:34:02 CET, Paolo Bonzini <pbonzini@redhat.com> ha scritto: >[first time writing to lkml from phone so I hope the formatting isn't too bad] > >Il 17 marzo 2024 11:36:37 CET, Marc Zyngier <maz@kernel.org> ha scritto: >>On Sat, 16 Mar 2024 16:01:47 +0000, >>Linus Torvalds <torvalds@linux-foundation.org> wrote: >>> > You can also make CONFIG_KVM_ARM64_RES_BITS_PARANOIA depend on !COMPILE_TEST. >>> >>> No. >>> >>> WTF is wrong with you? >>> >>> You're saying "let's turn off this compile-time sanity check when >>> we're doing compile testing". >>> https://lore.kernel.org/linux-next/20240222220349.1889c728@canb.auug.org.au/ >>> >>> and you're still trying to just *HIDE* this garbage? >>> >>> Stop it. >> >>Well, if you really need to shout at someone, it should be me, as I >>was the one who didn't get Stephen's hint last time. > >No problem with being shouted at, but "depends on !COMPILE_TEST" is actually something that *is* used for "maintainers will look at it, it shouldn't matter for linux-next compile testing". Most notably it's used for -Werror. > >When Stephen reported the failure, I should have noticed that the bandaid doesn't do anything to fix allyesconfig/allmodconfig. If there's anything I can blame you for, I thought/understood that you would be able to fix the failure between the report and the beginning of the merge window, so there's that small miscommunication but that's it. > >>I'll try to resurrect it as a selftest, or maybe just keep it out of >>tree for my own use. > >I still believe that "depends on !COMPILE_TEST" is what you want here, but yeah keeping out of tree or even under a special make target is an option if Linus disagrees. > >Selftests have the advantage that they can be marked XFAIL, but I am not sure they're a good match here (also because the flip side is that I think XPASS fails the run). > >Paolo Paolo
diff --cc arch/arm64/include/asm/kvm_arm.h index 7f45ce9170bb,a1769e415d72..000000000000 --- a/arch/arm64/include/asm/kvm_arm.h diff --cc arch/arm64/kernel/cpufeature.c index d6679d8b737e,f309fd542c20..000000000000 --- a/arch/arm64/kernel/cpufeature.c