Re: [PATCH 04/10] viridian: use hypercall_vpmask in hvcall_ipi()

From: Jan Beulich <jbeulich@suse.com>
To: Paul Durrant <paul@xen.org>
Cc: "Paul Durrant" <pdurrant@amazon.com>, "Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 04/10] viridian: use hypercall_vpmask in hvcall_ipi()
Date: Thu, 12 Nov 2020 09:49:35 +0100	[thread overview]
Message-ID: <0e05cc16-c7b9-34a1-8862-a4e7d581189f@suse.com> (raw)
In-Reply-To: <20201111200721.30551-5-paul@xen.org>

On 11.11.2020 21:07, Paul Durrant wrote:
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -533,6 +533,21 @@ static bool vpmask_test(struct hypercall_vpmask *vpmask, unsigned int vp)
>      return test_bit(vp, vpmask->mask);
>  }
>  
> +static unsigned int vpmask_first(struct hypercall_vpmask *vpmask)
> +{
> +    return find_first_bit(vpmask->mask, HVM_MAX_VCPUS);
> +}
> +
> +static unsigned int vpmask_next(struct hypercall_vpmask *vpmask, unsigned int vp)
> +{
> +    return find_next_bit(vpmask->mask, HVM_MAX_VCPUS, vp + 1);

Perhaps again assert on vp's range?

> +}
> +
> +#define for_each_vp(vpmask, vp) \
> +	for ((vp) = vpmask_first(vpmask); \
> +	     (vp) < HVM_MAX_VCPUS; \
> +	     (vp) = vpmask_next(vpmask, vp))

Nit again: Missing blanks here and ...

> @@ -669,17 +693,20 @@ static int hvcall_ipi(union hypercall_input *input,
>      if ( vector < 0x10 || vector > 0xff )
>          return -EINVAL;
>  
> -    for_each_vcpu ( currd, v )
> +    vpmask_empty(vpmask);
> +    for (vp = 0; vp < 64; vp++)

... here. I also wonder if the literal 64 couldn't be expressed in
some suitable way.

Jan