1. Patchew
  2. Xen
  3. View series

[PATCH] x86/hyperv: stash and use the configured max VP index

Wei Liu posted 1 patch 3 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200429104144.17816-1-liuwe@microsoft.com
Maintainers: "Roger Pau Monné" <roger.pau@citrix.com>, Jan Beulich <jbeulich@suse.com>, Wei Liu <wl@xen.org>, Paul Durrant <paul@xen.org>, Andrew Cooper <andrew.cooper3@citrix.com>
xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
xen/arch/x86/guest/hyperv/private.h | 1 +
xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
xen/arch/x86/guest/hyperv/util.c    | 2 +-
4 files changed, 7 insertions(+), 2 deletions(-)
[PATCH] x86/hyperv: stash and use the configured max VP index
Posted by Wei Liu 3 years, 12 months ago 
The value returned from CPUID is the maximum number for virtual
processors supported by Hyper-V. It could be larger than the maximum
number of virtual processors configured.

Stash the configured number into a variable and use it in calculations.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
 xen/arch/x86/guest/hyperv/private.h | 1 +
 xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
 xen/arch/x86/guest/hyperv/util.c    | 2 +-
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
index 91a6782cd986..84221b751453 100644
--- a/xen/arch/x86/guest/hyperv/hyperv.c
+++ b/xen/arch/x86/guest/hyperv/hyperv.c
@@ -33,6 +33,7 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
 DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
 DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
 
+unsigned int __read_mostly hv_max_vp_index;
 static bool __read_mostly hcall_page_ready;
 
 static uint64_t generate_guest_id(void)
@@ -143,6 +144,9 @@ static int setup_hypercall_pcpu_arg(void)
     rdmsrl(HV_X64_MSR_VP_INDEX, vp_index_msr);
     this_cpu(hv_vp_index) = vp_index_msr;
 
+    if ( vp_index_msr > hv_max_vp_index )
+        hv_max_vp_index = vp_index_msr;
+
     return 0;
 }
 
diff --git a/xen/arch/x86/guest/hyperv/private.h b/xen/arch/x86/guest/hyperv/private.h
index 354fc7f685a7..fea3e291e944 100644
--- a/xen/arch/x86/guest/hyperv/private.h
+++ b/xen/arch/x86/guest/hyperv/private.h
@@ -28,6 +28,7 @@
 DECLARE_PER_CPU(void *, hv_input_page);
 DECLARE_PER_CPU(void *, hv_vp_assist);
 DECLARE_PER_CPU(unsigned int, hv_vp_index);
