diff mbox series

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

Message ID 20221105045704.2315186-5-vipinsh@google.com
State New
Headers
Series Add Hyper-v extended hypercall support in KVM |

Commit Message

Vipin Sharma Nov. 5, 2022, 4:57 a.m. UTC 
  Make guest OS ID calculation common to all hyperv tests and similar to
hv_generate_guest_id().

Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
 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(-)

Comments

David Matlack Nov. 7, 2022, 7:08 p.m. UTC | #1 
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 <vipinsh@google.com>
> ---
>  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
>
Vipin Sharma Nov. 8, 2022, 1:45 a.m. UTC | #2 
On Mon, Nov 7, 2022 at 11:08 AM David Matlack <dmatlack@google.com> 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 <vipinsh@google.com>
> > ---
> >  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
> >
David Matlack Nov. 8, 2022, 5:56 p.m. UTC | #3 
On Mon, Nov 7, 2022 at 5:45 PM Vipin Sharma <vipinsh@google.com> wrote:
>
> On Mon, Nov 7, 2022 at 11:08 AM David Matlack <dmatlack@google.com> 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 <vipinsh@google.com>
> > > ---
> > >  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
> > >
Vitaly Kuznetsov Nov. 9, 2022, 1:48 p.m. UTC | #4 
Vipin Sharma <vipinsh@google.com> 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/20221101145426.251680-32-vkuznets@redhat.com/

>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
>  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. */
Vipin Sharma Nov. 9, 2022, 6:52 p.m. UTC | #5 
On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Vipin Sharma <vipinsh@google.com> 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/20221101145426.251680-32-vkuznets@redhat.com/
>

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.
Sean Christopherson Nov. 9, 2022, 8:18 p.m. UTC | #6 
On Wed, Nov 09, 2022, Vipin Sharma wrote:
> On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >
> > Vipin Sharma <vipinsh@google.com> 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/20221101145426.251680-32-vkuznets@redhat.com/
> >
> 
> 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 ;-)
Vitaly Kuznetsov Nov. 10, 2022, 10:02 a.m. UTC | #7 
Vipin Sharma <vipinsh@google.com> writes:

> On Wed, Nov 9, 2022 at 5:48 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Vipin Sharma <vipinsh@google.com> 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/20221101145426.251680-32-vkuznets@redhat.com/
>>
>
> 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)
diff mbox series

Patch

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. */