* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available [not found] <20201210083059.1277162-1-maz@kernel.org> @ 2021-01-04 15:47 ` Qian Cai 2021-01-04 16:08 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Qian Cai @ 2021-01-04 15:47 UTC (permalink / raw) To: Marc Zyngier, linux-arm-kernel, kvmarm Cc: kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: > We reset the guest's view of PMCR_EL0 unconditionally, based on > the host's view of this register. It is however legal for an > imnplementation not to provide any PMU, resulting in an UNDEF. > > The obvious fix is to skip the reset of this shadow register > when no PMU is available, sidestepping the issue entirely. > If no PMU is available, the guest is not able to request > a virtual PMU anyway, so not doing nothing is the right thing > to do! > > It is unlikely that this bug can hit any HW implementation > though, as they all provide a PMU. It has been found using nested > virt with the host KVM not implementing the PMU itself. > > Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR register") > Signed-off-by: Marc Zyngier <maz@kernel.org> Reverting this commit on the top of today's linux-next fixed a qemu-kvm coredump issue on TX2 while starting a guest. - host kernel .config: https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host -smp 2 -m 2g -drive if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd -device virtio-scsi -device scsi-hd,drive=hd -cdrom ./ubuntu-20.04-server-cloudimg.iso -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic -nic user,model=virtio,hostfwd=tcp::2222-:22 qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. > --- > arch/arm64/kvm/sys_regs.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index bc15246775d0..6c64d010102b 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -923,6 +923,10 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const > struct sys_reg_desc *r) > { > u64 pmcr, val; > > + /* No PMU available, PMCR_EL0 may UNDEF... */ > + if (!kvm_arm_support_pmu_v3()) > + return; > + > pmcr = read_sysreg(pmcr_el0); > /* > * Writable bits of PMCR_EL0 (ARMV8_PMU_PMCR_MASK) are reset to UNKNOWN ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 15:47 ` [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available Qian Cai @ 2021-01-04 16:08 ` Marc Zyngier 2021-01-04 16:22 ` Qian Cai 0 siblings, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2021-01-04 16:08 UTC (permalink / raw) To: Qian Cai Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On 2021-01-04 15:47, Qian Cai wrote: > On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: >> We reset the guest's view of PMCR_EL0 unconditionally, based on >> the host's view of this register. It is however legal for an >> imnplementation not to provide any PMU, resulting in an UNDEF. >> >> The obvious fix is to skip the reset of this shadow register >> when no PMU is available, sidestepping the issue entirely. >> If no PMU is available, the guest is not able to request >> a virtual PMU anyway, so not doing nothing is the right thing >> to do! >> >> It is unlikely that this bug can hit any HW implementation >> though, as they all provide a PMU. It has been found using nested >> virt with the host KVM not implementing the PMU itself. >> >> Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR >> register") >> Signed-off-by: Marc Zyngier <maz@kernel.org> > > Reverting this commit on the top of today's linux-next fixed a qemu-kvm > coredump > issue on TX2 while starting a guest. > > - host kernel .config: > https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config > > # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host > -smp 2 -m 2g > -drive > if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd > -device virtio-scsi -device scsi-hd,drive=hd -cdrom > ./ubuntu-20.04-server-cloudimg.iso > -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic > -nic user,model=virtio,hostfwd=tcp::2222-:22 > > qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: > pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. You don't have KVM_ARM_PMU selected in your config, so QEMU cannot access the PMU registers, and no counters are exposed. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 16:08 ` Marc Zyngier @ 2021-01-04 16:22 ` Qian Cai 2021-01-04 16:27 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Qian Cai @ 2021-01-04 16:22 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On Mon, 2021-01-04 at 16:08 +0000, Marc Zyngier wrote: > On 2021-01-04 15:47, Qian Cai wrote: > > On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: > > > We reset the guest's view of PMCR_EL0 unconditionally, based on > > > the host's view of this register. It is however legal for an > > > imnplementation not to provide any PMU, resulting in an UNDEF. > > > > > > The obvious fix is to skip the reset of this shadow register > > > when no PMU is available, sidestepping the issue entirely. > > > If no PMU is available, the guest is not able to request > > > a virtual PMU anyway, so not doing nothing is the right thing > > > to do! > > > > > > It is unlikely that this bug can hit any HW implementation > > > though, as they all provide a PMU. It has been found using nested > > > virt with the host KVM not implementing the PMU itself. > > > > > > Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR > > > register") > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > Reverting this commit on the top of today's linux-next fixed a qemu-kvm > > coredump > > issue on TX2 while starting a guest. > > > > - host kernel .config: > > https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config > > > > # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host > > -smp 2 -m 2g > > -drive > > if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd > > -device virtio-scsi -device scsi-hd,drive=hd -cdrom > > ./ubuntu-20.04-server-cloudimg.iso > > -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic > > -nic user,model=virtio,hostfwd=tcp::2222-:22 > > > > qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: > > pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. > > You don't have KVM_ARM_PMU selected in your config, so QEMU cannot > access the PMU registers, and no counters are exposed. Well, isn't it the rule that don't break the userspace? qemu works fine with KVM_ARM_PMU=n until this commit. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 16:22 ` Qian Cai @ 2021-01-04 16:27 ` Marc Zyngier 2021-01-04 18:20 ` Qian Cai 0 siblings, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2021-01-04 16:27 UTC (permalink / raw) To: Qian Cai Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On 2021-01-04 16:22, Qian Cai wrote: > On Mon, 2021-01-04 at 16:08 +0000, Marc Zyngier wrote: >> On 2021-01-04 15:47, Qian Cai wrote: >> > On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: >> > > We reset the guest's view of PMCR_EL0 unconditionally, based on >> > > the host's view of this register. It is however legal for an >> > > imnplementation not to provide any PMU, resulting in an UNDEF. >> > > >> > > The obvious fix is to skip the reset of this shadow register >> > > when no PMU is available, sidestepping the issue entirely. >> > > If no PMU is available, the guest is not able to request >> > > a virtual PMU anyway, so not doing nothing is the right thing >> > > to do! >> > > >> > > It is unlikely that this bug can hit any HW implementation >> > > though, as they all provide a PMU. It has been found using nested >> > > virt with the host KVM not implementing the PMU itself. >> > > >> > > Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR >> > > register") >> > > Signed-off-by: Marc Zyngier <maz@kernel.org> >> > >> > Reverting this commit on the top of today's linux-next fixed a qemu-kvm >> > coredump >> > issue on TX2 while starting a guest. >> > >> > - host kernel .config: >> > https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config >> > >> > # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host >> > -smp 2 -m 2g >> > -drive >> > if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd >> > -device virtio-scsi -device scsi-hd,drive=hd -cdrom >> > ./ubuntu-20.04-server-cloudimg.iso >> > -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic >> > -nic user,model=virtio,hostfwd=tcp::2222-:22 >> > >> > qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: >> > pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. >> >> You don't have KVM_ARM_PMU selected in your config, so QEMU cannot >> access the PMU registers, and no counters are exposed. > > Well, isn't it the rule that don't break the userspace? qemu works fine > with > KVM_ARM_PMU=n until this commit. No, it doesn't "work fine". It gets random data that potentially makes no sense, depending on the HW this runs on. Now, userspace tells you that your kernel is misconfigured. I see it as an improvement. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 16:27 ` Marc Zyngier @ 2021-01-04 18:20 ` Qian Cai 2021-01-04 18:26 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Qian Cai @ 2021-01-04 18:20 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On Mon, 2021-01-04 at 16:27 +0000, Marc Zyngier wrote: > On 2021-01-04 16:22, Qian Cai wrote: > > On Mon, 2021-01-04 at 16:08 +0000, Marc Zyngier wrote: > > > On 2021-01-04 15:47, Qian Cai wrote: > > > > On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: > > > > > We reset the guest's view of PMCR_EL0 unconditionally, based on > > > > > the host's view of this register. It is however legal for an > > > > > imnplementation not to provide any PMU, resulting in an UNDEF. > > > > > > > > > > The obvious fix is to skip the reset of this shadow register > > > > > when no PMU is available, sidestepping the issue entirely. > > > > > If no PMU is available, the guest is not able to request > > > > > a virtual PMU anyway, so not doing nothing is the right thing > > > > > to do! > > > > > > > > > > It is unlikely that this bug can hit any HW implementation > > > > > though, as they all provide a PMU. It has been found using nested > > > > > virt with the host KVM not implementing the PMU itself. > > > > > > > > > > Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR > > > > > register") > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > > > > > Reverting this commit on the top of today's linux-next fixed a qemu-kvm > > > > coredump > > > > issue on TX2 while starting a guest. > > > > > > > > - host kernel .config: > > > > https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config > > > > > > > > # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host > > > > -smp 2 -m 2g > > > > -drive > > > > if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd > > > > -device virtio-scsi -device scsi-hd,drive=hd -cdrom > > > > ./ubuntu-20.04-server-cloudimg.iso > > > > -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic > > > > -nic user,model=virtio,hostfwd=tcp::2222-:22 > > > > > > > > qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: > > > > pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. > > > > > > You don't have KVM_ARM_PMU selected in your config, so QEMU cannot > > > access the PMU registers, and no counters are exposed. > > > > Well, isn't it the rule that don't break the userspace? qemu works fine > > with > > KVM_ARM_PMU=n until this commit. > > No, it doesn't "work fine". It gets random data that potentially makes > no sense, > depending on the HW this runs on. > > Now, userspace tells you that your kernel is misconfigured. I see it as > an improvement. Marc, do you suggest that CONFIG_KVM=y should select KVM_ARM_PMU=y then? Otherwise, this is rather difficult for users to figure out and a core dump with an implicit error message from qemu is not that helpful. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 18:20 ` Qian Cai @ 2021-01-04 18:26 ` Marc Zyngier 2021-01-04 18:42 ` Qian Cai 0 siblings, 1 reply; 8+ messages in thread From: Marc Zyngier @ 2021-01-04 18:26 UTC (permalink / raw) To: Qian Cai Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On 2021-01-04 18:20, Qian Cai wrote: > On Mon, 2021-01-04 at 16:27 +0000, Marc Zyngier wrote: >> On 2021-01-04 16:22, Qian Cai wrote: >> > On Mon, 2021-01-04 at 16:08 +0000, Marc Zyngier wrote: >> > > On 2021-01-04 15:47, Qian Cai wrote: >> > > > On Thu, 2020-12-10 at 08:30 +0000, Marc Zyngier wrote: >> > > > > We reset the guest's view of PMCR_EL0 unconditionally, based on >> > > > > the host's view of this register. It is however legal for an >> > > > > imnplementation not to provide any PMU, resulting in an UNDEF. >> > > > > >> > > > > The obvious fix is to skip the reset of this shadow register >> > > > > when no PMU is available, sidestepping the issue entirely. >> > > > > If no PMU is available, the guest is not able to request >> > > > > a virtual PMU anyway, so not doing nothing is the right thing >> > > > > to do! >> > > > > >> > > > > It is unlikely that this bug can hit any HW implementation >> > > > > though, as they all provide a PMU. It has been found using nested >> > > > > virt with the host KVM not implementing the PMU itself. >> > > > > >> > > > > Fixes: ab9468340d2bc ("arm64: KVM: Add access handler for PMCR >> > > > > register") >> > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> >> > > > >> > > > Reverting this commit on the top of today's linux-next fixed a qemu-kvm >> > > > coredump >> > > > issue on TX2 while starting a guest. >> > > > >> > > > - host kernel .config: >> > > > https://cailca.coding.net/public/linux/mm/git/files/master/arm64.config >> > > > >> > > > # /usr/libexec/qemu-kvm -name ubuntu-20.04-server-cloudimg -cpu host >> > > > -smp 2 -m 2g >> > > > -drive >> > > > if=none,format=qcow2,file=./ubuntu-20.04-server-cloudimg.qcow2,id=hd >> > > > -device virtio-scsi -device scsi-hd,drive=hd -cdrom >> > > > ./ubuntu-20.04-server-cloudimg.iso >> > > > -bios /usr/share/AAVMF/AAVMF_CODE.fd -M gic-version=host -nographic >> > > > -nic user,model=virtio,hostfwd=tcp::2222-:22 >> > > > >> > > > qemu-kvm: /builddir/build/BUILD/qemu-4.2.0/target/arm/helper.c:1812: >> > > > pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed. >> > > >> > > You don't have KVM_ARM_PMU selected in your config, so QEMU cannot >> > > access the PMU registers, and no counters are exposed. >> > >> > Well, isn't it the rule that don't break the userspace? qemu works fine >> > with >> > KVM_ARM_PMU=n until this commit. >> >> No, it doesn't "work fine". It gets random data that potentially makes >> no sense, >> depending on the HW this runs on. >> >> Now, userspace tells you that your kernel is misconfigured. I see it >> as >> an improvement. > > Marc, do you suggest that CONFIG_KVM=y should select KVM_ARM_PMU=y > then? > Otherwise, this is rather difficult for users to figure out and a core > dump with > an implicit error message from qemu is not that helpful. What I'm suggesting is this [1], which is to get rid of KVM_ARM_PMU completely. At least, the kernel configuration will be consistent. Overall, I think there is an issue with KVM exposing more than it should to userspace when no PMU is defined, but I don't think that's the problem you are seeing. M. [1] https://lore.kernel.org/r/20210104172723.2014324-1-maz@kernel.org -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 18:26 ` Marc Zyngier @ 2021-01-04 18:42 ` Qian Cai 2021-01-04 19:32 ` Marc Zyngier 0 siblings, 1 reply; 8+ messages in thread From: Qian Cai @ 2021-01-04 18:42 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On Mon, 2021-01-04 at 18:26 +0000, Marc Zyngier wrote: > What I'm suggesting is this [1], which is to get rid of KVM_ARM_PMU > completely. At least, the kernel configuration will be consistent. > Do you have a patch for CONFIG_KVM to select HW_PERF_EVENTS then? I could cook one if not. > Overall, I think there is an issue with KVM exposing more than it > should to userspace when no PMU is defined, but I don't think that's > the problem you are seeing. > > M. > > [1] https://lore.kernel.org/r/20210104172723.2014324-1-maz@kernel.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available 2021-01-04 18:42 ` Qian Cai @ 2021-01-04 19:32 ` Marc Zyngier 0 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2021-01-04 19:32 UTC (permalink / raw) To: Qian Cai Cc: linux-arm-kernel, kvmarm, kernel-team, Stephen Rothwell, Linux Next Mailing List, Alexandru Elisei On 2021-01-04 18:42, Qian Cai wrote: > On Mon, 2021-01-04 at 18:26 +0000, Marc Zyngier wrote: >> What I'm suggesting is this [1], which is to get rid of KVM_ARM_PMU >> completely. At least, the kernel configuration will be consistent. >> > > Do you have a patch for CONFIG_KVM to select HW_PERF_EVENTS then? I > could cook > one if not. I don't think there should be such a patch. People do disable HW_PERF_EVENTS in production in some cases, and we should honor that. All I am trying to guarantee at the moment is that the KVM configuration is consistent, as I believe that's what broke in your particular case. What needs doing is to hide the PMU registers from userspace when no PMU is configured, or even available. I'll try and post something to that effect tomorrow (hey, I'm still officially on holiday...). M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-04 19:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20201210083059.1277162-1-maz@kernel.org>
2021-01-04 15:47 ` [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available Qian Cai
2021-01-04 16:08 ` Marc Zyngier
2021-01-04 16:22 ` Qian Cai
2021-01-04 16:27 ` Marc Zyngier
2021-01-04 18:20 ` Qian Cai
2021-01-04 18:26 ` Marc Zyngier
2021-01-04 18:42 ` Qian Cai
2021-01-04 19:32 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).