Re: [Xen-devel] [PATCH V3 8/8] xen/arm: make domain_max_vcpus be alias of max_vcpus in struct domain

[Chen Baozi <cbz@xxxxxxxxxx>]

Hi Andrew,

On Thu, May 28, 2015 at 09:50:38AM +0100, Andrew Cooper wrote:
> On 28/05/15 08:44, Chen Baozi wrote:
> > From: Chen Baozi <baozich@xxxxxxxxx>
> >
> > Since the maximum vcpu information is already saved in the struct domain,
> > there is no need for domain_max_vpus to return the fixed value.
> >
> > Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
> > ---
> >  xen/include/asm-arm/domain.h | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> > index 603a20b..b4e38a2 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -261,10 +261,7 @@ struct arch_vcpu
> >  void vcpu_show_execution_state(struct vcpu *);
> >  void vcpu_show_registers(const struct vcpu *);
> >  
> > -static inline unsigned int domain_max_vcpus(const struct domain *d)
> > -{
> > -    return MAX_VIRT_CPUS;
> > -}
> > +#define domain_max_vcpus(d) (d->max_vcpus)
> 
> First of all, don't go altering a properly typed static inline like this
> for a macro.  The former is better in all regards, save the number of
> lines it takes to express.
> 
> You appear to have missed the entire point of this function.
> d->max_vcpus is uninitialised at this point (although it will always
> have the value 0).

Ah, yes. I didn't noticed that it is called at the very beginning of
XEN_DOMCTL_max_vcpus, where d->max_vcpus is not initialized.

> 
> You have also missed the point of Juliens review, which is to say that
> an ARM64 build of Xen running a GICv2 domain must not claim to support
> 128 cpus.
> 

Due to my misunderstanding, I thought the d->max_vcpus would store the right
value which has already been ajusted according to vGIC's version.

I'll resend a V4 to fix it.

Thanks.

Baozi.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel