Re: [PATCH] KVM: arm64: Don't access PMCR_EL0 when no PMU is available

[Marc Zyngier <maz@kernel.org>]

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...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel