Re: [PATCH] viridian: Fix bank count counting

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/12/2025 4:29 pm, Teddy Astie wrote:
> hv_vpset_nr_banks() incorrectly counts the number of bank by using hweight64()
> instead of flsl(). This for instance problematic in case only the second bank
> is selected (i.e >64 vCPUs, where here hweight64 gives 1), causing only the 
> first
> bank to be checked (non-valid) and the second (meaningful) one to be skipped.
>
> Fixes: b4124682db6e ("viridian: add ExProcessorMasks variants of the flush 
> hypercalls")
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/viridian/viridian.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c 
> b/xen/arch/x86/hvm/viridian/viridian.c
> index 90e749ceb5..f5e526241d 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -601,7 +601,7 @@ static DEFINE_PER_CPU(union hypercall_vpset, 
> hypercall_vpset);
>  
>  static unsigned int hv_vpset_nr_banks(struct hv_vpset *vpset)
>  {
> -    return hweight64(vpset->valid_bank_mask);
> +    return fls64(vpset->valid_bank_mask);
>  }
>  
>  static int hv_vpset_to_vpmask(const struct hv_vpset *in, paddr_t bank_gpa,

https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vp_set#processor-set-example

This example is poor (it has several errors in the text), but the final
statement says fairly clearly that the banks are tightly packed and not
contiguous.

Therefore, hweight64() the correct calculation, and AFAICT, the single
use of this helper in Xen correct.

~Andrew