All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: selftests: Fixups for overhaul
@ 2022-06-13 16:19 Sean Christopherson
  2022-06-13 16:19 ` [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership Sean Christopherson
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 16:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andrew Jones, kvm, linux-kernel

Fixups for the overhaul, all of which come from Drew's code review.  The
first three should all squash cleanly, but the kvm_check_cap() patch will
not due to crossing the TEST_REQUIRE() boundary.

Sean Christopherson (4):
  KVM: selftests: Add a missing apostrophe in comment to show ownership
  KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce
    type
  KVM: selftests: Drop a duplicate TEST_ASSERT() in
    vm_nr_pages_required()
  KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible

 .../testing/selftests/kvm/aarch64/psci_test.c |  2 +-
 .../selftests/kvm/include/kvm_util_base.h     | 57 ++++++++++---------
 tools/testing/selftests/kvm/lib/kvm_util.c    |  6 +-
 .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
 .../selftests/kvm/s390x/sync_regs_test.c      |  2 +-
 .../kvm/x86_64/pmu_event_filter_test.c        |  2 +-
 .../selftests/kvm/x86_64/sev_migrate_tests.c  |  6 +-
 tools/testing/selftests/kvm/x86_64/smm_test.c |  2 +-
 .../testing/selftests/kvm/x86_64/state_test.c |  2 +-
 9 files changed, 42 insertions(+), 41 deletions(-)


base-commit: 8baacf67c76c560fed954ac972b63e6e59a6fba0
-- 
2.36.1.476.g0c4daa206d-goog


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

* [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
@ 2022-06-13 16:19 ` Sean Christopherson
  2022-06-13 19:13   ` Jim Mattson
  2022-06-13 16:19 ` [PATCH 2/4] KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce type Sean Christopherson
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 16:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andrew Jones, kvm, linux-kernel

Add an apostrophe in a comment about it being the caller's, not callers,
responsibility to free an object.

