public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] KVM: selftests: Fix wrmsr_safe()
Date: Wed, 13 Jul 2022 15:41:04 +0000	[thread overview]
Message-ID: <Ys7nkBcfYlSuF7rt@google.com> (raw)
In-Reply-To: <20220713150532.1012466-3-vkuznets@redhat.com>

On Wed, Jul 13, 2022, Vitaly Kuznetsov wrote:
> It seems to be a misconception that "A" places an u64 operand to
> EAX:EDX, at least with GCC11.

It's not a misconception, it's just that the "A" trick only works for 32-bit
binaries.  For 64-bit, the 64-bit integer fits into "rax" without needing to spill
into "rdx".

I swear I had fixed this, but apparently I had only done that locally and never
pushed/posted the changes :-/

> While writing a new test, I've noticed that wrmsr_safe() tries putting
> garbage to the upper bits of the MSR, e.g.:
> 
>   kvm_exit:             reason MSR_WRITE rip 0x402919 info 0 0
>   kvm_msr:              msr_write 40000118 = 0x60000000001 (#GP)
> ...
> when it was supposed to write '1'. Apparently, "A" works the same as
> "a" and not as EAX/EDX. Here's the relevant disassembled part:
> 
> With "A":
> 
> 	48 8b 43 08          	mov    0x8(%rbx),%rax
> 	49 b9 ba da ca ba 0a 	movabs $0xabacadaba,%r9
> 	00 00 00
> 	4c 8d 15 07 00 00 00 	lea    0x7(%rip),%r10        # 402f44 <guest_msr+0x34>
> 	4c 8d 1d 06 00 00 00 	lea    0x6(%rip),%r11        # 402f4a <guest_msr+0x3a>
> 	0f 30                	wrmsr
> 
> With "a"/"d":
> 
> 	48 8b 43 08          	mov    0x8(%rbx),%rax
> 	48 89 c2             	mov    %rax,%rdx
> 	48 c1 ea 20          	shr    $0x20,%rdx
> 	49 b9 ba da ca ba 0a 	movabs $0xabacadaba,%r9
> 	00 00 00
> 	4c 8d 15 07 00 00 00 	lea    0x7(%rip),%r10        # 402fc3 <guest_msr+0xb3>
> 	4c 8d 1d 06 00 00 00 	lea    0x6(%rip),%r11        # 402fc9 <guest_msr+0xb9>
> 	0f 30                	wrmsr
> 
> I was only able to find one online reference that "A" gives "eax and
> edx combined into a 64-bit integer", other places don't mention it at
> all.
> 
> Fixes: 3b23054cd3f5 ("KVM: selftests: Add x86-64 support for exception fixup")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  tools/testing/selftests/kvm/include/x86_64/processor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 79dcf6be1b47..3d412c578e78 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -612,7 +612,7 @@ static inline uint8_t rdmsr_safe(uint32_t msr, uint64_t *val)
>  
>  static inline uint8_t wrmsr_safe(uint32_t msr, uint64_t val)
>  {
> -	return kvm_asm_safe("wrmsr", "A"(val), "c"(msr));
> +	return kvm_asm_safe("wrmsr", "a"((u32)val), "d"(val >> 32), "c"(msr));
>  }
>  
>  uint64_t vm_get_page_table_entry(struct kvm_vm *vm, struct kvm_vcpu *vcpu,
> -- 
> 2.35.3
> 

  reply	other threads:[~2022-07-13 15:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-13 15:05 [PATCH 0/3] KVM: x86: Hyper-V invariant TSC control feature Vitaly Kuznetsov
2022-07-13 15:05 ` [PATCH 1/3] KVM: x86: Hyper-V invariant TSC control Vitaly Kuznetsov
2022-07-14  9:25   ` Maxim Levitsky
2022-07-14 15:05     ` Vitaly Kuznetsov
2022-07-13 15:05 ` [PATCH 2/3] KVM: selftests: Fix wrmsr_safe() Vitaly Kuznetsov
2022-07-13 15:41   ` Sean Christopherson [this message]
2022-07-14  0:52     ` Sean Christopherson
2022-07-13 15:05 ` [PATCH 3/3] KVM: selftests: Test Hyper-V invariant TSC control Vitaly Kuznetsov
2022-07-14  9:26   ` Maxim Levitsky
2022-07-14 14:57     ` Vitaly Kuznetsov
2022-07-14  9:24 ` [PATCH 0/3] KVM: x86: Hyper-V invariant TSC control feature Maxim Levitsky
2022-07-14 15:02   ` Vitaly Kuznetsov
2022-07-18 15:07     ` Michael Kelley (LINUX)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ys7nkBcfYlSuF7rt@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox