All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c
@ 2021-02-10  3:17 Ricardo Koller
  2021-02-10  4:09 ` Nathan Chancellor
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ricardo Koller @ 2021-02-10  3:17 UTC (permalink / raw)
  To: kvm
  Cc: Paolo Bonzini, Andrew Jones, Nathan Chancellor, Ricardo Koller,
	Sean Christopherson, Jim Mattson

Building the KVM selftests with LLVM's integrated assembler fails with:

  $ CFLAGS=-fintegrated-as make -C tools/testing/selftests/kvm CC=clang
  lib/x86_64/svm.c:77:16: error: too few operands for instruction
          asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
                        ^
  <inline asm>:1:2: note: instantiated into assembly here
          vmsave
          ^
  lib/x86_64/svm.c:134:3: error: too few operands for instruction
                  "vmload\n\t"
                  ^
  <inline asm>:1:2: note: instantiated into assembly here
          vmload
          ^
This is because LLVM IAS does not currently support calling vmsave,
vmload, or vmload without an explicit %rax operand.

Add an explicit operand to vmsave, vmload, and vmrum in svm.c. Fixing
this was suggested by Sean Christopherson.

Tested: building without this error in clang 11. The following patch
(not queued yet) needs to be applied to solve the other remaining error:
"selftests: kvm: remove reassignment of non-absolute variables".

Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/kvm/X+Df2oQczVBmwEzi@google.com/
Reviewed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Ricardo Koller <ricarkol@google.com>
---
 tools/testing/selftests/kvm/lib/x86_64/svm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
index 3a5c72ed2b79..827fe6028dd4 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
@@ -74,7 +74,7 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
 	wrmsr(MSR_VM_HSAVE_PA, svm->save_area_gpa);
 
 	memset(vmcb, 0, sizeof(*vmcb));
-	asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
+	asm volatile ("vmsave %0\n\t" : : "a" (vmcb_gpa) : "memory");
 	vmcb_set_seg(&save->es, get_es(), 0, -1U, data_seg_attr);
 	vmcb_set_seg(&save->cs, get_cs(), 0, -1U, code_seg_attr);
 	vmcb_set_seg(&save->ss, get_ss(), 0, -1U, data_seg_attr);
@@ -131,19 +131,19 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
 void run_guest(struct vmcb *vmcb, uint64_t vmcb_gpa)
 {
 	asm volatile (
-		"vmload\n\t"
+		"vmload %[vmcb_gpa]\n\t"
 		"mov rflags, %%r15\n\t"	// rflags
 		"mov %%r15, 0x170(%[vmcb])\n\t"
 		"mov guest_regs, %%r15\n\t"	// rax
 		"mov %%r15, 0x1f8(%[vmcb])\n\t"
 		LOAD_GPR_C
-		"vmrun\n\t"
+		"vmrun %[vmcb_gpa]\n\t"
 		SAVE_GPR_C
 		"mov 0x170(%[vmcb]), %%r15\n\t"	// rflags
 		"mov %%r15, rflags\n\t"
 		"mov 0x1f8(%[vmcb]), %%r15\n\t"	// rax
 		"mov %%r15, guest_regs\n\t"
-		"vmsave\n\t"
+		"vmsave %[vmcb_gpa]\n\t"
 		: : [vmcb] "r" (vmcb), [vmcb_gpa] "a" (vmcb_gpa)
 		: "r15", "memory");
 }
-- 
2.30.0.478.g8a0d178c01-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c
  2021-02-10  3:17 [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c Ricardo Koller
@ 2021-02-10  4:09 ` Nathan Chancellor
  2021-02-10 17:12 ` Paolo Bonzini
  2021-02-10 17:12 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2021-02-10  4:09 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: kvm, Paolo Bonzini, Andrew Jones, Sean Christopherson, Jim Mattson

On Wed, Feb 10, 2021 at 03:17:19AM +0000, Ricardo Koller wrote:
> Building the KVM selftests with LLVM's integrated assembler fails with:
> 
>   $ CFLAGS=-fintegrated-as make -C tools/testing/selftests/kvm CC=clang
>   lib/x86_64/svm.c:77:16: error: too few operands for instruction
>           asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
>                         ^
>   <inline asm>:1:2: note: instantiated into assembly here
>           vmsave
>           ^
>   lib/x86_64/svm.c:134:3: error: too few operands for instruction
>                   "vmload\n\t"
>                   ^
>   <inline asm>:1:2: note: instantiated into assembly here
>           vmload
>           ^
> This is because LLVM IAS does not currently support calling vmsave,
> vmload, or vmload without an explicit %rax operand.

It is worth noting that this has been fixed in LLVM already, available
in 12.0.0-rc1 (final should be out in about a month or so as far as I am
aware):

http://github.com/llvm/llvm-project/commit/f47b07315a3c308a214119244b216602c537a1b2

> Add an explicit operand to vmsave, vmload, and vmrum in svm.c. Fixing
> this was suggested by Sean Christopherson.
> 
> Tested: building without this error in clang 11. The following patch
> (not queued yet) needs to be applied to solve the other remaining error:
> "selftests: kvm: remove reassignment of non-absolute variables".
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Link: https://lore.kernel.org/kvm/X+Df2oQczVBmwEzi@google.com/
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Ricardo Koller <ricarkol@google.com>

Yes, same fix as commit f65cf84ee769 ("KVM: SVM: Add register operand to
vmsave call in sev_es_vcpu_load").

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  tools/testing/selftests/kvm/lib/x86_64/svm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> index 3a5c72ed2b79..827fe6028dd4 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> @@ -74,7 +74,7 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>  	wrmsr(MSR_VM_HSAVE_PA, svm->save_area_gpa);
>  
>  	memset(vmcb, 0, sizeof(*vmcb));
> -	asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
> +	asm volatile ("vmsave %0\n\t" : : "a" (vmcb_gpa) : "memory");
>  	vmcb_set_seg(&save->es, get_es(), 0, -1U, data_seg_attr);
>  	vmcb_set_seg(&save->cs, get_cs(), 0, -1U, code_seg_attr);
>  	vmcb_set_seg(&save->ss, get_ss(), 0, -1U, data_seg_attr);
> @@ -131,19 +131,19 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>  void run_guest(struct vmcb *vmcb, uint64_t vmcb_gpa)
>  {
>  	asm volatile (
> -		"vmload\n\t"
> +		"vmload %[vmcb_gpa]\n\t"
>  		"mov rflags, %%r15\n\t"	// rflags
>  		"mov %%r15, 0x170(%[vmcb])\n\t"
>  		"mov guest_regs, %%r15\n\t"	// rax
>  		"mov %%r15, 0x1f8(%[vmcb])\n\t"
>  		LOAD_GPR_C
> -		"vmrun\n\t"
> +		"vmrun %[vmcb_gpa]\n\t"
>  		SAVE_GPR_C
>  		"mov 0x170(%[vmcb]), %%r15\n\t"	// rflags
>  		"mov %%r15, rflags\n\t"
>  		"mov 0x1f8(%[vmcb]), %%r15\n\t"	// rax
>  		"mov %%r15, guest_regs\n\t"
> -		"vmsave\n\t"
> +		"vmsave %[vmcb_gpa]\n\t"
>  		: : [vmcb] "r" (vmcb), [vmcb_gpa] "a" (vmcb_gpa)
>  		: "r15", "memory");
>  }
> -- 
> 2.30.0.478.g8a0d178c01-goog
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c
  2021-02-10  3:17 [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c Ricardo Koller
  2021-02-10  4:09 ` Nathan Chancellor