Reported-by: Andrew Jones <drjones@redhat.com>
Fixes: 768e9a61856b ("KVM: selftests: Purge vm+vcpu_id == vcpu silliness")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 39f2f5f1338f..0c550fb0dab2 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1434,7 +1434,7 @@ void vcpu_run_complete_io(struct kvm_vcpu *vcpu)
 /*
  * Get the list of guest registers which are supported for
  * KVM_GET_ONE_REG/KVM_SET_ONE_REG ioctls.  Returns a kvm_reg_list pointer,
- * it is the callers responsibility to free the list.
+ * it is the caller's responsibility to free the list.
  */
 struct kvm_reg_list *vcpu_get_reg_list(struct kvm_vcpu *vcpu)
 {
-- 
2.36.1.476.g0c4daa206d-goog


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

* [PATCH 2/4] KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce type
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
  2022-06-13 16:19 ` [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership Sean Christopherson
@ 2022-06-13 16:19 ` Sean Christopherson
  2022-06-13 16:19 ` [PATCH 3/4] KVM: selftests: Drop a duplicate TEST_ASSERT() in vm_nr_pages_required() Sean Christopherson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 16:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andrew Jones, kvm, linux-kernel

Replace the goofy static_assert on the size of the @vm/@vcpu parameters
with a call to a dummy helper, i.e. let the compiler naturally complain
about an incompatible type instead of homebrewing a poor replacement.

Reported-by: Andrew Jones <drjones@redhat.com>
Fixes: fcba483e8246 ("KVM: selftests: Sanity check input to ioctls() at build time")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/kvm/include/kvm_util_base.h     | 57 ++++++++++---------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index cdaea2383543..7ebfc8c7de17 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -186,50 +186,55 @@ static inline bool kvm_has_cap(long cap)
 	ioctl(fd, cmd, arg);							\
 })
 
-#define __kvm_ioctl(kvm_fd, cmd, arg)						\
+#define __kvm_ioctl(kvm_fd, cmd, arg)				\
 	kvm_do_ioctl(kvm_fd, cmd, arg)
 
 
-#define _kvm_ioctl(kvm_fd, cmd, name, arg)					\
-({										\
-	int ret = __kvm_ioctl(kvm_fd, cmd, arg);				\
-										\
-	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));			\
+#define _kvm_ioctl(kvm_fd, cmd, name, arg)			\
+({								\
+	int ret = __kvm_ioctl(kvm_fd, cmd, arg);		\
+								\
+	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));	\
 })
 
 #define kvm_ioctl(kvm_fd, cmd, arg) \
 	_kvm_ioctl(kvm_fd, cmd, #cmd, arg)
 
-#define __vm_ioctl(vm, cmd, arg)						\
-({										\
-	static_assert(sizeof(*(vm)) == sizeof(struct kvm_vm), "");		\
-	kvm_do_ioctl((vm)->fd, cmd, arg);					\
+static __always_inline void static_assert_is_vm(struct kvm_vm *vm) { }
+
+#define __vm_ioctl(vm, cmd, arg)				\
+({								\
+	static_assert_is_vm(vm);				\
+	kvm_do_ioctl((vm)->fd, cmd, arg);			\
 })
 
-#define _vm_ioctl(vm, cmd, name, arg)						\
-({										\
-	int ret = __vm_ioctl(vm, cmd, arg);					\
-										\
-	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));			\
+#define _vm_ioctl(vm, cmd, name, arg)				\
+({								\
+	int ret = __vm_ioctl(vm, cmd, arg);			\
+								\
+	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));	\
 })
 
-#define vm_ioctl(vm, cmd, arg)							\
+#define vm_ioctl(vm, cmd, arg)					\
 	_vm_ioctl(vm, cmd, #cmd, arg)
 
-#define __vcpu_ioctl(vcpu, cmd, arg)						\
-({										\
-	static_assert(sizeof(*(vcpu)) == sizeof(struct kvm_vcpu), "");		\
-	kvm_do_ioctl((vcpu)->fd, cmd, arg);					\
+
+static __always_inline void static_assert_is_vcpu(struct kvm_vcpu *vcpu) { }
+
+#define __vcpu_ioctl(vcpu, cmd, arg)				\
+({								\
+	static_assert_is_vcpu(vcpu);				\
+	kvm_do_ioctl((vcpu)->fd, cmd, arg);			\
 })
 
-#define _vcpu_ioctl(vcpu, cmd, name, arg)					\
-({										\
-	int ret = __vcpu_ioctl(vcpu, cmd, arg);					\
-										\
-	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));			\
+#define _vcpu_ioctl(vcpu, cmd, name, arg)			\
+({								\
+	int ret = __vcpu_ioctl(vcpu, cmd, arg);			\
+								\
+	TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret));	\
 })
 
-#define vcpu_ioctl(vcpu, cmd, arg)						\
+#define vcpu_ioctl(vcpu, cmd, arg)				\
 	_vcpu_ioctl(vcpu, cmd, #cmd, arg)
 
 /*
-- 
2.36.1.476.g0c4daa206d-goog


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

* [PATCH 3/4] KVM: selftests: Drop a duplicate TEST_ASSERT() in vm_nr_pages_required()
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
  2022-06-13 16:19 ` [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership Sean Christopherson
  2022-06-13 16:19 ` [PATCH 2/4] KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce type Sean Christopherson
@ 2022-06-13 16:19 ` Sean Christopherson
  2022-06-13 16:19 ` [PATCH 4/4] KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible Sean Christopherson
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 16:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andrew Jones, kvm, linux-kernel

Remove a duplicate TEST_ASSERT() on the number of runnable vCPUs in
vm_nr_pages_required() that snuck in during a rebase gone bad.

Reported-by: Andrew Jones <drjones@redhat.com>
Fixes: 6e1d13bf3815 ("KVM: selftests: Move per-VM/per-vCPU nr pages calculation to __vm_create()")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 0c550fb0dab2..bceb668f2627 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -284,10 +284,6 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
 	 */
 	nr_pages += (nr_pages + extra_mem_pages) / PTES_PER_MIN_PAGE * 2;
 
-	TEST_ASSERT(nr_runnable_vcpus <= kvm_check_cap(KVM_CAP_MAX_VCPUS),
-		    "Host doesn't support %d vCPUs, max-vcpus = %d",
-		    nr_runnable_vcpus, kvm_check_cap(KVM_CAP_MAX_VCPUS));
-
 	return vm_adjust_num_guest_pages(mode, nr_pages);
 }
 
-- 
2.36.1.476.g0c4daa206d-goog


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

* [PATCH 4/4] KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
                   ` (2 preceding siblings ...)
  2022-06-13 16:19 ` [PATCH 3/4] KVM: selftests: Drop a duplicate TEST_ASSERT() in vm_nr_pages_required() Sean Christopherson
@ 2022-06-13 16:19 ` Sean Christopherson
  2022-06-14  7:51 ` [PATCH 0/4] KVM: selftests: Fixups for overhaul Andrew Jones
  2022-06-14 16:44 ` Paolo Bonzini
  5 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 16:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Andrew Jones, kvm, linux-kernel

Replace calls to kvm_check_cap() that treat its return as a boolean with
calls to kvm_has_cap().  Several instances of kvm_check_cap() were missed
when kvm_has_cap() was introduced.

Reported-by: Andrew Jones <drjones@redhat.com>
Fixes: 3ea9b809650b ("KVM: selftests: Add kvm_has_cap() to provide syntactic sugar")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/aarch64/psci_test.c            | 2 +-
 tools/testing/selftests/kvm/lib/x86_64/processor.c         | 4 ++--
 tools/testing/selftests/kvm/s390x/sync_regs_test.c         | 2 +-
 tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c | 2 +-
 tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c     | 6 +++---
 tools/testing/selftests/kvm/x86_64/smm_test.c              | 2 +-
 tools/testing/selftests/kvm/x86_64/state_test.c            | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/psci_test.c b/tools/testing/selftests/kvm/aarch64/psci_test.c
index a889e1cf5e4d..b665b534cb78 100644
--- a/tools/testing/selftests/kvm/aarch64/psci_test.c
+++ b/tools/testing/selftests/kvm/aarch64/psci_test.c
@@ -192,7 +192,7 @@ static void host_test_system_suspend(void)
 
 int main(void)
 {
-	TEST_REQUIRE(kvm_check_cap(KVM_CAP_ARM_SYSTEM_SUSPEND));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_SYSTEM_SUSPEND));
 
 	host_test_cpu_on();
 	host_test_system_suspend();
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 4a7de11d6f37..906132e70fa4 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -991,7 +991,7 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vcpu *vcpu)
 	vcpu_regs_get(vcpu, &state->regs);
 	vcpu_save_xsave_state(vcpu, state);
 
