[PATCH 0/3] PSCI support for r8a7790 SoC (Lager/Stout boards)

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Oleksandr Tyshchenko-2

On 09.02.19 18:35, Marek Vasut wrote:

Hi

> On 2/7/19 6:19 PM, Oleksandr wrote:
>
> [...]
>
>>>>>> +        /* Update registers with correct frequency */
>>>>>> +        writel(freq, TIMER_BASE + TIMER_CNTFID0);
>>>>>> +        asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
>>>>>> +
>>>>>> +        /* Make sure arch timer is started by setting bit 0 of
>>>>>> CNTCR */
>>>>>> +        writel(1, TIMER_BASE + TIMER_CNTCR);
>>>>> Shouldn't this be in the timer driver instead ?
>>>> Which timer driver? Probably, I missed something, but I didn't find any
>>>> arch timer driver being used for Gen2.
>>>>
>>>> I see that TMU timer (arch/sh/lib/time.c) is used as a system timer, but
>>>> it is yet another IP.
>>>>
>>>> Would it be correct, if I move arch timer updating code to
>>>> arch/arm/mach-rmobile directory?
>>>>
>>>> Actually, at the same location the corresponding code lives in Linux:
>>>>
>>>> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/setup-rcar-gen2.c#L61
>>>>
>>> Presumably if this is something else than TMU, it needs proper driver
>>> that goes into drivers/ .
>> Did I get your point correctly that new driver (specially for Gen2 arch
>> timer) should be introduced in U-Boot and
>>
>> the only one thing it is intended to do is to configure that timer for
>> the future use by Linux/Hypervisor?
>>
>> If yes, the only question I have is how that new driver is going to be
>> populated? There is no corresponding node for arch timer in the device
>> tree.
>>
>> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/boot/dts/r8a7790.dtsi
>>
>>
>> So, do I need specially for this case add arch timer node with reg and
>> compatible properties?
>>
>> Sorry if I ask obvious questions, not familiar enough with U-Boot.
> You would need a DT entry and a bit of code to start the timer in case
> the PSCI is enabled, yes. This would then fit into the DM/DT paradigm.

I understand your point, thank you.

Will create a simple driver for arch timer in V2.


>
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * In order not to break compilation if someone decides to build
>>>>>> with PSCI
>>>>>> + * support disabled, keep these dummy functions.
>>>>>> + */
>>>>> That's currently the only use-case.
>>>> Shall I drop this comment and dummy functions below?
>>> Since there is no PSCI support, yes.
>> Understand.
>>
>>> [...]
>>>
>>> I'd like to have a cover letter go with V2, which describes what you're
>>> trying to achieve here.
>> Yes, sure. Cover letter is present for the current V1 as well.
>>
>> I thought that I had CC-ed all.
>>
>> This is a link to it:
>>
>> http://u-boot.10912.n7.nabble.com/PATCH-0-3-PSCI-support-for-r8a7790-SoC-Lager-Stout-boards-td357352.html
> Thanks
>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Oleksandr Tyshchenko-2
In reply to this post by Marek Vasut

On 09.02.19 18:37, Marek Vasut wrote:

Hi

>>>>>>> diff --git a/arch/arm/mach-rmobile/Kconfig.32
>>>>>>> b/arch/arm/mach-rmobile/Kconfig.32
>>>>>>> index 076a019..a2e9e3d 100644
>>>>>>> --- a/arch/arm/mach-rmobile/Kconfig.32
>>>>>>> +++ b/arch/arm/mach-rmobile/Kconfig.32
>>>>>>> @@ -76,6 +76,8 @@ config TARGET_LAGER
>>>>>>>         select SUPPORT_SPL
>>>>>>>         select USE_TINY_PRINTF
>>>>>>>         imply CMD_DM
>>>>>>> +    select CPU_V7_HAS_NONSEC
>>>>>>> +    select CPU_V7_HAS_VIRT
>>>>>> Shouldn't this be a H2 CPU property instead of a board property ?
>>>>> Probably yes, sounds reasonable. I will move these options under
>>>>> "config
>>>>> R8A7790".
>>>>>
>>>>>> Does this apply to M2* too , since it has CA15 cores as well ?
>>>>> I think, yes. But, without PSCI support being implemented for M2*, I
>>>>> think it is not worth to select these options for it as well.
>>>> It's basically the same SoC with two CPU cores less, how would that PSCI
>>>> be any different on M2 ?
>>> I need some time to investigate. I will come up with an answer later.
>>  From the first look:
>>
>> 1. It should be different total number of CPUs:
>>
>> For R8A7790 we have the following:
>>
>> #define R8A7790_PSCI_NR_CPUS    8
>>
>> But for R8A7791 it seems we need to use:
>>
>> #define R8A7791_PSCI_NR_CPUS    2
> This information should be in the DT for each SoC, so you should extract
> it from there.
>
>> 2. It should be new pm-r8a7791.c file which will don't have any CA7
>> related stuff (CPU cores, SCU, etc).
> I'd like to have a generic pm-gen2.c file , which parses the DT and
> figures the configuration out that way. We are trying to get rid of all
> the ad-hoc hardcoded configuration stuff in favor of DT.
>
>> Or it should be the single pm-r8a779x.c which must distinguish what SoC
>> is being used and do proper things.
> Right


