All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests: kvm: Mmap the entire vcpu mmap area
@ 2021-02-10 16:50 Aaron Lewis
  2021-02-10 17:41 ` Andrew Jones
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Aaron Lewis @ 2021-02-10 16:50 UTC (permalink / raw)
  To: kvm; +Cc: Aaron Lewis, Steve Rutherford

The vcpu mmap area may consist of more than just the kvm_run struct.
Allocate enough space for the entire vcpu mmap area. Without this, on
x86, the PIO page, for example, will be missing.  This is problematic
when dealing with an unhandled exception from the guest as the exception
vector will be incorrectly reported as 0x0.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
Signed-off-by: Steve Rutherford <srutherford@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index fa5a90e6c6f0..859a0b57c683 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -21,6 +21,8 @@
 #define KVM_UTIL_PGS_PER_HUGEPG 512
 #define KVM_UTIL_MIN_PFN	2
 
+static int vcpu_mmap_sz(void);
+
 /* Aligns x up to the next multiple of size. Size must be a power of 2. */
 static void *align(void *x, size_t size)
 {
@@ -509,7 +511,7 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct vcpu *vcpu)
 		vcpu->dirty_gfns = NULL;
 	}
 
-	ret = munmap(vcpu->state, sizeof(*vcpu->state));
+	ret = munmap(vcpu->state, vcpu_mmap_sz());
 	TEST_ASSERT(ret == 0, "munmap of VCPU fd failed, rc: %i "
 		"errno: %i", ret, errno);
 	close(vcpu->fd);
@@ -978,7 +980,7 @@ void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
 	TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->state), "vcpu mmap size "
 		"smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
 		vcpu_mmap_sz(), sizeof(*vcpu->state));
-	vcpu->state = (struct kvm_run *) mmap(NULL, sizeof(*vcpu->state),
+	vcpu->state = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
 		PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
 	TEST_ASSERT(vcpu->state != MAP_FAILED, "mmap vcpu_state failed, "
 		"vcpu id: %u errno: %i", vcpuid, errno);
-- 
2.30.0.478.g8a0d178c01-goog


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

* Re: [PATCH] selftests: kvm: Mmap the entire vcpu mmap area
  2021-02-10 16:50 [PATCH] selftests: kvm: Mmap the entire vcpu mmap area Aaron Lewis
@ 2021-02-10 17:41 ` Andrew Jones
  2021-02-16 16:37 ` Sean Christopherson
  2021-02-26 10:18 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Andrew Jones @ 2021-02-10 17:41 UTC (permalink / raw)
  To: Aaron Lewis; +Cc: kvm, Steve Rutherford

On Wed, Feb 10, 2021 at 08:50:36AM -0800, Aaron Lewis wrote:
> The vcpu mmap area may consist of more than just the kvm_run struct.
> Allocate enough space for the entire vcpu mmap area. Without this, on
> x86, the PIO page, for example, will be missing.  This is problematic
> when dealing with an unhandled exception from the guest as the exception
> vector will be incorrectly reported as 0x0.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> Signed-off-by: Steve Rutherford <srutherford@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index fa5a90e6c6f0..859a0b57c683 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -21,6 +21,8 @@
>  #define KVM_UTIL_PGS_PER_HUGEPG 512
>  #define KVM_UTIL_MIN_PFN	2
>  
> +static int vcpu_mmap_sz(void);
> +
>  /* Aligns x up to the next multiple of size. Size must be a power of 2. */
>  static void *align(void *x, size_t size)
>  {
> @@ -509,7 +511,7 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct vcpu *vcpu)
>  		vcpu->dirty_gfns = NULL;
>  	}
>  
> -	ret = munmap(vcpu->state, sizeof(*vcpu->state));
> +	ret = munmap(vcpu->state, vcpu_mmap_sz());
>  	TEST_ASSERT(ret == 0, "munmap of VCPU fd failed, rc: %i "
>  		"errno: %i", ret, errno);
>  	close(vcpu->fd);
> @@ -978,7 +980,7 @@ void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
>  	TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->state), "vcpu mmap size "
>  		"smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
>  		vcpu_mmap_sz(), sizeof(*vcpu->state));
> -	vcpu->state = (struct kvm_run *) mmap(NULL, sizeof(*vcpu->state),
> +	vcpu->state = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
>  		PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
>  	TEST_ASSERT(vcpu->state != MAP_FAILED, "mmap vcpu_state failed, "
>  		"vcpu id: %u errno: %i", vcpuid, errno);
> -- 
> 2.30.0.478.g8a0d178c01-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>


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

* Re: [PATCH] selftests: kvm: Mmap the entire vcpu mmap area
  2021-02-10 16:50 [PATCH] selftests: kvm: Mmap the entire vcpu mmap area Aaron Lewis
  2021-02-10 17:41 ` Andrew Jones
