On Sun, Mar 02, 2025 at 02:00:15PM -0800, Dongli Zhang wrote:
> Date: Sun, 2 Mar 2025 14:00:15 -0800
> From: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
> Subject: [PATCH v2 07/10] target/i386/kvm: query kvm.enable_pmu parameter
> X-Mailer: git-send-email 2.43.5
>
> There is no way to distinguish between the following scenarios:
>
> (1) KVM_CAP_PMU_CAPABILITY is not supported.
> (2) KVM_CAP_PMU_CAPABILITY is supported but disabled via the module
> parameter kvm.enable_pmu=N.
>
> In scenario (1), there is no way to fully disable AMD PMU virtualization.
>
> In scenario (2), PMU virtualization is completely disabled by the KVM
> module.
KVM_CAP_PMU_CAPABILITY is introduced since ba7bb663f554 ("KVM: x86:
Provide per VM capability for disabling PMU virtualization") in v5.18,
so I understand you want to handle the old linux before v5.18.
Let's sort out all the cases:
1) v5.18 and after, if the parameter "enable_pmu" is Y and then
KVM_CAP_PMU_CAPABILITY exists, so everything could work.
2) v5.18 and after, "enable_pmu" is N and then KVM_CAP_PMU_CAPABILITY
doesn't exist, QEMU needs to helpe user disable vPMU.
3) v5.17 (since "enable_pmu" is introduced in v5.17 since 4732f2444acd
("KVM: x86: Making the module parameter of vPMU more common")),
there's no KVM_CAP_PMU_CAPABILITY and vPMU enablement depends on
"enable_pmu". QEMU's enable_pmu option should depend on kvm
parameter.
4) before v5.17, there's no "enable_pmu" so that there's no way to
fully disable AMD PMU.
IIUC, you want to distinguish 2) and 3). And your current codes won't
break old kernels on 4) because "kvm_pmu_disabled" defaults false.
Therefore, overall the idea of this patch is good for me.
But IMO, the logics all above can be compatible by:
* First check the KVM_CAP_PMU_CAPABILITY,
* Only if KVM_CAP_PMU_CAPABILITY doesn't exist, then check the kvm parameter
...instead of always checking the parameter as you are currently doing.
What about this change? :-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 4902694129f9..9a6044e41a82 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2055,13 +2055,34 @@ int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp)
* behavior on Intel platform because current "pmu" property works
* as expected.
*/
- if (has_pmu_cap && !X86_CPU(cpu)->enable_pmu) {
- ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_PMU_CAPABILITY, 0,
- KVM_PMU_CAP_DISABLE);
- if (ret < 0) {
- error_setg_errno(errp, -ret,
- "Failed to set KVM_PMU_CAP_DISABLE");
- return ret;
+ if (has_pmu_cap) {
+ if (!X86_CPU(cpu)->enable_pmu) {
+ ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_PMU_CAPABILITY, 0,
+ KVM_PMU_CAP_DISABLE);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Failed to set KVM_PMU_CAP_DISABLE");
+ return ret;
+ }
+ }
+ } else {
+ /*
+ * KVM_CAP_PMU_CAPABILITY is introduced in Linux v5.18. For old linux,
+ * we have to check enable_pmu parameter for vPMU support.
+ */
+ g_autofree char *kvm_enable_pmu;
+
+ /*
+ * The kvm.enable_pmu's permission is 0444. It does not change until a
+ * reload of the KVM module.
+ */
+ if (g_file_get_contents("/sys/module/kvm/parameters/enable_pmu",
+ &kvm_enable_pmu, NULL, NULL)) {
+ if (*kvm_enable_pmu == 'N' && !X86_CPU(cpu)->enable_pmu) {
+ error_setg(errp, "Failed to enable PMU since "
+ "KVM's enable_pmu parameter is disabled");
+ return -1;
+ }
}
}
}
---
This example not only eliminates the static variable “kvm_pmu_disabled”,
but also explicitly informs the user that vPMU is not available and
QEMU's "pmu" option doesn't work.
As a comparison, your patch 8 actually "silently" disables PMU (in the
kvm_init_pmu_info()) and user can only find it in Guest through PMU
exceptions.
Thanks,
Zhao
