[0/5] KVM: x86/pmu: Hide guest counter updates from the VMRUN instruction

Message

Like Xu March 10, 2023, 10:53 a.m. UTC

Hi,

Considering that developers are more likely to have access to AMD VMs
and use vPMU inside guest, there's a dark cloud that needs to rain.
The x86_64/pmu_event_filter_test always fails on Zen3 boxes:

  test_amd_deny_list: Branch instructions retired = 43 (expected 42)
  test_without_filter: Branch instructions retired = 43 (expected 42)
  test_member_allow_list: Branch instructions retired = 43 (expected 42)
  test_not_member_deny_list: Branch instructions retired = 43 (expected 42)

,which is not caused by the event_filter feature (otherwise it's zero).

After some dubious guessing and microtesting on Zen3+ pmu hardware,
we found that VMRUN or one of the instructions in __svm_vcpu_run()
causes a guest-only enabled counter for counting guest instruction (in the
pmu_event_filter case, the branch instruction) to always increase by one
right after each vm_entry.

This creates an inconsistency with the AMD64_EVENTSEL_GUESTONLY,
where the vPMU user in the VM does not expect to see any counter
changes due to the SVM transaction at all. This patch set provides a low
overhead software fix until HW change arrives or simply no fix planned.

Prerequisite:
KVM: x86/pmu/misc: Fix a typo on kvm_pmu_request_counter_reprogam()
KVM: x86/pmu: Prevent the PMU from counting disallowed events

Please feel free to run more tests, add more or share comments.

Thanks,

Like Xu (5):
  KVM: x86/pmu: Emulate CTR overflow directly in kvm_pmu_handle_event()
  KVM: x86/pmu: Add a helper to check if pmc has PEBS mode enabled
  KVM: x86/pmu: Move the overflow of a normal counter out of PMI context
  KVM: x86/pmu: Reorder functions to reduce unnecessary declarations
  KVM: x86/pmu: Hide guest counter updates from the VMRUN instruction

 arch/x86/include/asm/kvm_host.h |  4 ++
 arch/x86/kvm/pmu.c              | 81 +++++++++++++++++++++------------
 arch/x86/kvm/pmu.h              | 37 +++++++++++++--
 arch/x86/kvm/svm/pmu.c          |  1 +
 arch/x86/kvm/svm/svm.c          | 28 ++++++++++++
 5 files changed, 119 insertions(+), 32 deletions(-)


base-commit: e79412ef245c53b2157458754327f39f43dd6a49

Comments

Sandipan Das March 13, 2023, 10:57 a.m. UTC | #1

+CC: Santosh, Tom, Ananth

Hi Like,

On 3/10/2023 4:23 PM, Like Xu wrote:
> Considering that developers are more likely to have access to AMD VMs
> and use vPMU inside guest, there's a dark cloud that needs to rain.
> The x86_64/pmu_event_filter_test always fails on Zen3 boxes:
> 
>   test_amd_deny_list: Branch instructions retired = 43 (expected 42)
>   test_without_filter: Branch instructions retired = 43 (expected 42)
>   test_member_allow_list: Branch instructions retired = 43 (expected 42)
>   test_not_member_deny_list: Branch instructions retired = 43 (expected 42)
> 
> ,which is not caused by the event_filter feature (otherwise it's zero).
> 
> After some dubious guessing and microtesting on Zen3+ pmu hardware,
> we found that VMRUN or one of the instructions in __svm_vcpu_run()
> causes a guest-only enabled counter for counting guest instruction (in the
> pmu_event_filter case, the branch instruction) to always increase by one
> right after each vm_entry.
> 
> This creates an inconsistency with the AMD64_EVENTSEL_GUESTONLY,
> where the vPMU user in the VM does not expect to see any counter
> changes due to the SVM transaction at all. This patch set provides a low
> overhead software fix until HW change arrives or simply no fix planned.
> 

Yes, VMRUNs do get counted as retired branches in the guest context. My
understanding is that this behaviour applies to all generations of Zen
and even some older ones too, not just Zen 3 and later. I also do not
expect this to change in the near future.

- Sandipan

Like Xu March 23, 2023, 8:16 a.m. UTC | #2

On 13/3/2023 6:57 pm, Sandipan Das wrote:
> +CC: Santosh, Tom, Ananth
> 
> Hi Like,
> 
> On 3/10/2023 4:23 PM, Like Xu wrote:
>> Considering that developers are more likely to have access to AMD VMs
>> and use vPMU inside guest, there's a dark cloud that needs to rain.
>> The x86_64/pmu_event_filter_test always fails on Zen3 boxes:
>>
>>    test_amd_deny_list: Branch instructions retired = 43 (expected 42)
>>    test_without_filter: Branch instructions retired = 43 (expected 42)
>>    test_member_allow_list: Branch instructions retired = 43 (expected 42)
>>    test_not_member_deny_list: Branch instructions retired = 43 (expected 42)
>>
>> ,which is not caused by the event_filter feature (otherwise it's zero).
>>
>> After some dubious guessing and microtesting on Zen3+ pmu hardware,
>> we found that VMRUN or one of the instructions in __svm_vcpu_run()
>> causes a guest-only enabled counter for counting guest instruction (in the
>> pmu_event_filter case, the branch instruction) to always increase by one
>> right after each vm_entry.
>>
>> This creates an inconsistency with the AMD64_EVENTSEL_GUESTONLY,
>> where the vPMU user in the VM does not expect to see any counter
>> changes due to the SVM transaction at all. This patch set provides a low
>> overhead software fix until HW change arrives or simply no fix planned.
>>
> 
> Yes, VMRUNs do get counted as retired branches in the guest context. My
> understanding is that this behaviour applies to all generations of Zen
> and even some older ones too, not just Zen 3 and later. I also do not
> expect this to change in the near future.
> 
> - Sandipan
> 

Interesting, thanks for confirming this issue (I presume this is an official reply).

If this hardware behavior is not expected to change, this software-based correction
would be essential and urgent for those who only use vPMU inside AMD guests.

Let's see what will happen ...