Re: [PATCH v7 1/5] xen/arm: Add support for GIC v3

[Julien Grall <julien.grall@linaro.org>]

On 22/07/14 12:12, Vijay Kilari wrote:
> On Tue, Jul 22, 2014 at 4:25 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Tue, 2014-07-22 at 16:19 +0530, Vijay Kilari wrote:
>>>>
>>>>>>> + * Additional registers defined in GIC v3.
>>>>>>> + * Common GICD registers are defined in gic.h
>>>>>>> + */
>>>>>>> +
>>>>>>> +#define GICD_STATUSR                 (0x010)
>>>>>>> [...][
>>>>>>> +#define GICV3_GICD_PIDR0             (0x92)
>>>>>>
>>>>>> What is the distinction between variables with GIC[DR]_ prefixes and
>>>>>> those with GICV3_GIC[DR]_ ones?
>>>>>
>>>>> GICV3 is prefixed for indicating that there are values not the addresses.
>>>>> In anycase I will remove GICV3 prefixes and postfix _VAL
>>>>
>>>> You mean the value used when emulating a read, I think?
>>>
>>>     Yes, it is used in gicv3 driver for checking presence of re-distributor
>>
>> In the phsyical h/w? That sounds wrong. 0x92 is just one possible value
>> of this register (and the spec says it should be 0x94...). I don't see
>> you using this #define in that way anywhere though so perhaps I've
>> misunderstood.
>
> I am using 20.0 version. which specifies 0x92 for ARM implementations of GICv3.
> May be other vendors can use different value. So it is better propagate hw value
> for emulation
>
>   GICD_PIDR0:
>   Bits [7:0]. Bits [7:0] of the ARM-defined DevID field. This field is 0x92 in
> ARM implementations of a GICv3 or later Distributor.
>
> In any case, GICD_PIDR0 is not used by gicv3 driver. GICD_PIDR2 & GICR_PIDR2 are
>   used where check is made on ARCH_REV [7:4]
>
>>
>>> and also in vgicv3 for emulating read.
>>
>> I wonder if we should instead propagate the underlying hardware value?
>
> Yes,we can propagate hardware value.

I don't think we should propagate the hardware value. We may emulate a 
different distributor than the hardware one. In this case, 
implementation defined register won't work as expected by the kernel.

Regards,

-- 
Julien Grall