* [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode
@ 2019-06-21 20:02 Denis Obrezkov
2019-06-24 11:09 ` Julien Grall
0 siblings, 1 reply; 4+ messages in thread
From: Denis Obrezkov @ 2019-06-21 20:02 UTC (permalink / raw)
To: xen-devel; +Cc: julien.grall, tim, Denis Obrezkov
This function allows xen to bring secondary CPU cores into non-secure
HYP mode. This is done by using a Secure Monitor call.
Signed-off-by: Denis Obrezkov <denisobrezkov@gmail.com>
---
xen/arch/arm/arm32/head.S | 11 ++++++++++-
xen/arch/arm/platforms/omap5.c | 5 +++--
xen/include/asm-arm/platforms/omap5.h | 3 +++
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 5f817d473e..120e034934 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -36,6 +36,10 @@
#include EARLY_PRINTK_INC
#endif
+
+#define API_HYP_ENTRY 0x102
+#define AUX_CORE_BOOT0_PA 0x48281800
+
/*
* Common register usage in this file:
* r0 -
@@ -113,6 +117,12 @@ past_zImage:
b common_start
+GLOBAL(omap5_init_secondary)
+ ldr r12, =API_HYP_ENTRY
+ adr r0, init_secondary
+ dsb
+ smc #0
+
GLOBAL(init_secondary)
cpsid aif /* Disable all interrupts */
@@ -159,7 +169,6 @@ common_start:
PRINT("- CPU doesn't support the virtualization extensions -\r\n")
b fail
1:
-
/* Check that we're already in Hyp mode */
mrs r0, cpsr
and r0, r0, #0x1f /* Mode is in the low 5 bits of CPSR */
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index aee24e4d28..6b5cc15af3 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -128,8 +128,9 @@ static int __init omap5_smp_init(void)
}
printk("Set AuxCoreBoot1 to %"PRIpaddr" (%p)\n",
- __pa(init_secondary), init_secondary);
- writel(__pa(init_secondary), wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
+ __pa(omap5_init_secondary), omap5_init_secondary);
+ writel(__pa(omap5_init_secondary),
+ wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
printk("Set AuxCoreBoot0 to 0x20\n");
writel(0x20, wugen_base + OMAP_AUX_CORE_BOOT_0_OFFSET);
diff --git a/xen/include/asm-arm/platforms/omap5.h b/xen/include/asm-arm/platforms/omap5.h
index c559c84b61..732b27f403 100644
--- a/xen/include/asm-arm/platforms/omap5.h
+++ b/xen/include/asm-arm/platforms/omap5.h
@@ -22,6 +22,9 @@
#endif /* __ASM_ARM_PLATFORMS_OMAP5_H */
+/* Secondary cpu omap5 specific init routine */
+extern void omap5_init_secondary(void);
+
/*
* Local variables:
* mode: C
--
2.20.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode
2019-06-21 20:02 [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode Denis Obrezkov
@ 2019-06-24 11:09 ` Julien Grall
2019-06-24 12:03 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Julien Grall @ 2019-06-24 11:09 UTC (permalink / raw)
To: Denis Obrezkov, xen-devel
Cc: Hunyue Yau, julien.grall, Andre Przywara, tim, Iain Hunter, baozich
(+ GSOC mentors and Andre)
Hi Denis,
Thank you for the patch.
First of all, may I ask to CC the other mentors?
On 6/21/19 9:02 PM, Denis Obrezkov wrote:
> This function allows xen to bring secondary CPU cores into non-secure
> HYP mode. This is done by using a Secure Monitor call.
>
> Signed-off-by: Denis Obrezkov <denisobrezkov@gmail.com>
> ---
> xen/arch/arm/arm32/head.S | 11 ++++++++++-
> xen/arch/arm/platforms/omap5.c | 5 +++--
> xen/include/asm-arm/platforms/omap5.h | 3 +++
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 5f817d473e..120e034934 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -36,6 +36,10 @@
> #include EARLY_PRINTK_INC
> #endif
>
> +
> +#define API_HYP_ENTRY 0x102
> +#define AUX_CORE_BOOT0_PA 0x48281800
> +
I have thought a bit more about the placement of the code. I think it
would be best if it lives in a separate file (maybe platforms/omap5-head.S).
> /*
> * Common register usage in this file:
> * r0 -
> @@ -113,6 +117,12 @@ past_zImage:
>
> b common_start
>
> +GLOBAL(omap5_init_secondary)
> + ldr r12, =API_HYP_ENTRY
NIT: It is 3 spaces after ldr.
> + adr r0, init_secondary
Same here.
> + dsb
Why do you need the dsb here?
> + smc #0
> +
> GLOBAL(init_secondary)
> cpsid aif /* Disable all interrupts */
>
> @@ -159,7 +169,6 @@ common_start:
> PRINT("- CPU doesn't support the virtualization extensions -\r\n")
> b fail
> 1:
> -
This is a spurious change. Please remove it.
> /* Check that we're already in Hyp mode */
> mrs r0, cpsr
> and r0, r0, #0x1f /* Mode is in the low 5 bits of CPSR */
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index aee24e4d28..6b5cc15af3 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -128,8 +128,9 @@ static int __init omap5_smp_init(void)
> }
>
> printk("Set AuxCoreBoot1 to %"PRIpaddr" (%p)\n",
> - __pa(init_secondary), init_secondary);
> - writel(__pa(init_secondary), wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
> + __pa(omap5_init_secondary), omap5_init_secondary);
> + writel(__pa(omap5_init_secondary),
> + wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
I am trying to understand how this ever worked. omap5_smp_init is called
by two sets of platforms (based on compatible):
- ti,dra7: there were some hacks in U-boot to avoid the SMC. If I am
right, then I would not bother to support hacked U-boot.
- ti,omap5: [1] suggest that U-boot do the switch for us but it is
not clear whether this is upstreamed. @Chen, I know you did the port a
long time ago. Do you recall how this worked?
Linux seems to use the smc on any dra7 and omap54xx. So maybe we can use
safely here.
> printk("Set AuxCoreBoot0 to 0x20\n");
> writel(0x20, wugen_base + OMAP_AUX_CORE_BOOT_0_OFFSET);
> diff --git a/xen/include/asm-arm/platforms/omap5.h b/xen/include/asm-arm/platforms/omap5.h
> index c559c84b61..732b27f403 100644
> --- a/xen/include/asm-arm/platforms/omap5.h
> +++ b/xen/include/asm-arm/platforms/omap5.h
> @@ -22,6 +22,9 @@
>
> #endif /* __ASM_ARM_PLATFORMS_OMAP5_H */
>
> +/* Secondary cpu omap5 specific init routine */
> +extern void omap5_init_secondary(void);
> +
> /*
> * Local variables:
> * mode: C
>
[1]
https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/OMAP5432_uEVM
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode
2019-06-24 11:09 ` Julien Grall
@ 2019-06-24 12:03 ` Andrew Cooper
2019-06-25 9:57 ` Julien Grall
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2019-06-24 12:03 UTC (permalink / raw)
To: Julien Grall, Denis Obrezkov, xen-devel
Cc: Hunyue Yau, julien.grall, Andre Przywara, tim, Iain Hunter, baozich
[-- Attachment #1.1: Type: text/plain, Size: 2170 bytes --]
On 24/06/2019 12:09, Julien Grall wrote:
> (+ GSOC mentors and Andre)
>
> Hi Denis,
>
> Thank you for the patch.
>
> First of all, may I ask to CC the other mentors?
>
> On 6/21/19 9:02 PM, Denis Obrezkov wrote:
>> This function allows xen to bring secondary CPU cores into non-secure
>> HYP mode. This is done by using a Secure Monitor call.
>>
>> Signed-off-by: Denis Obrezkov <denisobrezkov@gmail.com>
>> ---
>> xen/arch/arm/arm32/head.S | 11 ++++++++++-
>> xen/arch/arm/platforms/omap5.c | 5 +++--
>> xen/include/asm-arm/platforms/omap5.h | 3 +++
>> 3 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>> index 5f817d473e..120e034934 100644
>> --- a/xen/arch/arm/arm32/head.S
>> +++ b/xen/arch/arm/arm32/head.S
>> @@ -36,6 +36,10 @@
>> #include EARLY_PRINTK_INC
>> #endif
>> +
>> +#define API_HYP_ENTRY 0x102
>> +#define AUX_CORE_BOOT0_PA 0x48281800
>> +
>
> I have thought a bit more about the placement of the code. I think it
> would be best if it lives in a separate file (maybe
> platforms/omap5-head.S).
For something this trivial, it is easy to put straight into omap5.c
Completely untested, but this ought to work:
diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index 6b5cc15af3..1dcc92d3a4 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -23,6 +23,16 @@
#include <xen/vmap.h>
#include <asm/io.h>
+void omap5_init_secondary(void);
+asm (
+".text \n\t"
+"omap5_init_secondary: \n\t"
+" ldr r12, =0x102 \n\t" /* API_HYP_ENTRY */
+" adr r0, init_secondary \n\t"
+" smc #0 \n\t"
+" b init_secondary \n\t"
+);
+
static uint16_t num_den[8][2] = {
{ 0, 0 }, /* not used */
{ 26 * 64, 26 * 125 }, /* 12.0 Mhz */
I personally find this favourable to introducing new stub files.
Ultimately it is Julien/Stefano's decision, but I'd like to point it out
as an option for anyone who is unaware.
~Andrew
[-- Attachment #1.2: Type: text/html, Size: 3247 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode
2019-06-24 12:03 ` Andrew Cooper
@ 2019-06-25 9:57 ` Julien Grall
0 siblings, 0 replies; 4+ messages in thread
From: Julien Grall @ 2019-06-25 9:57 UTC (permalink / raw)
To: Andrew Cooper, Denis Obrezkov, xen-devel
Cc: Hunyue Yau, julien.grall, Andre Przywara, tim, Iain Hunter, baozich
Hi Andrew,
On 24/06/2019 13:03, Andrew Cooper wrote:
> On 24/06/2019 12:09, Julien Grall wrote:
>> (+ GSOC mentors and Andre)
>>
>> Hi Denis,
>>
>> Thank you for the patch.
>>
>> First of all, may I ask to CC the other mentors?
>>
>> On 6/21/19 9:02 PM, Denis Obrezkov wrote:
>>> This function allows xen to bring secondary CPU cores into non-secure
>>> HYP mode. This is done by using a Secure Monitor call.
>>>
>>> Signed-off-by: Denis Obrezkov <denisobrezkov@gmail.com>
>>> ---
>>> xen/arch/arm/arm32/head.S | 11 ++++++++++-
>>> xen/arch/arm/platforms/omap5.c | 5 +++--
>>> xen/include/asm-arm/platforms/omap5.h | 3 +++
>>> 3 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>>> index 5f817d473e..120e034934 100644
>>> --- a/xen/arch/arm/arm32/head.S
>>> +++ b/xen/arch/arm/arm32/head.S
>>> @@ -36,6 +36,10 @@
>>> #include EARLY_PRINTK_INC
>>> #endif
>>> +
>>> +#define API_HYP_ENTRY 0x102
>>> +#define AUX_CORE_BOOT0_PA 0x48281800
>>> +
>>
>> I have thought a bit more about the placement of the code. I think it would be
>> best if it lives in a separate file (maybe platforms/omap5-head.S).
>
> For something this trivial, it is easy to put straight into omap5.c
>
> Completely untested, but this ought to work:
>
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index 6b5cc15af3..1dcc92d3a4 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -23,6 +23,16 @@
> #include <xen/vmap.h>
> #include <asm/io.h>
>
> +void omap5_init_secondary(void);
> +asm (
> +".text \n\t"
> +"omap5_init_secondary: \n\t"
> +" ldr r12, =0x102 \n\t" /* API_HYP_ENTRY */
> +" adr r0, init_secondary \n\t"
You cannot use adr on external address for Arm32. This is because the immediate
constant needs to have a specific format (see "Modified immediate constants in
ARM instructions" A5.2.4 in ARM DDI 406C.c).
Instead we would need something like:
omap5_init_secondary:
ldr r12, =0x102
adr r0, omap5_hyp
smc #0
omap5_hyp:
b init_secondary
Note similar code would be needed for the stub file.
> +" smc #0 \n\t"
> +" b init_secondary \n\t"
> +);
> +
> static uint16_t num_den[8][2] = {
> { 0, 0 }, /* not used */
> { 26 * 64, 26 * 125 }, /* 12.0 Mhz */
>
>
> I personally find this favourable to introducing new stub files.
>
> Ultimately it is Julien/Stefano's decision, but I'd like to point it out as an
> option for anyone who is unaware.
Thank you for the suggestion :). This was suggested last week, but no-one came
back explaining how it could be implemented.
The two are fine with me.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-25 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 20:02 [Xen-devel] [PATCH] xen/arm: Switch OMAP5 secondary cores into hyp mode Denis Obrezkov
2019-06-24 11:09 ` Julien Grall
2019-06-24 12:03 ` Andrew Cooper
2019-06-25 9:57 ` 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).