On 10/25/2024 4:26 AM, Chen, Zide wrote:
>
> On 7/31/2024 9:58 PM, Mingwei Zhang wrote:
>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 339742350b7a..34a420fa98c5 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -4394,6 +4394,97 @@ static u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx)
>> return pin_based_exec_ctrl;
>> }
>>
>> +static void vmx_set_perf_global_ctrl(struct vcpu_vmx *vmx)
>> +{
>> + u32 vmentry_ctrl = vm_entry_controls_get(vmx);
>> + u32 vmexit_ctrl = vm_exit_controls_get(vmx);
>> + struct vmx_msrs *m;
>> + int i;
>> +
>> + if (cpu_has_perf_global_ctrl_bug() ||
>> + !is_passthrough_pmu_enabled(&vmx->vcpu)) {
>> + vmentry_ctrl &= ~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
>> + vmexit_ctrl &= ~VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL;
>> + vmexit_ctrl &= ~VM_EXIT_SAVE_IA32_PERF_GLOBAL_CTRL;
>> + }
>> +
>> + if (is_passthrough_pmu_enabled(&vmx->vcpu)) {
>> + /*
>> + * Setup auto restore guest PERF_GLOBAL_CTRL MSR at vm entry.
>> + */
>> + if (vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL) {
>> + vmcs_write64(GUEST_IA32_PERF_GLOBAL_CTRL, 0);
> To save and restore Global Ctrl MSR at VMX transitions, I'm wondering if
> there are particular reasons why we prefer VMCS exec control over
> VMX-transition MSR areas? If no, I'd suggest to use the MSR area
> approach only for two reasons:
>
> 1. Simpler code. In this patch set, in total it takes ~100 LOC to handle
> the switch of this MSR.
> 2. With exec ctr approach, it needs one expensive VMCS read instruction
> to save guest Global Ctrl on every VM exit and one VMCS write in VM
> entry. (covered in patch 37)
In my opinion, there could be two reasons that we prefer to use VMCS exec
control to save/restore GLOBAL_CTRL MSR.
1. VMCS exec_control save/load is prior of MSR area save/load in
VM-Exit/VM-Entry process.
Take VM-Exit as example, the sequence (copy from SDM Chapter 28 "VM
EXITS") is
"
2. Processor state is saved in the guest-state area (Section 28.3).
3. MSRs may be saved in the VM-exit MSR-store area (Section 28.4).
4. The following may be performed in parallel and in any order (Section
28.5):
— Processor state is loaded based in part on the host-state area
and some VM-exit controls.
— Address-range monitoring is cleared.
5. MSRs may be loaded from the VM-exit MSR-load area (Section 28.6).
"
In our mediated vPMU implementation, we hope the guest counters are
disabled (Load 0 host global_ctrl) ASAP. That could help to avoid some race
condition issues, such as guest overflow PMI leaks into host in VM-Exit
process in theory, although we don't really observe it on Intel platforms.
2. Currently VMX save/restore the MSRs in MSR auto-load/restore area with
MSR index. As the recommendation from SDM, GLOBAL_CTRL MSR should be
enabled at last among all PMU MSRs. If there are multiple PMU MSRs in the
MSR auto-load/restore area, the restoring sequence may not meet this
requirement as GLOBAL_CTRL doesn't always have the largest MSR index. Of
course, we don't have this issue right now since current implementation
only save/restore one MSR global_ctrl with MSR auto-load/store area.
But yes, frequent vmcs_read()/vmcs_write() indeed brings some kind of
performance hit. We may need to do a performance evaluation and look at how
big the performance impact is.
>
>
>> + } else {
>> + m = &vmx->msr_autoload.guest;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i < 0) {
>> + i = m->nr++;
>> + vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->nr);
>> + }
>> + m->val[i].index = MSR_CORE_PERF_GLOBAL_CTRL;
>> + m->val[i].value = 0;
>> + }
>> + /*
>> + * Setup auto clear host PERF_GLOBAL_CTRL msr at vm exit.
>> + */
>> + if (vmexit_ctrl & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL) {
>> + vmcs_write64(HOST_IA32_PERF_GLOBAL_CTRL, 0);
> ditto.
>
>> + } else {
>> + m = &vmx->msr_autoload.host;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i < 0) {
>> + i = m->nr++;
>> + vmcs_write32(VM_EXIT_MSR_LOAD_COUNT, m->nr);
>> + }
>> + m->val[i].index = MSR_CORE_PERF_GLOBAL_CTRL;
>> + m->val[i].value = 0;
>> + }
>> + /*
>> + * Setup auto save guest PERF_GLOBAL_CTRL msr at vm exit
>> + */
>> + if (!(vmexit_ctrl & VM_EXIT_SAVE_IA32_PERF_GLOBAL_CTRL)) {
>> + m = &vmx->msr_autostore.guest;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i < 0) {
>> + i = m->nr++;
>> + vmcs_write32(VM_EXIT_MSR_STORE_COUNT, m->nr);
>> + }
>> + m->val[i].index = MSR_CORE_PERF_GLOBAL_CTRL;
>> + }
>> + } else {
>> + if (!(vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL)) {
>> + m = &vmx->msr_autoload.guest;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i >= 0) {
>> + m->nr--;
>> + m->val[i] = m->val[m->nr];
>> + vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->nr);
>> + }
>> + }
>> + if (!(vmexit_ctrl & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL)) {
>> + m = &vmx->msr_autoload.host;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i >= 0) {
>> + m->nr--;
>> + m->val[i] = m->val[m->nr];
>> + vmcs_write32(VM_EXIT_MSR_LOAD_COUNT, m->nr);
>> + }
>> + }
>> + if (!(vmexit_ctrl & VM_EXIT_SAVE_IA32_PERF_GLOBAL_CTRL)) {
>> + m = &vmx->msr_autostore.guest;
>> + i = vmx_find_loadstore_msr_slot(m, MSR_CORE_PERF_GLOBAL_CTRL);
>> + if (i >= 0) {
>> + m->nr--;
>> + m->val[i] = m->val[m->nr];
>> + vmcs_write32(VM_EXIT_MSR_STORE_COUNT, m->nr);
>> + }
>> + }
>> + }
>> +
>> + vm_entry_controls_set(vmx, vmentry_ctrl);
>> + vm_exit_controls_set(vmx, vmexit_ctrl);
>> +}
>> +
>> static u32 vmx_vmentry_ctrl(void)
>> {
>> u32 vmentry_ctrl = vmcs_config.vmentry_ctrl;
>
