2022-11-05 05:02:49

by Vipin Sharma

[permalink] [raw]
Subject: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

Make guest OS ID calculation common to all hyperv tests and similar to
hv_generate_guest_id().

Signed-off-by: Vipin Sharma <[email protected]>
---
tools/testing/selftests/kvm/include/x86_64/hyperv.h | 10 ++++++++++
tools/testing/selftests/kvm/x86_64/hyperv_clock.c | 2 +-
tools/testing/selftests/kvm/x86_64/hyperv_features.c | 6 ++----
tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c | 2 +-
4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
index 075fd29071a6..9d8c325af1d9 100644
--- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
+++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
@@ -9,6 +9,10 @@
#ifndef SELFTEST_KVM_HYPERV_H
#define SELFTEST_KVM_HYPERV_H

+#include <linux/version.h>
+
+#define HV_LINUX_VENDOR_ID 0x8100
+
#define HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS 0x40000000
#define HYPERV_CPUID_INTERFACE 0x40000001
#define HYPERV_CPUID_VERSION 0x40000002
@@ -189,4 +193,10 @@
/* hypercall options */
#define HV_HYPERCALL_FAST_BIT BIT(16)

+static inline uint64_t hv_linux_guest_id(void)
+{
+ return ((uint64_t)HV_LINUX_VENDOR_ID << 48) |
+ ((uint64_t)LINUX_VERSION_CODE << 16);
+}
+
#endif /* !SELFTEST_KVM_HYPERV_H */
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
index d576bc8ce823..f9112c5dc3f7 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
@@ -104,7 +104,7 @@ static void guest_main(struct ms_hyperv_tsc_page *tsc_page, vm_paddr_t tsc_page_

/* Set Guest OS id to enable Hyper-V emulation */
GUEST_SYNC(1);
- wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
+ wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
GUEST_SYNC(2);

check_tsc_msr_rdtsc();
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index 6b443ce456b6..b5a42cf1ad9d 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -13,8 +13,6 @@
#include "processor.h"
#include "hyperv.h"

-#define LINUX_OS_ID ((u64)0x8100 << 48)
-
static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
vm_vaddr_t output_address, uint64_t *hv_status)
{
@@ -71,7 +69,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)

GUEST_ASSERT(hcall->control);

- wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
+ wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);

if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
@@ -169,7 +167,7 @@ static void guest_test_msrs_access(void)
*/
msr->idx = HV_X64_MSR_GUEST_OS_ID;
msr->write = 1;
- msr->write_val = LINUX_OS_ID;
+ msr->write_val = hv_linux_guest_id();
msr->available = 1;
break;
case 3:
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
index a380ad7bb9b3..2c13a144b04c 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
@@ -69,7 +69,7 @@ static void __attribute__((__flatten__)) guest_code(struct svm_test_data *svm)

GUEST_SYNC(1);

- wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
+ wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());

GUEST_ASSERT(svm->vmcb_gpa);
/* Prepare for L2 execution. */
--
2.38.1.273.g43a17bfeac-goog



2022-11-07 19:28:49

by David Matlack

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

On Fri, Nov 04, 2022 at 09:57:02PM -0700, Vipin Sharma wrote:
> Make guest OS ID calculation common to all hyperv tests and similar to
> hv_generate_guest_id().

This commit makes the HV_LINUX_VENDOR_ID and adds LINUX_VERSION_CODE
to existing tests. Can you split out the latter to a separate commit?
Also what's the reason to add LINUX_VERSION_CODE to the mix?

