Test the KVM_GET_MSR_FEATURE_INDEX_LIST
and KVM_GET_MSR_INDEX_LIST ioctls.
Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
---
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/x86_64/get_msr_index_features.c | 124 ++++++++++++++++++
3 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 32b87cc77c8e..d99f3969d371 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -5,6 +5,7 @@
/s390x/resets
/s390x/sync_regs_test
/x86_64/cr4_cpuid_sync_test
+/x86_64/get_msr_index_features
/x86_64/debug_regs
/x86_64/evmcs_test
/x86_64/get_cpuid_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..c748b9650e28 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -39,6 +39,7 @@ LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
+TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
TEST_GEN_PROGS_x86_64 += x86_64/get_cpuid_test
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
new file mode 100644
index 000000000000..ad9972d99dfa
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test that KVM_GET_MSR_INDEX_LIST and
+ * KVM_GET_MSR_FEATURE_INDEX_LIST work as intended
+ *
+ * Copyright (C) 2020, Red Hat, Inc.
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "../lib/kvm_util_internal.h"
+
+static int kvm_num_index_msrs(int kvm_fd, int nmsrs)
+{
+ struct kvm_msr_list *list;
+ int r;
+
+ list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
+ list->nmsrs = nmsrs;
+ r = ioctl(kvm_fd, KVM_GET_MSR_INDEX_LIST, list);
+ TEST_ASSERT(r == -1 && errno == E2BIG,
+ "Unexpected result from KVM_GET_MSR_INDEX_LIST probe, r: %i",
+ r);
+
+ r = list->nmsrs;
+ free(list);
+ return r;
+}
+
+static void test_get_msr_index(void)
+{
+ int old_res, res, kvm_fd;
+
+ kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
+ if (kvm_fd < 0)
+ exit(KSFT_SKIP);
+
+ old_res = kvm_num_index_msrs(kvm_fd, 0);
+ TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
+
+ if (old_res != 1) {
+ res = kvm_num_index_msrs(kvm_fd, 1);
+ TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
+ TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
+ }
+
+ close(kvm_fd);
+}
+
+static int kvm_num_feature_msrs(int kvm_fd, int nmsrs)
+{
+ struct kvm_msr_list *list;
+ int r;
+
+ list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
+ list->nmsrs = nmsrs;
+ r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
+ TEST_ASSERT(r == -1 && errno == E2BIG,
+ "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST probe, r: %i",
+ r);
+
+ r = list->nmsrs;
+ free(list);
+ return r;
+}
+
+struct kvm_msr_list *kvm_get_msr_feature_list(int kvm_fd, int nmsrs)
+{
+ struct kvm_msr_list *list;
+ int r;
+
+ list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
+ list->nmsrs = nmsrs;
+ r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
+
+ TEST_ASSERT(r == 0,
+ "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST, r: %i",
+ r);
+
+ return list;
+}
+
+static void test_get_msr_feature(void)
+{
+ int res, old_res, i, kvm_fd;
+ struct kvm_msr_list *feature_list;
+
+ kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
+ if (kvm_fd < 0)
+ exit(KSFT_SKIP);
+
+ old_res = kvm_num_feature_msrs(kvm_fd, 0);
+ TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
+
+ if (old_res != 1) {
+ res = kvm_num_feature_msrs(kvm_fd, 1);
+ TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
+ TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
+ }
+
+ feature_list = kvm_get_msr_feature_list(kvm_fd, old_res);
+ TEST_ASSERT(old_res == feature_list->nmsrs,
+ "Unmatching number of msr indexes");
+
+ for (i = 0; i < feature_list->nmsrs; i++)
+ kvm_get_feature_msr(feature_list->indices[i]);
+
+ free(feature_list);
+ close(kvm_fd);
+}
+
+int main(int argc, char *argv[])
+{
+ if (kvm_check_cap(KVM_CAP_GET_MSR_FEATURES))
+ test_get_msr_feature();
+
+ test_get_msr_index();
+}
--
2.29.2
On 18/03/21 15:56, Emanuele Giuseppe Esposito wrote:
> Test the KVM_GET_MSR_FEATURE_INDEX_LIST
> and KVM_GET_MSR_INDEX_LIST ioctls.
>
> Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
Looks good, thanks.
I queued it with a little extra code to verify KVM_GET_MSR_INDEX_LIST in
the case that does not return E2BIG.
Paolo
> ---
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 1 +
> .../kvm/x86_64/get_msr_index_features.c | 124 ++++++++++++++++++
> 3 files changed, 126 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 32b87cc77c8e..d99f3969d371 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -5,6 +5,7 @@
> /s390x/resets
> /s390x/sync_regs_test
> /x86_64/cr4_cpuid_sync_test
> +/x86_64/get_msr_index_features
> /x86_64/debug_regs
> /x86_64/evmcs_test
> /x86_64/get_cpuid_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..c748b9650e28 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -39,6 +39,7 @@ LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
> LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
>
> TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
> +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
> TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
> TEST_GEN_PROGS_x86_64 += x86_64/get_cpuid_test
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
> diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
> new file mode 100644
> index 000000000000..ad9972d99dfa
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test that KVM_GET_MSR_INDEX_LIST and
> + * KVM_GET_MSR_FEATURE_INDEX_LIST work as intended
> + *
> + * Copyright (C) 2020, Red Hat, Inc.
> + */
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "../lib/kvm_util_internal.h"
> +
> +static int kvm_num_index_msrs(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_INDEX_LIST, list);
> + TEST_ASSERT(r == -1 && errno == E2BIG,
> + "Unexpected result from KVM_GET_MSR_INDEX_LIST probe, r: %i",
> + r);
> +
> + r = list->nmsrs;
> + free(list);
> + return r;
> +}
> +
> +static void test_get_msr_index(void)
> +{
> + int old_res, res, kvm_fd;
> +
> + kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
> + if (kvm_fd < 0)
> + exit(KSFT_SKIP);
> +
> + old_res = kvm_num_index_msrs(kvm_fd, 0);
> + TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
> +
> + if (old_res != 1) {
> + res = kvm_num_index_msrs(kvm_fd, 1);
> + TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
> + TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
> + }
> +
> + close(kvm_fd);
> +}
> +
> +static int kvm_num_feature_msrs(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
> + TEST_ASSERT(r == -1 && errno == E2BIG,
> + "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST probe, r: %i",
> + r);
> +
> + r = list->nmsrs;
> + free(list);
> + return r;
> +}
> +
> +struct kvm_msr_list *kvm_get_msr_feature_list(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
> +
> + TEST_ASSERT(r == 0,
> + "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST, r: %i",
> + r);
> +
> + return list;
> +}
> +
> +static void test_get_msr_feature(void)
> +{
> + int res, old_res, i, kvm_fd;
> + struct kvm_msr_list *feature_list;
> +
> + kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
> + if (kvm_fd < 0)
> + exit(KSFT_SKIP);
> +
> + old_res = kvm_num_feature_msrs(kvm_fd, 0);
> + TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
> +
> + if (old_res != 1) {
> + res = kvm_num_feature_msrs(kvm_fd, 1);
> + TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
> + TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
> + }
> +
> + feature_list = kvm_get_msr_feature_list(kvm_fd, old_res);
> + TEST_ASSERT(old_res == feature_list->nmsrs,
> + "Unmatching number of msr indexes");
> +
> + for (i = 0; i < feature_list->nmsrs; i++)
> + kvm_get_feature_msr(feature_list->indices[i]);
> +
> + free(feature_list);
> + close(kvm_fd);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + if (kvm_check_cap(KVM_CAP_GET_MSR_FEATURES))
> + test_get_msr_feature();
> +
> + test_get_msr_index();
> +}
>
On Thu, Mar 18, 2021 at 03:56:29PM +0100, Emanuele Giuseppe Esposito wrote:
> Test the KVM_GET_MSR_FEATURE_INDEX_LIST
> and KVM_GET_MSR_INDEX_LIST ioctls.
>
> Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
> ---
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 1 +
> .../kvm/x86_64/get_msr_index_features.c | 124 ++++++++++++++++++
> 3 files changed, 126 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 32b87cc77c8e..d99f3969d371 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -5,6 +5,7 @@
> /s390x/resets
> /s390x/sync_regs_test
> /x86_64/cr4_cpuid_sync_test
> +/x86_64/get_msr_index_features
> /x86_64/debug_regs
> /x86_64/evmcs_test
> /x86_64/get_cpuid_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..c748b9650e28 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -39,6 +39,7 @@ LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
> LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
>
> TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
> +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
Maybe we should give up trying to keep an alphabetic order.
> TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
> TEST_GEN_PROGS_x86_64 += x86_64/get_cpuid_test
> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
> diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
> new file mode 100644
> index 000000000000..ad9972d99dfa
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test that KVM_GET_MSR_INDEX_LIST and
> + * KVM_GET_MSR_FEATURE_INDEX_LIST work as intended
> + *
> + * Copyright (C) 2020, Red Hat, Inc.
> + */
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "../lib/kvm_util_internal.h"
I'm not sure why the original kvm selftests authors decided to do this
internal stuff, but we should either kill that or avoid doing stuff like
this.
> +
> +static int kvm_num_index_msrs(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_INDEX_LIST, list);
> + TEST_ASSERT(r == -1 && errno == E2BIG,
> + "Unexpected result from KVM_GET_MSR_INDEX_LIST probe, r: %i",
> + r);
Weird indentation
> +
> + r = list->nmsrs;
> + free(list);
> + return r;
> +}
> +
> +static void test_get_msr_index(void)
> +{
> + int old_res, res, kvm_fd;
> +
> + kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
> + if (kvm_fd < 0)
> + exit(KSFT_SKIP);
> +
> + old_res = kvm_num_index_msrs(kvm_fd, 0);
> + TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
> +
> + if (old_res != 1) {
> + res = kvm_num_index_msrs(kvm_fd, 1);
> + TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
> + TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
> + }
> +
> + close(kvm_fd);
> +}
> +
> +static int kvm_num_feature_msrs(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
> + TEST_ASSERT(r == -1 && errno == E2BIG,
> + "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST probe, r: %i",
> + r);
Weird indentation. I'd just leave it up on the last line. We don't care
about long lines.
> +
> + r = list->nmsrs;
> + free(list);
> + return r;
> +}
> +
> +struct kvm_msr_list *kvm_get_msr_feature_list(int kvm_fd, int nmsrs)
> +{
> + struct kvm_msr_list *list;
> + int r;
> +
> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
> + list->nmsrs = nmsrs;
> + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
> +
> + TEST_ASSERT(r == 0,
> + "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST, r: %i",
> + r);
> +
> + return list;
> +}
> +
> +static void test_get_msr_feature(void)
> +{
> + int res, old_res, i, kvm_fd;
> + struct kvm_msr_list *feature_list;
> +
> + kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
> + if (kvm_fd < 0)
> + exit(KSFT_SKIP);
> +
> + old_res = kvm_num_feature_msrs(kvm_fd, 0);
> + TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
> +
> + if (old_res != 1) {
> + res = kvm_num_feature_msrs(kvm_fd, 1);
> + TEST_ASSERT(res > 1, "Expecting nmsrs to be > 1");
> + TEST_ASSERT(res == old_res, "Expecting nmsrs to be identical");
> + }
> +
> + feature_list = kvm_get_msr_feature_list(kvm_fd, old_res);
> + TEST_ASSERT(old_res == feature_list->nmsrs,
> + "Unmatching number of msr indexes");
Weird indentation
> +
> + for (i = 0; i < feature_list->nmsrs; i++)
> + kvm_get_feature_msr(feature_list->indices[i]);
> +
> + free(feature_list);
> + close(kvm_fd);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + if (kvm_check_cap(KVM_CAP_GET_MSR_FEATURES))
> + test_get_msr_feature();
> +
> + test_get_msr_index();
Missing return
> +}
> --
> 2.29.2
>
Thanks,
drew
On 18/03/21 18:03, Andrew Jones wrote:
>>
>> TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
>> +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
>
> Maybe we should give up trying to keep an alphabetic order.
FWIW I had fixed that but yeah maybe we should just give up.
>> +int main(int argc, char *argv[])
>> +{
>> + if (kvm_check_cap(KVM_CAP_GET_MSR_FEATURES))
>> + test_get_msr_feature();
>> +
>> + test_get_msr_index();
> Missing return
>
>> +}
"main" is special, it's okay not to have a return there.
Paolo
On Thu, Mar 18, 2021 at 06:33:35PM +0100, Paolo Bonzini wrote:
> On 18/03/21 18:03, Andrew Jones wrote:
> > >
> > > TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
> > > +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
> >
> > Maybe we should give up trying to keep an alphabetic order.
>
> FWIW I had fixed that but yeah maybe we should just give up.
>
> > > +int main(int argc, char *argv[])
> > > +{
> > > + if (kvm_check_cap(KVM_CAP_GET_MSR_FEATURES))
> > > + test_get_msr_feature();
> > > +
> > > + test_get_msr_index();
> > Missing return
> >
> > > +}
>
> "main" is special, it's okay not to have a return there.
Hmm, yeah. I always assumed the compiler would complain or that you'd end
up with a garbage return code. But, I just checked, and indeed not only do
you not get a warning, even with -Wall -Wextra, but the compiler actually
emits code for a zero return value on your behalf. Looks weird to me
though to end an int function without a return, so I don't think I'm
going to adopt this practice myself.
Thanks,
drew
On Thu, Mar 18, 2021, Paolo Bonzini wrote:
> On 18/03/21 18:03, Andrew Jones wrote:
> > >
> > > TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
> > > +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
> >
> > Maybe we should give up trying to keep an alphabetic order.
>
> FWIW I had fixed that but yeah maybe we should just give up.
Never! What if we make offenders wear a dunce cap at the next KVM Forum?
Hi Andrew,
Thank you for the feedback (also in v1).
On 18/03/2021 18:03, Andrew Jones wrote:
> On Thu, Mar 18, 2021 at 03:56:29PM +0100, Emanuele Giuseppe Esposito wrote:
>> Test the KVM_GET_MSR_FEATURE_INDEX_LIST
>> and KVM_GET_MSR_INDEX_LIST ioctls.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
>> ---
>> tools/testing/selftests/kvm/.gitignore | 1 +
>> tools/testing/selftests/kvm/Makefile | 1 +
>> .../kvm/x86_64/get_msr_index_features.c | 124 ++++++++++++++++++
>> 3 files changed, 126 insertions(+)
>> create mode 100644 tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
>>
>> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
>> index 32b87cc77c8e..d99f3969d371 100644
>> --- a/tools/testing/selftests/kvm/.gitignore
>> +++ b/tools/testing/selftests/kvm/.gitignore
>> @@ -5,6 +5,7 @@
>> /s390x/resets
>> /s390x/sync_regs_test
>> /x86_64/cr4_cpuid_sync_test
>> +/x86_64/get_msr_index_features
>> /x86_64/debug_regs
>> /x86_64/evmcs_test
>> /x86_64/get_cpuid_test
>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
>> index a6d61f451f88..c748b9650e28 100644
>> --- a/tools/testing/selftests/kvm/Makefile
>> +++ b/tools/testing/selftests/kvm/Makefile
>> @@ -39,6 +39,7 @@ LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
>> LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
>>
>> TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
>> +TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
>
> Maybe we should give up trying to keep an alphabetic order.
My bad, I did not notice that it was in alphabetic order.
>
>> TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
>> TEST_GEN_PROGS_x86_64 += x86_64/get_cpuid_test
>> TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
>> diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
>> new file mode 100644
>> index 000000000000..ad9972d99dfa
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c
>> @@ -0,0 +1,124 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Test that KVM_GET_MSR_INDEX_LIST and
>> + * KVM_GET_MSR_FEATURE_INDEX_LIST work as intended
>> + *
>> + * Copyright (C) 2020, Red Hat, Inc.
>> + */
>> +#include <fcntl.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/ioctl.h>
>> +
>> +#include "test_util.h"
>> +#include "kvm_util.h"
>> +#include "processor.h"
>> +#include "../lib/kvm_util_internal.h"
>
> I'm not sure why the original kvm selftests authors decided to do this
> internal stuff, but we should either kill that or avoid doing stuff like
> this.
I need this include because of the KVM_DEV_PATH macro, to get the kvm_fd.
No other reason for including it in this test.
>> +
>> +static int kvm_num_feature_msrs(int kvm_fd, int nmsrs)
>> +{
>> + struct kvm_msr_list *list;
>> + int r;
>> +
>> + list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0]));
>> + list->nmsrs = nmsrs;
>> + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, list);
>> + TEST_ASSERT(r == -1 && errno == E2BIG,
>> + "Unexpected result from KVM_GET_MSR_FEATURE_INDEX_LIST probe, r: %i",
>> + r);
>
> Weird indentation. I'd just leave it up on the last line. We don't care
> about long lines.
Ok. I wanted avoid warnings from the checkpatch script.
Paolo, do you want me to send v2 with fixed indentation or you already
took care of it? I'll be happy to do so.
Emanuele
On 19/03/21 09:07, Emanuele Giuseppe Esposito wrote:
>>
>> I'm not sure why the original kvm selftests authors decided to do this
>> internal stuff, but we should either kill that or avoid doing stuff like
>> this.
>
> I need this include because of the KVM_DEV_PATH macro, to get the kvm_fd.
> No other reason for including it in this test.
I'll take care of moving that macro to the non-internal kvm_util.h
header, thanks.
No need to do anything, I'll send the pull request later today.
Paolo