executor: arm64: Init PMUs if enabling the PMU feature #5582
Conversation
| .group = KVM_ARM_VCPU_PMU_V3_CTRL, | ||
| .attr = KVM_ARM_VCPU_PMU_V3_INIT | ||
| }; | ||
| ioctl(cpufd, KVM_SET_DEVICE_ATTR, &attrs); |
There was a problem hiding this comment.
Is it possible to uninit it back with an ioctl?
I am worried that we are restricting completeness and all states that the fuzzer can achieve.
Perhaps it's better to pass a separate flag from the fuzzer that will say to initialize PMU or not.
There was a problem hiding this comment.
I'm not sure how easy it is to uninit it, but I think the flag makes sense since conceptually it's exactly what I'm trying to do. Could you please point me to some part of the code that sets up a flag like that so that I can follow that paradigm?
| if (opt_count == 1 && (features & pmu_bit)) { | ||
| struct kvm_device_attr attrs = { | ||
| .group = KVM_ARM_VCPU_PMU_V3_CTRL, | ||
| .attr = KVM_ARM_VCPU_PMU_V3_INIT |
There was a problem hiding this comment.
Hmm, according to https://www.kernel.org/doc/Documentation/virt/kvm/devices/vcpu.rst:
If using the PMUv3 with an in-kernel virtual GIC implementation, this must be done after initializing the in-kernel irqchip.
So looks like this call has to follow syz_kvm_vgic_v3_setup().
Do you have a syzlang program working for you with this change?
Maybe we can just factor ioctl$KVM_SET_DEVICE_ATTR() outside of syz_kvm_add_vcpu() in that program?
There was a problem hiding this comment.
Agreed, decoupling it from syz_kvm_add_vcpu() sounds good (I think it'll be quite clear using a flag as suggested above).
A sample program that works with this change is:
r0 = openat$kvm(0, &AUTO='/dev/kvm\x00', 0x0, 0x0)
r1 = ioctl$KVM_CREATE_VM(r0, AUTO, 0x0)
r2 = syz_kvm_setup_syzos_vm(r1, &(0x7f0000c00000/0x400000)=nil)
r3 = syz_kvm_add_vcpu(r2, &AUTO={0x0, &AUTO=[@msr={AUTO, AUTO, {0x603000000013c4f1, 0x8000}}], AUTO}, &AUTO=[@featur1={0x1, 0x8}], 0x1)
ioctl$KVM_RUN(r3, 0xae80, 0x0)
KVM requires initializing PMUs (via KVM_SET_DEVICE_ATTR) if the PMU feature is enabled in a VCPU (via KVM_ARM_VCPU_INIT). This fix ensures that whenever we enable the PMU feature, we also initialize PMUs.
Add sys/linux/test/arm64-syz_kvm_setup_syzos_vm-enable-pmu, a seed that enables PMU and touches PMINTENSET_EL1. It was inspired by google#5582 and led to a notable (+500) coverage increase, as the fuzzer couldn't guess it should pass KVM_ARM_VCPU_PMU_V3 when creating the vCPU and set the KVM_ARM_VCPU_PMU_V3_INIT attribute at the same time.
Add sys/linux/test/arm64-syz_kvm_setup_syzos_vm-enable-pmu, a seed that enables PMU and touches PMEVCNTR0_EL0. It was inspired by google#5582 and led to a notable (+500) coverage increase, as the fuzzer couldn't previously guess that it should pass KVM_ARM_VCPU_PMU_V3 when creating the vCPU and set the KVM_ARM_VCPU_PMU_V3_INIT attribute at the same time.
Add sys/linux/test/arm64-syz_kvm_setup_syzos_vm-enable-pmu, a seed that enables PMU and touches PMEVCNTR0_EL0. It was inspired by #5582 and led to a notable (+500) coverage increase, as the fuzzer couldn't previously guess that it should pass KVM_ARM_VCPU_PMU_V3 when creating the vCPU and set the KVM_ARM_VCPU_PMU_V3_INIT attribute at the same time.
|
Given that we have PMU seeds now, I think we can close this patch. |
3 participants
KVM requires initializing PMUs (via KVM_SET_DEVICE_ATTR) if the PMU feature is enabled in a VCPU (via KVM_ARM_VCPU_INIT). This fix ensures that whenever we enable the PMU feature, we also initialize PMUs.