On 31/05/15 19:05, Julien Grall wrote:
> Hi Chen,
>
> On 31/05/2015 16:31, Chen Baozi wrote:
>>> On May 31, 2015, at 21:35, Julien Grall <julien.gr...@citrix.com>
>>> wrote:
>>>> +
>>>> #define NR_GIC_LOCAL_IRQS NR_LOCAL_IRQS
>>>> #define NR_GIC_SGI 16
>>>> #define MAX_RDIST_COUNT 4
>>>> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
>>>> index 2f413e1..1157b04 100644
>>>> --- a/xen/include/asm-arm/vgic.h
>>>> +++ b/xen/include/asm-arm/vgic.h
>>>> @@ -110,6 +110,8 @@ struct vgic_ops {
>>>> struct vcpu *(*get_target_vcpu)(struct vcpu *v, unsigned int
>>>> irq);
>>>> /* vGIC sysreg emulation */
>>>> int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr
>>>> hsr);
>>>> + /* Get the maximum number of vCPU supported */
>>>> + int (*get_max_vcpus)(void);
>>>
>>> Why did you create a function? The value never change so a field
>>> value would have been better…
>>
>> But is it appropriate that we define a field value in a struct called
>> vgic_ops? I
>> thought ‘XXX_ops’ refers to a struct that is made up by function
>> points. (FIXME)
>
> Well if the only issue if the name, please rename the structure. IHMO,
> the name is fine for me, I will let Ian & Stefano decides about it.
>
> As the vGIC is always supporting N cpus and will never change, it's
> pointless to use a function (thinking about the "cost" vs access an
> field).
We have plenty of other structures with mixed function pointers and data.
A "const unsigned int max_vcpus;" should absolutely be used in a case
like this.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