I agree to have a single generic pm-gen2.c file which covers all Gen2 SoCs.

But, unfortunately, I only have Stout boards (H2), and therefore can
check only on them. This is why the current patch series adds support
for H2 SoC only.

Theoretically, I could add support for M2 as well, but, I wouldn't have
possibility to check.


I would focus on the r8a7790 for now, as the Stout board is our nearest
target which we want to support in Xen and the PSCI feature is a
mandatory option.

What do you think?



_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Oleksandr Tyshchenko-2

Hi, Marek

>>
>>> 2. It should be new pm-r8a7791.c file which will don't have any CA7
>>> related stuff (CPU cores, SCU, etc).
>> I'd like to have a generic pm-gen2.c file , which parses the DT and
>> figures the configuration out that way. We are trying to get rid of all
>> the ad-hoc hardcoded configuration stuff in favor of DT.
>>
>>> Or it should be the single pm-r8a779x.c which must distinguish what SoC
>>> is being used and do proper things.
>> Right
>
>
> I agree to have a single generic pm-gen2.c file which covers all Gen2
> SoCs.
>
> But, unfortunately, I only have Stout boards (H2), and therefore can
> check only on them. This is why the current patch series adds support
> for H2 SoC only.
>
> Theoretically, I could add support for M2 as well, but, I wouldn't
> have possibility to check.
>
>
> I would focus on the r8a7790 for now, as the Stout board is our
> nearest target which we want to support in Xen and the PSCI feature is
> a mandatory option.
>
> What do you think?

Could you, please, answer that question...


>
>
>
--
Regards,

Oleksandr Tyshchenko

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] ARM: rmobile: Add basic PSCI support for r8a7790 SoC

Oleksandr Tyshchenko-2
In reply to this post by Oleksandr Tyshchenko-2

Hi, Marek


