On Wed, May 18, 2022 at 3:46 PM Uros Bizjak <ubizjak@xxxxxxxxx> wrote:
>
> Use try_cmpxchg64 instead of cmpxchg64 (*ptr, old, new) != old
> in pi_try_set_control. cmpxchg returns success in ZF flag, so this
> change saves a compare after cmpxchg (and related move instruction
> in front of cmpxchg):
>
> b9: 88 44 24 60 mov %al,0x60(%rsp)
> bd: 48 89 c8 mov %rcx,%rax
> c0: c6 44 24 62 f2 movb $0xf2,0x62(%rsp)
> c5: 48 8b 74 24 60 mov 0x60(%rsp),%rsi
> ca: f0 49 0f b1 34 24 lock cmpxchg %rsi,(%r12)
> d0: 48 39 c1 cmp %rax,%rcx
> d3: 75 cf jne a4 <vmx_vcpu_pi_load+0xa4>
>
> patched:
>
> c1: 88 54 24 60 mov %dl,0x60(%rsp)
> c5: c6 44 24 62 f2 movb $0xf2,0x62(%rsp)
> ca: 48 8b 54 24 60 mov 0x60(%rsp),%rdx
> cf: f0 48 0f b1 13 lock cmpxchg %rdx,(%rbx)
> d4: 75 d5 jne ab <vmx_vcpu_pi_load+0xab>
>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
> ---
> Patch requires commits 0aa7be05d83cc584da0782405e8007e351dfb6cc
> and c2df0a6af177b6c06a859806a876f92b072dc624 from tip.git
> ---
> arch/x86/kvm/vmx/posted_intr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c
> index 3834bb30ce54..4d41d5994a26 100644
> --- a/arch/x86/kvm/vmx/posted_intr.c
> +++ b/arch/x86/kvm/vmx/posted_intr.c
> @@ -34,7 +34,7 @@ static inline struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
> return &(to_vmx(vcpu)->pi_desc);
> }
>
> -static int pi_try_set_control(struct pi_desc *pi_desc, u64 old, u64 new)
> +static int pi_try_set_control(struct pi_desc *pi_desc, u64 *pold, u64 new)
> {
> /*
> * PID.ON can be set at any time by a different vCPU or by hardware,
> @@ -42,7 +42,7 @@ static int pi_try_set_control(struct pi_desc *pi_desc, u64 old, u64 new)
> * update must be retried with a fresh snapshot an ON change causes
> * the cmpxchg to fail.
> */
> - if (cmpxchg64(&pi_desc->control, old, new) != old)
> + if (!try_cmpxchg64(&pi_desc->control, pold, new))
Actually, we can just use the address of local variable &old here, no
need to change function arguments.
Patch v2 incoming...
Uros.
