On 12/2/24 14:13, Tom Lendacky wrote: > On 11/27/24 18:43, Sean Christopherson wrote: >> Increment the "hypercalls" stat for KVM hypercalls as soon as KVM knows >> it will skip the guest instruction, i.e. once KVM is committed to emulating >> the hypercall. Waiting until completion adds no known value, and creates a >> discrepancy where the stat will be bumped if KVM exits to userspace as a >> result of trying to skip the instruction, but not if the hypercall itself >> exits. >> >> Handling the stat in common code will also avoid the need for another >> helper to dedup code when TDX comes along (TDX needs a separate completion >> path due to GPR usage differences). >> >> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > There's a comment in the KVM_HC_MAP_GPA_RANGE case that reads: > > /* stat is incremented on completion. */ > > that should probably be deleted, but otherwise: > > Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx> > > Also, if you want, you could get rid of the 'out' label, too, by doing: > > if (cpl) > return -KVM_EPERM; Until I saw the next patch... nevermind. Thanks, Tom > >> --- >> arch/x86/kvm/x86.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 13fe5d6eb8f3..11434752b467 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -9979,7 +9979,6 @@ static int complete_hypercall_exit(struct kvm_vcpu *vcpu) >> if (!is_64_bit_hypercall(vcpu)) >> ret = (u32)ret; >> kvm_rax_write(vcpu, ret); >> - ++vcpu->stat.hypercalls; >> return kvm_skip_emulated_instruction(vcpu); >> } >> >> @@ -9990,6 +9989,8 @@ unsigned long __kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr, >> { >> unsigned long ret; >> >> + ++vcpu->stat.hypercalls; >> + >> trace_kvm_hypercall(nr, a0, a1, a2, a3); >> >> if (!op_64_bit) { >> @@ -10070,7 +10071,6 @@ unsigned long __kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr, >> } >> >> out: >> - ++vcpu->stat.hypercalls; >> return ret; >> } >> EXPORT_SYMBOL_GPL(__kvm_emulate_hypercall);