>>>>>>> +}
>>>>>>> +
>>>>>>> +/*
>>>>>>> + * Reset vector for secondary CPUs.
>>>>>>> + * This will be mapped at address 0 by SBAR register.
>>>>>>> + * We need _long_ jump to the physical address.
>>>>>>> + */
>>>>>>> +asm("    .arm\n"
>>>>>>> +    "    .align 12\n"
>>>>>>> +    "    .globl shmobile_boot_vector\n"
>>>>>>> +    "shmobile_boot_vector:\n"
>>>>>>> +    "    ldr r1, 1f\n"
>>>>>>> +    "    bx    r1\n"
>>>>>>> +    "    .type shmobile_boot_vector, %function\n"
>>>>>>> +    "    .size shmobile_boot_vector, .-shmobile_boot_vector\n"
>>>>>>> +    "    .align    2\n"
>>>>>>> +    "    .globl    shmobile_boot_fn\n"
>>>>>>> +    "shmobile_boot_fn:\n"
>>>>>>> +    "1:    .space    4\n"
>>>>>>> +    "    .globl    shmobile_boot_size\n"
>>>>>>> +    "shmobile_boot_size:\n"
>>>>>>> +    "    .long    .-shmobile_boot_vector\n");
>>>>>> Why can't this be implemented in C ?
>>>>> This "reset vector" code was ported from Linux:
>>>>>
>>>>> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/headsmp.S#L21 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Really don't know whether it can be implemented in C.
>>>>>
>>>>> I was thinking of moving this code to a separate ASM file in order
>>>>> not
>>>>> to mix C and ASM. What do you think about it?
>>>> U-Boot already has a reset vector code, can't that be reused ?
>>> I don't think. Being honest, I couldn't find an obvious way how to
>>> reuse
>>> (I assume you meant arch/arm/cpu/armv7/start.S).
>> Maybe it needs some additional work first ?
>> It seems Altera socfpga somehow uses the U-Boot reset vectors for PSCI,
>> so it should at least be possible.
>
> Could you, please, point me in code? Unfortunately, I wasn't able to
> find.
>
>
>>
>>> The newly turned on secondary CPU entry should be common
>>> "psci_cpu_entry", which does proper things.
>>>
>>> And this reset vector is just "a small piece of code" to be located in
>>> on-chip RAM (with limited size) and used for the jump stub...
>> We already have the SPL reset vectors in SRAM, maybe that can be
>> recycled somehow ?
>
>
> The only idea I have, how it may be recycled (not sure whether it will
> work...)
>
>
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 0cb6dd39cc..69acf4677b 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -36,6 +36,12 @@
>  #endif
>
>  reset:
> +
> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> +       b psci_cpu_entry_jump
> +       /* return only if this is not "a newly turned on CPU" using
> PSCI) */
> +#endif
> +
>         /* Allow the board to save important registers */
>         b       save_boot_params
>  save_boot_params_ret:
> @@ -128,6 +134,21 @@ ENDPROC(switch_to_hypervisor)
>         .weak   switch_to_hypervisor
>  #endif
>
> +/*
> + * Each platform which implements psci_cpu_entry_jump function should
> perform
> + * in the following way:
> + *
> + * If the executing this call CPU is exactly that CPU we are
> expecting to be
> + * powered on, then jump to psci_cpu_entry and never return.
> + * Otherwise return to the caller.
> + */
> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> +ENTRY(psci_cpu_entry_jump)
> +       movs    pc, lr
> +ENDPROC(psci_cpu_entry_jump)
> +.weak psci_cpu_entry_jump
> +#endif
> +
>  /*************************************************************************
>
>   *
>   * cpu_init_cp15
>
>
> What do you think?
>
>
> It would be much appreciated, if you could provide some hints.


Don't want to be annoying, but it would be really nice to get my
questions answered...


--
Regards,

Oleksandr Tyshchenko

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Marek Vasut
In reply to this post by Oleksandr Tyshchenko-2
On 2/26/19 7:37 PM, Oleksandr wrote:
>
> Hi, Marek

Hi,

>>>
>>>> 2. It should be new pm-r8a7791.c file which will don't have any CA7
>>>> related stuff (CPU cores, SCU, etc).
>>> I'd like to have a generic pm-gen2.c file , which parses the DT and
>>> figures the configuration out that way. We are trying to get rid of all
>>> the ad-hoc hardcoded configuration stuff in favor of DT.
>>>
>>>> Or it should be the single pm-r8a779x.c which must distinguish what SoC
>>>> is being used and do proper things.
>>> Right
>>
>>
>> I agree to have a single generic pm-gen2.c file which covers all Gen2
>> SoCs.
>>
>> But, unfortunately, I only have Stout boards (H2), and therefore can
>> check only on them. This is why the current patch series adds support
>> for H2 SoC only.
>>
>> Theoretically, I could add support for M2 as well, but, I wouldn't
>> have possibility to check.
>>
>>
>> I would focus on the r8a7790 for now, as the Stout board is our
>> nearest target which we want to support in Xen and the PSCI feature is
>> a mandatory option.
>>
>> What do you think?
>
> Could you, please, answer that question...

I am sorry, I am too busy right now. I will answer that once I can
properly study the problem and give you a useful answer.

--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] ARM: rmobile: Add basic PSCI support for r8a7790 SoC

Marek Vasut
In reply to this post by Oleksandr Tyshchenko-2
On 2/26/19 8:37 PM, Oleksandr wrote:
>
> Hi, Marek

Hi,

>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> + * Reset vector for secondary CPUs.
>>>>>>>> + * This will be mapped at address 0 by SBAR register.
>>>>>>>> + * We need _long_ jump to the physical address.
>>>>>>>> + */
>>>>>>>> +asm("    .arm\n"
>>>>>>>> +    "    .align 12\n"
>>>>>>>> +    "    .globl shmobile_boot_vector\n"
>>>>>>>> +    "shmobile_boot_vector:\n"
>>>>>>>> +    "    ldr r1, 1f\n"
>>>>>>>> +    "    bx    r1\n"
>>>>>>>> +    "    .type shmobile_boot_vector, %function\n"
>>>>>>>> +    "    .size shmobile_boot_vector, .-shmobile_boot_vector\n"
>>>>>>>> +    "    .align    2\n"
>>>>>>>> +    "    .globl    shmobile_boot_fn\n"
>>>>>>>> +    "shmobile_boot_fn:\n"
>>>>>>>> +    "1:    .space    4\n"
>>>>>>>> +    "    .globl    shmobile_boot_size\n"
>>>>>>>> +    "shmobile_boot_size:\n"
>>>>>>>> +    "    .long    .-shmobile_boot_vector\n");
>>>>>>> Why can't this be implemented in C ?
>>>>>> This "reset vector" code was ported from Linux:
>>>>>>
>>>>>> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/headsmp.S#L21
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Really don't know whether it can be implemented in C.
>>>>>>
>>>>>> I was thinking of moving this code to a separate ASM file in order
>>>>>> not
>>>>>> to mix C and ASM. What do you think about it?
>>>>> U-Boot already has a reset vector code, can't that be reused ?
>>>> I don't think. Being honest, I couldn't find an obvious way how to
>>>> reuse
>>>> (I assume you meant arch/arm/cpu/armv7/start.S).
>>> Maybe it needs some additional work first ?
>>> It seems Altera socfpga somehow uses the U-Boot reset vectors for PSCI,
>>> so it should at least be possible.
>>
>> Could you, please, point me in code? Unfortunately, I wasn't able to
>> find.
>>
>>
>>>
>>>> The newly turned on secondary CPU entry should be common
>>>> "psci_cpu_entry", which does proper things.
>>>>
>>>> And this reset vector is just "a small piece of code" to be located in
>>>> on-chip RAM (with limited size) and used for the jump stub...
>>> We already have the SPL reset vectors in SRAM, maybe that can be
>>> recycled somehow ?
>>
>>
>> The only idea I have, how it may be recycled (not sure whether it will
>> work...)
>>
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index 0cb6dd39cc..69acf4677b 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>> @@ -36,6 +36,12 @@
>>  #endif
>>
>>  reset:
>> +
>> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
>> +       b psci_cpu_entry_jump
>> +       /* return only if this is not "a newly turned on CPU" using
>> PSCI) */
>> +#endif
>> +
>>         /* Allow the board to save important registers */
>>         b       save_boot_params
>>  save_boot_params_ret:
>> @@ -128,6 +134,21 @@ ENDPROC(switch_to_hypervisor)
>>         .weak   switch_to_hypervisor
>>  #endif
>>
>> +/*
>> + * Each platform which implements psci_cpu_entry_jump function should
>> perform
>> + * in the following way:
>> + *
>> + * If the executing this call CPU is exactly that CPU we are
>> expecting to be
>> + * powered on, then jump to psci_cpu_entry and never return.
>> + * Otherwise return to the caller.
>> + */
>> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
>> +ENTRY(psci_cpu_entry_jump)
>> +       movs    pc, lr
>> +ENDPROC(psci_cpu_entry_jump)
>> +.weak psci_cpu_entry_jump
>> +#endif
>> +
>>  /*************************************************************************
>>
>>   *
>>   * cpu_init_cp15
>>
>>
>> What do you think?
>>
>>
>> It would be much appreciated, if you could provide some hints.
>
>
> Don't want to be annoying, but it would be really nice to get my
> questions answered...

I am sorry, I am too busy right now. I will answer that once I can
properly study the problem and give you a useful answer.

--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/3] ARM: rmobile: Add basic PSCI support for r8a7790 SoC

Oleksandr Tyshchenko-2
Hi, Marek

sorry for possible format issue.


ср, 27 февр. 2019 г., 23:02 Marek Vasut <[hidden email]>:

> On 2/26/19 8:37 PM, Oleksandr wrote:
> >
> > Hi, Marek
>
> Hi,
>
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +/*
> >>>>>>>> + * Reset vector for secondary CPUs.
> >>>>>>>> + * This will be mapped at address 0 by SBAR register.
> >>>>>>>> + * We need _long_ jump to the physical address.
> >>>>>>>> + */
> >>>>>>>> +asm("    .arm\n"
> >>>>>>>> +    "    .align 12\n"
> >>>>>>>> +    "    .globl shmobile_boot_vector\n"
> >>>>>>>> +    "shmobile_boot_vector:\n"
> >>>>>>>> +    "    ldr r1, 1f\n"
> >>>>>>>> +    "    bx    r1\n"
> >>>>>>>> +    "    .type shmobile_boot_vector, %function\n"
> >>>>>>>> +    "    .size shmobile_boot_vector, .-shmobile_boot_vector\n"
> >>>>>>>> +    "    .align    2\n"
> >>>>>>>> +    "    .globl    shmobile_boot_fn\n"
> >>>>>>>> +    "shmobile_boot_fn:\n"
> >>>>>>>> +    "1:    .space    4\n"
> >>>>>>>> +    "    .globl    shmobile_boot_size\n"
> >>>>>>>> +    "shmobile_boot_size:\n"
> >>>>>>>> +    "    .long    .-shmobile_boot_vector\n");
> >>>>>>> Why can't this be implemented in C ?
> >>>>>> This "reset vector" code was ported from Linux:
> >>>>>>
> >>>>>>
> https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/headsmp.S#L21
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Really don't know whether it can be implemented in C.
> >>>>>>
> >>>>>> I was thinking of moving this code to a separate ASM file in order
> >>>>>> not
> >>>>>> to mix C and ASM. What do you think about it?
> >>>>> U-Boot already has a reset vector code, can't that be reused ?
> >>>> I don't think. Being honest, I couldn't find an obvious way how to
> >>>> reuse
> >>>> (I assume you meant arch/arm/cpu/armv7/start.S).
> >>> Maybe it needs some additional work first ?
> >>> It seems Altera socfpga somehow uses the U-Boot reset vectors for PSCI,
> >>> so it should at least be possible.
> >>
> >> Could you, please, point me in code? Unfortunately, I wasn't able to
> >> find.
> >>
> >>
> >>>
> >>>> The newly turned on secondary CPU entry should be common
> >>>> "psci_cpu_entry", which does proper things.
> >>>>
> >>>> And this reset vector is just "a small piece of code" to be located in
> >>>> on-chip RAM (with limited size) and used for the jump stub...
> >>> We already have the SPL reset vectors in SRAM, maybe that can be
> >>> recycled somehow ?
> >>
> >>
> >> The only idea I have, how it may be recycled (not sure whether it will
> >> work...)
> >>
> >>
> >> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >> index 0cb6dd39cc..69acf4677b 100644
> >> --- a/arch/arm/cpu/armv7/start.S
> >> +++ b/arch/arm/cpu/armv7/start.S
> >> @@ -36,6 +36,12 @@
> >>  #endif
> >>
> >>  reset:
> >> +
> >> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> >> +       b psci_cpu_entry_jump
> >> +       /* return only if this is not "a newly turned on CPU" using
> >> PSCI) */
> >> +#endif
> >> +
> >>         /* Allow the board to save important registers */
> >>         b       save_boot_params
> >>  save_boot_params_ret:
> >> @@ -128,6 +134,21 @@ ENDPROC(switch_to_hypervisor)
> >>         .weak   switch_to_hypervisor
> >>  #endif
> >>
> >> +/*
> >> + * Each platform which implements psci_cpu_entry_jump function should
> >> perform
> >> + * in the following way:
> >> + *
> >> + * If the executing this call CPU is exactly that CPU we are
> >> expecting to be
> >> + * powered on, then jump to psci_cpu_entry and never return.
> >> + * Otherwise return to the caller.
> >> + */
> >> +#if defined(CONFIG_ARMV7_PSCI) && !defined(CONFIG_SPL_BUILD)
> >> +ENTRY(psci_cpu_entry_jump)
> >> +       movs    pc, lr
> >> +ENDPROC(psci_cpu_entry_jump)
> >> +.weak psci_cpu_entry_jump
> >> +#endif
> >> +
> >>
>  /*************************************************************************
> >>
> >>   *
> >>   * cpu_init_cp15
> >>
> >>
> >> What do you think?
> >>
> >>
> >> It would be much appreciated, if you could provide some hints.
> >
> >
> > Don't want to be annoying, but it would be really nice to get my
> > questions answered...
>
> I am sorry, I am too busy right now. I will answer that once I can
> properly study the problem and give you a useful answer.
>

No problem, I will wait.

Thank you.


> --
> Best regards,
> Marek Vasut
>
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Marek Vasut
In reply to this post by Oleksandr Tyshchenko-2
On 2/12/19 8:52 PM, Oleksandr wrote:

Hi,

[...]

I was thinking about this whole PSCI situation and how it's all
implemented today. Basically what we do is generate a separate reduced
shred of U-Boot, which will remain resident in memory and which could be
called by some other software. That shred is a piece of U-Boot proper,
but a lot of stuff is just torn away at runtime, so it's not the whole
binary, just tattered remnants of it.

I really do not like this approach to the whole U-Boot PSCI in the first
place, it seems really fragile and dangerous.

But look, U-Boot already has support for U-Boot SPL/TPL, which is small,
can contain a full driver model, drivers and all the necessary support
functionality. It is built from the same sources, at the same time, but
it is not a shred of U-Boot proper but rather a separate entity.

So I wonder, can't we use this U-Boot SPL/TPL as the piece of code which
would remain resident in memory and handle the PSCI calls instead ? I
think U-Boot can load such a code or it could be somehow bundled with
U-Boot proper (using a fitImage maybe ?). This way you won't have to
re-implement all the drivers in arch/arm/, the DM/DT remains in place
and the whole PSCI handler would be self-contained, so no need to fiddle
with linker sections of U-Boot itself.

I am CCing other people who use this PSCI stuff in U-Boot, maybe they
have some thoughts on this approach.

And I apologize again for taking this long to reply.

[...]

--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Tan, Ley Foon
On Thu, 2019-03-14 at 01:16 +0100, Marek Vasut wrote:

> On 2/12/19 8:52 PM, Oleksandr wrote:
>
> Hi,
>
> [...]
>
> I was thinking about this whole PSCI situation and how it's all
> implemented today. Basically what we do is generate a separate
> reduced
> shred of U-Boot, which will remain resident in memory and which could
> be
> called by some other software. That shred is a piece of U-Boot
> proper,
> but a lot of stuff is just torn away at runtime, so it's not the
> whole
> binary, just tattered remnants of it.
>
> I really do not like this approach to the whole U-Boot PSCI in the
> first
> place, it seems really fragile and dangerous.
>
> But look, U-Boot already has support for U-Boot SPL/TPL, which is
> small,
> can contain a full driver model, drivers and all the necessary
> support
> functionality. It is built from the same sources, at the same time,
> but
> it is not a shred of U-Boot proper but rather a separate entity.
>
> So I wonder, can't we use this U-Boot SPL/TPL as the piece of code
> which
> would remain resident in memory and handle the PSCI calls instead ? I
> think U-Boot can load such a code or it could be somehow bundled with
> U-Boot proper (using a fitImage maybe ?). This way you won't have to
> re-implement all the drivers in arch/arm/, the DM/DT remains in place
> and the whole PSCI handler would be self-contained, so no need to
> fiddle
> with linker sections of U-Boot itself.
>
> I am CCing other people who use this PSCI stuff in U-Boot, maybe they
> have some thoughts on this approach.
>
> And I apologize again for taking this long to reply.
>
> [...]
>
Adding Chin Liang and Jeremy.


Regards
Ley Foon
_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Oleksandr Tyshchenko-2
In reply to this post by Marek Vasut

On 14.03.19 02:16, Marek Vasut wrote:
> On 2/12/19 8:52 PM, Oleksandr wrote:
>
> Hi,

Hi

>
> [...]
>
> I was thinking about this whole PSCI situation and how it's all
> implemented today. Basically what we do is generate a separate reduced
> shred of U-Boot, which will remain resident in memory and which could be
> called by some other software. That shred is a piece of U-Boot proper,
> but a lot of stuff is just torn away at runtime, so it's not the whole
> binary, just tattered remnants of it.
>
> I really do not like this approach to the whole U-Boot PSCI in the first
> place, it seems really fragile and dangerous.
>
> But look, U-Boot already has support for U-Boot SPL/TPL, which is small,
> can contain a full driver model, drivers and all the necessary support
> functionality. It is built from the same sources, at the same time, but
> it is not a shred of U-Boot proper but rather a separate entity.
>
> So I wonder, can't we use this U-Boot SPL/TPL as the piece of code which
> would remain resident in memory and handle the PSCI calls instead ? I
> think U-Boot can load such a code or it could be somehow bundled with
> U-Boot proper (using a fitImage maybe ?). This way you won't have to
> re-implement all the drivers in arch/arm/, the DM/DT remains in place
> and the whole PSCI handler would be self-contained, so no need to fiddle
> with linker sections of U-Boot itself.
>
> I am CCing other people who use this PSCI stuff in U-Boot, maybe they
> have some thoughts on this approach.

Thank you for your honest answer.

Let's see what the community will say.


>
> And I apologize again for taking this long to reply.

No problem.


>
> [...]
>
--
Regards,

Oleksandr Tyshchenko

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

Patrick DELAUNAY-2
Hi,

> From: Oleksandr <[hidden email]>
> Sent: jeudi 14 mars 2019 12:37
>
>
> On 14.03.19 02:16, Marek Vasut wrote:
> > On 2/12/19 8:52 PM, Oleksandr wrote:
> >
> > Hi,
>
> Hi
>
> >
> > [...]
> >
> > I was thinking about this whole PSCI situation and how it's all
> > implemented today. Basically what we do is generate a separate reduced
> > shred of U-Boot, which will remain resident in memory and which could
> > be called by some other software. That shred is a piece of U-Boot
> > proper, but a lot of stuff is just torn away at runtime, so it's not
> > the whole binary, just tattered remnants of it.
> >
> > I really do not like this approach to the whole U-Boot PSCI in the
> > first place, it seems really fragile and dangerous.
> >
> > But look, U-Boot already has support for U-Boot SPL/TPL, which is
> > small, can contain a full driver model, drivers and all the necessary
> > support functionality. It is built from the same sources, at the same
> > time, but it is not a shred of U-Boot proper but rather a separate entity.
> >
> > So I wonder, can't we use this U-Boot SPL/TPL as the piece of code
> > which would remain resident in memory and handle the PSCI calls
> > instead ? I think U-Boot can load such a code or it could be somehow
> > bundled with U-Boot proper (using a fitImage maybe ?). This way you
> > won't have to re-implement all the drivers in arch/arm/, the DM/DT
> > remains in place and the whole PSCI handler would be self-contained,
> > so no need to fiddle with linker sections of U-Boot itself.
> >
> > I am CCing other people who use this PSCI stuff in U-Boot, maybe they
> > have some thoughts on this approach.
>
> Thank you for your honest answer.
>
> Let's see what the community will say.

I will answer for STM32MP1 platform.

On STM32MP157 chip, the PSCI support is expected minimal in U-Boot for basic boot defconfig.
  ROM code => SPL => U-Boot (install PSCI monitor) => Kernel
  => only CPU_ON/OFF for hotplug needed by kernel but no power management.

For full PSCI support (standby modes), we are using TF-A secure monitor (trusted boot defconfig) with full power support or OP-TEE secure monitor
  ROM code => TF-A (BL2 install secure monitor = BL32 : SP min) => U-Boot (non secure world) => Kernel
  ROM code => TF-A (BL2)  =>  secure OS = OP-TEE => U-Boot (non secure world) => Kernel
In these 2 cases U-Boot access secure resource with SPCI request (SMC call).


But it in the future of the basic boot, if we want add a full PSCI support in U-Boot for power management, we need at least access to some resources:
- I2C driver
- STPMIC1 driver
- DDR driver (to switch the DDR in self refresh mode)
- Clock /reset driver

I agree all this part are already managed by U-Boot drivers / u-class.
But we need also a way to have access to board information / DT description.

It is also why I don't implement the function psci_system_off() in ./arch/arm/mach-stm32mp/psci.c
=> I don't want recode a I2C driver in PSCI code...

void __secure psci_system_off(u32 function_id)
{
        /* System Off is not managed, waiting user power off
         * TODO: handle I2C write in PMIC Main Control register bit 0 = SWOFF
         */
        while (1)
                wfi();
}

So have a PSCI firmware with driver model based on the U-Boot code (as it is done for SPL/TPL) seems for me a good solution.

Question: what the right moment to install  the secure monitor ?

+ SPL=> U-Boot then U-Boot can use PSCI installed firmware but no access of secure resource in U-Boot
+ U-Boot => Kernel then U-Boot is executed in secure world can't use the PSCI firmware (as today)

PS: the first solution is used for some board when the secure monitor of TF-A = BL32 is installed by U-Boot (see CONFIG_SPL_ATF)

        ROM code => SPL : install BL32 compiled from TF-A code => U-Boot (non secure world) => Kernel

>
> >
> > And I apologize again for taking this long to reply.
>
> No problem.
>
>
> >
> > [...]
> >
> --
> Regards,
>
> Oleksandr Tyshchenko

Regards

Patrick

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot
12