| Message ID | 20190523125756.4645-2-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: selftests: aarch64: use struct kvm_vcpu_init | expand |
On 23/05/2019 14.57, Andrew Jones wrote: > This frees up the name vm_vcpu_add for another use. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > tools/testing/selftests/kvm/include/kvm_util.h | 4 ++-- > tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +- > tools/testing/selftests/kvm/lib/kvm_util.c | 12 +++++++----- > tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 +- > tools/testing/selftests/kvm/x86_64/evmcs_test.c | 2 +- > .../selftests/kvm/x86_64/kvm_create_max_vcpus.c | 2 +- > tools/testing/selftests/kvm/x86_64/smm_test.c | 2 +- > tools/testing/selftests/kvm/x86_64/state_test.c | 2 +- > 8 files changed, 15 insertions(+), 13 deletions(-) [...] > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index e9113857f44e..937292dca81b 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -756,21 +756,23 @@ static int vcpu_mmap_sz(void) > } > > /* > - * VM VCPU Add > + * 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 > * > - * Creates and adds to the VM specified by vm and virtual CPU with > - * the ID given by vcpuid. > + * 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. > */ > -void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, > - int gdt_memslot) > +void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, > + int gdt_memslot) I think the naming and description of the function is somewhat unfortunate now. The function is not really about memslots, but about setting up some MMU tables in the memory (and for this you need a memslot). So maybe rather name it vm_vcpu_add_with_mmu() or something similar? Also it would be nice to give the reason for the memslots in the comment before the function. Thomas
On Mon, May 27, 2019 at 09:27:56AM +0200, Thomas Huth wrote: > On 23/05/2019 14.57, Andrew Jones wrote: > > This frees up the name vm_vcpu_add for another use. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > > tools/testing/selftests/kvm/include/kvm_util.h | 4 ++-- > > tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +- > > tools/testing/selftests/kvm/lib/kvm_util.c | 12 +++++++----- > > tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 +- > > tools/testing/selftests/kvm/x86_64/evmcs_test.c | 2 +- > > .../selftests/kvm/x86_64/kvm_create_max_vcpus.c | 2 +- > > tools/testing/selftests/kvm/x86_64/smm_test.c | 2 +- > > tools/testing/selftests/kvm/x86_64/state_test.c | 2 +- > > 8 files changed, 15 insertions(+), 13 deletions(-) > [...] > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > > index e9113857f44e..937292dca81b 100644 > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > > @@ -756,21 +756,23 @@ static int vcpu_mmap_sz(void) > > } > > > > /* > > - * VM VCPU Add > > + * 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 > > * > > - * Creates and adds to the VM specified by vm and virtual CPU with > > - * the ID given by vcpuid. > > + * 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. > > */ > > -void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, > > - int gdt_memslot) > > +void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, > > + int gdt_memslot) > > I think the naming and description of the function is somewhat > unfortunate now. The function is not really about memslots, but about > setting up some MMU tables in the memory (and for this you need a > memslot). So maybe rather name it vm_vcpu_add_with_mmu() or something > similar? Also it would be nice to give the reason for the memslots in > the comment before the function. > Peter Xu suggested almost the same name, so I'll do that for a v2. I'll add a couple more words to the comment too. Thanks, drew
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 8c6b9619797d..4e92f34cf46a 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -88,8 +88,8 @@ 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, int pgd_memslot, - int gdt_memslot); +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, uint32_t data_memslot, uint32_t pgd_memslot); void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr, diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index 19e667911496..713a0e6b0e08 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -243,7 +243,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) uint64_t stack_vaddr = vm_vaddr_alloc(vm, stack_size, DEFAULT_ARM64_GUEST_STACK_VADDR_MIN, 0, 0); - vm_vcpu_add(vm, vcpuid, 0, 0); + vm_vcpu_add_memslots(vm, vcpuid, 0, 0); set_reg(vm, vcpuid, ARM64_CORE_REG(sp_el1), stack_vaddr + stack_size); set_reg(vm, vcpuid, ARM64_CORE_REG(regs.pc), (uint64_t)guest_code); diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index e9113857f44e..937292dca81b 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -756,21 +756,23 @@ static int vcpu_mmap_sz(void) } /* - * VM VCPU Add + * 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 * - * Creates and adds to the VM specified by vm and virtual CPU with - * the ID given by vcpuid. + * 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. */ -void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, - int gdt_memslot) +void vm_vcpu_add_memslots(struct kvm_vm *vm, uint32_t vcpuid, int pgd_memslot, + int gdt_memslot) { struct vcpu *vcpu; diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index dc7fae9fa424..7779cdcc9159 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -658,7 +658,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code) DEFAULT_GUEST_STACK_VADDR_MIN, 0, 0); /* Create VCPU */ - vm_vcpu_add(vm, vcpuid, 0, 0); + vm_vcpu_add_memslots(vm, vcpuid, 0, 0); /* Setup guest general purpose registers */ vcpu_regs_get(vm, vcpuid, ®s); diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c index 36669684eca5..f4bce50ded95 100644 --- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c +++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c @@ -149,7 +149,7 @@ int main(int argc, char *argv[]) /* Restore state in a new VM. */ kvm_vm_restart(vm, O_RDWR); - vm_vcpu_add(vm, VCPU_ID, 0, 0); + vm_vcpu_add_memslots(vm, VCPU_ID, 0, 0); vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid()); vcpu_load_state(vm, VCPU_ID, state); run = vcpu_state(vm, VCPU_ID); diff --git a/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c b/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c index 50e92996f918..ded4e0272f8a 100644 --- a/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c +++ b/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c @@ -34,7 +34,7 @@ void test_vcpu_creation(int first_vcpu_id, int num_vcpus) int vcpu_id = first_vcpu_id + i; /* This asserts that the vCPU was created. */ - vm_vcpu_add(vm, vcpu_id, 0, 0); + vm_vcpu_add_memslots(vm, vcpu_id, 0, 0); } kvm_vm_free(vm); diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c b/tools/testing/selftests/kvm/x86_64/smm_test.c index fb8086964d83..b31d8c29b215 100644 --- a/tools/testing/selftests/kvm/x86_64/smm_test.c +++ b/tools/testing/selftests/kvm/x86_64/smm_test.c @@ -145,7 +145,7 @@ int main(int argc, char *argv[]) state = vcpu_save_state(vm, VCPU_ID); kvm_vm_release(vm); kvm_vm_restart(vm, O_RDWR); - vm_vcpu_add(vm, VCPU_ID, 0, 0); + vm_vcpu_add_memslots(vm, VCPU_ID, 0, 0); vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid()); vcpu_load_state(vm, VCPU_ID, state); run = vcpu_state(vm, VCPU_ID); diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c index e0a3c0204b7c..b564ebdd34ca 100644 --- a/tools/testing/selftests/kvm/x86_64/state_test.c +++ b/tools/testing/selftests/kvm/x86_64/state_test.c @@ -182,7 +182,7 @@ int main(int argc, char *argv[]) /* Restore state in a new VM. */ kvm_vm_restart(vm, O_RDWR); - vm_vcpu_add(vm, VCPU_ID, 0, 0); + vm_vcpu_add_memslots(vm, VCPU_ID, 0, 0); vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid()); vcpu_load_state(vm, VCPU_ID, state); run = vcpu_state(vm, VCPU_ID);
This frees up the name vm_vcpu_add for another use. Signed-off-by: Andrew Jones <drjones@redhat.com> --- tools/testing/selftests/kvm/include/kvm_util.h | 4 ++-- tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +- tools/testing/selftests/kvm/lib/kvm_util.c | 12 +++++++----- tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 +- tools/testing/selftests/kvm/x86_64/evmcs_test.c | 2 +- .../selftests/kvm/x86_64/kvm_create_max_vcpus.c | 2 +- tools/testing/selftests/kvm/x86_64/smm_test.c | 2 +- tools/testing/selftests/kvm/x86_64/state_test.c | 2 +- 8 files changed, 15 insertions(+), 13 deletions(-)