* [PATCH] selftests/kvm: Use num_vcpus when testing ICR in the xapic_state_test
@ 2022-09-03 7:39 ` Gautam Menghani
0 siblings, 0 replies; 6+ messages in thread
From: Gautam Menghani @ 2022-09-03 7:39 UTC (permalink / raw)
To: pbonzini, shuah
Cc: Gautam Menghani, seanjc, guang.zeng, kvm, linux-kselftest,
linux-kernel, skhan, linux-kernel-mentees
A TODO in xapic_state_test asks to use number of vCPUs instead of
vcpu.id + 1 in test_icr(). This patch adds support to get the number
of vCPUs from the VM created and use it.
Signed-off-by: Gautam Menghani <gautammenghani201@gmail.com>
---
.../selftests/kvm/x86_64/xapic_state_test.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
index 6f7a5ef66718..de934e8e5e41 100644
--- a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
@@ -96,9 +96,8 @@ static void __test_icr(struct xapic_vcpu *x, uint64_t val)
____test_icr(x, val & ~(u64)APIC_ICR_BUSY);
}
-static void test_icr(struct xapic_vcpu *x)
+static void test_icr(struct xapic_vcpu *x, int num_vcpus)
{
- struct kvm_vcpu *vcpu = x->vcpu;
uint64_t icr, i, j;
icr = APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_FIXED;
@@ -110,11 +109,11 @@ static void test_icr(struct xapic_vcpu *x)
__test_icr(x, icr | i);
/*
- * Send all flavors of IPIs to non-existent vCPUs. TODO: use number of
- * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff.
+ * Send all flavors of IPIs to non-existent vCPUs. Arbitrarily use vector 0xff.
*/
+
icr = APIC_INT_ASSERT | 0xff;
- for (i = vcpu->id + 1; i < 0xff; i++) {
+ for (i = num_vcpus; i < 0xff; i++) {
for (j = 0; j < 8; j++)
__test_icr(x, i << (32 + 24) | icr | (j << 8));
}
@@ -137,9 +136,13 @@ int main(int argc, char *argv[])
.is_x2apic = true,
};
struct kvm_vm *vm;
+ struct list_head *iter;
+ int nr_vcpus_created = 0;
vm = vm_create_with_one_vcpu(&x.vcpu, x2apic_guest_code);
- test_icr(&x);
+ list_for_each(iter, &vm->vcpus)
+ nr_vcpus_created++;
+ test_icr(&x, nr_vcpus_created);
kvm_vm_free(vm);
/*
@@ -153,6 +156,6 @@ int main(int argc, char *argv[])
vcpu_clear_cpuid_feature(x.vcpu, X86_FEATURE_X2APIC);
virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
- test_icr(&x);
+ test_icr(&x, nr_vcpus_created);
kvm_vm_free(vm);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] selftests/kvm: Use num_vcpus when testing ICR in the xapic_state_test
2022-09-03 7:39 ` Gautam Menghani
@ 2022-09-13 16:19 ` Gautam Menghani
-1 siblings, 0 replies; 6+ messages in thread
From: Gautam Menghani @ 2022-09-13 16:19 UTC (permalink / raw)
To: pbonzini, shuah
Cc: kvm, seanjc, linux-kernel, linux-kselftest, guang.zeng,
linux-kernel-mentees
On Sat, Sep 03, 2022 at 01:09:01PM +0530, Gautam Menghani wrote:
> A TODO in xapic_state_test asks to use number of vCPUs instead of
> vcpu.id + 1 in test_icr(). This patch adds support to get the number
> of vCPUs from the VM created and use it.
>
> Signed-off-by: Gautam Menghani <gautammenghani201@gmail.com>
> ---
> .../selftests/kvm/x86_64/xapic_state_test.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> index 6f7a5ef66718..de934e8e5e41 100644
> --- a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> @@ -96,9 +96,8 @@ static void __test_icr(struct xapic_vcpu *x, uint64_t val)
> ____test_icr(x, val & ~(u64)APIC_ICR_BUSY);
> }
>
> -static void test_icr(struct xapic_vcpu *x)
> +static void test_icr(struct xapic_vcpu *x, int num_vcpus)
> {
> - struct kvm_vcpu *vcpu = x->vcpu;
> uint64_t icr, i, j;
>
> icr = APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_FIXED;
> @@ -110,11 +109,11 @@ static void test_icr(struct xapic_vcpu *x)
> __test_icr(x, icr | i);
>
> /*
> - * Send all flavors of IPIs to non-existent vCPUs. TODO: use number of
> - * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff.
> + * Send all flavors of IPIs to non-existent vCPUs. Arbitrarily use vector 0xff.
> */
> +
> icr = APIC_INT_ASSERT | 0xff;
> - for (i = vcpu->id + 1; i < 0xff; i++) {
> + for (i = num_vcpus; i < 0xff; i++) {
> for (j = 0; j < 8; j++)
> __test_icr(x, i << (32 + 24) | icr | (j << 8));
> }
> @@ -137,9 +136,13 @@ int main(int argc, char *argv[])
> .is_x2apic = true,
> };
> struct kvm_vm *vm;
> + struct list_head *iter;
> + int nr_vcpus_created = 0;
>
> vm = vm_create_with_one_vcpu(&x.vcpu, x2apic_guest_code);
> - test_icr(&x);
> + list_for_each(iter, &vm->vcpus)
> + nr_vcpus_created++;
> + test_icr(&x, nr_vcpus_created);
> kvm_vm_free(vm);
>
> /*
> @@ -153,6 +156,6 @@ int main(int argc, char *argv[])
> vcpu_clear_cpuid_feature(x.vcpu, X86_FEATURE_X2APIC);
>
> virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
> - test_icr(&x);
> + test_icr(&x, nr_vcpus_created);
> kvm_vm_free(vm);
> }
> --
> 2.34.1
>
Hi,
Please review the above patch and let me know if any change is required.
Thanks,
Gautam
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] selftests/kvm: Use num_vcpus when testing ICR in the xapic_state_test
@ 2022-09-13 16:19 ` Gautam Menghani
0 siblings, 0 replies; 6+ messages in thread
From: Gautam Menghani @ 2022-09-13 16:19 UTC (permalink / raw)
To: pbonzini, shuah
Cc: seanjc, guang.zeng, kvm, linux-kselftest, linux-kernel, skhan,
linux-kernel-mentees
On Sat, Sep 03, 2022 at 01:09:01PM +0530, Gautam Menghani wrote:
> A TODO in xapic_state_test asks to use number of vCPUs instead of
> vcpu.id + 1 in test_icr(). This patch adds support to get the number
> of vCPUs from the VM created and use it.
>
> Signed-off-by: Gautam Menghani <gautammenghani201@gmail.com>
> ---
> .../selftests/kvm/x86_64/xapic_state_test.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> index 6f7a5ef66718..de934e8e5e41 100644
> --- a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> @@ -96,9 +96,8 @@ static void __test_icr(struct xapic_vcpu *x, uint64_t val)
> ____test_icr(x, val & ~(u64)APIC_ICR_BUSY);
> }
>
> -static void test_icr(struct xapic_vcpu *x)
> +static void test_icr(struct xapic_vcpu *x, int num_vcpus)
> {
> - struct kvm_vcpu *vcpu = x->vcpu;
> uint64_t icr, i, j;
>
> icr = APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_FIXED;
> @@ -110,11 +109,11 @@ static void test_icr(struct xapic_vcpu *x)
> __test_icr(x, icr | i);
>
> /*
> - * Send all flavors of IPIs to non-existent vCPUs. TODO: use number of
> - * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff.
> + * Send all flavors of IPIs to non-existent vCPUs. Arbitrarily use vector 0xff.
> */
> +
> icr = APIC_INT_ASSERT | 0xff;
> - for (i = vcpu->id + 1; i < 0xff; i++) {
> + for (i = num_vcpus; i < 0xff; i++) {
> for (j = 0; j < 8; j++)
> __test_icr(x, i << (32 + 24) | icr | (j << 8));
> }
> @@ -137,9 +136,13 @@ int main(int argc, char *argv[])
> .is_x2apic = true,
> };
> struct kvm_vm *vm;
> + struct list_head *iter;
> + int nr_vcpus_created = 0;
>
> vm = vm_create_with_one_vcpu(&x.vcpu, x2apic_guest_code);
> - test_icr(&x);
> + list_for_each(iter, &vm->vcpus)
> + nr_vcpus_created++;
> + test_icr(&x, nr_vcpus_created);
> kvm_vm_free(vm);
>
> /*
> @@ -153,6 +156,6 @@ int main(int argc, char *argv[])
> vcpu_clear_cpuid_feature(x.vcpu, X86_FEATURE_X2APIC);
>
> virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
> - test_icr(&x);
> + test_icr(&x, nr_vcpus_created);
> kvm_vm_free(vm);
> }
> --
> 2.34.1
>
Hi,
Please review the above patch and let me know if any change is required.
Thanks,
Gautam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/kvm: Use num_vcpus when testing ICR in the xapic_state_test
2022-09-03 7:39 ` Gautam Menghani
@ 2022-09-20 19:51 ` Sean Christopherson
-1 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson via Linux-kernel-mentees @ 2022-09-20 19:51 UTC (permalink / raw)
To: Gautam Menghani
Cc: kvm, linux-kernel-mentees, linux-kernel, linux-kselftest,
guang.zeng, pbonzini, shuah
On Sat, Sep 03, 2022, Gautam Menghani wrote:
> A TODO in xapic_state_test asks to use number of vCPUs instead of
> vcpu.id + 1 in test_icr(). This patch adds support to get the number
> of vCPUs from the VM created and use it.
>
> Signed-off-by: Gautam Menghani <gautammenghani201@gmail.com>
> ---
> .../selftests/kvm/x86_64/xapic_state_test.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> index 6f7a5ef66718..de934e8e5e41 100644
> --- a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> @@ -96,9 +96,8 @@ static void __test_icr(struct xapic_vcpu *x, uint64_t val)
> ____test_icr(x, val & ~(u64)APIC_ICR_BUSY);
> }
>
> -static void test_icr(struct xapic_vcpu *x)
> +static void test_icr(struct xapic_vcpu *x, int num_vcpus)
> {
> - struct kvm_vcpu *vcpu = x->vcpu;
> uint64_t icr, i, j;
>
> icr = APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_FIXED;
> @@ -110,11 +109,11 @@ static void test_icr(struct xapic_vcpu *x)
> __test_icr(x, icr | i);
>
> /*
> - * Send all flavors of IPIs to non-existent vCPUs. TODO: use number of
> - * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff.
> + * Send all flavors of IPIs to non-existent vCPUs. Arbitrarily use vector 0xff.
> */
> +
> icr = APIC_INT_ASSERT | 0xff;
> - for (i = vcpu->id + 1; i < 0xff; i++) {
> + for (i = num_vcpus; i < 0xff; i++) {
Hrm, this is a bad TODO, which is 100% my fault. I think past me wanted to play
nice with the possibility of creating multiple vCPUs, but using the number of
vCPUs to do that is the wrong approach as it makes assumptions on the vcpu_id.
One of the goals of the refactoring that led to vm_create_with_one_vcpu() was
to have tests stop making assumptions about vCPU IDs.
What would make sense though is to not assume vcpu_id == 0, i.e.
if (i = 0; i < 0xff; i++) {
if (i == vcpu->id)
continue;
for (j = 0; j < 8; j++)
__test_icr(x, i << (32 + 24) | icr | (j << 8));
}
> for (j = 0; j < 8; j++)
> __test_icr(x, i << (32 + 24) | icr | (j << 8));
> }
> @@ -137,9 +136,13 @@ int main(int argc, char *argv[])
> .is_x2apic = true,
> };
> struct kvm_vm *vm;
> + struct list_head *iter;
> + int nr_vcpus_created = 0;
>
> vm = vm_create_with_one_vcpu(&x.vcpu, x2apic_guest_code);
> - test_icr(&x);
> + list_for_each(iter, &vm->vcpus)
> + nr_vcpus_created++;
> + test_icr(&x, nr_vcpus_created);
Computing the number of vCPUs created is rather silly, the test _knows_ that it
has created exacty one vCPU. Should be a moot point though.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] selftests/kvm: Use num_vcpus when testing ICR in the xapic_state_test
@ 2022-09-20 19:51 ` Sean Christopherson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-09-20 19:51 UTC (permalink / raw)
To: Gautam Menghani
Cc: pbonzini, shuah, guang.zeng, kvm, linux-kselftest, linux-kernel,
skhan, linux-kernel-mentees
On Sat, Sep 03, 2022, Gautam Menghani wrote:
> A TODO in xapic_state_test asks to use number of vCPUs instead of
> vcpu.id + 1 in test_icr(). This patch adds support to get the number
> of vCPUs from the VM created and use it.
>
> Signed-off-by: Gautam Menghani <gautammenghani201@gmail.com>
> ---
> .../selftests/kvm/x86_64/xapic_state_test.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> index 6f7a5ef66718..de934e8e5e41 100644
> --- a/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xapic_state_test.c
> @@ -96,9 +96,8 @@ static void __test_icr(struct xapic_vcpu *x, uint64_t val)
> ____test_icr(x, val & ~(u64)APIC_ICR_BUSY);
> }
>
> -static void test_icr(struct xapic_vcpu *x)
> +static void test_icr(struct xapic_vcpu *x, int num_vcpus)
> {
> - struct kvm_vcpu *vcpu = x->vcpu;
> uint64_t icr, i, j;
>
> icr = APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_FIXED;
> @@ -110,11 +109,11 @@ static void test_icr(struct xapic_vcpu *x)
> __test_icr(x, icr | i);
>
> /*
> - * Send all flavors of IPIs to non-existent vCPUs. TODO: use number of
> - * vCPUs, not vcpu.id + 1. Arbitrarily use vector 0xff.
> + * Send all flavors of IPIs to non-existent vCPUs. Arbitrarily use vector 0xff.
> */
> +
> icr = APIC_INT_ASSERT | 0xff;
> - for (i = vcpu->id + 1; i < 0xff; i++) {
> + for (i = num_vcpus; i < 0xff; i++) {
Hrm, this is a bad TODO, which is 100% my fault. I think past me wanted to play
nice with the possibility of creating multiple vCPUs, but using the number of
vCPUs to do that is the wrong approach as it makes assumptions on the vcpu_id.
One of the goals of the refactoring that led to vm_create_with_one_vcpu() was
to have tests stop making assumptions about vCPU IDs.
What would make sense though is to not assume vcpu_id == 0, i.e.
if (i = 0; i < 0xff; i++) {
if (i == vcpu->id)
continue;
for (j = 0; j < 8; j++)
__test_icr(x, i << (32 + 24) | icr | (j << 8));
}
> for (j = 0; j < 8; j++)
> __test_icr(x, i << (32 + 24) | icr | (j << 8));
> }
> @@ -137,9 +136,13 @@ int main(int argc, char *argv[])
> .is_x2apic = true,
> };
> struct kvm_vm *vm;
> + struct list_head *iter;
> + int nr_vcpus_created = 0;
>
> vm = vm_create_with_one_vcpu(&x.vcpu, x2apic_guest_code);
> - test_icr(&x);
> + list_for_each(iter, &vm->vcpus)
> + nr_vcpus_created++;
> + test_icr(&x, nr_vcpus_created);
Computing the number of vCPUs created is rather silly, the test _knows_ that it
has created exacty one vCPU. Should be a moot point though.
^ permalink raw reply [flat|nested] 6+ messages in thread