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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxxxxxx>
> > > ---
> > >  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@xxxxxxxxxx>

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.