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

[Michal Orzel <michal.orzel@arm.com>]

On 17.05.2022 10:34, Julien Grall wrote:
> 
> 
> 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?
> 
It is not a subject of any requirement. It was my personal thought.
As it is not a common practice in Xen, I will keep that in mind for the future.

>>
>>> +    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@arm.com/
> [2] https://lore.kernel.org/xen-devel/20220221102218.33785-5-julien@xen.org/
> 
>>
>> Cheers,
>> Michal
> 

Cheers,
Michal