@ 2021-02-10 17:12 ` Paolo Bonzini
  2021-02-10 17:12 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-02-10 17:12 UTC (permalink / raw)
  To: Ricardo Koller, kvm
  Cc: Andrew Jones, Nathan Chancellor, Sean Christopherson, Jim Mattson

On 10/02/21 04:17, Ricardo Koller wrote:
> Building the KVM selftests with LLVM's integrated assembler fails with:
> 
>    $ CFLAGS=-fintegrated-as make -C tools/testing/selftests/kvm CC=clang
>    lib/x86_64/svm.c:77:16: error: too few operands for instruction
>            asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
>                          ^
>    <inline asm>:1:2: note: instantiated into assembly here
>            vmsave
>            ^
>    lib/x86_64/svm.c:134:3: error: too few operands for instruction
>                    "vmload\n\t"
>                    ^
>    <inline asm>:1:2: note: instantiated into assembly here
>            vmload
>            ^
> This is because LLVM IAS does not currently support calling vmsave,
> vmload, or vmload without an explicit %rax operand.
> 
> Add an explicit operand to vmsave, vmload, and vmrum in svm.c. Fixing
> this was suggested by Sean Christopherson.
> 
> Tested: building without this error in clang 11. The following patch
> (not queued yet) needs to be applied to solve the other remaining error:
> "selftests: kvm: remove reassignment of non-absolute variables".
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Link: https://lore.kernel.org/kvm/X+Df2oQczVBmwEzi@google.com/
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Ricardo Koller <ricarkol@google.com>
> ---
>   tools/testing/selftests/kvm/lib/x86_64/svm.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> index 3a5c72ed2b79..827fe6028dd4 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> @@ -74,7 +74,7 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>   	wrmsr(MSR_VM_HSAVE_PA, svm->save_area_gpa);
>   
>   	memset(vmcb, 0, sizeof(*vmcb));
> -	asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
> +	asm volatile ("vmsave %0\n\t" : : "a" (vmcb_gpa) : "memory");
>   	vmcb_set_seg(&save->es, get_es(), 0, -1U, data_seg_attr);
>   	vmcb_set_seg(&save->cs, get_cs(), 0, -1U, code_seg_attr);
>   	vmcb_set_seg(&save->ss, get_ss(), 0, -1U, data_seg_attr);
> @@ -131,19 +131,19 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>   void run_guest(struct vmcb *vmcb, uint64_t vmcb_gpa)
>   {
>   	asm volatile (
> -		"vmload\n\t"
> +		"vmload %[vmcb_gpa]\n\t"
>   		"mov rflags, %%r15\n\t"	// rflags
>   		"mov %%r15, 0x170(%[vmcb])\n\t"
>   		"mov guest_regs, %%r15\n\t"	// rax
>   		"mov %%r15, 0x1f8(%[vmcb])\n\t"
>   		LOAD_GPR_C
> -		"vmrun\n\t"
> +		"vmrun %[vmcb_gpa]\n\t"
>   		SAVE_GPR_C
>   		"mov 0x170(%[vmcb]), %%r15\n\t"	// rflags
>   		"mov %%r15, rflags\n\t"
>   		"mov 0x1f8(%[vmcb]), %%r15\n\t"	// rax
>   		"mov %%r15, guest_regs\n\t"
> -		"vmsave\n\t"
> +		"vmsave %[vmcb_gpa]\n\t"
>   		: : [vmcb] "r" (vmcb), [vmcb_gpa] "a" (vmcb_gpa)
>   		: "r15", "memory");
>   }
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c
  2021-02-10  3:17 [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c Ricardo Koller
  2021-02-10  4:09 ` Nathan Chancellor
  2021-02-10 17:12 ` Paolo Bonzini
@ 2021-02-10 17:12 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-02-10 17:12 UTC (permalink / raw)
  To: Ricardo Koller, kvm
  Cc: Andrew Jones, Nathan Chancellor, Sean Christopherson, Jim Mattson

On 10/02/21 04:17, Ricardo Koller wrote:
> Building the KVM selftests with LLVM's integrated assembler fails with:
> 
>    $ CFLAGS=-fintegrated-as make -C tools/testing/selftests/kvm CC=clang
>    lib/x86_64/svm.c:77:16: error: too few operands for instruction
>            asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
>                          ^
>    <inline asm>:1:2: note: instantiated into assembly here
>            vmsave
>            ^
>    lib/x86_64/svm.c:134:3: error: too few operands for instruction
>                    "vmload\n\t"
>                    ^
>    <inline asm>:1:2: note: instantiated into assembly here
>            vmload
>            ^
> This is because LLVM IAS does not currently support calling vmsave,
> vmload, or vmload without an explicit %rax operand.
> 
> Add an explicit operand to vmsave, vmload, and vmrum in svm.c. Fixing
> this was suggested by Sean Christopherson.
> 
> Tested: building without this error in clang 11. The following patch
> (not queued yet) needs to be applied to solve the other remaining error:
> "selftests: kvm: remove reassignment of non-absolute variables".
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Link: https://lore.kernel.org/kvm/X+Df2oQczVBmwEzi@google.com/
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Ricardo Koller <ricarkol@google.com>
> ---
>   tools/testing/selftests/kvm/lib/x86_64/svm.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> index 3a5c72ed2b79..827fe6028dd4 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> @@ -74,7 +74,7 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>   	wrmsr(MSR_VM_HSAVE_PA, svm->save_area_gpa);
>   
>   	memset(vmcb, 0, sizeof(*vmcb));
> -	asm volatile ("vmsave\n\t" : : "a" (vmcb_gpa) : "memory");
> +	asm volatile ("vmsave %0\n\t" : : "a" (vmcb_gpa) : "memory");
>   	vmcb_set_seg(&save->es, get_es(), 0, -1U, data_seg_attr);
>   	vmcb_set_seg(&save->cs, get_cs(), 0, -1U, code_seg_attr);
>   	vmcb_set_seg(&save->ss, get_ss(), 0, -1U, data_seg_attr);
> @@ -131,19 +131,19 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
>   void run_guest(struct vmcb *vmcb, uint64_t vmcb_gpa)
>   {
>   	asm volatile (
> -		"vmload\n\t"
> +		"vmload %[vmcb_gpa]\n\t"
>   		"mov rflags, %%r15\n\t"	// rflags
>   		"mov %%r15, 0x170(%[vmcb])\n\t"
>   		"mov guest_regs, %%r15\n\t"	// rax
>   		"mov %%r15, 0x1f8(%[vmcb])\n\t"
>   		LOAD_GPR_C
> -		"vmrun\n\t"
> +		"vmrun %[vmcb_gpa]\n\t"
>   		SAVE_GPR_C
>   		"mov 0x170(%[vmcb]), %%r15\n\t"	// rflags
>   		"mov %%r15, rflags\n\t"
>   		"mov 0x1f8(%[vmcb]), %%r15\n\t"	// rax
>   		"mov %%r15, guest_regs\n\t"
> -		"vmsave\n\t"
> +		"vmsave %[vmcb_gpa]\n\t"
>   		: : [vmcb] "r" (vmcb), [vmcb_gpa] "a" (vmcb_gpa)
>   		: "r15", "memory");
>   }
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-10 17:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10  3:17 [PATCH] KVM: selftests: Add operand to vmsave/vmload/vmrun in svm.c Ricardo Koller
2021-02-10  4:09 ` Nathan Chancellor
2021-02-10 17:12 ` Paolo Bonzini
2021-02-10 17:12 ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.