From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen/arm: remove check for generic timer support for arm64 Date: Mon, 02 Jun 2014 14:39:35 +0100 Message-ID: <538C7E97.30702@linaro.org> References: <1401698241-20722-1-git-send-email-vijay.kilari@gmail.com> <1401707388.11045.6.camel@kazak.uk.xensource.com> <538C5C64.3080805@linaro.org> <1401714226.19553.11.camel@kazak.uk.xensource.com> <538C7AC6.30900@linaro.org> <1401715834.21534.7.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1401715834.21534.7.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: vijay.kilari@gmail.com, stefano.stabellini@eu.citrix.com, Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.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 >>>>>> >>>>>> 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