linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
@ 2019-05-03 11:17 Oleksandr Tyshchenko
  2019-05-03 11:38 ` Geert Uytterhoeven
  2019-05-06 15:37 ` Julien Grall
  0 siblings, 2 replies; 6+ messages in thread
From: Oleksandr Tyshchenko @ 2019-05-03 11:17 UTC (permalink / raw)
  To: linux-renesas-soc, linux-kernel
  Cc: julien.grall, horms, magnus.damm, linux, biju.das, Oleksandr Tyshchenko

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

If PSCI is available then most likely we are running on PSCI-enabled
U-Boot which, we assume, has already taken care of resetting CNTVOFF
before switching to non-secure mode and we don't need to.

Also, don't init CNTVOFF if we are running on top of Xen hypervisor,
as CNTVOFF is controlled by hypervisor itself and shouldn't be touched
by Dom0 in such case.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
CC: Julien Grall <julien.grall@arm.com>

---
   You can find previous discussion here:
   https://lkml.org/lkml/2019/4/17/810

   Changes in v2:
      - Clarify patch subject/description
      - Don't use CONFIG_ARM_PSCI option, check whether the PSCI is available,
        by using psci_smp_available()
      - Check whether we are running on top of Xen, by using xen_domain()
---
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index eea60b2..bc8537b 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -17,7 +17,9 @@
 #include <linux/of.h>
 #include <linux/of_fdt.h>
 #include <linux/of_platform.h>
+#include <xen/xen.h>
 #include <asm/mach/arch.h>
+#include <asm/psci.h>
 #include <asm/secure_cntvoff.h>
 #include "common.h"
 #include "rcar-gen2.h"
@@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
 	void __iomem *base;
 	u32 freq;
 