@ 2021-02-16 16:37 ` Sean Christopherson
  2021-02-26 10:18 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2021-02-16 16:37 UTC (permalink / raw)
  To: Aaron Lewis; +Cc: kvm, Steve Rutherford

On Wed, Feb 10, 2021, Aaron Lewis wrote:
> The vcpu mmap area may consist of more than just the kvm_run struct.
> Allocate enough space for the entire vcpu mmap area. Without this, on
> x86, the PIO page, for example, will be missing.  This is problematic
> when dealing with an unhandled exception from the guest as the exception
> vector will be incorrectly reported as 0x0.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> Signed-off-by: Steve Rutherford <srutherford@google.com>

Assuming Steve is the Co-author, this needs Co-developed-by.  And your SOB
should come after Steve's since, by virtue of sending the actual email, you're
the most recent handler of the patch.  See 'When to use Acked-by:, Cc:, and
Co-developed-by:' in Documentation/process/submitting-patches.rst.

  Co-developed-by: Steve Rutherford <srutherford@google.com>
  Signed-off-by: Steve Rutherford <srutherford@google.com>
  Signed-off-by: Aaron Lewis <aaronlewis@google.com>

> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index fa5a90e6c6f0..859a0b57c683 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -21,6 +21,8 @@
>  #define KVM_UTIL_PGS_PER_HUGEPG 512
>  #define KVM_UTIL_MIN_PFN	2
>  
> +static int vcpu_mmap_sz(void);

I'd vote to hoist the helper up instead of adding a forward declaration, but
either way works.

> +
>  /* Aligns x up to the next multiple of size. Size must be a power of 2. */
>  static void *align(void *x, size_t size)
>  {
> @@ -509,7 +511,7 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct vcpu *vcpu)
>  		vcpu->dirty_gfns = NULL;
>  	}
>  
> -	ret = munmap(vcpu->state, sizeof(*vcpu->state));
> +	ret = munmap(vcpu->state, vcpu_mmap_sz());
>  	TEST_ASSERT(ret == 0, "munmap of VCPU fd failed, rc: %i "
>  		"errno: %i", ret, errno);
>  	close(vcpu->fd);
> @@ -978,7 +980,7 @@ void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
>  	TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->state), "vcpu mmap size "
>  		"smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
>  		vcpu_mmap_sz(), sizeof(*vcpu->state));
> -	vcpu->state = (struct kvm_run *) mmap(NULL, sizeof(*vcpu->state),
> +	vcpu->state = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
>  		PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
>  	TEST_ASSERT(vcpu->state != MAP_FAILED, "mmap vcpu_state failed, "
>  		"vcpu id: %u errno: %i", vcpuid, errno);
> -- 
> 2.30.0.478.g8a0d178c01-goog
> 

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

* Re: [PATCH] selftests: kvm: Mmap the entire vcpu mmap area
  2021-02-10 16:50 [PATCH] selftests: kvm: Mmap the entire vcpu mmap area Aaron Lewis
  2021-02-10 17:41 ` Andrew Jones
  2021-02-16 16:37 ` Sean Christopherson
@ 2021-02-26 10:18 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-02-26 10:18 UTC (permalink / raw)
  To: Aaron Lewis, kvm; +Cc: Steve Rutherford

On 10/02/21 17:50, Aaron Lewis wrote:
> The vcpu mmap area may consist of more than just the kvm_run struct.
> Allocate enough space for the entire vcpu mmap area. Without this, on
> x86, the PIO page, for example, will be missing.  This is problematic
> when dealing with an unhandled exception from the guest as the exception
> vector will be incorrectly reported as 0x0.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> Signed-off-by: Steve Rutherford <srutherford@google.com>
> ---
>   tools/testing/selftests/kvm/lib/kvm_util.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index fa5a90e6c6f0..859a0b57c683 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -21,6 +21,8 @@
>   #define KVM_UTIL_PGS_PER_HUGEPG 512
>   #define KVM_UTIL_MIN_PFN	2
>   
> +static int vcpu_mmap_sz(void);
> +
>   /* Aligns x up to the next multiple of size. Size must be a power of 2. */
>   static void *align(void *x, size_t size)
>   {
> @@ -509,7 +511,7 @@ static void vm_vcpu_rm(struct kvm_vm *vm, struct vcpu *vcpu)
>   		vcpu->dirty_gfns = NULL;
>   	}
>   
> -	ret = munmap(vcpu->state, sizeof(*vcpu->state));
> +	ret = munmap(vcpu->state, vcpu_mmap_sz());
>   	TEST_ASSERT(ret == 0, "munmap of VCPU fd failed, rc: %i "
>   		"errno: %i", ret, errno);
>   	close(vcpu->fd);
> @@ -978,7 +980,7 @@ void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
>   	TEST_ASSERT(vcpu_mmap_sz() >= sizeof(*vcpu->state), "vcpu mmap size "
>   		"smaller than expected, vcpu_mmap_sz: %i expected_min: %zi",
>   		vcpu_mmap_sz(), sizeof(*vcpu->state));
> -	vcpu->state = (struct kvm_run *) mmap(NULL, sizeof(*vcpu->state),
> +	vcpu->state = (struct kvm_run *) mmap(NULL, vcpu_mmap_sz(),
>   		PROT_READ | PROT_WRITE, MAP_SHARED, vcpu->fd, 0);
>   	TEST_ASSERT(vcpu->state != MAP_FAILED, "mmap vcpu_state failed, "
>   		"vcpu id: %u errno: %i", vcpuid, errno);
> 

Queued, with SoB chain fixed as suggested by Sean.

Paolo


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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 16:50 [PATCH] selftests: kvm: Mmap the entire vcpu mmap area Aaron Lewis
2021-02-10 17:41 ` Andrew Jones
2021-02-16 16:37 ` Sean Christopherson
2021-02-26 10:18 ` 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.