-	if (kvm_check_cap(KVM_CAP_XCRS))
+	if (kvm_has_cap(KVM_CAP_XCRS))
 		vcpu_xcrs_get(vcpu, &state->xcrs);
 
 	vcpu_sregs_get(vcpu, &state->sregs);
@@ -1022,7 +1022,7 @@ void vcpu_load_state(struct kvm_vcpu *vcpu, struct kvm_x86_state *state)
 	vcpu_sregs_set(vcpu, &state->sregs);
 	vcpu_msrs_set(vcpu, &state->msrs);
 
-	if (kvm_check_cap(KVM_CAP_XCRS))
+	if (kvm_has_cap(KVM_CAP_XCRS))
 		vcpu_xcrs_set(vcpu, &state->xcrs);
 
 	vcpu_xsave_set(vcpu,  state->xsave);
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index b69710822c47..3fdb6e2598eb 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -229,7 +229,7 @@ int main(int argc, char *argv[])
 	struct kvm_vm *vm;
 	int idx;
 
-	TEST_REQUIRE(kvm_check_cap(KVM_CAP_SYNC_REGS));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_SYNC_REGS));
 
 	/* Tell stdout not to buffer its content */
 	setbuf(stdout, NULL);
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
index 786b3a794f84..530a75fee92c 100644
--- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
@@ -450,7 +450,7 @@ int main(int argc, char *argv[])
 	/* Tell stdout not to buffer its content */
 	setbuf(stdout, NULL);
 
-	TEST_REQUIRE(kvm_check_cap(KVM_CAP_PMU_EVENT_FILTER));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_PMU_EVENT_FILTER));
 
 	TEST_REQUIRE(use_intel_pmu() || use_amd_pmu());
 	guest_code = use_intel_pmu() ? intel_guest_code : amd_guest_code;
diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
index 76ba6fc80e37..46018b247a04 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
+++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
@@ -411,16 +411,16 @@ int main(int argc, char *argv[])
 
 	have_sev_es = !!(cpuid->eax & X86_FEATURE_SEV_ES);
 
-	if (kvm_check_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) {
+	if (kvm_has_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) {
 		test_sev_migrate_from(/* es= */ false);
 		if (have_sev_es)
 			test_sev_migrate_from(/* es= */ true);
 		test_sev_migrate_locking();
 		test_sev_migrate_parameters();
-		if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM))
+		if (kvm_has_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM))
 			test_sev_move_copy();
 	}
-	if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) {
+	if (kvm_has_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) {
 		test_sev_mirror(/* es= */ false);
 		if (have_sev_es)
 			test_sev_mirror(/* es= */ true);
diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c b/tools/testing/selftests/kvm/x86_64/smm_test.c
index 3cd1da388b52..921cbf117329 100644
--- a/tools/testing/selftests/kvm/x86_64/smm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
@@ -153,7 +153,7 @@ int main(int argc, char *argv[])
 
 	vcpu_set_msr(vcpu, MSR_IA32_SMBASE, SMRAM_GPA);
 
-	if (kvm_check_cap(KVM_CAP_NESTED_STATE)) {
+	if (kvm_has_cap(KVM_CAP_NESTED_STATE)) {
 		if (nested_svm_supported())
 			vcpu_alloc_svm(vm, &nested_gva);
 		else if (nested_vmx_supported())
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 0bcd78cf7c79..e2f1f35e51ff 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -169,7 +169,7 @@ int main(int argc, char *argv[])
 
 	vcpu_regs_get(vcpu, &regs1);
 
-	if (kvm_check_cap(KVM_CAP_NESTED_STATE)) {
+	if (kvm_has_cap(KVM_CAP_NESTED_STATE)) {
 		if (nested_svm_supported())
 			vcpu_alloc_svm(vm, &nested_gva);
 		else if (nested_vmx_supported())
-- 
2.36.1.476.g0c4daa206d-goog


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

* Re: [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership
  2022-06-13 16:19 ` [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership Sean Christopherson
@ 2022-06-13 19:13   ` Jim Mattson
  2022-06-13 19:32     ` Sean Christopherson
  0 siblings, 1 reply; 11+ messages in thread
From: Jim Mattson @ 2022-06-13 19:13 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, Andrew Jones, kvm, linux-kernel

On Mon, Jun 13, 2022 at 12:01 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Add an apostrophe in a comment about it being the caller's, not callers,
> responsibility to free an object.
>
> Reported-by: Andrew Jones <drjones@redhat.com>
> Fixes: 768e9a61856b ("KVM: selftests: Purge vm+vcpu_id == vcpu silliness")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 39f2f5f1338f..0c550fb0dab2 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1434,7 +1434,7 @@ void vcpu_run_complete_io(struct kvm_vcpu *vcpu)
>  /*
>   * Get the list of guest registers which are supported for
>   * KVM_GET_ONE_REG/KVM_SET_ONE_REG ioctls.  Returns a kvm_reg_list pointer,
> - * it is the callers responsibility to free the list.
> + * it is the caller's responsibility to free the list.
>   */
Shouldn't that be callers'? Or are you assuming there is only ever
going to be one caller?

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

* Re: [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership
  2022-06-13 19:13   ` Jim Mattson
@ 2022-06-13 19:32     ` Sean Christopherson
  2022-06-13 19:35       ` Jim Mattson
  0 siblings, 1 reply; 11+ messages in thread
From: Sean Christopherson @ 2022-06-13 19:32 UTC (permalink / raw)
  To: Jim Mattson; +Cc: Paolo Bonzini, Andrew Jones, kvm, linux-kernel

On Mon, Jun 13, 2022, Jim Mattson wrote:
> On Mon, Jun 13, 2022 at 12:01 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Add an apostrophe in a comment about it being the caller's, not callers,
> > responsibility to free an object.
> >
> > Reported-by: Andrew Jones <drjones@redhat.com>
> > Fixes: 768e9a61856b ("KVM: selftests: Purge vm+vcpu_id == vcpu silliness")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 39f2f5f1338f..0c550fb0dab2 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -1434,7 +1434,7 @@ void vcpu_run_complete_io(struct kvm_vcpu *vcpu)
> >  /*
> >   * Get the list of guest registers which are supported for
> >   * KVM_GET_ONE_REG/KVM_SET_ONE_REG ioctls.  Returns a kvm_reg_list pointer,
> > - * it is the callers responsibility to free the list.
> > + * it is the caller's responsibility to free the list.
> >   */
> Shouldn't that be callers'? Or are you assuming there is only ever
> going to be one caller?

No?  Regardless of the number of users of the function, for any given invocation
and allocation, there is exactly one caller.

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

* Re: [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership
  2022-06-13 19:32     ` Sean Christopherson
@ 2022-06-13 19:35       ` Jim Mattson
  0 siblings, 0 replies; 11+ messages in thread
From: Jim Mattson @ 2022-06-13 19:35 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, Andrew Jones, kvm, linux-kernel

On Mon, Jun 13, 2022 at 12:32 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Jun 13, 2022, Jim Mattson wrote:
> > On Mon, Jun 13, 2022 at 12:01 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Add an apostrophe in a comment about it being the caller's, not callers,
> > > responsibility to free an object.
> > >
> > > Reported-by: Andrew Jones <drjones@redhat.com>
> > > Fixes: 768e9a61856b ("KVM: selftests: Purge vm+vcpu_id == vcpu silliness")
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >  tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > index 39f2f5f1338f..0c550fb0dab2 100644
> > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > > @@ -1434,7 +1434,7 @@ void vcpu_run_complete_io(struct kvm_vcpu *vcpu)
> > >  /*
> > >   * Get the list of guest registers which are supported for
> > >   * KVM_GET_ONE_REG/KVM_SET_ONE_REG ioctls.  Returns a kvm_reg_list pointer,
> > > - * it is the callers responsibility to free the list.
> > > + * it is the caller's responsibility to free the list.
> > >   */
> > Shouldn't that be callers'? Or are you assuming there is only ever
> > going to be one caller?
>
> No?  Regardless of the number of users of the function, for any given invocation
> and allocation, there is exactly one caller.

Statically, there may be multiple callers, and each is responsible for
freeing the list, right?

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

* Re: [PATCH 0/4] KVM: selftests: Fixups for overhaul
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
                   ` (3 preceding siblings ...)
  2022-06-13 16:19 ` [PATCH 4/4] KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible Sean Christopherson
@ 2022-06-14  7:51 ` Andrew Jones
  2022-06-14  7:52   ` Andrew Jones
  2022-06-14 16:44 ` Paolo Bonzini
  5 siblings, 1 reply; 11+ messages in thread
From: Andrew Jones @ 2022-06-14  7:51 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel

On Mon, Jun 13, 2022 at 04:19:38PM +0000, Sean Christopherson wrote:
> Fixups for the overhaul, all of which come from Drew's code review.  The
> first three should all squash cleanly, but the kvm_check_cap() patch will
> not due to crossing the TEST_REQUIRE() boundary.
> 
> Sean Christopherson (4):
>   KVM: selftests: Add a missing apostrophe in comment to show ownership
>   KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce
>     type
>   KVM: selftests: Drop a duplicate TEST_ASSERT() in
>     vm_nr_pages_required()
>   KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible
> 
>  .../testing/selftests/kvm/aarch64/psci_test.c |  2 +-
>  .../selftests/kvm/include/kvm_util_base.h     | 57 ++++++++++---------
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  6 +-
>  .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
>  .../selftests/kvm/s390x/sync_regs_test.c      |  2 +-
>  .../kvm/x86_64/pmu_event_filter_test.c        |  2 +-
>  .../selftests/kvm/x86_64/sev_migrate_tests.c  |  6 +-
>  tools/testing/selftests/kvm/x86_64/smm_test.c |  2 +-
>  .../testing/selftests/kvm/x86_64/state_test.c |  2 +-
>  9 files changed, 42 insertions(+), 41 deletions(-)
> 
> 
> base-commit: 8baacf67c76c560fed954ac972b63e6e59a6fba0
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

All these patches look good to me. For the series

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

There's still one more comment I made on the overhaul though, which
is that the expressions using i and j in kvm_binary_stats_test.c
for the vcpus and vcpu_stat_test indices have i and j swapped.

Thanks,
drew


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

* Re: [PATCH 0/4] KVM: selftests: Fixups for overhaul
  2022-06-14  7:51 ` [PATCH 0/4] KVM: selftests: Fixups for overhaul Andrew Jones
@ 2022-06-14  7:52   ` Andrew Jones
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Jones @ 2022-06-14  7:52 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, linux-kernel

On Tue, Jun 14, 2022 at 09:51:06AM +0200, Andrew Jones wrote:
> On Mon, Jun 13, 2022 at 04:19:38PM +0000, Sean Christopherson wrote:
> > Fixups for the overhaul, all of which come from Drew's code review.  The
> > first three should all squash cleanly, but the kvm_check_cap() patch will
> > not due to crossing the TEST_REQUIRE() boundary.
> > 
> > Sean Christopherson (4):
> >   KVM: selftests: Add a missing apostrophe in comment to show ownership
> >   KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce
> >     type
> >   KVM: selftests: Drop a duplicate TEST_ASSERT() in
> >     vm_nr_pages_required()
> >   KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible
> > 
> >  .../testing/selftests/kvm/aarch64/psci_test.c |  2 +-
> >  .../selftests/kvm/include/kvm_util_base.h     | 57 ++++++++++---------
> >  tools/testing/selftests/kvm/lib/kvm_util.c    |  6 +-
> >  .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
> >  .../selftests/kvm/s390x/sync_regs_test.c      |  2 +-
> >  .../kvm/x86_64/pmu_event_filter_test.c        |  2 +-
> >  .../selftests/kvm/x86_64/sev_migrate_tests.c  |  6 +-
> >  tools/testing/selftests/kvm/x86_64/smm_test.c |  2 +-
> >  .../testing/selftests/kvm/x86_64/state_test.c |  2 +-
> >  9 files changed, 42 insertions(+), 41 deletions(-)
> > 
> > 
> > base-commit: 8baacf67c76c560fed954ac972b63e6e59a6fba0
> > -- 
> > 2.36.1.476.g0c4daa206d-goog
> >
> 
> All these patches look good to me. For the series
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
> There's still one more comment I made on the overhaul though, which
> is that the expressions using i and j in kvm_binary_stats_test.c
> for the vcpus and vcpu_stat_test indices have i and j swapped.
>

I'll just go ahead and send that i,j patch myself now.

Thanks,
drew 


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

* Re: [PATCH 0/4] KVM: selftests: Fixups for overhaul
  2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
                   ` (4 preceding siblings ...)
  2022-06-14  7:51 ` [PATCH 0/4] KVM: selftests: Fixups for overhaul Andrew Jones
@ 2022-06-14 16:44 ` Paolo Bonzini
  5 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2022-06-14 16:44 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: Andrew Jones, kvm, linux-kernel

Queued, thanks; also queued the i/j fixup at "KVM: selftests: kvm_binary_stats_test:
Fix index expressions".

Paolo



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

end of thread, other threads:[~2022-06-14 16:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 16:19 [PATCH 0/4] KVM: selftests: Fixups for overhaul Sean Christopherson
2022-06-13 16:19 ` [PATCH 1/4] KVM: selftests: Add a missing apostrophe in comment to show ownership Sean Christopherson
2022-06-13 19:13   ` Jim Mattson
2022-06-13 19:32     ` Sean Christopherson
2022-06-13 19:35       ` Jim Mattson
2022-06-13 16:19 ` [PATCH 2/4] KVM: selftests: Call a dummy helper in VM/vCPU ioctls() to enforce type Sean Christopherson
2022-06-13 16:19 ` [PATCH 3/4] KVM: selftests: Drop a duplicate TEST_ASSERT() in vm_nr_pages_required() Sean Christopherson
2022-06-13 16:19 ` [PATCH 4/4] KVM: selftests: Use kvm_has_cap(), not kvm_check_cap(), where possible Sean Christopherson
2022-06-14  7:51 ` [PATCH 0/4] KVM: selftests: Fixups for overhaul Andrew Jones
2022-06-14  7:52   ` Andrew Jones
2022-06-14 16:44 ` 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.