-	secure_cntvoff_init();
+	/*
+	 * If PSCI is available then most likely we are running on PSCI-enabled
+	 * U-Boot which, we assume, has already taken care of resetting CNTVOFF
+	 * before switching to non-secure mode and we don't need to.
+	 * Another check is to be sure that we are not running on top of Xen
+	 * hypervisor, as CNTVOFF is controlled by hypervisor itself and
+	 * shouldn't be touched by Dom0 in such case.
+	 */
+	if (!psci_smp_available() && !xen_domain())
+		secure_cntvoff_init();
 
 	if (of_machine_is_compatible("renesas,r8a7745") ||
 	    of_machine_is_compatible("renesas,r8a77470") ||
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
  2019-05-03 11:17 [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available Oleksandr Tyshchenko
@ 2019-05-03 11:38 ` Geert Uytterhoeven
  2019-05-03 12:28   ` Oleksandr
  2019-05-06 15:37 ` Julien Grall
  1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2019-05-03 11:38 UTC (permalink / raw)
  To: Oleksandr Tyshchenko
  Cc: Linux-Renesas, Linux Kernel Mailing List, Julien Grall,
	Simon Horman, Magnus Damm, Russell King, Biju Das,
	Oleksandr Tyshchenko

Hi Oleksandr,

On Fri, May 3, 2019 at 1:21 PM Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> If PSCI is available then most likely we are running on PSCI-enabled
> U-Boot which, we assume, has already taken care of resetting CNTVOFF
> before switching to non-secure mode and we don't need to.
>
> Also, don't init CNTVOFF if we are running on top of Xen hypervisor,
> as CNTVOFF is controlled by hypervisor itself and shouldn't be touched
> by Dom0 in such case.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> CC: Julien Grall <julien.grall@arm.com>

Thanks for your patch!

> ---
>    You can find previous discussion here:
>    https://lkml.org/lkml/2019/4/17/810
>
>    Changes in v2:
>       - Clarify patch subject/description
>       - Don't use CONFIG_ARM_PSCI option, check whether the PSCI is available,
>         by using psci_smp_available()
>       - Check whether we are running on top of Xen, by using xen_domain()
> ---
>  arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> index eea60b2..bc8537b 100644
> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> @@ -17,7 +17,9 @@
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
>  #include <linux/of_platform.h>
> +#include <xen/xen.h>
>  #include <asm/mach/arch.h>
> +#include <asm/psci.h>
>  #include <asm/secure_cntvoff.h>
>  #include "common.h"
>  #include "rcar-gen2.h"
> @@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
>         void __iomem *base;
>         u32 freq;
>
> -       secure_cntvoff_init();
> +       /*
> +        * If PSCI is available then most likely we are running on PSCI-enabled
> +        * U-Boot which, we assume, has already taken care of resetting CNTVOFF
> +        * before switching to non-secure mode and we don't need to.
> +        * Another check is to be sure that we are not running on top of Xen
> +        * hypervisor, as CNTVOFF is controlled by hypervisor itself and
> +        * shouldn't be touched by Dom0 in such case.
> +        */
> +       if (!psci_smp_available() && !xen_domain())
> +               secure_cntvoff_init();
>
>         if (of_machine_is_compatible("renesas,r8a7745") ||
>             of_machine_is_compatible("renesas,r8a77470") ||

How do you prevent secure_cntvoff_init() from being called for secondary
CPUs in arch/arm/mach-shmobile/headsmp-apmu.S?

With PSCI, it is not called if "enable-method" in DT is "psci"', so that case
is covered, I guess.

What about XEN? Do you override the "enable-method"?
If yes, perhaps a check for "renesas,apmu" is more appropriate?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
  2019-05-03 11:38 ` Geert Uytterhoeven
@ 2019-05-03 12:28   ` Oleksandr
  0 siblings, 0 replies; 6+ messages in thread
From: Oleksandr @ 2019-05-03 12:28 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux-Renesas, Linux Kernel Mailing List, Julien Grall,
	Simon Horman, Magnus Damm, Russell King, Biju Das,
	Oleksandr Tyshchenko


On 03.05.19 14:38, Geert Uytterhoeven wrote:
> Hi Oleksandr,

Hi Geert


>
> On Fri, May 3, 2019 at 1:21 PM Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> If PSCI is available then most likely we are running on PSCI-enabled
>> U-Boot which, we assume, has already taken care of resetting CNTVOFF
>> before switching to non-secure mode and we don't need to.
>>
>> Also, don't init CNTVOFF if we are running on top of Xen hypervisor,
>> as CNTVOFF is controlled by hypervisor itself and shouldn't be touched
>> by Dom0 in such case.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> CC: Julien Grall <julien.grall@arm.com>
> Thanks for your patch!

Thank you for review!


>
>> ---
>>     You can find previous discussion here:
>>     https://lkml.org/lkml/2019/4/17/810
>>
>>     Changes in v2:
>>        - Clarify patch subject/description
>>        - Don't use CONFIG_ARM_PSCI option, check whether the PSCI is available,
>>          by using psci_smp_available()
>>        - Check whether we are running on top of Xen, by using xen_domain()
>> ---
>>   arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> index eea60b2..bc8537b 100644
>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> @@ -17,7 +17,9 @@
>>   #include <linux/of.h>
>>   #include <linux/of_fdt.h>
>>   #include <linux/of_platform.h>
>> +#include <xen/xen.h>
>>   #include <asm/mach/arch.h>
>> +#include <asm/psci.h>
>>   #include <asm/secure_cntvoff.h>
>>   #include "common.h"
>>   #include "rcar-gen2.h"
>> @@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
>>          void __iomem *base;
>>          u32 freq;
>>
>> -       secure_cntvoff_init();
>> +       /*
>> +        * If PSCI is available then most likely we are running on PSCI-enabled
>> +        * U-Boot which, we assume, has already taken care of resetting CNTVOFF
>> +        * before switching to non-secure mode and we don't need to.
>> +        * Another check is to be sure that we are not running on top of Xen
>> +        * hypervisor, as CNTVOFF is controlled by hypervisor itself and
>> +        * shouldn't be touched by Dom0 in such case.
>> +        */
>> +       if (!psci_smp_available() && !xen_domain())
>> +               secure_cntvoff_init();
>>
>>          if (of_machine_is_compatible("renesas,r8a7745") ||
>>              of_machine_is_compatible("renesas,r8a77470") ||
> How do you prevent secure_cntvoff_init() from being called for secondary
> CPUs in arch/arm/mach-shmobile/headsmp-apmu.S?

Good question.


>
> With PSCI, it is not called if "enable-method" in DT is "psci"', so that case
> is covered, I guess.

Yes.

>
> What about XEN? Do you override the "enable-method"?
> If yes, perhaps a check for "renesas,apmu" is more appropriate?

No, I don't override.


The correct way to run Xen would be to use PSCI, so Linux shouldn't do 
any platform low level operation with secondary CPU cores (on/off, 
reset, etc) by itself when running on top of Xen hypervisor.

As Xen brings available secondary cores up before starting first domain 
(Dom0) using PSCI CPU_ON call to FW, and these cores are entered Xen in 
Hyp mode, so any attempts from Dom0 to perform CPU power management 
directly (using APMU, RST, etc) may result in something not good. I 
think that in case when someone wants to run Xen on R-Car Gen2 system 
(which is SMP) without PSCI enabled, we need to forbid even trying to 
start secondary cores using APMU.

What do you think?


>
> Gr{oetje,eeting}s,
>
>                          Geert
>
-- 
Regards,

Oleksandr Tyshchenko


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
  2019-05-03 11:17 [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available Oleksandr Tyshchenko
  2019-05-03 11:38 ` Geert Uytterhoeven
@ 2019-05-06 15:37 ` Julien Grall
  2019-05-10 12:27   ` Oleksandr
  1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2019-05-06 15:37 UTC (permalink / raw)
  To: Oleksandr Tyshchenko, linux-renesas-soc, linux-kernel
  Cc: horms, magnus.damm, linux, biju.das, Oleksandr Tyshchenko

Hi Oleksandr,

On 5/3/19 12:17 PM, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> If PSCI is available then most likely we are running on PSCI-enabled
> U-Boot which, we assume, has already taken care of resetting CNTVOFF
> before switching to non-secure mode and we don't need to.
> 
> Also, don't init CNTVOFF if we are running on top of Xen hypervisor,
> as CNTVOFF is controlled by hypervisor itself and shouldn't be touched
> by Dom0 in such case.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> CC: Julien Grall <julien.grall@arm.com>
> 
> ---
>     You can find previous discussion here:
>     https://lkml.org/lkml/2019/4/17/810
> 
>     Changes in v2:
>        - Clarify patch subject/description
>        - Don't use CONFIG_ARM_PSCI option, check whether the PSCI is available,
>          by using psci_smp_available()
>        - Check whether we are running on top of Xen, by using xen_domain()
> ---
>   arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> index eea60b2..bc8537b 100644
> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> @@ -17,7 +17,9 @@
>   #include <linux/of.h>
>   #include <linux/of_fdt.h>
>   #include <linux/of_platform.h>
> +#include <xen/xen.h>
>   #include <asm/mach/arch.h>
> +#include <asm/psci.h>
>   #include <asm/secure_cntvoff.h>
>   #include "common.h"
>   #include "rcar-gen2.h"
> @@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
>   	void __iomem *base;
>   	u32 freq;
>   
> -	secure_cntvoff_init();
> +	/*
> +	 * If PSCI is available then most likely we are running on PSCI-enabled
> +	 * U-Boot which, we assume, has already taken care of resetting CNTVOFF
> +	 * before switching to non-secure mode and we don't need to.
> +	 * Another check is to be sure that we are not running on top of Xen
> +	 * hypervisor, as CNTVOFF is controlled by hypervisor itself and
> +	 * shouldn't be touched by Dom0 in such case.

If you are running on top of Xen, then PSCI will be available. So is 
there any real value to check the presence of Xen as well?

Cheers,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
  2019-05-06 15:37 ` Julien Grall
@ 2019-05-10 12:27   ` Oleksandr
  2019-05-10 12:53     ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Oleksandr @ 2019-05-10 12:27 UTC (permalink / raw)
  To: Julien Grall, linux-renesas-soc, linux-kernel
  Cc: horms, magnus.damm, linux, biju.das, Oleksandr Tyshchenko


On 06.05.19 18:37, Julien Grall wrote:
> Hi Oleksandr,

Hi Julien


>> ---
>>   arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c 
>> b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> index eea60b2..bc8537b 100644
>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>> @@ -17,7 +17,9 @@
>>   #include <linux/of.h>
>>   #include <linux/of_fdt.h>
>>   #include <linux/of_platform.h>
>> +#include <xen/xen.h>
>>   #include <asm/mach/arch.h>
>> +#include <asm/psci.h>
>>   #include <asm/secure_cntvoff.h>
>>   #include "common.h"
>>   #include "rcar-gen2.h"
>> @@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
>>       void __iomem *base;
>>       u32 freq;
>>   -    secure_cntvoff_init();
>> +    /*
>> +     * If PSCI is available then most likely we are running on 
>> PSCI-enabled
>> +     * U-Boot which, we assume, has already taken care of resetting 
>> CNTVOFF
>> +     * before switching to non-secure mode and we don't need to.
>> +     * Another check is to be sure that we are not running on top of 
>> Xen
>> +     * hypervisor, as CNTVOFF is controlled by hypervisor itself and
>> +     * shouldn't be touched by Dom0 in such case.
>
> If you are running on top of Xen, then PSCI will be available. So is 
> there any real value to check the presence of Xen as well?

I think, no. If the PSCI is a must to run Xen, then it is certainly an 
excessive check. I will drop it in V3.


>
> Cheers,
>
-- 
Regards,

Oleksandr Tyshchenko


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available
  2019-05-10 12:27   ` Oleksandr
@ 2019-05-10 12:53     ` Julien Grall
  0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2019-05-10 12:53 UTC (permalink / raw)
  To: Oleksandr, linux-renesas-soc, linux-kernel
  Cc: horms, magnus.damm, linux, biju.das, Oleksandr Tyshchenko



On 10/05/2019 13:27, Oleksandr wrote:
> 
> On 06.05.19 18:37, Julien Grall wrote:
>> Hi Oleksandr,
> 
> Hi Julien
> 
> 
>>> ---
>>>   arch/arm/mach-shmobile/setup-rcar-gen2.c | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c 
>>> b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> index eea60b2..bc8537b 100644
>>> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
>>> @@ -17,7 +17,9 @@
>>>   #include <linux/of.h>
>>>   #include <linux/of_fdt.h>
>>>   #include <linux/of_platform.h>
>>> +#include <xen/xen.h>
>>>   #include <asm/mach/arch.h>
>>> +#include <asm/psci.h>
>>>   #include <asm/secure_cntvoff.h>
>>>   #include "common.h"
>>>   #include "rcar-gen2.h"
>>> @@ -63,7 +65,16 @@ void __init rcar_gen2_timer_init(void)
>>>       void __iomem *base;
>>>       u32 freq;
>>>   -    secure_cntvoff_init();
>>> +    /*
>>> +     * If PSCI is available then most likely we are running on PSCI-enabled
>>> +     * U-Boot which, we assume, has already taken care of resetting CNTVOFF
>>> +     * before switching to non-secure mode and we don't need to.
>>> +     * Another check is to be sure that we are not running on top of Xen
>>> +     * hypervisor, as CNTVOFF is controlled by hypervisor itself and
>>> +     * shouldn't be touched by Dom0 in such case.
>>
>> If you are running on top of Xen, then PSCI will be available. So is there any 
>> real value to check the presence of Xen as well?
> 
> I think, no. If the PSCI is a must to run Xen, then it is certainly an excessive 
> check. I will drop it in V3.

Xen does not offer another interface for SMP bring-up.

Cheers,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-05-10 12:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03 11:17 [PATCH V2] ARM: mach-shmobile: Don't init CNTVOFF if PSCI is available Oleksandr Tyshchenko
2019-05-03 11:38 ` Geert Uytterhoeven
2019-05-03 12:28   ` Oleksandr
2019-05-06 15:37 ` Julien Grall
2019-05-10 12:27   ` Oleksandr
2019-05-10 12:53     ` Julien Grall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).