diff mbox series

[2/4] kvm: selftests: introduce vm_vcpu_add

Message ID 20190523125756.4645-3-drjones@redhat.com (mailing list archive)
State New, archived
Headers show
Series kvm: selftests: aarch64: use struct kvm_vcpu_init | expand

Commit Message

Andrew Jones May 23, 2019, 12:57 p.m. UTC
vm_vcpu_add() just adds a vcpu to the vm, but doesn't do any
additional vcpu setup.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  1 +
 tools/testing/selftests/kvm/lib/kvm_util.c    | 32 +++++++++++++++----
 2 files changed, 26 insertions(+), 7 deletions(-)

Comments

Peter Xu May 27, 2019, 6:35 a.m. UTC | #1
On Thu, May 23, 2019 at 02:57:54PM +0200, Andrew Jones wrote:
> vm_vcpu_add() just adds a vcpu to the vm, but doesn't do any
> additional vcpu setup.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c    | 32 +++++++++++++++----
>  2 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 4e92f34cf46a..32fabbc98803 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -88,6 +88,7 @@ int _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long ioctl,
>  		void *arg);
>  void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
>  void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
> +void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
>  void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
>  			  int gdt_memslot);
>  vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 937292dca81b..ae6d4b274ddd 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -756,23 +756,20 @@ static int vcpu_mmap_sz(void)
>  }
>  
>  /*
> - * VM VCPU Add with provided memslots
> + * VM VCPU Add
>   *
>   * Input Args:
>   *   vm - Virtual Machine
>   *   vcpuid - VCPU ID
> - *   pgd_memslot - Memory region slot for new virtual translation tables
> - *   gdt_memslot - Memory region slot for data pages

Would it make sense to squash the first two patches together?  They
are somehow related, and also no lines will be added and quickly removed.

Nitpicking on the name: vm_vcpu_add_memslots() makes me feel like
"vcpu is adding memslots" rather than adding vcpu itself.  How about
vm_vcpu_add_with_memslots()?

Thanks,

>   *
>   * Output Args: None
>   *
>   * Return: None
>   *
> - * Adds a virtual CPU to the VM specified by vm with the ID given by vcpuid
> - * and then sets it up with vcpu_setup() and the provided memslots.
> + * Adds a virtual CPU to the VM specified by vm with the ID given by vcpuid.
> + * No additional VCPU setup is done.
>   */
> -void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
> -			  int gdt_memslot)
> +void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
>  {
>  	struct vcpu *vcpu;
>  
> @@ -806,7 +803,28 @@ void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
>  		vm->vcpu_head->prev = vcpu;
>  	vcpu->next = vm->vcpu_head;
>  	vm->vcpu_head = vcpu;
> +}
>  
> +/*
> + * VM VCPU Add with provided memslots
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   pgd_memslot - Memory region slot for new virtual translation tables
> + *   gdt_memslot - Memory region slot for data pages
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Adds a virtual CPU the VM specified by vm with the ID given by vcpuid
> + * and then sets it up with vcpu_setup() and the provided memslots.
> + */
> +void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
> +			  int gdt_memslot)
> +{
> +	vm_vcpu_add(vm, vcpuid);
>  	vcpu_setup(vm, vcpuid, pgd_memslot, gdt_memslot);
>  }
>  
> -- 
> 2.20.1
> 

Regards,
Andrew Jones May 27, 2019, 7:09 a.m. UTC | #2
On Mon, May 27, 2019 at 02:35:52PM +0800, Peter Xu wrote:
> On Thu, May 23, 2019 at 02:57:54PM +0200, Andrew Jones wrote:
> > vm_vcpu_add() just adds a vcpu to the vm, but doesn't do any
> > additional vcpu setup.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  .../testing/selftests/kvm/include/kvm_util.h  |  1 +
> >  tools/testing/selftests/kvm/lib/kvm_util.c    | 32 +++++++++++++++----
> >  2 files changed, 26 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 4e92f34cf46a..32fabbc98803 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -88,6 +88,7 @@ int _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long ioctl,
> >  		void *arg);
> >  void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
> >  void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
> > +void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
> >  void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
> >  			  int gdt_memslot);
> >  vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 937292dca81b..ae6d4b274ddd 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -756,23 +756,20 @@ static int vcpu_mmap_sz(void)
> >  }
> >  
> >  /*
> > - * VM VCPU Add with provided memslots
> > + * VM VCPU Add
> >   *
> >   * Input Args:
> >   *   vm - Virtual Machine
> >   *   vcpuid - VCPU ID
> > - *   pgd_memslot - Memory region slot for new virtual translation tables
> > - *   gdt_memslot - Memory region slot for data pages
> 
> Would it make sense to squash the first two patches together?  They
> are somehow related, and also no lines will be added and quickly removed.

I had them separated for easier review. If Paolo wants to squash on merge,
I've got no problem with that.

> 
> Nitpicking on the name: vm_vcpu_add_memslots() makes me feel like
> "vcpu is adding memslots" rather than adding vcpu itself.  How about
> vm_vcpu_add_with_memslots()?

I can do that, although it's getting pretty verbose. Anybody else want
to vote on the name?

Thanks,
drew
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 4e92f34cf46a..32fabbc98803 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -88,6 +88,7 @@  int _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, unsigned long ioctl,
 		void *arg);
 void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
 void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t flags);
+void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
 void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
 			  int gdt_memslot);
 vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 937292dca81b..ae6d4b274ddd 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -756,23 +756,20 @@  static int vcpu_mmap_sz(void)
 }
 
 /*
- * VM VCPU Add with provided memslots
+ * VM VCPU Add
  *
  * Input Args:
  *   vm - Virtual Machine
  *   vcpuid - VCPU ID
- *   pgd_memslot - Memory region slot for new virtual translation tables
- *   gdt_memslot - Memory region slot for data pages
  *
  * Output Args: None
  *
  * Return: None
  *
- * Adds a virtual CPU to the VM specified by vm with the ID given by vcpuid
- * and then sets it up with vcpu_setup() and the provided memslots.
+ * Adds a virtual CPU to the VM specified by vm with the ID given by vcpuid.
+ * No additional VCPU setup is done.
  */
-void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
-			  int gdt_memslot)
+void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid)
 {
 	struct vcpu *vcpu;
 
@@ -806,7 +803,28 @@  void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
 		vm->vcpu_head->prev = vcpu;
 	vcpu->next = vm->vcpu_head;
 	vm->vcpu_head = vcpu;
+}
 
+/*
+ * VM VCPU Add with provided memslots
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - VCPU ID
+ *   pgd_memslot - Memory region slot for new virtual translation tables
+ *   gdt_memslot - Memory region slot for data pages
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Adds a virtual CPU the VM specified by vm with the ID given by vcpuid
+ * and then sets it up with vcpu_setup() and the provided memslots.
+ */
+void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot,
+			  int gdt_memslot)
+{
+	vm_vcpu_add(vm, vcpuid);
 	vcpu_setup(vm, vcpuid, pgd_memslot, gdt_memslot);
 }