Re: [PATCH v2] xen/arm: gic-v3-lpi: Allocate the pending table while preparing the CPU

[Julien Grall <julien@xxxxxxx>]

On 17/05/2022 07:47, Michal Orzel wrote:
> Hi Julien,
Hi Michal,

> On 16.05.2022 19:02, Julien Grall wrote:
> > +static int cpu_callback(struct notifier_block *nfb, unsigned long action,
> > +                        void *hcpu)
> > +{
> > +    unsigned long cpu = (unsigned long)hcpu;
> As cpu does not change in this function, shouldn't we mark it as const?
As discussed in [1], this is not a common practice to use const for 
variable that just hold integer.
I would personally only used it if I think there is a security issue 
risk (e.g. the variable is used to flush the TLB like in [2]).
Would you be able to explain why you want to use const? Is it a 
requirement for some certification?
> > +    int rc = 0;
> > +
> > +    switch ( action )
> > +    {
> > +    case CPU_UP_PREPARE:
> > +        rc = gicv3_lpi_allocate_pendtable(cpu);
> > +        if ( rc )
> > +            printk(XENLOG_ERR "Unable to allocate the pendtable for CPU%u\n",
> %u requires unsigned int but cpu is unsigned long.
> FWICS this will cause a compilation error, so you should change to %lu.
Whoops. I will fix it on the next version.

> > +                   cpu);
> > +        break;
> >       }
> >   
> > -    return gicv3_lpi_set_proptable(rdist_base);
> > +    return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
> >   }
> >   
Cheers,

[1] 
https://lore.kernel.org/xen-devel/20220505103601.322110-2-michal.orzel@xxxxxxx/
[2] https://lore.kernel.org/xen-devel/20220221102218.33785-5-julien@xxxxxxx/

> Cheers,
> Michal
--
Julien Grall