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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx
--
Jazz is not dead. It just smells funny...