>
> Signed-off-by: Vipin Sharma <[email protected]>
> ---
> tools/testing/selftests/kvm/include/x86_64/hyperv.h | 10 ++++++++++
> tools/testing/selftests/kvm/x86_64/hyperv_clock.c | 2 +-
> tools/testing/selftests/kvm/x86_64/hyperv_features.c | 6 ++----
> tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c | 2 +-
> 4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> index 075fd29071a6..9d8c325af1d9 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> @@ -9,6 +9,10 @@
> #ifndef SELFTEST_KVM_HYPERV_H
> #define SELFTEST_KVM_HYPERV_H
>
> +#include <linux/version.h>
> +
> +#define HV_LINUX_VENDOR_ID 0x8100
> +
> #define HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS 0x40000000
> #define HYPERV_CPUID_INTERFACE 0x40000001
> #define HYPERV_CPUID_VERSION 0x40000002
> @@ -189,4 +193,10 @@
> /* hypercall options */
> #define HV_HYPERCALL_FAST_BIT BIT(16)
>
> +static inline uint64_t hv_linux_guest_id(void)
> +{
> + return ((uint64_t)HV_LINUX_VENDOR_ID << 48) |
> + ((uint64_t)LINUX_VERSION_CODE << 16);

This can be a compile-time constant (i.e. macro).

> +}
> +
> #endif /* !SELFTEST_KVM_HYPERV_H */
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> index d576bc8ce823..f9112c5dc3f7 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> @@ -104,7 +104,7 @@ static void guest_main(struct ms_hyperv_tsc_page *tsc_page, vm_paddr_t tsc_page_
>
> /* Set Guest OS id to enable Hyper-V emulation */
> GUEST_SYNC(1);
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> GUEST_SYNC(2);
>
> check_tsc_msr_rdtsc();
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index 6b443ce456b6..b5a42cf1ad9d 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -13,8 +13,6 @@
> #include "processor.h"
> #include "hyperv.h"
>
> -#define LINUX_OS_ID ((u64)0x8100 << 48)
> -
> static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
> vm_vaddr_t output_address, uint64_t *hv_status)
> {
> @@ -71,7 +69,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
>
> GUEST_ASSERT(hcall->control);
>
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
>
> if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> @@ -169,7 +167,7 @@ static void guest_test_msrs_access(void)
> */
> msr->idx = HV_X64_MSR_GUEST_OS_ID;
> msr->write = 1;
> - msr->write_val = LINUX_OS_ID;
> + msr->write_val = hv_linux_guest_id();
> msr->available = 1;
> break;
> case 3:
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> index a380ad7bb9b3..2c13a144b04c 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> @@ -69,7 +69,7 @@ static void __attribute__((__flatten__)) guest_code(struct svm_test_data *svm)
>
> GUEST_SYNC(1);
>
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
>
> GUEST_ASSERT(svm->vmcb_gpa);
> /* Prepare for L2 execution. */
> --
> 2.38.1.273.g43a17bfeac-goog
>

2022-11-08 02:29:56

by Vipin Sharma

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

On Mon, Nov 7, 2022 at 11:08 AM David Matlack <[email protected]> wrote:
>
> On Fri, Nov 04, 2022 at 09:57:02PM -0700, Vipin Sharma wrote:
> > Make guest OS ID calculation common to all hyperv tests and similar to
> > hv_generate_guest_id().
>
> This commit makes the HV_LINUX_VENDOR_ID and adds LINUX_VERSION_CODE
> to existing tests. Can you split out the latter to a separate commit?
> Also what's the reason to add LINUX_VERSION_CODE to the mix?
>

I looked at the implementation in selftest and what is in the
include/asm-generic/mshyperv.h for the hv_generate_guest_id()
function, both looked different. I thought it would be nice to have
consistent logic at both places.

I can remove LINUX_VERISON_CODE if you prefer.

> >
> > Signed-off-by: Vipin Sharma <[email protected]>
> > ---
> > tools/testing/selftests/kvm/include/x86_64/hyperv.h | 10 ++++++++++
> > tools/testing/selftests/kvm/x86_64/hyperv_clock.c | 2 +-
> > tools/testing/selftests/kvm/x86_64/hyperv_features.c | 6 ++----
> > tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c | 2 +-
> > 4 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > index 075fd29071a6..9d8c325af1d9 100644
> > --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > @@ -9,6 +9,10 @@
> > #ifndef SELFTEST_KVM_HYPERV_H
> > #define SELFTEST_KVM_HYPERV_H
> >
> > +#include <linux/version.h>
> > +
> > +#define HV_LINUX_VENDOR_ID 0x8100
> > +
> > #define HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS 0x40000000
> > #define HYPERV_CPUID_INTERFACE 0x40000001
> > #define HYPERV_CPUID_VERSION 0x40000002
> > @@ -189,4 +193,10 @@
> > /* hypercall options */
> > #define HV_HYPERCALL_FAST_BIT BIT(16)
> >
> > +static inline uint64_t hv_linux_guest_id(void)
> > +{
> > + return ((uint64_t)HV_LINUX_VENDOR_ID << 48) |
> > + ((uint64_t)LINUX_VERSION_CODE << 16);
>
> This can be a compile-time constant (i.e. macro).
>

Yes, I will make it a macro in the next version.

> > +}
> > +
> > #endif /* !SELFTEST_KVM_HYPERV_H */
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > index d576bc8ce823..f9112c5dc3f7 100644
> > --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > @@ -104,7 +104,7 @@ static void guest_main(struct ms_hyperv_tsc_page *tsc_page, vm_paddr_t tsc_page_
> >
> > /* Set Guest OS id to enable Hyper-V emulation */
> > GUEST_SYNC(1);
> > - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > GUEST_SYNC(2);
> >
> > check_tsc_msr_rdtsc();
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > index 6b443ce456b6..b5a42cf1ad9d 100644
> > --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > @@ -13,8 +13,6 @@
> > #include "processor.h"
> > #include "hyperv.h"
> >
> > -#define LINUX_OS_ID ((u64)0x8100 << 48)
> > -
> > static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
> > vm_vaddr_t output_address, uint64_t *hv_status)
> > {
> > @@ -71,7 +69,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
> >
> > GUEST_ASSERT(hcall->control);
> >
> > - wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
> >
> > if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> > @@ -169,7 +167,7 @@ static void guest_test_msrs_access(void)
> > */
> > msr->idx = HV_X64_MSR_GUEST_OS_ID;
> > msr->write = 1;
> > - msr->write_val = LINUX_OS_ID;
> > + msr->write_val = hv_linux_guest_id();
> > msr->available = 1;
> > break;
> > case 3:
> > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > index a380ad7bb9b3..2c13a144b04c 100644
> > --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > @@ -69,7 +69,7 @@ static void __attribute__((__flatten__)) guest_code(struct svm_test_data *svm)
> >
> > GUEST_SYNC(1);
> >
> > - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> >
> > GUEST_ASSERT(svm->vmcb_gpa);
> > /* Prepare for L2 execution. */
> > --
> > 2.38.1.273.g43a17bfeac-goog
> >

2022-11-08 18:13:16

by David Matlack

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

On Mon, Nov 7, 2022 at 5:45 PM Vipin Sharma <[email protected]> wrote:
>
> On Mon, Nov 7, 2022 at 11:08 AM David Matlack <[email protected]> wrote:
> >
> > On Fri, Nov 04, 2022 at 09:57:02PM -0700, Vipin Sharma wrote:
> > > Make guest OS ID calculation common to all hyperv tests and similar to
> > > hv_generate_guest_id().
> >
> > This commit makes the HV_LINUX_VENDOR_ID and adds LINUX_VERSION_CODE
> > to existing tests. Can you split out the latter to a separate commit?
> > Also what's the reason to add LINUX_VERSION_CODE to the mix?
> >
>
> I looked at the implementation in selftest and what is in the
> include/asm-generic/mshyperv.h for the hv_generate_guest_id()
> function, both looked different. I thought it would be nice to have
> consistent logic at both places.
>
> I can remove LINUX_VERISON_CODE if you prefer.

Using LINUX_VERSION_CODE here assumes the selftest will run on the
same kernel that it was compiled with. This might not be the case in
practice, e.g. if anyone runs the latest upstream selftests against
their internal kernel (something we've discussed doing recently).

The right way to incorporate the Linux version code would be for the
selftest to query it from the host dynamically and use that (at which
point hv_linux_guest_id() would actually have to be a function).

But since you don't strictly need it, it's probably best to just omit
it for the time being.



>
> > >
> > > Signed-off-by: Vipin Sharma <[email protected]>
> > > ---
> > > tools/testing/selftests/kvm/include/x86_64/hyperv.h | 10 ++++++++++
> > > tools/testing/selftests/kvm/x86_64/hyperv_clock.c | 2 +-
> > > tools/testing/selftests/kvm/x86_64/hyperv_features.c | 6 ++----
> > > tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c | 2 +-
> > > 4 files changed, 14 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > > index 075fd29071a6..9d8c325af1d9 100644
> > > --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > > +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> > > @@ -9,6 +9,10 @@
> > > #ifndef SELFTEST_KVM_HYPERV_H
> > > #define SELFTEST_KVM_HYPERV_H
> > >
> > > +#include <linux/version.h>
> > > +
> > > +#define HV_LINUX_VENDOR_ID 0x8100
> > > +
> > > #define HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS 0x40000000
> > > #define HYPERV_CPUID_INTERFACE 0x40000001
> > > #define HYPERV_CPUID_VERSION 0x40000002
> > > @@ -189,4 +193,10 @@
> > > /* hypercall options */
> > > #define HV_HYPERCALL_FAST_BIT BIT(16)
> > >
> > > +static inline uint64_t hv_linux_guest_id(void)
> > > +{
> > > + return ((uint64_t)HV_LINUX_VENDOR_ID << 48) |
> > > + ((uint64_t)LINUX_VERSION_CODE << 16);
> >
> > This can be a compile-time constant (i.e. macro).
> >
>
> Yes, I will make it a macro in the next version.
>
> > > +}
> > > +
> > > #endif /* !SELFTEST_KVM_HYPERV_H */
> > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > > index d576bc8ce823..f9112c5dc3f7 100644
> > > --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> > > @@ -104,7 +104,7 @@ static void guest_main(struct ms_hyperv_tsc_page *tsc_page, vm_paddr_t tsc_page_
> > >
> > > /* Set Guest OS id to enable Hyper-V emulation */
> > > GUEST_SYNC(1);
> > > - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> > > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > > GUEST_SYNC(2);
> > >
> > > check_tsc_msr_rdtsc();
> > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > > index 6b443ce456b6..b5a42cf1ad9d 100644
> > > --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> > > @@ -13,8 +13,6 @@
> > > #include "processor.h"
> > > #include "hyperv.h"
> > >
> > > -#define LINUX_OS_ID ((u64)0x8100 << 48)
> > > -
> > > static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
> > > vm_vaddr_t output_address, uint64_t *hv_status)
> > > {
> > > @@ -71,7 +69,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
> > >
> > > GUEST_ASSERT(hcall->control);
> > >
> > > - wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> > > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > > wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
> > >
> > > if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> > > @@ -169,7 +167,7 @@ static void guest_test_msrs_access(void)
> > > */
> > > msr->idx = HV_X64_MSR_GUEST_OS_ID;
> > > msr->write = 1;
> > > - msr->write_val = LINUX_OS_ID;
> > > + msr->write_val = hv_linux_guest_id();
> > > msr->available = 1;
> > > break;
> > > case 3:
> > > diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > > index a380ad7bb9b3..2c13a144b04c 100644
> > > --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > > +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> > > @@ -69,7 +69,7 @@ static void __attribute__((__flatten__)) guest_code(struct svm_test_data *svm)
> > >
> > > GUEST_SYNC(1);
> > >
> > > - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> > > + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> > >
> > > GUEST_ASSERT(svm->vmcb_gpa);
> > > /* Prepare for L2 execution. */
> > > --
> > > 2.38.1.273.g43a17bfeac-goog
> > >

2022-11-09 14:09:04

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

Vipin Sharma <[email protected]> writes:

> Make guest OS ID calculation common to all hyperv tests and similar to
> hv_generate_guest_id().

A similar (but without hv_linux_guest_id()) patch is present in my
Hyper-V TLB flush update:

https://lore.kernel.org/kvm/[email protected]/

>
> Signed-off-by: Vipin Sharma <[email protected]>
> ---
> tools/testing/selftests/kvm/include/x86_64/hyperv.h | 10 ++++++++++
> tools/testing/selftests/kvm/x86_64/hyperv_clock.c | 2 +-
> tools/testing/selftests/kvm/x86_64/hyperv_features.c | 6 ++----
> tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c | 2 +-
> 4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/hyperv.h b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> index 075fd29071a6..9d8c325af1d9 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/hyperv.h
> @@ -9,6 +9,10 @@
> #ifndef SELFTEST_KVM_HYPERV_H
> #define SELFTEST_KVM_HYPERV_H
>
> +#include <linux/version.h>
> +
> +#define HV_LINUX_VENDOR_ID 0x8100
> +
> #define HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS 0x40000000
> #define HYPERV_CPUID_INTERFACE 0x40000001
> #define HYPERV_CPUID_VERSION 0x40000002
> @@ -189,4 +193,10 @@
> /* hypercall options */
> #define HV_HYPERCALL_FAST_BIT BIT(16)
>
> +static inline uint64_t hv_linux_guest_id(void)
> +{
> + return ((uint64_t)HV_LINUX_VENDOR_ID << 48) |
> + ((uint64_t)LINUX_VERSION_CODE << 16);
> +}
> +
> #endif /* !SELFTEST_KVM_HYPERV_H */
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> index d576bc8ce823..f9112c5dc3f7 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> @@ -104,7 +104,7 @@ static void guest_main(struct ms_hyperv_tsc_page *tsc_page, vm_paddr_t tsc_page_
>
> /* Set Guest OS id to enable Hyper-V emulation */
> GUEST_SYNC(1);
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> GUEST_SYNC(2);
>
> check_tsc_msr_rdtsc();
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index 6b443ce456b6..b5a42cf1ad9d 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -13,8 +13,6 @@
> #include "processor.h"
> #include "hyperv.h"
>
> -#define LINUX_OS_ID ((u64)0x8100 << 48)
> -
> static inline uint8_t hypercall(u64 control, vm_vaddr_t input_address,
> vm_vaddr_t output_address, uint64_t *hv_status)
> {
> @@ -71,7 +69,7 @@ static void guest_hcall(vm_vaddr_t pgs_gpa, struct hcall_data *hcall)
>
> GUEST_ASSERT(hcall->control);
>
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, LINUX_OS_ID);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
> wrmsr(HV_X64_MSR_HYPERCALL, pgs_gpa);
>
> if (!(hcall->control & HV_HYPERCALL_FAST_BIT)) {
> @@ -169,7 +167,7 @@ static void guest_test_msrs_access(void)
> */
> msr->idx = HV_X64_MSR_GUEST_OS_ID;
> msr->write = 1;
> - msr->write_val = LINUX_OS_ID;
> + msr->write_val = hv_linux_guest_id();
> msr->available = 1;
> break;
> case 3:
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> index a380ad7bb9b3..2c13a144b04c 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> @@ -69,7 +69,7 @@ static void __attribute__((__flatten__)) guest_code(struct svm_test_data *svm)
>
> GUEST_SYNC(1);
>
> - wrmsr(HV_X64_MSR_GUEST_OS_ID, (u64)0x8100 << 48);
> + wrmsr(HV_X64_MSR_GUEST_OS_ID, hv_linux_guest_id());
>
> GUEST_ASSERT(svm->vmcb_gpa);
> /* Prepare for L2 execution. */

--
Vitaly


2022-11-09 19:27:11

by Vipin Sharma

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <[email protected]> wrote:
>
> Vipin Sharma <[email protected]> writes:
>
> > Make guest OS ID calculation common to all hyperv tests and similar to
> > hv_generate_guest_id().
>
> A similar (but without hv_linux_guest_id()) patch is present in my
> Hyper-V TLB flush update:
>
> https://lore.kernel.org/kvm/[email protected]/
>

After getting feedback from David, I decided to remove
LINUX_VERSION_CODE in v2. Our patches are functionally identical now.

@Sean, Paolo, Vitaly
Should I be rebasing my v2 on top of TLB flush patch series and remove
patch 4 and 5 from my series? I am not sure how these situations are
handled.

@Vitaly
Are you planning to send v14?

If yes, then for v13 Patch 31 (KVM: selftests: Move HYPERV_LINUX_OS_ID
definition to a common header) will you keep it same or will you
modify it to add HYPERV_LINUX_OS_ID in hyperv_clock.c and
hyperv_svm_test.c?

If not, then I can add a patch in my series to change those two files
if I end up rebasing on top of your series.

2022-11-09 20:54:53

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

On Wed, Nov 09, 2022, Vipin Sharma wrote:
> On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <[email protected]> wrote:
> >
> > Vipin Sharma <[email protected]> writes:
> >
> > > Make guest OS ID calculation common to all hyperv tests and similar to
> > > hv_generate_guest_id().
> >
> > A similar (but without hv_linux_guest_id()) patch is present in my
> > Hyper-V TLB flush update:
> >
> > https://lore.kernel.org/kvm/[email protected]/
> >
>
> After getting feedback from David, I decided to remove
> LINUX_VERSION_CODE in v2. Our patches are functionally identical now.
>
> @Sean, Paolo, Vitaly
> Should I be rebasing my v2 on top of TLB flush patch series and remove
> patch 4 and 5 from my series? I am not sure how these situations are
> handled.

In this case, unless Paolo is NOT planning on merging Vitaly's series for 6.2, I
would just wait for Vitaly's series to get pushed to kvm/queue. I'm banking on
Paolo going on a queueing spree soon ;-)

2022-11-10 11:02:48

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH 4/6] KVM: selftests: Make Hyper-V guest OS ID common

Vipin Sharma <[email protected]> writes:

> On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <[email protected]> wrote:
>>
>> Vipin Sharma <[email protected]> writes:
>>
>> > Make guest OS ID calculation common to all hyperv tests and similar to
>> > hv_generate_guest_id().
>>
>> A similar (but without hv_linux_guest_id()) patch is present in my
>> Hyper-V TLB flush update:
>>
>> https://lore.kernel.org/kvm/[email protected]/
>>
>
> After getting feedback from David, I decided to remove
> LINUX_VERSION_CODE in v2. Our patches are functionally identical now.
>
> @Sean, Paolo, Vitaly
> Should I be rebasing my v2 on top of TLB flush patch series and remove
> patch 4 and 5 from my series? I am not sure how these situations are
> handled.
>
> @Vitaly
> Are you planning to send v14?
>
> If yes, then for v13 Patch 31 (KVM: selftests: Move HYPERV_LINUX_OS_ID
> definition to a common header) will you keep it same or will you
> modify it to add HYPERV_LINUX_OS_ID in hyperv_clock.c and
> hyperv_svm_test.c?
>
> If not, then I can add a patch in my series to change those two files
> if I end up rebasing on top of your series.
>

Rumor has it that v13 is going to be merged to kvm/queue soon so I have
no plans for v14 at this point. Fingers crossed)

--
Vitaly