+extern unsigned int hv_max_vp_index;
 
 static inline unsigned int hv_vp_index(unsigned int cpu)
 {
diff --git a/xen/arch/x86/guest/hyperv/tlb.c b/xen/arch/x86/guest/hyperv/tlb.c
index 1d723d6ee679..0a44071481bd 100644
--- a/xen/arch/x86/guest/hyperv/tlb.c
+++ b/xen/arch/x86/guest/hyperv/tlb.c
@@ -166,7 +166,7 @@ int hyperv_flush_tlb(const cpumask_t *mask, const void *va,
         {
             unsigned int vpid = hv_vp_index(cpu);
 
-            if ( vpid >= ms_hyperv.max_vp_index )
+            if ( vpid >= hv_max_vp_index )
             {
                 local_irq_restore(irq_flags);
                 return -ENXIO;
diff --git a/xen/arch/x86/guest/hyperv/util.c b/xen/arch/x86/guest/hyperv/util.c
index bec61c2afd87..2c5f421b7bd9 100644
--- a/xen/arch/x86/guest/hyperv/util.c
+++ b/xen/arch/x86/guest/hyperv/util.c
@@ -33,7 +33,7 @@ int cpumask_to_vpset(struct hv_vpset *vpset,
 {
     int nr = 1;
     unsigned int cpu, vcpu_bank, vcpu_offset;
-    unsigned int max_banks = ms_hyperv.max_vp_index / 64;
+    unsigned int max_banks = hv_max_vp_index / 64;
 
     /* Up to 64 banks can be represented by valid_bank_mask */
     if ( max_banks > 64 )
-- 
2.20.1
Re: [PATCH] x86/hyperv: stash and use the configured max VP index
Posted by Wei Liu 3 years, 12 months ago 
On Wed, Apr 29, 2020 at 11:41:44AM +0100, Wei Liu wrote:
> The value returned from CPUID is the maximum number for virtual
> processors supported by Hyper-V. It could be larger than the maximum
> number of virtual processors configured.
> 
> Stash the configured number into a variable and use it in calculations.
> 
> Signed-off-by: Wei Liu <liuwe@microsoft.com>
> ---
>  xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
>  xen/arch/x86/guest/hyperv/private.h | 1 +
>  xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
>  xen/arch/x86/guest/hyperv/util.c    | 2 +-
>  4 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> index 91a6782cd986..84221b751453 100644
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -33,6 +33,7 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
>  DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
>  DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
>  
> +unsigned int __read_mostly hv_max_vp_index;
>  static bool __read_mostly hcall_page_ready;
>  
>  static uint64_t generate_guest_id(void)
> @@ -143,6 +144,9 @@ static int setup_hypercall_pcpu_arg(void)
>      rdmsrl(HV_X64_MSR_VP_INDEX, vp_index_msr);
>      this_cpu(hv_vp_index) = vp_index_msr;
>  
> +    if ( vp_index_msr > hv_max_vp_index )
> +        hv_max_vp_index = vp_index_msr;
> +
>      return 0;
>  }
>  
> diff --git a/xen/arch/x86/guest/hyperv/private.h b/xen/arch/x86/guest/hyperv/private.h
> index 354fc7f685a7..fea3e291e944 100644
> --- a/xen/arch/x86/guest/hyperv/private.h
> +++ b/xen/arch/x86/guest/hyperv/private.h
> @@ -28,6 +28,7 @@
>  DECLARE_PER_CPU(void *, hv_input_page);
>  DECLARE_PER_CPU(void *, hv_vp_assist);
>  DECLARE_PER_CPU(unsigned int, hv_vp_index);
> +extern unsigned int hv_max_vp_index;
>  
>  static inline unsigned int hv_vp_index(unsigned int cpu)
>  {
> diff --git a/xen/arch/x86/guest/hyperv/tlb.c b/xen/arch/x86/guest/hyperv/tlb.c
> index 1d723d6ee679..0a44071481bd 100644
> --- a/xen/arch/x86/guest/hyperv/tlb.c
> +++ b/xen/arch/x86/guest/hyperv/tlb.c
> @@ -166,7 +166,7 @@ int hyperv_flush_tlb(const cpumask_t *mask, const void *va,
>          {
>              unsigned int vpid = hv_vp_index(cpu);
>  
> -            if ( vpid >= ms_hyperv.max_vp_index )
> +            if ( vpid >= hv_max_vp_index )

I think the >= should be changed to > here.

Wei.

>              {
>                  local_irq_restore(irq_flags);
>                  return -ENXIO;
> diff --git a/xen/arch/x86/guest/hyperv/util.c b/xen/arch/x86/guest/hyperv/util.c
> index bec61c2afd87..2c5f421b7bd9 100644
> --- a/xen/arch/x86/guest/hyperv/util.c
> +++ b/xen/arch/x86/guest/hyperv/util.c
> @@ -33,7 +33,7 @@ int cpumask_to_vpset(struct hv_vpset *vpset,
>  {
>      int nr = 1;
>      unsigned int cpu, vcpu_bank, vcpu_offset;
> -    unsigned int max_banks = ms_hyperv.max_vp_index / 64;
> +    unsigned int max_banks = hv_max_vp_index / 64;
>  
>      /* Up to 64 banks can be represented by valid_bank_mask */
>      if ( max_banks > 64 )
> -- 
> 2.20.1
>
Re: [PATCH] x86/hyperv: stash and use the configured max VP index
Posted by Roger Pau Monné 3 years, 12 months ago 
On Wed, Apr 29, 2020 at 11:47:18AM +0000, Wei Liu wrote:
> On Wed, Apr 29, 2020 at 11:41:44AM +0100, Wei Liu wrote:
> > The value returned from CPUID is the maximum number for virtual
> > processors supported by Hyper-V. It could be larger than the maximum
> > number of virtual processors configured.
> > 
> > Stash the configured number into a variable and use it in calculations.
> > 
> > Signed-off-by: Wei Liu <liuwe@microsoft.com>
> > ---
> >  xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
> >  xen/arch/x86/guest/hyperv/private.h | 1 +
> >  xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
> >  xen/arch/x86/guest/hyperv/util.c    | 2 +-
> >  4 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> > index 91a6782cd986..84221b751453 100644
> > --- a/xen/arch/x86/guest/hyperv/hyperv.c
> > +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> > @@ -33,6 +33,7 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
> >  DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
> >  DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
> >  
> > +unsigned int __read_mostly hv_max_vp_index;
> >  static bool __read_mostly hcall_page_ready;
> >  
> >  static uint64_t generate_guest_id(void)
> > @@ -143,6 +144,9 @@ static int setup_hypercall_pcpu_arg(void)
> >      rdmsrl(HV_X64_MSR_VP_INDEX, vp_index_msr);
> >      this_cpu(hv_vp_index) = vp_index_msr;
> >  
> > +    if ( vp_index_msr > hv_max_vp_index )
> > +        hv_max_vp_index = vp_index_msr;
> > +
> >      return 0;
> >  }
> >  
> > diff --git a/xen/arch/x86/guest/hyperv/private.h b/xen/arch/x86/guest/hyperv/private.h
> > index 354fc7f685a7..fea3e291e944 100644
> > --- a/xen/arch/x86/guest/hyperv/private.h
> > +++ b/xen/arch/x86/guest/hyperv/private.h
> > @@ -28,6 +28,7 @@
> >  DECLARE_PER_CPU(void *, hv_input_page);
> >  DECLARE_PER_CPU(void *, hv_vp_assist);
> >  DECLARE_PER_CPU(unsigned int, hv_vp_index);
> > +extern unsigned int hv_max_vp_index;
> >  
> >  static inline unsigned int hv_vp_index(unsigned int cpu)
> >  {
> > diff --git a/xen/arch/x86/guest/hyperv/tlb.c b/xen/arch/x86/guest/hyperv/tlb.c
> > index 1d723d6ee679..0a44071481bd 100644
> > --- a/xen/arch/x86/guest/hyperv/tlb.c
> > +++ b/xen/arch/x86/guest/hyperv/tlb.c
> > @@ -166,7 +166,7 @@ int hyperv_flush_tlb(const cpumask_t *mask, const void *va,
> >          {
> >              unsigned int vpid = hv_vp_index(cpu);
> >  
> > -            if ( vpid >= ms_hyperv.max_vp_index )
> > +            if ( vpid >= hv_max_vp_index )
> 
> I think the >= should be changed to > here.

I agree. With this fixed:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks!
Re: [PATCH] x86/hyperv: stash and use the configured max VP index
Posted by Roger Pau Monné 3 years, 12 months ago 
On Thu, Apr 30, 2020 at 12:15:58PM +0200, Roger Pau Monné wrote:
> On Wed, Apr 29, 2020 at 11:47:18AM +0000, Wei Liu wrote:
> > On Wed, Apr 29, 2020 at 11:41:44AM +0100, Wei Liu wrote:
> > > The value returned from CPUID is the maximum number for virtual
> > > processors supported by Hyper-V. It could be larger than the maximum
> > > number of virtual processors configured.
> > > 
> > > Stash the configured number into a variable and use it in calculations.
> > > 
> > > Signed-off-by: Wei Liu <liuwe@microsoft.com>
> > > ---
> > >  xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
> > >  xen/arch/x86/guest/hyperv/private.h | 1 +
> > >  xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
> > >  xen/arch/x86/guest/hyperv/util.c    | 2 +-
> > >  4 files changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> > > index 91a6782cd986..84221b751453 100644
> > > --- a/xen/arch/x86/guest/hyperv/hyperv.c
> > > +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> > > @@ -33,6 +33,7 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
> > >  DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
> > >  DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
> > >  
> > > +unsigned int __read_mostly hv_max_vp_index;
> > >  static bool __read_mostly hcall_page_ready;
> > >  
> > >  static uint64_t generate_guest_id(void)
> > > @@ -143,6 +144,9 @@ static int setup_hypercall_pcpu_arg(void)
> > >      rdmsrl(HV_X64_MSR_VP_INDEX, vp_index_msr);
> > >      this_cpu(hv_vp_index) = vp_index_msr;
> > >  
> > > +    if ( vp_index_msr > hv_max_vp_index )
> > > +        hv_max_vp_index = vp_index_msr;
> > > +
> > >      return 0;
> > >  }
> > >  
> > > diff --git a/xen/arch/x86/guest/hyperv/private.h b/xen/arch/x86/guest/hyperv/private.h
> > > index 354fc7f685a7..fea3e291e944 100644
> > > --- a/xen/arch/x86/guest/hyperv/private.h
> > > +++ b/xen/arch/x86/guest/hyperv/private.h
> > > @@ -28,6 +28,7 @@
> > >  DECLARE_PER_CPU(void *, hv_input_page);
> > >  DECLARE_PER_CPU(void *, hv_vp_assist);
> > >  DECLARE_PER_CPU(unsigned int, hv_vp_index);
> > > +extern unsigned int hv_max_vp_index;
> > >  
> > >  static inline unsigned int hv_vp_index(unsigned int cpu)
> > >  {
> > > diff --git a/xen/arch/x86/guest/hyperv/tlb.c b/xen/arch/x86/guest/hyperv/tlb.c
> > > index 1d723d6ee679..0a44071481bd 100644
> > > --- a/xen/arch/x86/guest/hyperv/tlb.c
> > > +++ b/xen/arch/x86/guest/hyperv/tlb.c
> > > @@ -166,7 +166,7 @@ int hyperv_flush_tlb(const cpumask_t *mask, const void *va,
> > >          {
> > >              unsigned int vpid = hv_vp_index(cpu);
> > >  
> > > -            if ( vpid >= ms_hyperv.max_vp_index )
> > > +            if ( vpid >= hv_max_vp_index )
> > 
> > I think the >= should be changed to > here.
> 
> I agree. With this fixed:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

FWIW, I think it should also be nice to add an ASSERT_UNREACHABLE in
the if body, as now it shouldn't be possible for vpid to be greater
than hv_max_vp_index unless something has gone really wrong?

Roger.
Re: [PATCH] x86/hyperv: stash and use the configured max VP index
Posted by Wei Liu 3 years, 11 months ago 
On Thu, Apr 30, 2020 at 12:21:18PM +0200, Roger Pau Monné wrote:
> On Thu, Apr 30, 2020 at 12:15:58PM +0200, Roger Pau Monné wrote:
> > On Wed, Apr 29, 2020 at 11:47:18AM +0000, Wei Liu wrote:
> > > On Wed, Apr 29, 2020 at 11:41:44AM +0100, Wei Liu wrote:
> > > > The value returned from CPUID is the maximum number for virtual
> > > > processors supported by Hyper-V. It could be larger than the maximum
> > > > number of virtual processors configured.
> > > > 
> > > > Stash the configured number into a variable and use it in calculations.
> > > > 
> > > > Signed-off-by: Wei Liu <liuwe@microsoft.com>
> > > > ---
> > > >  xen/arch/x86/guest/hyperv/hyperv.c  | 4 ++++
> > > >  xen/arch/x86/guest/hyperv/private.h | 1 +
> > > >  xen/arch/x86/guest/hyperv/tlb.c     | 2 +-
> > > >  xen/arch/x86/guest/hyperv/util.c    | 2 +-
> > > >  4 files changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> > > > index 91a6782cd986..84221b751453 100644
> > > > --- a/xen/arch/x86/guest/hyperv/hyperv.c
> > > > +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> > > > @@ -33,6 +33,7 @@ DEFINE_PER_CPU_READ_MOSTLY(void *, hv_input_page);
> > > >  DEFINE_PER_CPU_READ_MOSTLY(void *, hv_vp_assist);
> > > >  DEFINE_PER_CPU_READ_MOSTLY(unsigned int, hv_vp_index);
> > > >  
> > > > +unsigned int __read_mostly hv_max_vp_index;
> > > >  static bool __read_mostly hcall_page_ready;
> > > >  
> > > >  static uint64_t generate_guest_id(void)
> > > > @@ -143,6 +144,9 @@ static int setup_hypercall_pcpu_arg(void)
> > > >      rdmsrl(HV_X64_MSR_VP_INDEX, vp_index_msr);
> > > >      this_cpu(hv_vp_index) = vp_index_msr;
> > > >  
> > > > +    if ( vp_index_msr > hv_max_vp_index )
> > > > +        hv_max_vp_index = vp_index_msr;
> > > > +
> > > >      return 0;
> > > >  }
> > > >  
> > > > diff --git a/xen/arch/x86/guest/hyperv/private.h b/xen/arch/x86/guest/hyperv/private.h
> > > > index 354fc7f685a7..fea3e291e944 100644
> > > > --- a/xen/arch/x86/guest/hyperv/private.h
> > > > +++ b/xen/arch/x86/guest/hyperv/private.h
> > > > @@ -28,6 +28,7 @@
> > > >  DECLARE_PER_CPU(void *, hv_input_page);
> > > >  DECLARE_PER_CPU(void *, hv_vp_assist);
> > > >  DECLARE_PER_CPU(unsigned int, hv_vp_index);
> > > > +extern unsigned int hv_max_vp_index;
> > > >  
> > > >  static inline unsigned int hv_vp_index(unsigned int cpu)
> > > >  {
> > > > diff --git a/xen/arch/x86/guest/hyperv/tlb.c b/xen/arch/x86/guest/hyperv/tlb.c
> > > > index 1d723d6ee679..0a44071481bd 100644
> > > > --- a/xen/arch/x86/guest/hyperv/tlb.c
> > > > +++ b/xen/arch/x86/guest/hyperv/tlb.c
> > > > @@ -166,7 +166,7 @@ int hyperv_flush_tlb(const cpumask_t *mask, const void *va,
> > > >          {
> > > >              unsigned int vpid = hv_vp_index(cpu);
> > > >  
> > > > -            if ( vpid >= ms_hyperv.max_vp_index )
> > > > +            if ( vpid >= hv_max_vp_index )
> > > 
> > > I think the >= should be changed to > here.
> > 
> > I agree. With this fixed:
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> FWIW, I think it should also be nice to add an ASSERT_UNREACHABLE in
> the if body, as now it shouldn't be possible for vpid to be greater
> than hv_max_vp_index unless something has gone really wrong?

At some point I will initialise vpid to (uint32_t)-1 so it could go over
hv_max_vp_index if there is a bug in code.

Wei.

> 
> Roger.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.