Re: [PATCH] xen/arm: remove check for generic timer support for arm64

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

On 06/02/2014 02:30 PM, Ian Campbell wrote:
> On Mon, 2014-06-02 at 14:23 +0100, Julien Grall wrote:
>> On 06/02/2014 02:03 PM, Ian Campbell wrote:
>>> On Mon, 2014-06-02 at 12:13 +0100, Julien Grall wrote:
>>>> On 06/02/2014 12:09 PM, Ian Campbell wrote:
>>>>> On Mon, 2014-06-02 at 14:07 +0530, vijay.kilari@gmail.com wrote:
>>>>>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>>>>>
>>>>>> arm64 always supports generic timer. So check is not required
>>>>>> for arm64. For platforms which supports only aarch64 mode this
>>>>>> check always passes and panics
>>>>>
>>>>> Ah, because the relevant feature flag/register is 32-bit only.
>>>>>
>>>>> I'd prefer to see this done in the cpufeature header as:
>>>>>         #ifdef CONFIG_ARM_32
>>>>>         #define cpu_has_gentimer  (boot_cpu_feature32(gentimer) == 1)
>>>>>         #else
>>>>>         #define cpu_has_gentimer  (1)
>>>>>         #endif
>>>>> rather than adding #ifdef to the common code. Likewise for any similar
>>>>> "always on for aarch64" features.
>>>>
>>>> AFAIU, the feature flag exists on ARMv8 platform with aarch32 support.
>>>> So an ifdef may not be the correct solution here.
>>>
>>> The flag might exist in the AArch32 feature registers (for compat with
>>> v7) but AIUI the feature is not actually optional on v8.
>>
>> The manual says the ARM Generic Timer is an optional extension to an
>> ARMv8 implementation.
> 
> So it does. In that case cpu_has_gentimer should turn into a check of
> ID_PFR1_EL1.GenTimer for arm64 builds.

This is already the case (without Vijay's patch). But on AArch64-only
implementation, this register is RAZ.

It looks like ID_PFR{0,1}_EL1 are only used for ARMv7 and ARMv8 which
support 32-bit.

Regards,

-- 
Julien Grall