Re: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI is enabled

[Julien Grall <julien.grall@arm.com>]

Hi,

On 18/04/2019 12:15, Oleksandr wrote:
> 
> On 18.04.19 12:32, Julien Grall wrote:
>>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c 
>>> b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> index eea60b2..bac4490 100644
>>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> @@ -32,7 +32,7 @@ static const struct of_device_id cpg_matches[] __initconst = {
>>>       { /* sentinel */ }
>>>   };
>>>   -static unsigned int __init get_extal_freq(void)
>>> +static unsigned int __init __maybe_unused get_extal_freq(void)
>>>   {
>>>       const struct of_device_id *match;
>>>       struct device_node *cpg, *extal;
>>> @@ -60,6 +60,12 @@ static unsigned int __init get_extal_freq(void)
>>>     void __init rcar_gen2_timer_init(void)
>>>   {
>>> +/*
>>> + * If CONFIG_PSCI is enabled then most likely we are running on PSCI-enabled
>>> + * U-Boot which, we assume, has already taken care of configuring ARCH timer
>>> + * stuff before switching to non-secure mode.
>>> + */
>>
>> I disagree with that statement. CONFIG_ARM_PSCI may be selected in your 
>> .config for multiple reasons:
>>      - CONFIG_XEN will selected it
>>      - you build a .config targeting multiple platforms
>>
>> Yet you may still want to boot the kernel on older U-boot. Such change would 
>> make it impossible.
>>
>> If there are code that should not be called when PSCI is used, then you need 
>> to make sure the function is not called at runtime.
> 
> I got your point.
> 
> 
> As I answered in a previous email:
> 
>  > But "#if !defined(CONFIG_ARM_PSCI)" wraps secure_cntvoff_init() call as
>  > well, which resets CNTVOFF,
> 
>  > and this is something I tried to avoid the first from being executed on
>  > a CPU, which is in SVC mode (when running on top of Xen).
> 
> What would be a proper way to avoid calling secure_cntvoff_init() at runtime?
> 
> Check that we are already in non-secure mode and/or cntvoff is already 0?

CNTVOFF is controlled by the hypervisor, so you cannot access it from Dom0.

If none of this code should be necessary when PSCI is enabled, then you can 
check the presence of PSCI (see psci_smp_available()).

Another solution is to check whether we are running on xen (see xen_domain()).

In any case, I would still surround the two checks because the "armgcnt" should 
really not be touched by Dom0.

Cheers,

-- 
Julien Grall