RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 12 November 2020 12:49
>To: Pragnesh Patel ; Simon Glass
>
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>anup.pa...@wdc.com; Sagar Kadam ;
>r...@andestech.com; Sean Anderson
>Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>On 11/12/20 8:09 AM, Heinrich Schuchardt wrote:
>> On 11/12/20 7:34 AM, Pragnesh Patel wrote:
>>> Hi Heinrich,
>>>
-Original Message-
From: Heinrich Schuchardt
Sent: 11 November 2020 19:18
To: Pragnesh Patel
Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
u-boot@lists.denx.de; bmeng...@gmail.com; Paul Walmsley ( Sifive)
; anup.pa...@wdc.com; Sagar Kadam
; r...@andestech.com; Sean Anderson
; Simon Glass
Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
[External Email] Do not click links or attachments unless you
recognize the sender and know the content is safe
On 11/11/20 12:56 PM, Pragnesh Patel wrote:
> Hi Heinrich,
>
>> -Original Message-
>> From: Heinrich Schuchardt
>> Sent: 11 November 2020 16:51
>> To: Pragnesh Patel ;
>> u-boot@lists.denx.de
>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>> bmeng...@gmail.com; Paul Walmsley ( Sifive)
>> ; anup.pa...@wdc.com; Sagar Kadam
>> ; r...@andestech.com; Sean Anderson
>> ; Simon Glass
>> Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>>
>> [External Email] Do not click links or attachments unless you
>> recognize the sender and know the content is safe
>>
>> On 11.11.20 11:14, Pragnesh Patel wrote:
>>> Add timer_get_us() which is useful for tracing.
>>> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer
>>> ticks and For M-mode U-Boot, mtime register will provide the same.
>>>
>>> Signed-off-by: Pragnesh Patel
>>
>> The default implementation of get_timer_us() in lib/time.c calls
>> get_ticks() which calls timer_get_count(). The get_count()
>> operation is implemented in drivers/timer/andes_plmt_timer.c,
>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>>
>> Why do you need special timer_get_us() implementations?
>> Isn't it enough to supply the get_count() operation in the drivers?
>
> get_ticks() is depend on gd->timer and there are 2 cases
>
> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
> call functions which are not marked with "notrace" so tracing got stuck.
Thanks for the background information.
Please, identify the existing functions that lack "notrace" and fix
them. This will give us a solution for all existing boards and not for a
small
>selection.
Furthermore it will avoid code duplication.
>>>
>>> I tried but There are so many functions which need "notrace".
>>
>> Let's start with the problem statement:
>>
>> When tracing functions is enabled this adds calls to
>> __cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
>> functions.
>>
>> __cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
>> timer_get_us() to record the entry and exit time.
>>
>> If timer_get_us() or any function used to implement does not carry
>> __attribute__((no_instrument_function)) this will lead to an
>> indefinite recursion.
>>
>> We have to change __cyg_profile_func_enter() and
>> __cyg_profile_func_exit() such that during their execution no function
>> is traced. We can do so by temporarily setting trace_enabled to false.
>>
>> Does this match your observation?
>
>I just tried to put this into a patch
>
>[PATCH 1/1] trace: avoid infinite recursion https://lists.denx.de/pipermail/u-
>boot/2020-November/432589.html
>https://patchwork.ozlabs.org/project/uboot/patch/20201112071411.4202-1-
>xypron.g...@gmx.de/
>
>Does this solve you problem?
Getting error, "Could not initialize timer (err -11)" from get_ticks() function.
Below are my boot logs,
U-Boot 2021.01-rc1-00244-g794c155581-dirty (Nov 12 2020 - 12:56:27 +0530)
CPU: rv64imafdc
Model: SiFive HiFive Unleashed A00
DRAM: 8 GiB
trace: enabled
Could not initialize timer (err -11)
Could not initialize timer (err -11)
Could not initialize timer (err -11)
Could not initialize timer (err -11)
>
>Best regards
>
>Heinrich
>>
>>>
>>> Why don’t we consider removing gd->timer=NULL in initr_dm()
>>> initr_dm()
>>> {
>>> #ifdef CONFIG_TIMER
>>> gd->timer = NULL;
>>> #endif
>>> }
>>>
>>> Or I can use TIMER_EARLY and return ticks and rate by adding
>>> timer_early_get_count() and timer_early_get_rate() repectively.
>>>
>>> Suggestions are welcome
>>>
In lib/trace.c consider replacing
RE: [PATCH 1/1] trace: avoid infinite recursion
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 12 November 2020 12:44
>To: Simon Glass
>Cc: Pragnesh Patel ; u-boot@lists.denx.de;
>Heinrich Schuchardt
>Subject: [PATCH 1/1] trace: avoid infinite recursion
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>When tracing functions is enabled this adds calls to
>__cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
>functions.
>
>__cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
Replace one of __cyg_profile_func_exit() with __cyg_profile_func_enter()
>timer_get_us() to record the entry and exit time.
>
>If timer_get_us() or any function used to implement does not carry
>__attribute__((no_instrument_function)) this will lead to an indefinite
>recursion.
>
>The patch changes __cyg_profile_func_enter() and
>__cyg_profile_func_exit() such that during their execution no function is
>traced
>by temporarily setting trace_enabled to false.
>
>Reported-by: Pragnesh Patel
>Signed-off-by: Heinrich Schuchardt
>---
> lib/trace.c | 10 ++
> 1 file changed, 10 insertions(+)
>
>diff --git a/lib/trace.c b/lib/trace.c
>index defc9716d8..b84b9fbfef 100644
>--- a/lib/trace.c
>+++ b/lib/trace.c
>@@ -141,9 +141,12 @@ static void __attribute__((no_instrument_function))
>add_textbase(void) void __attribute__((no_instrument_function))
>__cyg_profile_func_enter(
>void *func_ptr, void *caller) {
>+
No need for new line
>if (trace_enabled) {
>int func;
>+ char trace_enabled_old = trace_enabled;
>
>+ trace_enabled = 0;
>trace_swap_gd();
>add_ftrace(func_ptr, caller, FUNCF_ENTRY);
>func = func_ptr_to_num(func_ptr); @@ -157,6 +160,7 @@ void
>__attribute__((no_instrument_function)) __cyg_profile_func_enter(
>if (hdr->depth > hdr->depth_limit)
>hdr->max_depth = hdr->depth;
>trace_swap_gd();
>+ trace_enabled = trace_enabled_old;
>}
> }
>
>@@ -169,11 +173,17 @@ void __attribute__((no_instrument_function))
>__cyg_profile_func_enter( void __attribute__((no_instrument_function))
>__cyg_profile_func_exit(
>void *func_ptr, void *caller) {
>+ trace_enabled
Is this necessary ?
>+
>if (trace_enabled) {
>+ char trace_enabled_old = trace_enabled;
>+
>+ trace_enabled = 0;
>trace_swap_gd();
>add_ftrace(func_ptr, caller, FUNCF_EXIT);
>hdr->depth--;
>trace_swap_gd();
>+ trace_enabled = trace_enabled_old;
>}
> }
>
>--
>2.28.0
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/12/20 8:09 AM, Heinrich Schuchardt wrote:
> On 11/12/20 7:34 AM, Pragnesh Patel wrote:
>> Hi Heinrich,
>>
>>> -Original Message-
>>> From: Heinrich Schuchardt
>>> Sent: 11 November 2020 19:18
>>> To: Pragnesh Patel
>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>>> bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>>> anup.pa...@wdc.com; Sagar Kadam ;
>>> r...@andestech.com; Sean Anderson ; Simon Glass
>>>
>>> Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>>>
>>> [External Email] Do not click links or attachments unless you recognize the
>>> sender and know the content is safe
>>>
>>> On 11/11/20 12:56 PM, Pragnesh Patel wrote:
Hi Heinrich,
> -Original Message-
> From: Heinrich Schuchardt
> Sent: 11 November 2020 16:51
> To: Pragnesh Patel ;
> u-boot@lists.denx.de
> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
> bmeng...@gmail.com; Paul Walmsley ( Sifive)
> ; anup.pa...@wdc.com; Sagar Kadam
> ; r...@andestech.com; Sean Anderson
> ; Simon Glass
> Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>
> [External Email] Do not click links or attachments unless you
> recognize the sender and know the content is safe
>
> On 11.11.20 11:14, Pragnesh Patel wrote:
>> Add timer_get_us() which is useful for tracing.
>> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
>> and For M-mode U-Boot, mtime register will provide the same.
>>
>> Signed-off-by: Pragnesh Patel
>
> The default implementation of get_timer_us() in lib/time.c calls
> get_ticks() which calls timer_get_count(). The get_count() operation
> is implemented in drivers/timer/andes_plmt_timer.c,
> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>
> Why do you need special timer_get_us() implementations?
> Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will
call functions which are not marked with "notrace" so tracing got stuck.
>>>
>>> Thanks for the background information.
>>>
>>> Please, identify the existing functions that lack "notrace" and fix them.
>>> This will
>>> give us a solution for all existing boards and not for a small selection.
>>> Furthermore it will avoid code duplication.
>>
>> I tried but There are so many functions which need "notrace".
>
> Let's start with the problem statement:
>
> When tracing functions is enabled this adds calls to
> __cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
> functions.
>
> __cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
> timer_get_us() to record the entry and exit time.
>
> If timer_get_us() or any function used to implement does not carry
> __attribute__((no_instrument_function)) this will lead to an indefinite
> recursion.
>
> We have to change __cyg_profile_func_enter() and
> __cyg_profile_func_exit() such that during their execution no function
> is traced. We can do so by temporarily setting trace_enabled to false.
>
> Does this match your observation?
I just tried to put this into a patch
[PATCH 1/1] trace: avoid infinite recursion
https://lists.denx.de/pipermail/u-boot/2020-November/432589.html
https://patchwork.ozlabs.org/project/uboot/patch/20201112071411.4202-1-xypron.g...@gmx.de/
Does this solve you problem?
Best regards
Heinrich
>
>>
>> Why don’t we consider removing gd->timer=NULL in initr_dm()
>> initr_dm()
>> {
>> #ifdef CONFIG_TIMER
>> gd->timer = NULL;
>> #endif
>> }
>>
>> Or I can use TIMER_EARLY and return ticks and rate by adding
>> timer_early_get_count()
>> and timer_early_get_rate() repectively.
>>
>> Suggestions are welcome
>>
>>>
>>> In lib/trace.c consider replacing
>>> "__attribute__((no_instrument_function))" by "notrace".
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
>
> Best regards
>
> Heinrich
>
>> ---
>>
>> Changes in v3:
>> - Added gd->arch.plmt in global data
>> - For timer_get_us(), use readq() instead of andes_plmt_get_count()
>>and sifive_clint_get_count()
>>
>> Changes in v2:
>> - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
>>and andes_plmt_timer.c.
>>
>>
>> arch/riscv/include/asm/global_data.h | 3 +++
>> drivers/timer/andes_plmt_timer.c | 19 ++-
>> drivers/timer/riscv_timer.c | 14 +-
>>
[PATCH 1/1] trace: avoid infinite recursion
When tracing functions is enabled this adds calls to
__cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
functions.
__cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
timer_get_us() to record the entry and exit time.
If timer_get_us() or any function used to implement does not carry
__attribute__((no_instrument_function)) this will lead to an indefinite
recursion.
The patch changes __cyg_profile_func_enter() and
__cyg_profile_func_exit() such that during their execution no function is
traced by temporarily setting trace_enabled to false.
Reported-by: Pragnesh Patel
Signed-off-by: Heinrich Schuchardt
---
lib/trace.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/lib/trace.c b/lib/trace.c
index defc9716d8..b84b9fbfef 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -141,9 +141,12 @@ static void __attribute__((no_instrument_function))
add_textbase(void)
void __attribute__((no_instrument_function)) __cyg_profile_func_enter(
void *func_ptr, void *caller)
{
+
if (trace_enabled) {
int func;
+ char trace_enabled_old = trace_enabled;
+ trace_enabled = 0;
trace_swap_gd();
add_ftrace(func_ptr, caller, FUNCF_ENTRY);
func = func_ptr_to_num(func_ptr);
@@ -157,6 +160,7 @@ void __attribute__((no_instrument_function))
__cyg_profile_func_enter(
if (hdr->depth > hdr->depth_limit)
hdr->max_depth = hdr->depth;
trace_swap_gd();
+ trace_enabled = trace_enabled_old;
}
}
@@ -169,11 +173,17 @@ void __attribute__((no_instrument_function))
__cyg_profile_func_enter(
void __attribute__((no_instrument_function)) __cyg_profile_func_exit(
void *func_ptr, void *caller)
{
+ trace_enabled
+
if (trace_enabled) {
+ char trace_enabled_old = trace_enabled;
+
+ trace_enabled = 0;
trace_swap_gd();
add_ftrace(func_ptr, caller, FUNCF_EXIT);
hdr->depth--;
trace_swap_gd();
+ trace_enabled = trace_enabled_old;
}
}
--
2.28.0
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/12/20 7:34 AM, Pragnesh Patel wrote:
> Hi Heinrich,
>
>> -Original Message-
>> From: Heinrich Schuchardt
>> Sent: 11 November 2020 19:18
>> To: Pragnesh Patel
>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>> bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>> anup.pa...@wdc.com; Sagar Kadam ;
>> r...@andestech.com; Sean Anderson ; Simon Glass
>>
>> Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>>
>> [External Email] Do not click links or attachments unless you recognize the
>> sender and know the content is safe
>>
>> On 11/11/20 12:56 PM, Pragnesh Patel wrote:
>>> Hi Heinrich,
>>>
-Original Message-
From: Heinrich Schuchardt
Sent: 11 November 2020 16:51
To: Pragnesh Patel ;
u-boot@lists.denx.de
Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
bmeng...@gmail.com; Paul Walmsley ( Sifive)
; anup.pa...@wdc.com; Sagar Kadam
; r...@andestech.com; Sean Anderson
; Simon Glass
Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
[External Email] Do not click links or attachments unless you
recognize the sender and know the content is safe
On 11.11.20 11:14, Pragnesh Patel wrote:
> Add timer_get_us() which is useful for tracing.
> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
> and For M-mode U-Boot, mtime register will provide the same.
>
> Signed-off-by: Pragnesh Patel
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation
is implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
>>>
>>> get_ticks() is depend on gd->timer and there are 2 cases
>>>
>>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
>>> call functions which are not marked with "notrace" so tracing got stuck.
>>
>> Thanks for the background information.
>>
>> Please, identify the existing functions that lack "notrace" and fix them.
>> This will
>> give us a solution for all existing boards and not for a small selection.
>> Furthermore it will avoid code duplication.
>
> I tried but There are so many functions which need "notrace".
Let's start with the problem statement:
When tracing functions is enabled this adds calls to
__cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
functions.
__cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
timer_get_us() to record the entry and exit time.
If timer_get_us() or any function used to implement does not carry
__attribute__((no_instrument_function)) this will lead to an indefinite
recursion.
We have to change __cyg_profile_func_enter() and
__cyg_profile_func_exit() such that during their execution no function
is traced. We can do so by temporarily setting trace_enabled to false.
Does this match your observation?
Best regards
Heinrich
>
> Why don’t we consider removing gd->timer=NULL in initr_dm()
> initr_dm()
> {
> #ifdef CONFIG_TIMER
> gd->timer = NULL;
> #endif
> }
>
> Or I can use TIMER_EARLY and return ticks and rate by adding
> timer_early_get_count()
> and timer_early_get_rate() repectively.
>
> Suggestions are welcome
>
>>
>> In lib/trace.c consider replacing
>> "__attribute__((no_instrument_function))" by "notrace".
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> 2) if gd->timer is already initialized then still initr_dm() will make
>>> gd->timer = NULL;
>>>
>>> initr_dm()
>>> {
>>> #ifdef CONFIG_TIMER
>>> gd->timer = NULL;
>>> #endif
>>> }
>>>
>>> So again dm_timer_init() gets called and tracing got stuck.
>>>
>>> So I thought better to implement timer_get_us().
>>>
Best regards
Heinrich
> ---
>
> Changes in v3:
> - Added gd->arch.plmt in global data
> - For timer_get_us(), use readq() instead of andes_plmt_get_count()
>and sifive_clint_get_count()
>
> Changes in v2:
> - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
>and andes_plmt_timer.c.
>
>
> arch/riscv/include/asm/global_data.h | 3 +++
> drivers/timer/andes_plmt_timer.c | 19 ++-
> drivers/timer/riscv_timer.c | 14 +-
> drivers/timer/sifive_clint_timer.c | 19 ++-
> 4 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/global_data.h
> b/arch/riscv/include/asm/global_data.h
> index d3a0b1d221..4e22ceb83f 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
>void __iomem *plic; /* plic base
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 11 November 2020 19:18
>To: Pragnesh Patel
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>anup.pa...@wdc.com; Sagar Kadam ;
>r...@andestech.com; Sean Anderson ; Simon Glass
>
>Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>On 11/11/20 12:56 PM, Pragnesh Patel wrote:
>> Hi Heinrich,
>>
>>> -Original Message-
>>> From: Heinrich Schuchardt
>>> Sent: 11 November 2020 16:51
>>> To: Pragnesh Patel ;
>>> u-boot@lists.denx.de
>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>>> bmeng...@gmail.com; Paul Walmsley ( Sifive)
>>> ; anup.pa...@wdc.com; Sagar Kadam
>>> ; r...@andestech.com; Sean Anderson
>>> ; Simon Glass
>>> Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>>>
>>> [External Email] Do not click links or attachments unless you
>>> recognize the sender and know the content is safe
>>>
>>> On 11.11.20 11:14, Pragnesh Patel wrote:
Add timer_get_us() which is useful for tracing.
For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
and For M-mode U-Boot, mtime register will provide the same.
Signed-off-by: Pragnesh Patel
>>>
>>> The default implementation of get_timer_us() in lib/time.c calls
>>> get_ticks() which calls timer_get_count(). The get_count() operation
>>> is implemented in drivers/timer/andes_plmt_timer.c,
>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>>>
>>> Why do you need special timer_get_us() implementations?
>>> Isn't it enough to supply the get_count() operation in the drivers?
>>
>> get_ticks() is depend on gd->timer and there are 2 cases
>>
>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
>> call functions which are not marked with "notrace" so tracing got stuck.
>
>Thanks for the background information.
>
>Please, identify the existing functions that lack "notrace" and fix them. This
>will
>give us a solution for all existing boards and not for a small selection.
>Furthermore it will avoid code duplication.
I tried but There are so many functions which need "notrace".
Why don’t we consider removing gd->timer=NULL in initr_dm()
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
Or I can use TIMER_EARLY and return ticks and rate by adding
timer_early_get_count()
and timer_early_get_rate() repectively.
Suggestions are welcome
>
>In lib/trace.c consider replacing
>"__attribute__((no_instrument_function))" by "notrace".
>
>Best regards
>
>Heinrich
>
>>
>> 2) if gd->timer is already initialized then still initr_dm() will make
>> gd->timer = NULL;
>>
>> initr_dm()
>> {
>> #ifdef CONFIG_TIMER
>> gd->timer = NULL;
>> #endif
>> }
>>
>> So again dm_timer_init() gets called and tracing got stuck.
>>
>> So I thought better to implement timer_get_us().
>>
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
---
Changes in v3:
- Added gd->arch.plmt in global data
- For timer_get_us(), use readq() instead of andes_plmt_get_count()
and sifive_clint_get_count()
Changes in v2:
- Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
and andes_plmt_timer.c.
arch/riscv/include/asm/global_data.h | 3 +++
drivers/timer/andes_plmt_timer.c | 19 ++-
drivers/timer/riscv_timer.c | 14 +-
drivers/timer/sifive_clint_timer.c | 19 ++-
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/global_data.h
b/arch/riscv/include/asm/global_data.h
index d3a0b1d221..4e22ceb83f 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
void __iomem *plic; /* plic base address */
#endif
+#ifdef CONFIG_ANDES_PLMT
+ void __iomem *plmt; /* plmt base address */
+#endif
#if CONFIG_IS_ENABLED(SMP)
struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git
a/drivers/timer/andes_plmt_timer.c
b/drivers/timer/andes_plmt_timer.c
index cec86718c7..7c50c46d9e 100644
--- a/drivers/timer/andes_plmt_timer.c
+++ b/drivers/timer/andes_plmt_timer.c
@@ -13,11 +13,12 @@
#include
#include
#include
+#include
/* mtime register */
#define MTIME_REG(base) ((ulong)(base))
-static u64 andes_plmt_get_count(struct udevice *dev)
+static u64 notrace andes_plmt_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv)); } @@
-26,12
+27,28 @@ static const struct timer_ops
Re: i.MX USB SDP broken on imx6q-sabresd
Hello Fabio, Am 12.11.2020 um 03:11 schrieb Fabio Estevam: Hi, I am trying to load SPL and u-boot-dtb.img via USB using the imx_usb_loader tool. Running the latest mainline U-Boot on a imx6q-sabresd I get: $ sudo ./imx_usb SPL On the console: U-Boot SPL 2021.01-rc2-5-g832bfad7451e (Nov 11 2020 - 23:00:05 -0300) Trying to boot from USB SDP SDP: initialize... SDP: handle requests... Then I try to load u-boot-dtb.img and the following is seen on the host PC: $ sudo ./imx_usb u-boot-dtb.img config file vid=0x066f pid=0x3780 file_name=mx23_usb_work.conf vid=0x15a2 pid=0x004f file_name=mx28_usb_work.conf vid=0x15a2 pid=0x0052 file_name=mx50_usb_work.conf vid=0x15a2 pid=0x0054 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0061 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0063 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0071 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x007d file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0080 file_name=mx6_usb_work.conf vid=0x1fc9 pid=0x0128 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0076 file_name=mx7_usb_work.conf vid=0x1fc9 pid=0x0126 file_name=mx7ulp_usb_work.conf vid=0x15a2 pid=0x0041 file_name=mx51_usb_work.conf vid=0x15a2 pid=0x004e file_name=mx53_usb_work.conf vid=0x15a2 pid=0x006a file_name=vybrid_usb_work.conf vid=0x066f pid=0x37ff file_name=linux_gadget.conf vid=0x1b67 pid=0x4fff file_name=mx6_usb_sdp_spl.conf vid=0x0525 pid=0xb4a4 file_name=mx6_usb_sdp_spl.conf vid=0x1fc9 pid=0x012b file_name=mx8mq_usb_work.conf config file parse /usr/etc/imx-loader.d//mx6_usb_sdp_spl.conf Trying to open device vid=0x0525 pid=0xb4a4 Interface 0 claimed HAB security state: development mode (0x56787856) == work item filename u-boot-dtb.img load_size 0 bytes load_addr 0x dcd 1 clear_dcd 0 plug 1 jump_mode 3 jump_addr 0x == end work item header not found 8400:2e77d044, 4000 do_download failed, err=-22 HAB security state: development mode (0x56787856) And nothing else appears in the console. I tried to bisect, but the latest version I got this to run was v2019.01, which was not using the driver model and prevents bisection. Does anyone have any suggestions? Hmm... just tried cuurent mainline on an imx6ull based board, works fine for me, see log [1] ... Ok, I boot signed image: u-boot-ivt.img.signed (Board is not yet in mainline ... but no special change regarding USB, only dts and board code ...) bye, Heiko [1] boot SPL/U-Boot with imx_usb_loader $ tbot @argsk30rfdevboard-local interactive_uboot -f usbloader tbot starting ... ├─Flags: │ 'do_power', 'usbloader', 'local', 'threadripper-big-build' ├─Calling interactive_uboot ... │ ├─[lab3] sudo ifconfig eth0 down 192.168.3.1 up │ ├─[lab3] sudo ifconfig eth0 down 192.168.3.1 up │ ├─[lab3] kermit /home/pi/kermrc_k30rf-devboard │ ├─POWERON (k30rf-devboard) │ ├─[lab3] sispmctl -D 01:01:5c:29:39 -o 2 │ │## Accessing Gembird #0 USB device 005 │ │## Switched outlet 2 on │ ├─[lab3] sudo /home/pi/source/imx_usb_loader/imx_usb /srv/tftpboot/imx6ull_pat_defconfig/20201016/SPL.signed │ │## config file │ │## vid=0x066f pid=0x3780 file_name=mx23_usb_work.conf │ │## vid=0x15a2 pid=0x004f file_name=mx28_usb_work.conf │ │## vid=0x15a2 pid=0x0052 file_name=mx50_usb_work.conf │ │## vid=0x15a2 pid=0x0054 file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x0061 file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x0063 file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x0071 file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x007d file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x0080 file_name=mx6ull_usb_work.conf │ │## vid=0x1fc9 pid=0x0128 file_name=mx6_usb_work.conf │ │## vid=0x15a2 pid=0x0076 file_name=mx7_usb_work.conf │ │## vid=0x1fc9 pid=0x0126 file_name=mx7ulp_usb_work.conf │ │## vid=0x15a2 pid=0x0041 file_name=mx51_usb_work.conf │ │## vid=0x15a2 pid=0x004e file_name=mx53_usb_work.conf │ │## vid=0x15a2 pid=0x006a file_name=vybrid_usb_work.conf │ │## vid=0x066f pid=0x37ff file_name=linux_gadget.conf │ │## vid=0x1b67 pid=0x4fff file_name=mx6_usb_sdp_spl.conf │ │## vid=0x0525 pid=0xb4a4 file_name=mx6_usb_sdp_spl.conf │ │## vid=0x1fc9 pid=0x012b file_name=mx8mq_usb_work.conf │ │## vid=0x1fc9 pid=0x0134 file_name=mx8mm_usb_work.conf │ │## vid=0x1fc9 pid=0x013e file_name=mx8mn_usb_work.conf │ │## vid=0x3016 pid=0x1001 file_name=mx8m_usb_sdp_spl.conf │ │## config file │ │## parse /home/pi/source/imx_usb_loader//mx6ull_usb_work.conf │ │## Trying to open device vid=0x15a2 pid=0x0080 │ │## Interface 0 claimed │ │## HAB security state: development mode (0x56787856) │ │## == work item │ │## filename /srv/tftpboot/imx6ull_pat_defconfig/20201016/SPL.signed │ │## load_size 0 bytes │ │## load_addr 0x │ │## dcd 1 │ │## clear_dcd 0 │ │## plug 1 │ │## jump_mode 3 │ │## jump_addr
Re: [PATCH] riscv: Fix efi header for RV32
On Wed, Nov 11, 2020 at 04:25:53PM +0800, Leo Liang wrote: > Hi Atish and Heinrich, > > On Tue, Oct 13, 2020 at 12:23:31PM -0700, Atish Patra wrote: > > RV32 should use PE32 format instead of PE32+ as the efi header format. > > This requires following changes > > 1. A different header magic value > > 2. An additional parameter known as BaseOfData. Currently, it is set to > >zero in absence of any usage. > > > > Signed-off-by: Atish Patra > > Reviewed-by: Bin Meng > > Reviewed-by: Rick Chen > > --- > > arch/riscv/lib/crt0_riscv_efi.S | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/lib/crt0_riscv_efi.S > > b/arch/riscv/lib/crt0_riscv_efi.S > > index 87fe1e56f906..4aaa49ad0777 100644 > > --- a/arch/riscv/lib/crt0_riscv_efi.S > > +++ b/arch/riscv/lib/crt0_riscv_efi.S > > @@ -15,11 +15,13 @@ > > #define SAVE_LONG(reg, idx)sd reg, (idx*SIZE_LONG)(sp) > > #define LOAD_LONG(reg, idx)ld reg, (idx*SIZE_LONG)(sp) > > #define PE_MACHINE IMAGE_FILE_MACHINE_RISCV64 > > +#define PE_MAGICIMAGE_NT_OPTIONAL_HDR64_MAGIC > > #else > > #define SIZE_LONG 4 > > #define SAVE_LONG(reg, idx)sw reg, (idx*SIZE_LONG)(sp) > > #define LOAD_LONG(reg, idx)lw reg, (idx*SIZE_LONG)(sp) > > #define PE_MACHINE IMAGE_FILE_MACHINE_RISCV32 > > +#define PE_MAGICIMAGE_NT_OPTIONAL_HDR32_MAGIC > > #endif > > > > > > @@ -48,7 +50,7 @@ coff_header: > > IMAGE_FILE_LOCAL_SYMS_STRIPPED | \ > > IMAGE_FILE_DEBUG_STRIPPED) > > optional_header: > > - .short IMAGE_NT_OPTIONAL_HDR64_MAGIC /* PE32+ format */ > > + .short PE_MAGIC/* PE32+ format */ > > .byte 0x02/* MajorLinkerVersion */ > > .byte 0x14/* MinorLinkerVersion */ > > .long _edata - _start /* SizeOfCode */ > > @@ -56,6 +58,9 @@ optional_header: > > .long 0 /* SizeOfUninitializedData */ > > .long _start - ImageBase /* AddressOfEntryPoint */ > > .long _start - ImageBase /* BaseOfCode */ > > +#if __riscv_xlen == 32 > > + .long 0 /* BaseOfData */ > > +#endif > > > > extra_header_fields: > > .quad 0 /* ImageBase */ > > diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S > index 4aaa49ad07..f47c4a6d82 100644 > --- a/arch/riscv/lib/crt0_riscv_efi.S > +++ b/arch/riscv/lib/crt0_riscv_efi.S > @@ -63,7 +63,11 @@ optional_header: > #endif > > extra_header_fields: > +#if __riscv_xlen == 32 > + .long 0 /* ImageBase */ > +#else if > .quad 0 /* ImageBase */ > +#endif > .long 0x20/* SectionAlignment */ > .long 0x8 /* FileAlignment */ > .short 0 /* > MajorOperatingSystemVersion */ > @@ -83,10 +87,17 @@ extra_header_fields: > .long 0 /* CheckSum */ > .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ > .short 0 /* DllCharacteristics */ > +#if __riscv_xlen == 32 > + .long 0 /* SizeOfStackReserve */ > + .long 0 /* SizeOfStackCommit */ > + .long 0 /* SizeOfHeapReserve */ > + .long 0 /* SizeOfHeapCommit */ > +#else if > .quad 0 /* SizeOfStackReserve */ > .quad 0 /* SizeOfStackCommit */ > .quad 0 /* SizeOfHeapReserve */ > .quad 0 /* SizeOfHeapCommit */ > +#endif > .long 0 /* LoaderFlags */ > .long 0x6 /* NumberOfRvaAndSizes */ > > Based on Atish's patch, > would the above modification fit more closely to the specification that > Heinrich provided ? > > Best regards, > Leo Sending a new patch seems more reasonable. I will send a new patch later based on this patch. Best regards, Leo
i.MX USB SDP broken on imx6q-sabresd
Hi, I am trying to load SPL and u-boot-dtb.img via USB using the imx_usb_loader tool. Running the latest mainline U-Boot on a imx6q-sabresd I get: $ sudo ./imx_usb SPL On the console: U-Boot SPL 2021.01-rc2-5-g832bfad7451e (Nov 11 2020 - 23:00:05 -0300) Trying to boot from USB SDP SDP: initialize... SDP: handle requests... Then I try to load u-boot-dtb.img and the following is seen on the host PC: $ sudo ./imx_usb u-boot-dtb.img config file vid=0x066f pid=0x3780 file_name=mx23_usb_work.conf vid=0x15a2 pid=0x004f file_name=mx28_usb_work.conf vid=0x15a2 pid=0x0052 file_name=mx50_usb_work.conf vid=0x15a2 pid=0x0054 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0061 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0063 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0071 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x007d file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0080 file_name=mx6_usb_work.conf vid=0x1fc9 pid=0x0128 file_name=mx6_usb_work.conf vid=0x15a2 pid=0x0076 file_name=mx7_usb_work.conf vid=0x1fc9 pid=0x0126 file_name=mx7ulp_usb_work.conf vid=0x15a2 pid=0x0041 file_name=mx51_usb_work.conf vid=0x15a2 pid=0x004e file_name=mx53_usb_work.conf vid=0x15a2 pid=0x006a file_name=vybrid_usb_work.conf vid=0x066f pid=0x37ff file_name=linux_gadget.conf vid=0x1b67 pid=0x4fff file_name=mx6_usb_sdp_spl.conf vid=0x0525 pid=0xb4a4 file_name=mx6_usb_sdp_spl.conf vid=0x1fc9 pid=0x012b file_name=mx8mq_usb_work.conf config file parse /usr/etc/imx-loader.d//mx6_usb_sdp_spl.conf Trying to open device vid=0x0525 pid=0xb4a4 Interface 0 claimed HAB security state: development mode (0x56787856) == work item filename u-boot-dtb.img load_size 0 bytes load_addr 0x dcd 1 clear_dcd 0 plug 1 jump_mode 3 jump_addr 0x == end work item header not found 8400:2e77d044, 4000 do_download failed, err=-22 HAB security state: development mode (0x56787856) And nothing else appears in the console. I tried to bisect, but the latest version I got this to run was v2019.01, which was not using the driver model and prevents bisection. Does anyone have any suggestions? Thanks, Fabio Estevam
Re: [PATCH] env: mmc: Correct partition comparison in mmc_offset_try_partition
Hi, On 11/11/20 7:25 PM, 김호연지기 wrote: > On Wed, Nov 11, 2020 at 7:07 PM Jorge Ramirez-Ortiz, Gmail > wrote: >> >> On 11/11/20, Jaehoon Chung wrote: >>> On 11/10/20 11:28 PM, Hoyeonjiki Kim wrote: The function mmc_offset_try_partition searches MMC partition to save the environment data by name. However, it only compares the first word-size bytes (size of 'const char *'), which may make the function to find unintended partition. Correct the function not to partially compare the partition name with config "u-boot,,mmc-env-partition". Signed-off-by: Hoyeonjiki Kim --- env/mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/mmc.c b/env/mmc.c index 4e67180b23..505f7aa2b8 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -42,7 +42,7 @@ static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val) if (ret < 0) return ret; - if (!strncmp((const char *)info.name, str, sizeof(str))) + if (!strcmp((const char *)info.name, str)) >>> >>> Using "strlen(str)" is better than changing to strcmp. >>> >>> strncmp(..., ..., strlen(str)) >> >> absolutely. >> maybe also modify the commit like to indicate a fix to the current bug? > > Thanks for the feedback for both. > > However, when we do so, isn't there still the possibility for the > function searching incorrect partition if, > 1) "str" is shorter than "info.name", and > 2) the first "strlen(str)" letters of "info.name" is same with those of "str"? Make sense. You're right. :) There are two approach. One is that choose one of those length what is longer. Other is your approach. I don't have any objection to fix whatever. Just resend your patch with Jorge's comment, plz. Best Regards, Jaehoon Chung > > This commit is for fixing the current bug, but also I wanna make sure > that partition name matches fully. > > Let me know your opinion. > > Best Regards, > Hoeyonjiki Kim > >> >>> >>> >>> Best Regards, >>> Jaehoon Chung >>> break; } >>> >
[PATCH v2 1/4] sandbox: add handler for exceptions
Add a handler for SIGILL, SIGBUS, SIGSEGV.
When an exception occurs print the program counter and the loaded
UEFI binaries and reset the system if CONFIG_SANDBOX_CRASH_RESET=y
or exit to the OS otherwise.
Signed-off-by: Heinrich Schuchardt
---
v2:
add a customizing switch
set SA_NODEFER flag for sigaction
---
arch/sandbox/Kconfig | 9
arch/sandbox/cpu/os.c | 40 +++
arch/sandbox/cpu/start.c | 4
arch/sandbox/lib/interrupts.c | 35 ++
include/os.h | 17 +++
5 files changed, 105 insertions(+)
diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index 65f988e736..f83282d9d5 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -51,6 +51,15 @@ config HOST_64BIT
endchoice
+config SANDBOX_CRASH_RESET
+ bool "Reset on crash"
+ help
+ If an illegal instruction or an illegal memory access occurs, the
+ sandbox by default writes a crash dump and exits. If you set this
+ flag, the sandbox is reset instead. This may be useful when running
+ test suites like the UEFI self certification test which continue
+ with the next test after a crash.
+
config SANDBOX_BITS_PER_LONG
int
default 32 if HOST_32BIT
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 0d8efd83f6..b56fa04a34 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -3,6 +3,8 @@
* Copyright (c) 2011 The Chromium OS Authors.
*/
+#define _GNU_SOURCE
+
#include
#include
#include
@@ -15,11 +17,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
@@ -191,6 +195,42 @@ static void os_sigint_handler(int sig)
raise(SIGINT);
}
+static void os_signal_handler(int sig, siginfo_t *info, void *con)
+{
+ ucontext_t __maybe_unused *context = con;
+ unsigned long pc;
+
+#if defined(__x86_64__)
+ pc = context->uc_mcontext.gregs[REG_RIP];
+#elif defined(__aarch64__)
+ pc = context->uc_mcontext.pc;
+#elif defined(__riscv)
+ pc = context->uc_mcontext.__gregs[REG_PC];
+#else
+ const char msg[] =
+ "\nUnsupported architecture, cannot read program counter\n";
+
+ os_write(1, msg, sizeof(msg));
+ pc = 0;
+#endif
+
+ os_signal_action(sig, pc);
+}
+
+int os_setup_signal_handlers(void)
+{
+ struct sigaction act;
+
+ act.sa_sigaction = os_signal_handler;
+ sigemptyset(_mask);
+ act.sa_flags = SA_SIGINFO | SA_NODEFER;
+ if (sigaction(SIGILL, , NULL) ||
+ sigaction(SIGBUS, , NULL) ||
+ sigaction(SIGSEGV, , NULL))
+ return -1;
+ return 0;
+}
+
/* Put tty into raw mode so and work */
void os_tty_raw(int fd, bool allow_sigs)
{
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index a03e5aa0b3..f6c98545e0 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -451,6 +451,10 @@ int main(int argc, char *argv[])
if (ret)
goto err;
+ ret = os_setup_signal_handlers();
+ if (ret)
+ goto err;
+
#if CONFIG_VAL(SYS_MALLOC_F_LEN)
gd->malloc_base = CONFIG_MALLOC_F_ADDR;
#endif
diff --git a/arch/sandbox/lib/interrupts.c b/arch/sandbox/lib/interrupts.c
index 21f761ac3b..9c2c60b8c6 100644
--- a/arch/sandbox/lib/interrupts.c
+++ b/arch/sandbox/lib/interrupts.c
@@ -6,7 +6,13 @@
*/
#include
+#include
#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
int interrupt_init(void)
{
@@ -21,3 +27,32 @@ int disable_interrupts(void)
{
return 0;
}
+
+void os_signal_action(int sig, unsigned long pc)
+{
+ efi_restore_gd();
+
+ switch (sig) {
+ case SIGILL:
+ printf("\nIllegal instruction\n");
+ break;
+ case SIGBUS:
+ printf("\nBus error\n");
+ break;
+ case SIGSEGV:
+ printf("\nSegmentation violation\n");
+ break;
+ default:
+ break;
+ }
+ printf("pc = 0x%lx, ", pc);
+ printf("pc_reloc = 0x%lx\n\n", pc - gd->reloc_off);
+ efi_print_image_infos((void *)pc);
+
+ if (IS_ENABLED(CONFIG_SANDBOX_CRASH_RESET)) {
+ printf("resetting ...\n\n");
+ sandbox_reset();
+ } else {
+ sandbox_exit();
+ }
+}
diff --git a/include/os.h b/include/os.h
index 1fe44f3510..0913b47b3a 100644
--- a/include/os.h
+++ b/include/os.h
@@ -407,4 +407,21 @@ void *os_find_text_base(void);
*/
void os_relaunch(char *argv[]);
+/**
+ * os_setup_signal_handlers() - setup signal handlers
+ *
+ * Install signal handlers for SIGBUS and SIGSEGV.
+ *
+ * Return: 0 for success
+ */
+int os_setup_signal_handlers(void);
+
+/**
+ * os_signal_action() - handle a signal
+ *
+ * @sig: signal
+ * @pc:
[PATCH v2 2/4] cmd: sandbox: implement exception command
Implement the commands
* exception undefined - execute an illegal instruction
* exception sigsegv - cause a segment violation
Here is a possible output:
=> exception undefined
Illegal instruction
pc = 0x55eb8d0a7575, pc_reloc = 0x57575
Resetting ...
Signed-off-by: Heinrich Schuchardt
Reviewed-by: Simon Glass
---
v2:
no change
---
cmd/Kconfig | 2 +-
cmd/Makefile| 1 +
cmd/sandbox/Makefile| 3 +++
cmd/sandbox/exception.c | 41 +
4 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 cmd/sandbox/Makefile
create mode 100644 cmd/sandbox/exception.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1595de999b..f9b72449c5 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1682,7 +1682,7 @@ config CMD_EFIDEBUG
config CMD_EXCEPTION
bool "exception - raise exception"
- depends on ARM || RISCV || X86
+ depends on ARM || RISCV || SANDBOX || X86
help
Enable the 'exception' command which allows to raise an exception.
diff --git a/cmd/Makefile b/cmd/Makefile
index dd86675bf2..5b3400a840 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -193,6 +193,7 @@ obj-$(CONFIG_CMD_AVB) += avb.o
obj-$(CONFIG_ARM) += arm/
obj-$(CONFIG_RISCV) += riscv/
+obj-$(CONFIG_SANDBOX) += sandbox/
obj-$(CONFIG_X86) += x86/
obj-$(CONFIG_ARCH_MVEBU) += mvebu/
diff --git a/cmd/sandbox/Makefile b/cmd/sandbox/Makefile
new file mode 100644
index 00..24df023ece
--- /dev/null
+++ b/cmd/sandbox/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-$(CONFIG_CMD_EXCEPTION) += exception.o
diff --git a/cmd/sandbox/exception.c b/cmd/sandbox/exception.c
new file mode 100644
index 00..1aa1d673ae
--- /dev/null
+++ b/cmd/sandbox/exception.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2020, Heinrich Schuchardt
+ */
+
+#include
+#include
+
+static int do_sigsegv(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ u8 *ptr = NULL;
+
+ *ptr = 0;
+ return CMD_RET_FAILURE;
+}
+
+static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ asm volatile (".word 0x\n");
+ return CMD_RET_FAILURE;
+}
+
+static struct cmd_tbl cmd_sub[] = {
+ U_BOOT_CMD_MKENT(sigsegv, CONFIG_SYS_MAXARGS, 1, do_sigsegv,
+"", ""),
+ U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
+"", ""),
+};
+
+static char exception_help_text[] =
+ "\n"
+ " The following exceptions are available:\n"
+ " undefined - undefined instruction\n"
+ " sigsegv- illegal memory access\n"
+ ;
+
+#include
--
2.28.0
[PATCH v2 3/4] efi_selftest: implement exception test for sandbox
Provide a unit test that causes an illegal instruction to occur.
The test can be run with the following commands:
=> setenv efi_selftest exception
=> bootefi selftest
This might be the output:
Executing 'exception'
EFI application triggers exception.
Illegal instruction
pc = 0x1444d016, pc_reloc = 0xaa078e8dd016
UEFI image [0x:0x] '/\selftest'
UEFI image [0x1444b000:0x14451fff] pc=0x2016 '/bug.efi'
Resetting ...
It would tell us that the exception was triggered by an instruction
0x2016 bytes after the load address of the binary with filename /bug.efi.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_selftest/efi_selftest_miniapp_exception.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_miniapp_exception.c
b/lib/efi_selftest/efi_selftest_miniapp_exception.c
index 63c63d75f1..59b7e5100a 100644
--- a/lib/efi_selftest/efi_selftest_miniapp_exception.c
+++ b/lib/efi_selftest/efi_selftest_miniapp_exception.c
@@ -33,6 +33,8 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
asm volatile (".word 0xe7f7defb\n");
#elif defined(CONFIG_RISCV)
asm volatile (".word 0x\n");
+#elif defined(CONFIG_SANDBOX)
+ asm volatile (".word 0x\n");
#elif defined(CONFIG_X86)
asm volatile (".word 0x\n");
#endif
--
2.28.0
[PATCH v2 4/4] test: unit test for exception command
Test that an exception SIGILL is answered by a reset on the sandbox if
CONFIG_SANDBOX_CRASH_RESET=y or by exiting to the OS otherwise.
Signed-off-by: Heinrich Schuchardt
---
v2:
new patch
---
arch/Kconfig | 1 +
test/py/tests/test_sandbox_exit.py | 24
2 files changed, 25 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 3aa99e08fc..b936d2dffc 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -112,6 +112,7 @@ config SANDBOX
imply BITREVERSE
select BLOBLIST
imply CMD_DM
+ imply CMD_EXCEPTION
imply CMD_GETTIME
imply CMD_HASH
imply CMD_IO
diff --git a/test/py/tests/test_sandbox_exit.py
b/test/py/tests/test_sandbox_exit.py
index 2d242ae0f6..706f5fa359 100644
--- a/test/py/tests/test_sandbox_exit.py
+++ b/test/py/tests/test_sandbox_exit.py
@@ -19,3 +19,27 @@ def test_ctrl_c(u_boot_console):
u_boot_console.kill(signal.SIGINT)
assert(u_boot_console.validate_exited())
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_exception')
+@pytest.mark.buildconfigspec('sandbox_crash_reset')
+def test_exception_reset(u_boot_console):
+"""Test that SIGILL causes a reset."""
+
+u_boot_console.run_command('exception undefined', wait_for_prompt=False)
+m = u_boot_console.p.expect(['resetting ...', 'U-Boot'])
+if m != 0:
+raise Exception('SIGILL did not lead to reset')
+m = u_boot_console.p.expect(['U-Boot', '=>'])
+if m != 0:
+raise Exception('SIGILL did not lead to reset')
+u_boot_console.restart_uboot()
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_exception')
+@pytest.mark.notbuildconfigspec('sandbox_crash_reset')
+def test_exception_exit(u_boot_console):
+"""Test that SIGILL causes a reset."""
+
+u_boot_console.run_command('exception undefined', wait_for_prompt=False)
+assert(u_boot_console.validate_exited())
--
2.28.0
[PATCH v2 0/4] sandbox: exception handling
Currently if an exception SIGILL, SIGBUS, SIGSEGV occurs the sandbox stops execution. This does not match the behavior on other architectures. Instead print the current program counter and if any the involved UEFI binaries. Then depending on a customizing system either reset the system or exit to the OS. When testing UEFI binaries like the Self Certification Test exceptions may occur. Without information about the UEFI binary where the exception was caused it is difficult to analyze the cause. The exception command is implemented for the sandbox. This command allows to trigger SIGILL or SIGSEGV. v2: add a customizing switch set SA_NODEFER flag for sigaction provide a unit test for the exception command Heinrich Schuchardt (4): sandbox: add handler for exceptions cmd: sandbox: implement exception command efi_selftest: implement exception test for sandbox test: unit test for exception command arch/Kconfig | 1 + arch/sandbox/Kconfig | 9 arch/sandbox/cpu/os.c | 40 ++ arch/sandbox/cpu/start.c | 4 ++ arch/sandbox/lib/interrupts.c | 35 cmd/Kconfig | 2 +- cmd/Makefile | 1 + cmd/sandbox/Makefile | 3 ++ cmd/sandbox/exception.c | 41 +++ include/os.h | 17 .../efi_selftest_miniapp_exception.c | 2 + test/py/tests/test_sandbox_exit.py| 24 +++ 12 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 cmd/sandbox/Makefile create mode 100644 cmd/sandbox/exception.c -- 2.28.0
Re: [PATCH 1/3] sandbox: add handler for exceptions
Hi Heinrich, On Wed, 11 Nov 2020 at 13:47, Heinrich Schuchardt wrote: > > On 11.11.20 15:32, Simon Glass wrote: > > Hi Heinrich, > > > > On Tue, 10 Nov 2020 at 16:09, Heinrich Schuchardt > > wrote: > >> > >> Add a handler for SIGILL, SIGBUS, SIGSEGV. > >> > >> When an exception occurs print the program counter and the loaded > >> UEFI binaries and reset the system. > >> > >> Signed-off-by: Heinrich Schuchardt > >> --- > >> arch/sandbox/cpu/os.c | 31 +++ > >> arch/sandbox/cpu/start.c | 4 > >> arch/sandbox/lib/interrupts.c | 30 ++ > >> include/os.h | 17 + > >> 4 files changed, 82 insertions(+) > > > > Generally we want to stop execution, because it indicates a bug. Then > > we might want to run it again with gdb to debug it. > > > > So I think this feature should be enabled by a flag. > > > > Regards, > > Simon > > > > I understand that if I call os_launch should be customizable. > > Providing the output should be helpful in any case. I'm not sure what you mean here, but what I am saying is that by default sandbox should crash. We don't really want the caller to have to kill it if a test fails, etc. Regards, Simon
Re: [PATCH 1/3] sandbox: add handler for exceptions
On 11.11.20 15:32, Simon Glass wrote: > Hi Heinrich, > > On Tue, 10 Nov 2020 at 16:09, Heinrich Schuchardt wrote: >> >> Add a handler for SIGILL, SIGBUS, SIGSEGV. >> >> When an exception occurs print the program counter and the loaded >> UEFI binaries and reset the system. >> >> Signed-off-by: Heinrich Schuchardt >> --- >> arch/sandbox/cpu/os.c | 31 +++ >> arch/sandbox/cpu/start.c | 4 >> arch/sandbox/lib/interrupts.c | 30 ++ >> include/os.h | 17 + >> 4 files changed, 82 insertions(+) > > Generally we want to stop execution, because it indicates a bug. Then > we might want to run it again with gdb to debug it. > > So I think this feature should be enabled by a flag. > > Regards, > Simon > I understand that if I call os_launch should be customizable. Providing the output should be helpful in any case. Best regards Heinrich
[PATCH] ARM: mach-at91: fix timer.o compile condition
The AT91 architecture now has two possible timer blocks, the old PIT timer and the new PIT64B. The timer.c file has an old non DM driver that works for platforms that do not use the ATMEL_PIT_TIMER DM-based driver. Update the Makefile to select this old driver in case neither of the ATMEL_PIT_TIMER and the MCHP_PIT64B_TIMER are selected. Suggested-by: Claudiu Beznea Signed-off-by: Eugen Hristev --- arch/arm/mach-at91/armv7/Makefile | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-at91/armv7/Makefile b/arch/arm/mach-at91/armv7/Makefile index b46b90b28e..f5b2665957 100644 --- a/arch/arm/mach-at91/armv7/Makefile +++ b/arch/arm/mach-at91/armv7/Makefile @@ -12,6 +12,9 @@ obj-$(CONFIG_SAMA5D4) += sama5d4_devices.o clock.o obj-$(CONFIG_SAMA7G5) += sama7g5_devices.o obj-y += cpu.o obj-y += reset.o -ifeq ($(CONFIG_ATMEL_PIT_TIMER),) +ifneq ($(CONFIG_ATMEL_PIT_TIMER),y) +ifneq ($(CONFIG_MCHP_PIT64B_TIMER),y) +# old non-DM timer driver obj-y += timer.o endif +endif -- 2.25.1
[PATCH] ARM: at91: armv7: sama7g5 uses CCF clock driver
From: Nicolas Ferre
SAMA7G5 uses CCF driver under drivers/clk/at91/ and not the custom older
at91 clock.c driver. Remove it from the compilation list and adapt cpu.c
arch_cpu_init() to avoid calling at91_clock_init() which is wrong
anyway.
Signed-off-by: Nicolas Ferre
Reviewed-by: Claudiu Beznea
---
arch/arm/mach-at91/armv7/Makefile | 7 +++
arch/arm/mach-at91/armv7/cpu.c| 4
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-at91/armv7/Makefile
b/arch/arm/mach-at91/armv7/Makefile
index 55477560a8..b46b90b28e 100644
--- a/arch/arm/mach-at91/armv7/Makefile
+++ b/arch/arm/mach-at91/armv7/Makefile
@@ -6,11 +6,10 @@
# (C) Copyright 2013
# Bo Shen
-obj-$(CONFIG_SAMA5D2) += sama5d2_devices.o
-obj-$(CONFIG_SAMA5D3) += sama5d3_devices.o
-obj-$(CONFIG_SAMA5D4) += sama5d4_devices.o
+obj-$(CONFIG_SAMA5D2) += sama5d2_devices.o clock.o
+obj-$(CONFIG_SAMA5D3) += sama5d3_devices.o clock.o
+obj-$(CONFIG_SAMA5D4) += sama5d4_devices.o clock.o
obj-$(CONFIG_SAMA7G5) += sama7g5_devices.o
-obj-y += clock.o
obj-y += cpu.o
obj-y += reset.o
ifeq ($(CONFIG_ATMEL_PIT_TIMER),)
diff --git a/arch/arm/mach-at91/armv7/cpu.c b/arch/arm/mach-at91/armv7/cpu.c
index 8b7355042b..9b3753491e 100644
--- a/arch/arm/mach-at91/armv7/cpu.c
+++ b/arch/arm/mach-at91/armv7/cpu.c
@@ -24,7 +24,11 @@
int arch_cpu_init(void)
{
+#if defined(CONFIG_CLK_CCF)
+ return 0;
+#else
return at91_clock_init(CONFIG_SYS_AT91_MAIN_CLOCK);
+#endif
}
void arch_preboot_os(void)
--
2.25.1
Re: Re-syncing U-Boot DTS with the kernel again
On 23.10.2020 19:05, Tom Rini wrote: Hello Tom, The at91/atmel related devicetrees should be fixed now in 2021.01-rc2. Do you plan to update the DTC for 2021.01 final or for a future release ? Thanks, Eugen
Re: [PATCH] libfdt: Fix signedness comparison warnings
On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote: > This is a combination of upstream libfdt commits to fix warnings about > comparing signed and unsigned integers: > == > scripts/dtc/libfdt/fdt.c: In function ‘fdt_offset_ptr’: > scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and > unsigned integer expressions [-Wsign-compare] >if ((absoffset < offset) > ... > == > > For a detailed description of the fixes, see the dtc repo: > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808 > > For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000 > have been combined and adjusted for the slight differences in U-Boot's > libfdt code base. > > Signed-off-by: Andre Przywara So, I've applied this to u-boot/master now. These warnings do show up with gcc-10 and it's worthwhile to silence them. I'm working with upstream dtc now so that when we resync next we'll be able to avoid the size and performance penalties of making all fdt loads unaligned safe. A further resync will also require us to fixup a number of dts warnings again. These are the main reasons that I'm setting aside my suggestion of a full resync for now. Thanks! -- Tom signature.asc Description: PGP signature
Re: Re-syncing U-Boot DTS with the kernel again
On Wed, Nov 11, 2020 at 11:07:11AM +, eugen.hris...@microchip.com wrote: > On 23.10.2020 19:05, Tom Rini wrote: > > Hello Tom, > > The at91/atmel related devicetrees should be fixed now in 2021.01-rc2. > > Do you plan to update the DTC for 2021.01 final or for a future release ? I think the safest plan is to take the patch that fixes gcc-10 warnings now (and only has minor actual code changes) and resync dtc again post v2021.01. I'm quite happy to see the dts resyncs that have happened, but there's a few more that need to happen before we can move to a newer version, without disabling warnings at least. And I think I need to dig out / post the sandbox ones. -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2 v4] efi: Add basic EFI_TCG2_PROTOCOL support
Hi Simon, On Wed, Nov 11, 2020 at 07:42:31AM -0700, Simon Glass wrote: > Hi Ilias, > > On Wed, 11 Nov 2020 at 02:18, Ilias Apalodimas > wrote: > > > > Since U-boot EFI implementation is getting richer it makes sense to > > add support for EFI_TCG2_PROTOCOL taking advantage of any hardware TPM > > available on the device. > > > > This is the initial implementation of the protocol which only adds > > support for GetCapability(). It's limited in the newer and safer > > TPMv2 devices. > > > > Signed-off-by: Ilias Apalodimas > > --- > > * changes since v3: > > - added check for maximum number of PCRs allowed > > - replaced multiple return Xl with goto out tags > > * changes since v2: > > - added description about include/efi_tcg2.h > > - switch bool to u8 for tpm_present_flag > > - removed superfluous 'default n' from Kconfig > > - use 'goto 'tag' when possible > > > > * changes since v1: > > - change return variable of platform_get_tpm2_device() when used > > - since more headers were included in patch #2 use them in offset > > calculations for all tpm commands > > - change the size of the response buffer regardless of what > > tpm2_get_capability() is doing > > include/efi_loader.h | 2 + > > include/efi_tcg2.h | 94 +++ > > lib/efi_loader/Kconfig | 7 + > > lib/efi_loader/Makefile| 1 + > > lib/efi_loader/efi_setup.c | 7 + > > lib/efi_loader/efi_tcg2.c | 539 + > > 6 files changed, 650 insertions(+) > > create mode 100644 include/efi_tcg2.h > > create mode 100644 lib/efi_loader/efi_tcg2.c > > I will let Heinrich review this one. I do feel that the overly long > identifiers make the code hard to read. I completely agree. The reason I kept them that long, is that the TCG specs are quite confusing to follow, so I tried to adhere to the naming as much as possible. Regards /Ilias > > Regards, > Simon
Re: [PATCH] tpm: spi: Cleanup source code
On Mon, 9 Nov 2020 at 07:46, Michal Simek wrote: > > There is no need for GD to be used and priv variable is unused. > > Signed-off-by: Michal Simek > --- > > drivers/tpm/tpm2_tis_spi.c | 3 --- > 1 file changed, 3 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 2/2 v4] efi: Add basic EFI_TCG2_PROTOCOL support
Hi Ilias, On Wed, 11 Nov 2020 at 02:18, Ilias Apalodimas wrote: > > Since U-boot EFI implementation is getting richer it makes sense to > add support for EFI_TCG2_PROTOCOL taking advantage of any hardware TPM > available on the device. > > This is the initial implementation of the protocol which only adds > support for GetCapability(). It's limited in the newer and safer > TPMv2 devices. > > Signed-off-by: Ilias Apalodimas > --- > * changes since v3: > - added check for maximum number of PCRs allowed > - replaced multiple return Xl with goto out tags > * changes since v2: > - added description about include/efi_tcg2.h > - switch bool to u8 for tpm_present_flag > - removed superfluous 'default n' from Kconfig > - use 'goto 'tag' when possible > > * changes since v1: > - change return variable of platform_get_tpm2_device() when used > - since more headers were included in patch #2 use them in offset > calculations for all tpm commands > - change the size of the response buffer regardless of what > tpm2_get_capability() is doing > include/efi_loader.h | 2 + > include/efi_tcg2.h | 94 +++ > lib/efi_loader/Kconfig | 7 + > lib/efi_loader/Makefile| 1 + > lib/efi_loader/efi_setup.c | 7 + > lib/efi_loader/efi_tcg2.c | 539 + > 6 files changed, 650 insertions(+) > create mode 100644 include/efi_tcg2.h > create mode 100644 lib/efi_loader/efi_tcg2.c I will let Heinrich review this one. I do feel that the overly long identifiers make the code hard to read. Regards, Simon
Re: [PATCH 1/2 v4] tpm: Add some headers from the spec
On Wed, 11 Nov 2020 at 02:18, Ilias Apalodimas wrote: > > A following patch introduces EFI_TCG2_PROTOCOL. > Add the required TPMv2 headers to support it. > > Signed-off-by: Ilias Apalodimas > Reviewed-by: Heinrich Schuchardt > --- > * changes since v3: > - Add TPM2 prefix on PT_GROUP, PT_FIXED > * changes since v2: > - Added description and pointers to TCG specs > - updated copyright info > include/tpm-v2.h | 77 > 1 file changed, 77 insertions(+) Reviewed-by: Simon Glass
[Patch] fdt: Use phandle to distinguish DT nodes with same name
While assigning the sequence number to subsystem instances by reading the
aliases property, only DT nodes names are compared and not the complete
path. This causes a problem when there are two DT nodes with same name but
have different paths.
Fix it by comparing the phandles of DT nodes after the node names match.
Signed-off-by: Aswath Govindraju
---
lib/fdtdec.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 2015907dee7d..9e1bfe0b519e 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -478,6 +478,11 @@ int fdtdec_get_alias_seq(const void *blob, const char
*base, int offset,
slash = strrchr(prop, '/');
if (strcmp(slash + 1, find_name))
continue;
+
+ if (fdt_get_phandle(blob, offset) !=
+ fdt_get_phandle(blob, fdt_path_offset(blob, prop)))
+ continue;
+
val = trailing_strtol(name);
if (val != -1) {
*seqp = val;
--
2.17.1
Re: [PATCH 2/3] log: use debug uart to output trace before LOG init
+Heinrich Schuchardt
On Fri, 6 Nov 2020 at 10:55, Patrick Delaunay wrote:
>
> Use the debug uart functions to output the traces before
> the log initialization (when CONFIG_LOG is not activated)
> as it is done in puts/putc function in console.c.
>
> This patch allows to display the first U-Boot traces
> (with macro debug) when CONFIG_DEBUG_UART is activated
> and not only drop them.
>
> For example for traces in board_f.c requested by the macro debug,
> when LOG_DEBUG is defined and CONFIG_LOG is activated.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> common/log.c | 11 +++
> 1 file changed, 11 insertions(+)
Reviewed-by: Simon Glass
Again this needs a sandbox test
>
> diff --git a/common/log.c b/common/log.c
> index aadf533fb2..aa5505943f 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -245,6 +246,16 @@ int _log(enum log_category_t cat, enum log_level_t
> level, const char *file,
>
> if (!(gd->flags & GD_FLG_LOG_READY)) {
> gd->log_drop_count++;
> +
> + /* manage droppped trace at default level with debug uart */
dropped
> + if (IS_ENABLED(CONFIG_DEBUG_UART) &&
> + (rec.level <= CONFIG_LOG_DEFAULT_LEVEL ||
> rec.force_debug)) {
> + va_start(args, fmt);
> + vsnprintf(buf, sizeof(buf), fmt, args);
> + printascii(buf);
> + va_end(args);
> + }
> +
> return -ENOSYS;
> }
> va_start(args, fmt);
> --
> 2.17.1
>
Re: [PATCH 1/3 v2] tpm: Change response length of tpm2_get_capability()
On Thu, 5 Nov 2020 at 14:58, Ilias Apalodimas wrote: > > For implementing the EFI_TCG2_PROTOCOL we need the count field returned by > the TPM when reading capabilities via tpm2_get_capability(). > > Adjust the implementation of the 'tpm2 get_capability' command accordingly. > > Suggested-by: Heinrich Schuchardt > Signed-off-by: Ilias Apalodimas > --- > Changes since v1: > - Unconditionally get the extra 4 bytes on the response and account for them > in do_tpm_get_capability() > cmd/tpm-v2.c | 4 ++-- > lib/tpm-v2.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/1] test: test/lib/test_print.c depends on CONSOLE_RECORD
Hi Heinrich, On Wed, 4 Nov 2020 at 17:29, Heinrich Schuchardt wrote: > > The tests in test/lib/test_print.c fail without CONFIG_CONSOLE_RECORD=y. > > Add a build dependency. > > Signed-off-by: Heinrich Schuchardt > --- > test/lib/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass But this is a strange thing to depend on. I wonder if we should add a Kconfig for this test? I haven't done it so far as I was worried about a proliferation of Kconfigs. > > diff --git a/test/lib/Makefile b/test/lib/Makefile > index 98a9abf40e..97c11e35a8 100644 > --- a/test/lib/Makefile > +++ b/test/lib/Makefile > @@ -7,7 +7,7 @@ obj-$(CONFIG_EFI_LOADER) += efi_device_path.o > obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o > obj-y += hexdump.o > obj-y += lmb.o > -obj-y += test_print.o > +obj-$(CONFIG_CONSOLE_RECORD) += test_print.o > obj-$(CONFIG_SSCANF) += sscanf.o > obj-y += string.o > obj-$(CONFIG_ERRNO_STR) += test_errno_str.o > -- > 2.28.0 >
Re: [PATCH 1/3] log: don't build the trace buffer when log is not ready
Hi Patrick, On Fri, 6 Nov 2020 at 10:55, Patrick Delaunay wrote: > > Update _log function to drop any traces when log is yet initialized: > vsnprintf is no more executed in this case. > > This patch allows to reduce the cost for the dropped early debug trace. > > Signed-off-by: Patrick Delaunay > --- > > common/log.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass But this needs a test.
Re: [PATCH 1/2 v3] tpm: Add some headers from the spec
On Tue, 10 Nov 2020 at 00:49, Ilias Apalodimas wrote: > > A following patch introduces EFI_TCG2_PROTOCOL. > Add the required TPMv2 headers to support it. > > Signed-off-by: Ilias Apalodimas > --- > changes since v2: > - Added description and pointers to TCG specs > - updated copyright info > include/tpm-v2.h | 77 > 1 file changed, 77 insertions(+) Reviewed-by: Simon Glass (Heinrich has already requested the TPM2 prefix be used everywhere)
Re: [PATCH 3/3] log: call vsnprintf only when it is needed to emit trace
On Fri, 6 Nov 2020 at 10:55, Patrick Delaunay wrote: > > Reduce the log overhead when the traces are filtered, > by moving the vsnprintf call from _log() to log_dispatch(). > > This patch avoids the printf treatment when LOG features is > activated, but trace is filtered, for example when > MAX_LOG_LEVEL=8 and LOG_DEFAULT_LEVEL=6. > > Signed-off-by: Patrick Delaunay > > # Conflicts: > # common/log.c > --- > > common/log.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH] cros_ec: Support keyboard scanning with EC_CMD_GET_NEXT_EVENT
Hi Alper,
On Mon, 9 Nov 2020 at 17:55, Alper Nebi Yasak wrote:
>
> On 09/11/2020 22:37, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 9 Nov 2020 at 12:34, Heinrich Schuchardt wrote:
> >>
> >> On 10/30/20 6:25 PM, Alper Nebi Yasak wrote:
> >>> The cros_ec_keyb driver currently uses EC_CMD_MKBP_STATE to scan the
> >>> keyboard, but this host command was superseded by EC_CMD_GET_NEXT_EVENT
> >>
> >> This patch has been applied to origin/master.
> >>
> >> Now when I compile sandbox_defconfig and run './u-boot -D' it spits out
> >> zillions of
> >>
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>** Unknown EC command 0x67
> >>
> >> When I revert the patch the messages are gone.
> >>
> >> So something is really missing in this patch.
> >
> > Test coverage, for a start!
> >
> > I think the EC emulator needs to be updated for the new command.
> >
> > Alper, can you please take a look?
> >
>
> I can get the messages to stop with the following, does it look good to
> you?
>
> case EC_CMD_ENTERING_MODE:
> len = 0;
> break;
> + case EC_CMD_GET_NEXT_EVENT: {
> + struct ec_response_get_next_event *resp = resp_data;
> + resp->event_type = EC_MKBP_EVENT_KEY_MATRIX;
> + cros_ec_keyscan(ec, resp->data.key_matrix);
> + len = sizeof(*resp);
> + break;
> + }
> default:
> printf(" ** Unknown EC command %#02x\n", req_hdr->command);
> return -1;
>
> That's more or less only what the cros-ec-keyb counterpart expects. But
> it doesn't test the -EC_RES_UNAVAILABLE thing or the fallback to the old
> method.
Yes that looks good. We don't need to test the fallback for now as
this is just an emulator anyway.
Regards,
Simon
Re: [PATCH 1/1] cros_ec: Handling EC_CMD_GET_NEXT_EVENT
Hi,
On Mon, 9 Nov 2020 at 14:25, Heinrich Schuchardt wrote:
>
> On 11/9/20 10:13 PM, Alper Nebi Yasak wrote:
> > On 09/11/2020 23:34, Heinrich Schuchardt wrote:
> >> With commit 690079767803 ("cros_ec: Support keyboard scanning with
> >> EC_CMD_GET_NEXT_EVENT") check_for_keys() tries to read keyboard
> >> strokes using EC_CMD_GET_NEXT_EVENT. But the sandbox driver does
> >> not understand this command. We need to reply with
> >> -EC_RES_INVALID_COMMAND to force check_for_keys() to fall back to
> >> use EC_CMD_MKBP_STATE. Currently the driver prints
> >>
> >> ** Unknown EC command 0x67
> >>
> >> in this case. With the patch the message is suppressed.
> >>
> >> In a future patch we should upgrade the sandbox driver to provide
> >> EC_CMD_GET_NEXT_EVENT support.
> >>
> >> Fixes: 690079767803 ("cros_ec: Support keyboard scanning with
> >> EC_CMD_GET_NEXT_EVENT")
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> process_cmd() should always return an appropriate negative enum ec_status
> >> in case of an error and not simply -1. But fixing the return values is
> >> beyond the scope of this patch.
> >
> > (Looks to me like -1 is already == -EC_RES_INVALID_COMMAND from
> > include/ec_commands.h definitions, but I'd agree the latter form should
> > be preferred.)
>
> If you look at the complete function, you will find other "return -1;"
> statements where return codes other than -EC_RES_INVALID_COMMAND make
> more sense. E.g. after
>
> printf("** Unknown flash region %d\n", req->region);
>
> it would be reasonable to return EC_RES_INVALID_PARAM.
>
> Best regards
>
> Heinrich
>
> >
> >> ---
> >> drivers/misc/cros_ec_sandbox.c | 10 ++
> >> 1 file changed, 10 insertions(+)
Shall we take Alper's patch to implement this command?
Regards,
Simon
Re: [PATCH 3/3] efi_selftest: implement exception test for sandbox
On Tue, 10 Nov 2020 at 16:09, Heinrich Schuchardt wrote: > > Provide a unit test that causes an illegal instruction to occur. > > The test can be run with the following commands: > > => setenv efi_selftest exception > => bootefi selftest > > This might be the output: > > Executing 'exception' > EFI application triggers exception. > Illegal instruction > pc = 0x1444d016, pc_reloc = 0xaa078e8dd016 > UEFI image [0x:0x] '/\selftest' > UEFI image [0x1444b000:0x14451fff] pc=0x2016 '/bug.efi' > Resetting ... > > It would tell us that the exception was triggered by an instruction > 0x2016 bytes after the load address of the binary with filename /bug.efi. > > Signed-off-by: Heinrich Schuchardt > --- > lib/efi_selftest/efi_selftest_miniapp_exception.c | 2 ++ > 1 file changed, 2 insertions(+) Test for U-Boot code should be via U-Boot directly and not EFI. It is OK to have an EFI test as well if you like, but I really don't want to rely on the EFI complexity to test U-Boot itself. Regards, Simon
Re: [PATCH 1/1] cmd: CMD_CPU depends on CPU
On Wed, 4 Nov 2020 at 16:29, Heinrich Schuchardt wrote: > > Compiling with CONFIG_CMD_CPU=y and CONFIG_CPU=n fails with: > > aarch64-linux-gnu-ld.bfd: cmd/built-in.o: in function `print_cpu_list': > /home/user/u-boot/cmd/cpu.c:34: undefined reference to `cpu_get_desc' > aarch64-linux-gnu-ld.bfd: > /home/user/u-boot/cmd/cpu.c:39: undefined reference to `cpu_get_info' > Segmentation fault (core dumped) > make: *** [Makefile:1757: u-boot] Error 139 > make: *** Deleting file 'u-boot' > > See error report in https://stackoverflow.com/questions/64678347. > > Add the missing build dependency. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/Kconfig | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Simon Glass
Re: [PATCH 1/3] sandbox: add handler for exceptions
Hi Heinrich, On Tue, 10 Nov 2020 at 16:09, Heinrich Schuchardt wrote: > > Add a handler for SIGILL, SIGBUS, SIGSEGV. > > When an exception occurs print the program counter and the loaded > UEFI binaries and reset the system. > > Signed-off-by: Heinrich Schuchardt > --- > arch/sandbox/cpu/os.c | 31 +++ > arch/sandbox/cpu/start.c | 4 > arch/sandbox/lib/interrupts.c | 30 ++ > include/os.h | 17 + > 4 files changed, 82 insertions(+) Generally we want to stop execution, because it indicates a bug. Then we might want to run it again with gdb to debug it. So I think this feature should be enabled by a flag. Regards, Simon
Re: [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
On Tue, 10 Nov 2020 at 07:00, Alper Nebi Yasak wrote:
>
> Since commit 690079767803 ("cros_ec: Support keyboard scanning with
> EC_CMD_GET_NEXT_EVENT") the cros-ec-keyb driver has started using this
> command, but the sandbox EC emulator does not recognize it and
> continuously prints:
>
> ** Unknown EC command 0x67
>
> This patch makes the sandbox driver send basic responses to the command,
> but the response only supports keyboard scans for now.
>
> Fixes: 690079767803 ("cros_ec: Support keyboard scanning with
> EC_CMD_GET_NEXT_EVENT")
> Reported-by: Heinrich Schuchardt
> Signed-off-by: Alper Nebi Yasak
> ---
> This doesn't test the -EC_RES_UNAVAILABLE part, which looks like an
> event queue on the EC side. As far as I understand sandbox is getting a
> single keyboard state from SDL after discarding pending events. I think
> things would need to be hooked into sandbox_sdl_poll_events, looks
> possible but harder.
>
> Also, now that this command would work, the fallback to the old one
> would never trigger and wouldn't be tested.
>
> drivers/misc/cros_ec_sandbox.c | 8
> 1 file changed, 8 insertions(+)
>
Reviewed-by: Simon Glass
> diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c
> index a191f061b898..d72db3eace98 100644
> --- a/drivers/misc/cros_ec_sandbox.c
> +++ b/drivers/misc/cros_ec_sandbox.c
> @@ -460,6 +460,14 @@ static int process_cmd(struct ec_state *ec,
> case EC_CMD_ENTERING_MODE:
> len = 0;
> break;
> + case EC_CMD_GET_NEXT_EVENT: {
> + struct ec_response_get_next_event *resp = resp_data;
> +
> + resp->event_type = EC_MKBP_EVENT_KEY_MATRIX;
> + cros_ec_keyscan(ec, resp->data.key_matrix);
> + len = sizeof(*resp);
> + break;
> + }
> default:
> printf(" ** Unknown EC command %#02x\n", req_hdr->command);
> return -1;
> --
> 2.29.2
>
Re: [PATCH 2/3] cmd: sandbox: implement exception command
On Tue, 10 Nov 2020 at 16:09, Heinrich Schuchardt wrote: > > Implement the commands > > * exception undefined - execute an illegal instruction > * exception sigsegv - cause a segment violation > > Here is a possible output: > > => exception undefined > Illegal instruction > pc = 0x55eb8d0a7575, pc_reloc = 0x57575 > Resetting ... > > Signed-off-by: Heinrich Schuchardt > --- > cmd/Kconfig | 2 +- > cmd/Makefile| 1 + > cmd/sandbox/Makefile| 3 +++ > cmd/sandbox/exception.c | 41 + > 4 files changed, 46 insertions(+), 1 deletion(-) > create mode 100644 cmd/sandbox/Makefile > create mode 100644 cmd/sandbox/exception.c Reviewed-by: Simon Glass But it does need a test
Re: [PATCH 2/2] test: Only enable bloblist test when supported
Hi Heinrich, On Mon, 9 Nov 2020 at 00:42, Heinrich Schuchardt wrote: > > On 09.11.20 05:08, Simon Glass wrote: > > This test cannot work unless CONFIG_BLOBLIST is enabled. Update it to add > > that condition. > > > > Reported-by: Kever Yang > > Signed-off-by: Simon Glass > > --- > > > > test/Makefile | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/test/Makefile b/test/Makefile > > index 1c930b31485..cdbfa61cda3 100644 > > --- a/test/Makefile > > +++ b/test/Makefile > > @@ -2,7 +2,9 @@ > > # > > # (C) Copyright 2012 The Chromium Authors > > > > +ifneq ($(CONFIG_$(SPL_)BLOBLIST),) > > obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o > > +endif > > obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/ > > obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o > > obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o > > > > This patch conflicts with my patch > > [1/1] test: test/bloblist.c depends on asm/state.h > https://patchwork.ozlabs.org/project/uboot/patch/20201031073806.30736-1-xypron.g...@gmx.de/ > > It seems that my patch is obsoleted by the first patch of Kever's series: > > test: Avoid assuming sandbox board for bloblist test > https://patchwork.ozlabs.org/project/uboot/patch/20201108210834.1.I08cec35c8482583834811d48894f358277c662aa@changeid/ I believe your patch was already applied, so there should not be a patch conflict. Yes there was a request to be able to run the test on boards other than sandbox. I think we should try to do that if possible. Regards, Simon
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/11/20 12:56 PM, Pragnesh Patel wrote:
Hi Heinrich,
-Original Message-
From: Heinrich Schuchardt
Sent: 11 November 2020 16:51
To: Pragnesh Patel ; u-boot@lists.denx.de
Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
Paul Walmsley ( Sifive) ; anup.pa...@wdc.com;
Sagar Kadam ; r...@andestech.com; Sean
Anderson ; Simon Glass
Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
[External Email] Do not click links or attachments unless you recognize the
sender and know the content is safe
On 11.11.20 11:14, Pragnesh Patel wrote:
Add timer_get_us() which is useful for tracing.
For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
and For M-mode U-Boot, mtime register will provide the same.
Signed-off-by: Pragnesh Patel
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation is
implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will call
functions
which are not marked with "notrace" so tracing got stuck.
Thanks for the background information.
Please, identify the existing functions that lack "notrace" and fix
them. This will give us a solution for all existing boards and not for a
small selection. Furthermore it will avoid code duplication.
In lib/trace.c consider replacing
"__attribute__((no_instrument_function))" by "notrace".
Best regards
Heinrich
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
Best regards
Heinrich
---
Changes in v3:
- Added gd->arch.plmt in global data
- For timer_get_us(), use readq() instead of andes_plmt_get_count()
and sifive_clint_get_count()
Changes in v2:
- Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
and andes_plmt_timer.c.
arch/riscv/include/asm/global_data.h | 3 +++
drivers/timer/andes_plmt_timer.c | 19 ++-
drivers/timer/riscv_timer.c | 14 +-
drivers/timer/sifive_clint_timer.c | 19 ++-
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/global_data.h
b/arch/riscv/include/asm/global_data.h
index d3a0b1d221..4e22ceb83f 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
void __iomem *plic; /* plic base address */
#endif
+#ifdef CONFIG_ANDES_PLMT
+ void __iomem *plmt; /* plmt base address */
+#endif
#if CONFIG_IS_ENABLED(SMP)
struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git
a/drivers/timer/andes_plmt_timer.c b/drivers/timer/andes_plmt_timer.c
index cec86718c7..7c50c46d9e 100644
--- a/drivers/timer/andes_plmt_timer.c
+++ b/drivers/timer/andes_plmt_timer.c
@@ -13,11 +13,12 @@
#include
#include
#include
+#include
/* mtime register */
#define MTIME_REG(base) ((ulong)(base))
-static u64 andes_plmt_get_count(struct udevice *dev)
+static u64 notrace andes_plmt_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv)); } @@ -26,12
+27,28 @@ static const struct timer_ops andes_plmt_ops = {
.get_count = andes_plmt_get_count, };
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_get_us(void) {
+ u64 ticks;
+
+ /* FIXME: gd->arch.plmt should contain valid base address */
+ if (gd->arch.plmt) {
+ ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
+ do_div(ticks, CONFIG_SYS_HZ);
+ }
+
+ return ticks;
+}
+#endif
+
static int andes_plmt_probe(struct udevice *dev) {
dev->priv = dev_read_addr_ptr(dev);
if (!dev->priv)
return -EINVAL;
+ gd->arch.plmt = dev->priv;
return timer_timebase_fallback(dev); }
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 21ae184057..7fa8773da3 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -15,8 +15,9 @@
#include
#include
#include
+#include
-static u64 riscv_timer_get_count(struct udevice *dev)
+static u64 notrace riscv_timer_get_count(struct udevice *dev)
{
__maybe_unused u32 hi, lo;
@@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
return ((u64)hi << 32) | lo;
}
+#if CONFIG_IS_ENABLED(RISCV_SMODE)
+unsigned long notrace timer_get_us(void) {
+ u64 ticks;
+
+ ticks = riscv_timer_get_count(NULL);
+
[PATCH] mpc83xx: Fix dcache setup in lock_ram_in_cache
Lock dcache before clearing INIT_RAM. More importantly, invalidate dcache contents before using it as RAM. Signed-off-by: Joakim Tjernlund --- Something odd happend, on a stable mpc8321 board, small unrelated code changes made the board unbootable. Debugging with an emulator it looked like something pulled away the dcache while it was in early boot stages. I found that if I let the emulator init the DDR before executing, all was OK again. To me this looked like there was some garbage in the dcache causing some DDR access. All this led me to lock_ram_in_cache and the the resluting changes in this patch. 86xx could possible need the same fix arch/powerpc/cpu/mpc83xx/start.S | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index c00bb31363..a1f3b56433 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -1071,18 +1071,22 @@ lock_ram_in_cache: ori r3, r3, (CONFIG_SYS_INIT_RAM_ADDR & ~31)@l li r4, ((CONFIG_SYS_INIT_RAM_SIZE & ~31) + \ (CONFIG_SYS_INIT_RAM_ADDR & 31) + 31) / 32 + /* Lock the data cache */ + mfspr r0, HID0 + ori r0, r0, HID0_DLOCK|HID0_DCFI + sync + mtspr HID0, r0 + rlwinm r0, r0, 0, ~HID0_DCFI /* Clear HID0_DCFI */ + sync + mtspr HID0, r0 + sync + mtctr r4 1: dcbzr0, r3 addir3, r3, 32 bdnz1b - /* Lock the data cache */ - mfspr r0, HID0 - ori r0, r0, HID0_DLOCK - sync - mtspr HID0, r0 - sync blr #ifndef MINIMAL_SPL -- 2.26.2
[PATCH v2 1/2] tools: mkimage: Add Allwinner eGON support
So far we used the separate mksunxiboot tool for generating a bootable
image for Allwinner SPLs, probably just for historical reasons.
Use the mkimage framework to generate a so called eGON image the
Allwinner BROM expects.
The new image type is called "sunxi_egon", to differentiate it
from the (still to be implemented) secure boot TOC0 image.
Signed-off-by: Andre Przywara
Reviewed-by: Simon Glass
Reviewed-by: Jagan Teki
Tested-by: Jagan Teki # BPI-M64
---
common/image.c | 1 +
include/image.h| 1 +
tools/Makefile | 1 +
tools/sunxi_egon.c | 133 +
4 files changed, 136 insertions(+)
create mode 100644 tools/sunxi_egon.c
diff --git a/common/image.c b/common/image.c
index 451fc689a89..6923dac7c07 100644
--- a/common/image.c
+++ b/common/image.c
@@ -189,6 +189,7 @@ static const table_entry_t uimage_type[] = {
{ IH_TYPE_STM32IMAGE, "stm32image", "STMicroelectronics STM32
Image" },
{ IH_TYPE_MTKIMAGE, "mtk_image", "MediaTek BootROM loadable
Image" },
{ IH_TYPE_COPRO, "copro", "Coprocessor Image"},
+ { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image"
},
{ -1, "", "", },
};
diff --git a/include/image.h b/include/image.h
index 00bc03bebec..89bfca2155a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -308,6 +308,7 @@ enum {
IH_TYPE_IMX8MIMAGE, /* Freescale IMX8MBoot Image*/
IH_TYPE_IMX8IMAGE, /* Freescale IMX8Boot Image */
IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/
+ IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
IH_TYPE_COUNT, /* Number of image types */
};
diff --git a/tools/Makefile b/tools/Makefile
index 51123fd9298..2c9a0c4c4e7 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -104,6 +104,7 @@ dumpimage-mkimage-objs := aisimage.o \
stm32image.o \
$(ROCKCHIP_OBS) \
socfpgaimage.o \
+ sunxi_egon.o \
lib/crc16.o \
lib/sha1.o \
lib/sha256.o \
diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
new file mode 100644
index 000..8dcc4c87413
--- /dev/null
+++ b/tools/sunxi_egon.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2018 Arm Ltd.
+ */
+
+#include "imagetool.h"
+#include
+
+#include
+
+/* checksum initialiser value */
+#define STAMP_VALUE 0x5F0A6C39
+
+/*
+ * NAND requires 8K padding. SD/eMMC gets away with 512 bytes,
+ * but let's use the larger padding to cover both.
+ */
+#define PAD_SIZE 8192
+
+static int egon_check_params(struct image_tool_params *params)
+{
+ /* We just need a binary image file. */
+ return !params->dflag;
+}
+
+static int egon_verify_header(unsigned char *ptr, int image_size,
+ struct image_tool_params *params)
+{
+ const struct boot_file_head *header = (void *)ptr;
+ uint32_t length;
+
+ /* First 4 bytes must be an ARM branch instruction. */
+ if ((le32_to_cpu(header->b_instruction) & 0xff00) != 0xea00)
+ return EXIT_FAILURE;
+
+ if (memcmp(header->magic, BOOT0_MAGIC, 8))
+ return EXIT_FAILURE;
+
+ length = le32_to_cpu(header->length);
+ /* Must be at least 512 byte aligned. */
+ if (length & 511)
+ return EXIT_FAILURE;
+
+ /*
+* Image could also contain U-Boot proper, so could be bigger.
+* But it must not be shorter.
+*/
+ if (image_size < length)
+ return EXIT_FAILURE;
+
+ return EXIT_SUCCESS;
+}
+
+static void egon_print_header(const void *buf)
+{
+ const struct boot_file_head *header = buf;
+
+ printf("Allwinner eGON header, size: %d bytes\n",
+ le32_to_cpu(header->length));
+ if (!memcmp(header->spl_signature, SPL_SIGNATURE, 3))
+ printf("\tSPL header version %d.%d\n",
+ header->spl_signature[3] >> SPL_MINOR_BITS,
+ header->spl_signature[3] & ((1U << SPL_MINOR_BITS) - 1));
+ if (header->spl_signature[3] >= SPL_DT_HEADER_VERSION)
+ printf("\tDT name: %s\n",
+ (char *)buf + le32_to_cpu(header->dt_name_offset));
+}
+
+static void egon_set_header(void *buf, struct stat *sbuf, int infd,
+ struct image_tool_params *params)
+{
+ struct boot_file_head *header = buf;
+ uint32_t *buf32 = buf;
+ uint32_t checksum = 0, value;
+ int i;
+
+ /* Generate an ARM branch instruction to jump over the header. */
+ value = 0xea00 | (sizeof(struct boot_file_head) / 4 - 2);
+ header->b_instruction = cpu_to_le32(value);
+
+
[PATCH v2 0/2] tools/sunxi: Use mkimage for SPL generation
Hi, digging this out after two years [1] ;-) I fixed the minor comments from v1 and rebased it, then repeated the tests. Apart from two trivial fixes there were no changes, so I kept the R-b: tags. For a changelog see below. == So far creating a bootable SPL image for Allwinner based boards uses the mksunxiboot tool. Most other platforms seemed to have integrated this kind of functionality into the common mkimage tool. Since there is nothing special about the Allwinner image in this respect, just add support for the so-called "eGON" image type into mkimage. If there was a particular reason this hasn't been done before, please let me know. This will eventually allow us to remove mksunxiboot, but I leave it around for now in case of regressions and since some people depend on it from external projects. Patch 1/2 adds the actual support to mkimage, patch 2/2 then switches the Makefile to use mkimage instead of mksunxiboot. I tested all 152 Allwinner boards by building each u-boot-sunxi-with-spl.bin and comparing them against the version created using mksunxiboot (using SOURCE_DATE_EPOCH and .scmversion to create reproducible builds, and by reverting just patch 2/2). All files before and after were identical. Cheers, Andre [1] https://lists.denx.de/pipermail/u-boot/2018-December/352798.html Changelog v1 .. v2: - Drop already merged cleanup patch (v1 1/3) - replace relative include path - remove already defined ALIGN macro - rebase against current master Andre Przywara (2): tools: mkimage: Add Allwinner eGON support sunxi: Use mkimage for SPL boot image generation common/image.c | 1 + include/image.h | 1 + scripts/Makefile.spl | 8 +-- tools/Makefile | 1 + tools/sunxi_egon.c | 133 +++ 5 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 tools/sunxi_egon.c -- 2.17.5
[PATCH v2 2/2] sunxi: Use mkimage for SPL boot image generation
Switch the SPL boot image generation from using mksunxiboot to the new sunxi_egon format of mkimage. Verified to create identical results for all 152 Allwinner boards. Signed-off-by: Andre Przywara Reviewed-by: Simon Glass --- scripts/Makefile.spl | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 9f1f7445d71..db95bd998a8 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -382,11 +382,11 @@ endif $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE $(call if_changed,mkimage) -quiet_cmd_mksunxiboot = MKSUNXI $@ -cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \ - --default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@ +MKIMAGEFLAGS_sunxi-spl.bin = -T sunxi_egon \ + -n $(CONFIG_DEFAULT_DEVICE_TREE) + $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin FORCE - $(call if_changed,mksunxiboot) + $(call if_changed,mkimage) quiet_cmd_sunxi_spl_image_builder = SUNXI_SPL_IMAGE_BUILDER $@ cmd_sunxi_spl_image_builder = $(objtree)/tools/sunxi-spl-image-builder \ -- 2.17.5
drivers/ram/rockchip/sdram_common.c :: sdram_detect_dbw() LPDDR3/2 calculations seem wrong
Hi,
I got a nanopc-t4 amongst others which shipped with:
DDR Version 1.15 20181010
Channel 0: LPDDR3, 933MHz
Bus Width=32 Col=10 Bank=8 Row=15/15 CS=2 Die Bus-Width=32 Size=2048MB
..
I have since upgraded to more recent u-boot versions:
U-Boot TPL 2020.07 (Sep 27 2020 - 12:34:15)
Channel 0: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
U-Boot TPL 2020.10 (Nov 10 2020 - 13:37:45)
Channel 0: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
The machine was highly instable showing memory and locking issues.
When only using two little cores, it was a lot more stable.
I went and also tried:
DDR Version 1.24 20191016
Channel 0: LPDDR3, 933MHz
Bus Width=32 Col=10 Bank=8 Row=15/15 CS=2 Die Bus-Width=16 Size=2048MB
which seems to match recent u-boot but all of them are different to the
original Die BW of 32 which I currently assume to be correct for the
Samsung K4E6E304EC-EGCG (so possibly the error also migrated into
rokchip-linux/rkbin ?).
Looking at sdram_common.c::sdram_detect_dbw()
300 cs_cap = (1 << (row + col + bk + bw - 20));
301 if (bw == 2) {
302 if (cs_cap <= 0x200) /* 256Mb */
303 die_bw_0 = (col < 9) ? 2 : 1;
304 else if (cs_cap <= 0x1000) /* 2Gb
*/
305 die_bw_0 = (col < 10) ? 2 : 1;
306 else if (cs_cap <= 0x4000) /* 8Gb
*/
307 die_bw_0 = (col < 11) ? 2 : 1;
308 else
309 die_bw_0 = (col < 12) ? 2 : 1;
310 if (cs > 1) {
311 row = cap_info->cs1_row;
312 cs_cap = (1 << (row + col + bk
+ bw - 20));
313 if (cs_cap <= 0x200) /*
256Mb */
314 die_bw_0 = (col < 9) ?
2 : 1;
315 else if (cs_cap <= 0x1000)
/* 2Gb */
316 die_bw_0 = (col < 10) ?
2 : 1;
317 else if (cs_cap <= 0x4000)
/* 8Gb */
318 die_bw_0 = (col < 11) ?
2 : 1;
319 else
320 die_bw_0 = (col < 12) ?
2 : 1;
321 }
322 } else {
ca_cap is off by 20 bits compared to the values you are comparing to;
in my case 0x400 and not 0x4000:
type 6 row 15 col 10 bk 3 cs 2 bw 2 cs_cap 8 cs1_row 15
1 << (15 + 10 + 3 + 2 - 20) == 1 << 10 == 0x400
And similar in the 2nd case with cs1_row given cs > 1.
Now I know very little about all the memory chips out there but it seems
very unlikely to regain these 20 bits in these calculations.
So either the “-20” goes or the cs_cap <= values need adjustment.
The problem now comes from the fact that cap_info->dbw gets the wrong
value from die_bw_0 this way and given it is LPDDR3 I assume that
set_cap_relate_config() in sdram_rk3399.c later restores the wrong
values for the “memdata_ratio”.
There might be more problems lingering, but changing this, my machine
got a lot more reliable, though I still see memory errors when I push it
to its (temperature) limits running on all 6 cores, even with decent
cooling, but that might be a secondary problem.
Can someone with a lot more insight into this magic have a look and if
needed please fix it?
/bz
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 11 November 2020 16:51
>To: Pragnesh Patel ; u-boot@lists.denx.de
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
>Paul Walmsley ( Sifive) ; anup.pa...@wdc.com;
>Sagar Kadam ; r...@andestech.com; Sean
>Anderson ; Simon Glass
>Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>On 11.11.20 11:14, Pragnesh Patel wrote:
>> Add timer_get_us() which is useful for tracing.
>> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
>> and For M-mode U-Boot, mtime register will provide the same.
>>
>> Signed-off-by: Pragnesh Patel
>
>The default implementation of get_timer_us() in lib/time.c calls
>get_ticks() which calls timer_get_count(). The get_count() operation is
>implemented in drivers/timer/andes_plmt_timer.c,
>drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>
>Why do you need special timer_get_us() implementations?
>Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will call
functions
which are not marked with "notrace" so tracing got stuck.
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
>
>Best regards
>
>Heinrich
>
>> ---
>>
>> Changes in v3:
>> - Added gd->arch.plmt in global data
>> - For timer_get_us(), use readq() instead of andes_plmt_get_count()
>> and sifive_clint_get_count()
>>
>> Changes in v2:
>> - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
>> and andes_plmt_timer.c.
>>
>>
>> arch/riscv/include/asm/global_data.h | 3 +++
>> drivers/timer/andes_plmt_timer.c | 19 ++-
>> drivers/timer/riscv_timer.c | 14 +-
>> drivers/timer/sifive_clint_timer.c | 19 ++-
>> 4 files changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/global_data.h
>> b/arch/riscv/include/asm/global_data.h
>> index d3a0b1d221..4e22ceb83f 100644
>> --- a/arch/riscv/include/asm/global_data.h
>> +++ b/arch/riscv/include/asm/global_data.h
>> @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
>> void __iomem *plic; /* plic base address */
>> #endif
>> +#ifdef CONFIG_ANDES_PLMT
>> + void __iomem *plmt; /* plmt base address */
>> +#endif
>> #if CONFIG_IS_ENABLED(SMP)
>> struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git
>> a/drivers/timer/andes_plmt_timer.c b/drivers/timer/andes_plmt_timer.c
>> index cec86718c7..7c50c46d9e 100644
>> --- a/drivers/timer/andes_plmt_timer.c
>> +++ b/drivers/timer/andes_plmt_timer.c
>> @@ -13,11 +13,12 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> /* mtime register */
>> #define MTIME_REG(base) ((ulong)(base))
>>
>> -static u64 andes_plmt_get_count(struct udevice *dev)
>> +static u64 notrace andes_plmt_get_count(struct udevice *dev)
>> {
>> return readq((void __iomem *)MTIME_REG(dev->priv)); } @@ -26,12
>> +27,28 @@ static const struct timer_ops andes_plmt_ops = {
>> .get_count = andes_plmt_get_count, };
>>
>> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
>> +unsigned long notrace timer_get_us(void) {
>> + u64 ticks;
>> +
>> + /* FIXME: gd->arch.plmt should contain valid base address */
>> + if (gd->arch.plmt) {
>> + ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
>> + do_div(ticks, CONFIG_SYS_HZ);
>> + }
>> +
>> + return ticks;
>> +}
>> +#endif
>> +
>> static int andes_plmt_probe(struct udevice *dev) {
>> dev->priv = dev_read_addr_ptr(dev);
>> if (!dev->priv)
>> return -EINVAL;
>>
>> + gd->arch.plmt = dev->priv;
>> return timer_timebase_fallback(dev); }
>>
>> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
>> index 21ae184057..7fa8773da3 100644
>> --- a/drivers/timer/riscv_timer.c
>> +++ b/drivers/timer/riscv_timer.c
>> @@ -15,8 +15,9 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> -static u64 riscv_timer_get_count(struct udevice *dev)
>> +static u64 notrace riscv_timer_get_count(struct udevice *dev)
>> {
>> __maybe_unused u32 hi, lo;
>>
>> @@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
>> return ((u64)hi << 32) | lo;
>> }
>>
>> +#if CONFIG_IS_ENABLED(RISCV_SMODE)
>> +unsigned long notrace timer_get_us(void) {
>> + u64 ticks;
>> +
>> + ticks = riscv_timer_get_count(NULL);
>> + do_div(ticks, CONFIG_SYS_HZ);
>> + return ticks;
>> +}
>> +#endif
>> +
>> static int riscv_timer_probe(struct udevice
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11.11.20 11:14, Pragnesh Patel wrote:
> Add timer_get_us() which is useful for tracing.
> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide
> a timer ticks and For M-mode U-Boot, mtime register will
> provide the same.
>
> Signed-off-by: Pragnesh Patel
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation is
implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
Best regards
Heinrich
> ---
>
> Changes in v3:
> - Added gd->arch.plmt in global data
> - For timer_get_us(), use readq() instead of andes_plmt_get_count()
> and sifive_clint_get_count()
>
> Changes in v2:
> - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
> and andes_plmt_timer.c.
>
>
> arch/riscv/include/asm/global_data.h | 3 +++
> drivers/timer/andes_plmt_timer.c | 19 ++-
> drivers/timer/riscv_timer.c | 14 +-
> drivers/timer/sifive_clint_timer.c | 19 ++-
> 4 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/global_data.h
> b/arch/riscv/include/asm/global_data.h
> index d3a0b1d221..4e22ceb83f 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -24,6 +24,9 @@ struct arch_global_data {
> #ifdef CONFIG_ANDES_PLIC
> void __iomem *plic; /* plic base address */
> #endif
> +#ifdef CONFIG_ANDES_PLMT
> + void __iomem *plmt; /* plmt base address */
> +#endif
> #if CONFIG_IS_ENABLED(SMP)
> struct ipi_data ipi[CONFIG_NR_CPUS];
> #endif
> diff --git a/drivers/timer/andes_plmt_timer.c
> b/drivers/timer/andes_plmt_timer.c
> index cec86718c7..7c50c46d9e 100644
> --- a/drivers/timer/andes_plmt_timer.c
> +++ b/drivers/timer/andes_plmt_timer.c
> @@ -13,11 +13,12 @@
> #include
> #include
> #include
> +#include
>
> /* mtime register */
> #define MTIME_REG(base) ((ulong)(base))
>
> -static u64 andes_plmt_get_count(struct udevice *dev)
> +static u64 notrace andes_plmt_get_count(struct udevice *dev)
> {
> return readq((void __iomem *)MTIME_REG(dev->priv));
> }
> @@ -26,12 +27,28 @@ static const struct timer_ops andes_plmt_ops = {
> .get_count = andes_plmt_get_count,
> };
>
> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
> +unsigned long notrace timer_get_us(void)
> +{
> + u64 ticks;
> +
> + /* FIXME: gd->arch.plmt should contain valid base address */
> + if (gd->arch.plmt) {
> + ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
> + do_div(ticks, CONFIG_SYS_HZ);
> + }
> +
> + return ticks;
> +}
> +#endif
> +
> static int andes_plmt_probe(struct udevice *dev)
> {
> dev->priv = dev_read_addr_ptr(dev);
> if (!dev->priv)
> return -EINVAL;
>
> + gd->arch.plmt = dev->priv;
> return timer_timebase_fallback(dev);
> }
>
> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
> index 21ae184057..7fa8773da3 100644
> --- a/drivers/timer/riscv_timer.c
> +++ b/drivers/timer/riscv_timer.c
> @@ -15,8 +15,9 @@
> #include
> #include
> #include
> +#include
>
> -static u64 riscv_timer_get_count(struct udevice *dev)
> +static u64 notrace riscv_timer_get_count(struct udevice *dev)
> {
> __maybe_unused u32 hi, lo;
>
> @@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
> return ((u64)hi << 32) | lo;
> }
>
> +#if CONFIG_IS_ENABLED(RISCV_SMODE)
> +unsigned long notrace timer_get_us(void)
> +{
> + u64 ticks;
> +
> + ticks = riscv_timer_get_count(NULL);
> + do_div(ticks, CONFIG_SYS_HZ);
> + return ticks;
> +}
> +#endif
> +
> static int riscv_timer_probe(struct udevice *dev)
> {
> struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> diff --git a/drivers/timer/sifive_clint_timer.c
> b/drivers/timer/sifive_clint_timer.c
> index 00ce0f08d6..c341f7789b 100644
> --- a/drivers/timer/sifive_clint_timer.c
> +++ b/drivers/timer/sifive_clint_timer.c
> @@ -10,11 +10,12 @@
> #include
> #include
> #include
> +#include
>
> /* mtime register */
> #define MTIME_REG(base) ((ulong)(base) + 0xbff8)
>
> -static u64 sifive_clint_get_count(struct udevice *dev)
> +static u64 notrace sifive_clint_get_count(struct udevice *dev)
> {
> return readq((void __iomem *)MTIME_REG(dev->priv));
> }
> @@ -23,12 +24,28 @@ static const struct timer_ops sifive_clint_ops = {
> .get_count = sifive_clint_get_count,
> };
>
> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
> +unsigned long notrace timer_get_us(void)
> +{
> + u64 ticks;
> +
> + /* FIXME: gd->arch.clint should contain valid base address */
> + if (gd->arch.clint) {
> + ticks = readq((void __iomem *)MTIME_REG(gd->arch.clint));
> +
[PATCH 1/3] tools/Makefile: FIT_CIPHER requires libssl
If CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n then mkimage and dumpimage will fail to link: /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data': image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob' /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x10): undefined reference to `EVP_aes_128_cbc' /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x40): undefined reference to `EVP_aes_192_cbc' /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x70): undefined reference to `EVP_aes_256_cbc' /usr/bin/ld: tools/lib/aes/aes-encrypt.o: in function `image_aes_encrypt': aes-encrypt.c:(.text+0x22): undefined reference to `EVP_CIPHER_CTX_new' /usr/bin/ld: aes-encrypt.c:(.text+0x6f): undefined reference to `EVP_EncryptInit_ex' /usr/bin/ld: aes-encrypt.c:(.text+0x8d): undefined reference to `EVP_EncryptUpdate' /usr/bin/ld: aes-encrypt.c:(.text+0xac): undefined reference to `EVP_CIPHER_CTX_free' /usr/bin/ld: aes-encrypt.c:(.text+0xf2): undefined reference to `EVP_EncryptFinal_ex' collect2: error: ld returned 1 exit status Signed-off-by: Joel Stanley --- tools/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/Makefile b/tools/Makefile index 51123fd92983..103b3ab8a7f2 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -154,7 +154,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE endif # MXSImage needs LibSSL -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE),) +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),) HOSTCFLAGS_kwbimage.o += \ $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "") HOSTLDLIBS_mkimage += \ -- 2.28.0
[PATCH 2/3] image-cipher: Fix FIT_CIPHER linking
When CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n is there is no
implementation of image_get_host_blob for mkimage or dumpimage:
/usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
The implementation is the same as common/image-fit-sig.c.
Signed-off-by: Joel Stanley
---
common/image-cipher.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/common/image-cipher.c b/common/image-cipher.c
index 4ca9eec4ef15..fcbbceb847a5 100644
--- a/common/image-cipher.c
+++ b/common/image-cipher.c
@@ -15,6 +15,20 @@ DECLARE_GLOBAL_DATA_PTR;
#include
#include
+#ifdef USE_HOSTCC
+void *host_blob;
+
+void image_set_host_blob(void *blob)
+{
+ host_blob = blob;
+}
+
+void *image_get_host_blob(void)
+{
+ return host_blob;
+}
+#endif
+
struct cipher_algo cipher_algos[] = {
{
.name = "aes128",
--
2.28.0
[PATCH 0/3] mkimage usability fixes
Hello, I was learning about signed FIT today and was unable to convince mkimage to produce a signature node in my control device tree. It turns out that Debian's packaged mkimage doesn't enable FIT_SIGNATURE, so the options I was passing were silently ignored. There's an existing Debian bug for that[1] but in the mean time this changes mkimage's behaviour to fail loudly when FIT_SIGNATURE is disabled. The first two patches fix some issues I found when testing the third patch under sandbox_defconfig with FIT_SIGNATURE=n. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=972513 Joel Stanley (3): tools/Makefile: FIT_CIPHER requires libssl image-cipher: Fix FIT_CIPHER linking mkimge: Reject signing-related flags without FIT_SIGNATURE common/image-cipher.c | 14 ++ tools/Makefile| 2 +- tools/mkimage.c | 9 +++-- 3 files changed, 22 insertions(+), 3 deletions(-) -- 2.28.0
[PATCH 3/3] mkimge: Reject signing-related flags without FIT_SIGNATURE
When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
user is careful they will notice this when looking at the help output.
If they are not careful they will waste several hours wondering what
they got wrong, as mkimage will silently ignore the signing related
options.
Make it obvious that the commands don't work by removing them from the
getopt opt_string.
The tool will now behave as follows:
$ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
mkimage: invalid option -- 'k'
Error: Invalid option
Signed-off-by: Joel Stanley
---
tools/mkimage.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index e78608293e72..10a1b3dc8c18 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -142,6 +142,12 @@ static int add_content(int type, const char *fname)
return 0;
}
+#ifdef CONFIG_FIT_SIGNATURE
+#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
+#else
+#define OPT_STRING "a:A:b:C:d:D:e:f:i:ln:O:R:qstT:vVx"
+#endif
+
static void process_args(int argc, char **argv)
{
char *ptr;
@@ -149,8 +155,7 @@ static void process_args(int argc, char **argv)
char *datafile = NULL;
int opt;
- while ((opt = getopt(argc, argv,
- "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
+ while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
switch (opt) {
case 'a':
params.addr = strtoull(optarg, , 16);
--
2.28.0
Re: [PATCH] env: mmc: Correct partition comparison in mmc_offset_try_partition
On Wed, Nov 11, 2020 at 7:07 PM Jorge Ramirez-Ortiz, Gmail wrote: > > On 11/11/20, Jaehoon Chung wrote: > > On 11/10/20 11:28 PM, Hoyeonjiki Kim wrote: > > > The function mmc_offset_try_partition searches MMC partition to save the > > > environment data by name. However, it only compares the first word-size > > > bytes (size of 'const char *'), which may make the function to find > > > unintended partition. > > > > > > Correct the function not to partially compare the partition name with > > > config "u-boot,,mmc-env-partition". > > > > > > Signed-off-by: Hoyeonjiki Kim > > > --- > > > env/mmc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/env/mmc.c b/env/mmc.c > > > index 4e67180b23..505f7aa2b8 100644 > > > --- a/env/mmc.c > > > +++ b/env/mmc.c > > > @@ -42,7 +42,7 @@ static inline int mmc_offset_try_partition(const char > > > *str, int copy, s64 *val) > > > if (ret < 0) > > > return ret; > > > > > > - if (!strncmp((const char *)info.name, str, sizeof(str))) > > > + if (!strcmp((const char *)info.name, str)) > > > > Using "strlen(str)" is better than changing to strcmp. > > > > strncmp(..., ..., strlen(str)) > > absolutely. > maybe also modify the commit like to indicate a fix to the current bug? Thanks for the feedback for both. However, when we do so, isn't there still the possibility for the function searching incorrect partition if, 1) "str" is shorter than "info.name", and 2) the first "strlen(str)" letters of "info.name" is same with those of "str"? This commit is for fixing the current bug, but also I wanna make sure that partition name matches fully. Let me know your opinion. Best Regards, Hoeyonjiki Kim > > > > > > > Best Regards, > > Jaehoon Chung > > > > > break; > > > } > > > > > > > >
[PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Add timer_get_us() which is useful for tracing.
For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide
a timer ticks and For M-mode U-Boot, mtime register will
provide the same.
Signed-off-by: Pragnesh Patel
---
Changes in v3:
- Added gd->arch.plmt in global data
- For timer_get_us(), use readq() instead of andes_plmt_get_count()
and sifive_clint_get_count()
Changes in v2:
- Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
and andes_plmt_timer.c.
arch/riscv/include/asm/global_data.h | 3 +++
drivers/timer/andes_plmt_timer.c | 19 ++-
drivers/timer/riscv_timer.c | 14 +-
drivers/timer/sifive_clint_timer.c | 19 ++-
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/global_data.h
b/arch/riscv/include/asm/global_data.h
index d3a0b1d221..4e22ceb83f 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -24,6 +24,9 @@ struct arch_global_data {
#ifdef CONFIG_ANDES_PLIC
void __iomem *plic; /* plic base address */
#endif
+#ifdef CONFIG_ANDES_PLMT
+ void __iomem *plmt; /* plmt base address */
+#endif
#if CONFIG_IS_ENABLED(SMP)
struct ipi_data ipi[CONFIG_NR_CPUS];
#endif
diff --git a/drivers/timer/andes_plmt_timer.c b/drivers/timer/andes_plmt_timer.c
index cec86718c7..7c50c46d9e 100644
--- a/drivers/timer/andes_plmt_timer.c
+++ b/drivers/timer/andes_plmt_timer.c
@@ -13,11 +13,12 @@
#include
#include
#include
+#include
/* mtime register */
#define MTIME_REG(base)((ulong)(base))
-static u64 andes_plmt_get_count(struct udevice *dev)
+static u64 notrace andes_plmt_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv));
}
@@ -26,12 +27,28 @@ static const struct timer_ops andes_plmt_ops = {
.get_count = andes_plmt_get_count,
};
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_get_us(void)
+{
+ u64 ticks;
+
+ /* FIXME: gd->arch.plmt should contain valid base address */
+ if (gd->arch.plmt) {
+ ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
+ do_div(ticks, CONFIG_SYS_HZ);
+ }
+
+ return ticks;
+}
+#endif
+
static int andes_plmt_probe(struct udevice *dev)
{
dev->priv = dev_read_addr_ptr(dev);
if (!dev->priv)
return -EINVAL;
+ gd->arch.plmt = dev->priv;
return timer_timebase_fallback(dev);
}
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 21ae184057..7fa8773da3 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -15,8 +15,9 @@
#include
#include
#include
+#include
-static u64 riscv_timer_get_count(struct udevice *dev)
+static u64 notrace riscv_timer_get_count(struct udevice *dev)
{
__maybe_unused u32 hi, lo;
@@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
return ((u64)hi << 32) | lo;
}
+#if CONFIG_IS_ENABLED(RISCV_SMODE)
+unsigned long notrace timer_get_us(void)
+{
+ u64 ticks;
+
+ ticks = riscv_timer_get_count(NULL);
+ do_div(ticks, CONFIG_SYS_HZ);
+ return ticks;
+}
+#endif
+
static int riscv_timer_probe(struct udevice *dev)
{
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
diff --git a/drivers/timer/sifive_clint_timer.c
b/drivers/timer/sifive_clint_timer.c
index 00ce0f08d6..c341f7789b 100644
--- a/drivers/timer/sifive_clint_timer.c
+++ b/drivers/timer/sifive_clint_timer.c
@@ -10,11 +10,12 @@
#include
#include
#include
+#include
/* mtime register */
#define MTIME_REG(base)((ulong)(base) + 0xbff8)
-static u64 sifive_clint_get_count(struct udevice *dev)
+static u64 notrace sifive_clint_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv));
}
@@ -23,12 +24,28 @@ static const struct timer_ops sifive_clint_ops = {
.get_count = sifive_clint_get_count,
};
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_get_us(void)
+{
+ u64 ticks;
+
+ /* FIXME: gd->arch.clint should contain valid base address */
+ if (gd->arch.clint) {
+ ticks = readq((void __iomem *)MTIME_REG(gd->arch.clint));
+ do_div(ticks, CONFIG_SYS_HZ);
+ }
+
+ return ticks;
+}
+#endif
+
static int sifive_clint_probe(struct udevice *dev)
{
dev->priv = dev_read_addr_ptr(dev);
if (!dev->priv)
return -EINVAL;
+ gd->arch.clint = dev->priv;
return timer_timebase_fallback(dev);
}
--
2.17.1
[PATCH v3 0/1] RISC-V tracing support
This series add a support of tracing for RISC-V arch. This series is also available here [1] for testing. [1] https://github.com/pragnesh26992/u-boot/tree/trace How to test this patch: 1) Enable tracing in "configs/sifive_fu540_defconfig" CONFIG_TRACE=y CONFIG_TRACE_BUFFER_SIZE=0x0100 CONFIG_TRACE_CALL_DEPTH_LIMIT=15 CONFIG_CMD_TRACE=y 2) make FTRACE=1 sifive_fu540_defconfig 3) make FTRACE=1 Changes in v3: - Added gd->arch.plmt in global data - For timer_get_us(), use readq() instead of andes_plmt_get_count() and sifive_clint_get_count() Changes in v2: - Remove newly added timer file (arch/riscv/lib/timer.c) - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c and andes_plmt_timer.c. Following are the boot messages on FU540 five cores SMP platform: U-Boot SPL 2021.01-rc1-00244-g88b5af756c-dirty (Nov 11 2020 - 14:57:25 +0530) Trying to boot from MMC1 U-Boot 2021.01-rc1-00244-g88b5af756c-dirty (Nov 11 2020 - 14:57:25 +0530) CPU: rv64imafdc Model: SiFive HiFive Unleashed A00 DRAM: 8 GiB trace: enabled MMC: spi@1005:mmc@0: 0 *** Warning - bad CRC, using default environment In:serial@1001 Out: serial@1001 Err: serial@1001 Board serial number should not be 0 !! Net: Error: ethernet@1009 address not set. No ethernet found. Hit any key to stop autoboot: 0 => trace stats 178,556 function sites 15,443,168 function calls 1 untracked function calls 1,279,056 traced function calls (14135744 dropped due to overflow) 19 maximum observed call depth 15 call depth limit 15,633,052 calls not traced due to depth => Pragnesh Patel (1): riscv: Add timer_get_us() for tracing arch/riscv/include/asm/global_data.h | 3 +++ drivers/timer/andes_plmt_timer.c | 19 ++- drivers/timer/riscv_timer.c | 14 +- drivers/timer/sifive_clint_timer.c | 19 ++- 4 files changed, 52 insertions(+), 3 deletions(-) -- 2.17.1
Re: [PATCH] env: mmc: Correct partition comparison in mmc_offset_try_partition
On 11/11/20, Jaehoon Chung wrote: > On 11/10/20 11:28 PM, Hoyeonjiki Kim wrote: > > The function mmc_offset_try_partition searches MMC partition to save the > > environment data by name. However, it only compares the first word-size > > bytes (size of 'const char *'), which may make the function to find > > unintended partition. > > > > Correct the function not to partially compare the partition name with > > config "u-boot,,mmc-env-partition". > > > > Signed-off-by: Hoyeonjiki Kim > > --- > > env/mmc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/env/mmc.c b/env/mmc.c > > index 4e67180b23..505f7aa2b8 100644 > > --- a/env/mmc.c > > +++ b/env/mmc.c > > @@ -42,7 +42,7 @@ static inline int mmc_offset_try_partition(const char > > *str, int copy, s64 *val) > > if (ret < 0) > > return ret; > > > > - if (!strncmp((const char *)info.name, str, sizeof(str))) > > + if (!strcmp((const char *)info.name, str)) > > Using "strlen(str)" is better than changing to strcmp. > > strncmp(..., ..., strlen(str)) absolutely. maybe also modify the commit like to indicate a fix to the current bug? > > > Best Regards, > Jaehoon Chung > > > break; > > } > > > > >
[PATCH 2/2 v4] efi: Add basic EFI_TCG2_PROTOCOL support
Since U-boot EFI implementation is getting richer it makes sense to
add support for EFI_TCG2_PROTOCOL taking advantage of any hardware TPM
available on the device.
This is the initial implementation of the protocol which only adds
support for GetCapability(). It's limited in the newer and safer
TPMv2 devices.
Signed-off-by: Ilias Apalodimas
---
* changes since v3:
- added check for maximum number of PCRs allowed
- replaced multiple return Xl with goto out tags
* changes since v2:
- added description about include/efi_tcg2.h
- switch bool to u8 for tpm_present_flag
- removed superfluous 'default n' from Kconfig
- use 'goto 'tag' when possible
* changes since v1:
- change return variable of platform_get_tpm2_device() when used
- since more headers were included in patch #2 use them in offset
calculations for all tpm commands
- change the size of the response buffer regardless of what
tpm2_get_capability() is doing
include/efi_loader.h | 2 +
include/efi_tcg2.h | 94 +++
lib/efi_loader/Kconfig | 7 +
lib/efi_loader/Makefile| 1 +
lib/efi_loader/efi_setup.c | 7 +
lib/efi_loader/efi_tcg2.c | 539 +
6 files changed, 650 insertions(+)
create mode 100644 include/efi_tcg2.h
create mode 100644 lib/efi_loader/efi_tcg2.c
diff --git a/include/efi_loader.h b/include/efi_loader.h
index f550ced56876..e5015d865ec9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -405,6 +405,8 @@ efi_status_t efi_console_register(void);
efi_status_t efi_disk_register(void);
/* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
efi_status_t efi_rng_register(void);
+/* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
+efi_status_t efi_tcg2_register(void);
/* Create handles and protocols for the partitions of a block device */
int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
const char *if_typename, int diskid,
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
new file mode 100644
index ..4214f767eaba
--- /dev/null
+++ b/include/efi_tcg2.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Defines data structures and APIs that allow an OS to interact with UEFI
+ * firmware to query information about the device
+ *
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#if !defined _EFI_TCG2_PROTOCOL_H_
+#define _EFI_TCG2_PROTOCOL_H_
+
+#include
+
+#define EFI_TCG2_PROTOCOL_GUID \
+ EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \
+0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f)
+
+/* TPMV2 only */
+#define TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
+
+/* SHA1, SHA256, SHA384, SHA512, TPM_ALG_SM3_256 */
+#define MAX_HASH_COUNT 5
+/* Algorithm Registry */
+#define EFI_TCG2_BOOT_HASH_ALG_SHA10x0001
+#define EFI_TCG2_BOOT_HASH_ALG_SHA256 0x0002
+#define EFI_TCG2_BOOT_HASH_ALG_SHA384 0x0004
+#define EFI_TCG2_BOOT_HASH_ALG_SHA512 0x0008
+#define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x0010
+
+typedef u32 efi_tcg_event_log_bitmap;
+typedef u32 efi_tcg_event_log_format;
+typedef u32 efi_tcg_event_algorithm_bitmap;
+
+struct efi_tcg2_version {
+ u8 major;
+ u8 minor;
+};
+
+struct efi_tcg2_event_header {
+ u32 header_size;
+ u16 header_version;
+ u32 pcr_index;
+ u32 event_type;
+} __packed;
+
+struct efi_tcg2_event {
+ u32 size;
+ struct efi_tcg2_event_header header;
+ u8 event[];
+} __packed;
+
+struct efi_tcg2_boot_service_capability {
+ u8 size;
+ struct efi_tcg2_version structure_version;
+ struct efi_tcg2_version protocol_version;
+ efi_tcg_event_algorithm_bitmap hash_algorithm_bitmap;
+ efi_tcg_event_log_bitmap supported_event_logs;
+ u8 tpm_present_flag;
+ u16 max_command_size;
+ u16 max_response_size;
+ u32 manufacturer_id;
+ u32 number_of_pcr_banks;
+ efi_tcg_event_algorithm_bitmap active_pcr_banks;
+};
+
+#define boot_service_capability_min \
+ sizeof(struct efi_tcg2_boot_service_capability) - \
+ offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
+
+struct efi_tcg2_protocol {
+ efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
+ struct
efi_tcg2_boot_service_capability *capability);
+ efi_status_t (EFIAPI * get_eventlog)(struct efi_tcg2_protocol *this,
+efi_tcg_event_log_format
log_format,
+u64 *event_log_location, u64
*event_log_last_entry,
+bool *event_log_truncated);
+ efi_status_t (EFIAPI * hash_log_extend_event)(struct efi_tcg2_protocol
*this,
+ u64 flags, u64
data_to_hash,
+ u64 data_to_hash_len,
+
[PATCH 1/2 v4] tpm: Add some headers from the spec
A following patch introduces EFI_TCG2_PROTOCOL.
Add the required TPMv2 headers to support it.
Signed-off-by: Ilias Apalodimas
Reviewed-by: Heinrich Schuchardt
---
* changes since v3:
- Add TPM2 prefix on PT_GROUP, PT_FIXED
* changes since v2:
- Added description and pointers to TCG specs
- updated copyright info
include/tpm-v2.h | 77
1 file changed, 77 insertions(+)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index f6c045d35480..74c14fe7c51d 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -1,6 +1,13 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/*
+ * Defines APIs and structures that allow software to interact with a
+ * TPM2 device
+ *
+ * Copyright (c) 2020 Linaro
* Copyright (c) 2018 Bootlin
+ *
+ *
https://trustedcomputinggroup.org/resource/tss-overview-common-structures-specification/
+ *
* Author: Miquel Raynal
*/
@@ -11,6 +18,74 @@
#define TPM2_DIGEST_LEN32
+#define TPM2_MAX_PCRS 32
+#define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
+#define TPM2_MAX_CAP_BUFFER 1024
+#define TPM2_MAX_TPM_PROPERTIES ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /*
TPM2_CAP */ - \
+sizeof(u32)) / sizeof(struct
tpms_tagged_property))
+
+/*
+ * We deviate from this draft of the specification by increasing the value of
+ * TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2
+ * implementations that have enabled a larger than typical number of PCR
+ * banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included
+ * in a future revision of the specification.
+ */
+#define TPM2_NUM_PCR_BANKS 16
+
+/* Definition of (UINT32) TPM2_CAP Constants */
+#define TPM2_CAP_PCRS 0x0005U
+#define TPM2_CAP_TPM_PROPERTIES 0x0006U
+
+/* Definition of (UINT32) TPM2_PT Constants */
+#define TPM2_PT_GROUP (u32)(0x0100)
+#define TPM2_PT_FIXED (u32)(TPM2_PT_GROUP * 1)
+#define TPM2_PT_MANUFACTURER (u32)(TPM2_PT_FIXED + 5)
+#define TPM2_PT_PCR_COUNT (u32)(TPM2_PT_FIXED + 18)
+#define TPM2_PT_MAX_COMMAND_SIZE (u32)(TPM2_PT_FIXED + 30)
+#define TPM2_PT_MAX_RESPONSE_SIZE (u32)(TPM2_PT_FIXED + 31)
+
+/* TPMS_TAGGED_PROPERTY Structure */
+struct tpms_tagged_property {
+ u32 property;
+ u32 value;
+} __packed;
+
+/* TPMS_PCR_SELECTION Structure */
+struct tpms_pcr_selection {
+ u16 hash;
+ u8 size_of_select;
+ u8 pcr_select[TPM2_PCR_SELECT_MAX];
+} __packed;
+
+/* TPML_PCR_SELECTION Structure */
+struct tpml_pcr_selection {
+ u32 count;
+ struct tpms_pcr_selection selection[TPM2_NUM_PCR_BANKS];
+} __packed;
+
+/* TPML_TAGGED_TPM_PROPERTY Structure */
+struct tpml_tagged_tpm_property {
+ u32 count;
+ struct tpms_tagged_property tpm_property[TPM2_MAX_TPM_PROPERTIES];
+} __packed;
+
+/* TPMU_CAPABILITIES Union */
+union tpmu_capabilities {
+ /*
+* Non exhaustive. Only added the structs needed for our
+* current code
+*/
+ struct tpml_pcr_selection assigned_pcr;
+ struct tpml_tagged_tpm_property tpm_properties;
+} __packed;
+
+/* TPMS_CAPABILITY_DATA Structure */
+struct tpms_capability_data {
+ u32 capability;
+ union tpmu_capabilities data;
+} __packed;
+
/**
* TPM2 Structure Tags for command/response buffers.
*
@@ -123,11 +198,13 @@ enum tpm2_return_codes {
* TPM2 algorithms.
*/
enum tpm2_algorithms {
+ TPM2_ALG_SHA1 = 0x04,
TPM2_ALG_XOR= 0x0A,
TPM2_ALG_SHA256 = 0x0B,
TPM2_ALG_SHA384 = 0x0C,
TPM2_ALG_SHA512 = 0x0D,
TPM2_ALG_NULL = 0x10,
+ TPM2_ALG_SM3_256= 0x12,
};
/* NV index attributes */
--
2.29.2
RE: [RESEND,PATCH v2 1/1] riscv: Add timer_get_us() for tracing
Hi Rick,
>-Original Message-
>From: Rick Chen
>Sent: 10 November 2020 13:17
>To: Pragnesh Patel
>Cc: U-Boot Mailing List ; Atish Patra
>; Bin Meng ; Paul Walmsley (
>Sifive) ; Anup Patel ; Sagar
>Kadam ; Simon Glass ; Sean
>Anderson ; palmerdabb...@google.com; rick
>; Alan Kao
>Subject: Re: [RESEND,PATCH v2 1/1] riscv: Add timer_get_us() for tracing
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>Hi Pragnesh
>
>> Hi Rick,
>>
>> >-Original Message-
>> >From: Rick Chen
>> >Sent: 09 November 2020 13:44
>> >To: Pragnesh Patel
>> >Cc: U-Boot Mailing List ; Atish Patra
>> >; Bin Meng ; Paul Walmsley (
>> >Sifive) ; Anup Patel ;
>> >Sagar Kadam ; Simon Glass
>> >; Sean Anderson ;
>> >palmerdabb...@google.com; rick ; Alan Kao
>> >
>> >Subject: Re: [RESEND,PATCH v2 1/1] riscv: Add timer_get_us() for
>> >tracing
>> >
>> >[External Email] Do not click links or attachments unless you
>> >recognize the sender and know the content is safe
>> >
>> >> From: Pragnesh Patel [mailto:pragnesh.pa...@sifive.com]
>> >> Sent: Thursday, November 05, 2020 7:31 PM
>> >> To: u-boot@lists.denx.de
>> >> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>> >bmeng...@gmail.com;
>> >> paul.walms...@sifive.com; anup.pa...@wdc.com;
>> >> sagar.ka...@sifive.com; Rick Jian-Zhi Chen(陳建志); Pragnesh Patel;
>> >> Sean Anderson; Claudiu Beznea; Simon Glass
>> >> Subject: [RESEND,PATCH v2 1/1] riscv: Add timer_get_us() for
>> >> tracing
>> >>
>> >> Add timer_get_us() which is useful for tracing.
>> >> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer
>> >> ticks and For M-mode U-Boot, mtime register will provide the same.
>> >>
>> >> Signed-off-by: Pragnesh Patel
>> >> ---
>> >> drivers/timer/andes_plmt_timer.c | 16 +++-
>> >> drivers/timer/riscv_timer.c| 14 +-
>> >> drivers/timer/sifive_clint_timer.c | 16 +++-
>> >> 3 files changed, 43 insertions(+), 3 deletions(-)
>> >>
>> >
>> >I verify it fail as below:
>> >
>> >U-Boot 2020.10-20532-g0910882 (Nov 09 2020 - 15:51:31 +0800)
>> >
>> >DRAM: 1 GiB
>> >trace: enabled
>> >
>> >Do you have any suggestion ?
>>
>> On which platform ?
>>
>> Do you follow this
>https://patchwork.ozlabs.org/project/uboot/cover/20201105113032.26234-1-
>pragnesh.pa...@sifive.com/ ?
>>
>
>I verify on AE350 platform with the following configurations:
>
>CONFIG_TRACE=y
>CONFIG_TRACE_BUFFER_SIZE=0x0100
>CONFIG_TRACE_CALL_DEPTH_LIMIT=15
>CONFIG_CMD_TRACE=y
>
>make FTRACE=1 ae350_rv64_defconfig
>make FTRACE=1
>
>After dig in and I found the root cause.
>You can't use gd->arch.plic to record plmt base.
>It conflicts with andes plic.
O nice catch, will update in v3.
>
>Thanks,
>Rick
>
>> >
>> >Thanks,
>> >Rick
>> >
>> >
>> >> diff --git a/drivers/timer/andes_plmt_timer.c
>> >> b/drivers/timer/andes_plmt_timer.c
>> >> index cec86718c7..9d663e036e 100644
>> >> --- a/drivers/timer/andes_plmt_timer.c
>> >> +++ b/drivers/timer/andes_plmt_timer.c
>> >> @@ -13,11 +13,12 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >>
>> >> /* mtime register */
>> >> #define MTIME_REG(base)((ulong)(base))
>> >>
>> >> -static u64 andes_plmt_get_count(struct udevice *dev)
>> >> +static u64 notrace andes_plmt_get_count(struct udevice *dev)
>> >> {
>> >> return readq((void __iomem *)MTIME_REG(dev->priv)); } @@
>> >> -26,12 +27,25 @@ static const struct timer_ops andes_plmt_ops = {
>> >> .get_count = andes_plmt_get_count, };
>> >>
>> >> +#if CONFIG_IS_ENABLED(RISCV_MMODE) unsigned long notrace
>> >> +timer_get_us(void) {
>> >> + u64 ticks;
>> >> +
>> >> + /* FIXME: gd->arch.plic should contain valid base address */
Any comment here for FIXME ?
>> >> + ticks = andes_plmt_get_count(gd->arch.plic);
>>
>> Here andes_plmt_get_count() should be replaced with MTIME_REG() macro.
>Will update in v3.
>>
>> >> + do_div(ticks, CONFIG_SYS_HZ);
>> >> + return ticks;
>> >> +}
>> >> +#endif
>> >> +
>> >> static int andes_plmt_probe(struct udevice *dev) {
>> >> dev->priv = dev_read_addr_ptr(dev);
>> >> if (!dev->priv)
>> >> return -EINVAL;
>> >>
>> >> + gd->arch.plic = dev->priv;
>> >> return timer_timebase_fallback(dev); }
>> >>
>> >> diff --git a/drivers/timer/riscv_timer.c
>> >> b/drivers/timer/riscv_timer.c index 21ae184057..7fa8773da3 100644
>> >> --- a/drivers/timer/riscv_timer.c
>> >> +++ b/drivers/timer/riscv_timer.c
>> >> @@ -15,8 +15,9 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >>
>> >> -static u64 riscv_timer_get_count(struct udevice *dev)
>> >> +static u64 notrace riscv_timer_get_count(struct udevice *dev)
>> >> {
>> >> __maybe_unused u32 hi, lo;
>> >>
>> >> @@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice
>*dev)
>> >> return ((u64)hi << 32) | lo; }
>> >>
>> >> +#if
Re: [PATCH] riscv: Fix efi header for RV32
Hi Atish and Heinrich, On Tue, Oct 13, 2020 at 12:23:31PM -0700, Atish Patra wrote: > RV32 should use PE32 format instead of PE32+ as the efi header format. > This requires following changes > 1. A different header magic value > 2. An additional parameter known as BaseOfData. Currently, it is set to >zero in absence of any usage. > > Signed-off-by: Atish Patra > Reviewed-by: Bin Meng > Reviewed-by: Rick Chen > --- > arch/riscv/lib/crt0_riscv_efi.S | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S > index 87fe1e56f906..4aaa49ad0777 100644 > --- a/arch/riscv/lib/crt0_riscv_efi.S > +++ b/arch/riscv/lib/crt0_riscv_efi.S > @@ -15,11 +15,13 @@ > #define SAVE_LONG(reg, idx) sd reg, (idx*SIZE_LONG)(sp) > #define LOAD_LONG(reg, idx) ld reg, (idx*SIZE_LONG)(sp) > #define PE_MACHINE IMAGE_FILE_MACHINE_RISCV64 > +#define PE_MAGICIMAGE_NT_OPTIONAL_HDR64_MAGIC > #else > #define SIZE_LONG4 > #define SAVE_LONG(reg, idx) sw reg, (idx*SIZE_LONG)(sp) > #define LOAD_LONG(reg, idx) lw reg, (idx*SIZE_LONG)(sp) > #define PE_MACHINE IMAGE_FILE_MACHINE_RISCV32 > +#define PE_MAGICIMAGE_NT_OPTIONAL_HDR32_MAGIC > #endif > > > @@ -48,7 +50,7 @@ coff_header: >IMAGE_FILE_LOCAL_SYMS_STRIPPED | \ >IMAGE_FILE_DEBUG_STRIPPED) > optional_header: > - .short IMAGE_NT_OPTIONAL_HDR64_MAGIC /* PE32+ format */ > + .short PE_MAGIC/* PE32+ format */ > .byte 0x02/* MajorLinkerVersion */ > .byte 0x14/* MinorLinkerVersion */ > .long _edata - _start /* SizeOfCode */ > @@ -56,6 +58,9 @@ optional_header: > .long 0 /* SizeOfUninitializedData */ > .long _start - ImageBase /* AddressOfEntryPoint */ > .long _start - ImageBase /* BaseOfCode */ > +#if __riscv_xlen == 32 > + .long 0 /* BaseOfData */ > +#endif > > extra_header_fields: > .quad 0 /* ImageBase */ diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S index 4aaa49ad07..f47c4a6d82 100644 --- a/arch/riscv/lib/crt0_riscv_efi.S +++ b/arch/riscv/lib/crt0_riscv_efi.S @@ -63,7 +63,11 @@ optional_header: #endif extra_header_fields: +#if __riscv_xlen == 32 + .long 0 /* ImageBase */ +#else if .quad 0 /* ImageBase */ +#endif .long 0x20/* SectionAlignment */ .long 0x8 /* FileAlignment */ .short 0 /* MajorOperatingSystemVersion */ @@ -83,10 +87,17 @@ extra_header_fields: .long 0 /* CheckSum */ .short IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */ .short 0 /* DllCharacteristics */ +#if __riscv_xlen == 32 + .long 0 /* SizeOfStackReserve */ + .long 0 /* SizeOfStackCommit */ + .long 0 /* SizeOfHeapReserve */ + .long 0 /* SizeOfHeapCommit */ +#else if .quad 0 /* SizeOfStackReserve */ .quad 0 /* SizeOfStackCommit */ .quad 0 /* SizeOfHeapReserve */ .quad 0 /* SizeOfHeapCommit */ +#endif .long 0 /* LoaderFlags */ .long 0x6 /* NumberOfRvaAndSizes */ Based on Atish's patch, would the above modification fit more closely to the specification that Heinrich provided ? Best regards, Leo
Re: [PATCH] env: mmc: Correct partition comparison in mmc_offset_try_partition
On 11/10/20 11:28 PM, Hoyeonjiki Kim wrote: > The function mmc_offset_try_partition searches MMC partition to save the > environment data by name. However, it only compares the first word-size > bytes (size of 'const char *'), which may make the function to find > unintended partition. > > Correct the function not to partially compare the partition name with > config "u-boot,,mmc-env-partition". > > Signed-off-by: Hoyeonjiki Kim > --- > env/mmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/env/mmc.c b/env/mmc.c > index 4e67180b23..505f7aa2b8 100644 > --- a/env/mmc.c > +++ b/env/mmc.c > @@ -42,7 +42,7 @@ static inline int mmc_offset_try_partition(const char *str, > int copy, s64 *val) > if (ret < 0) > return ret; > > - if (!strncmp((const char *)info.name, str, sizeof(str))) > + if (!strcmp((const char *)info.name, str)) Using "strlen(str)" is better than changing to strcmp. strncmp(..., ..., strlen(str)) Best Regards, Jaehoon Chung > break; > } > >
RE: [PATCH] usb: xhci: fix event trb handling missed
Hi Bin On Tuesday, November 10, 2020 5:55 PM, Ran Wang wrote: > > Hi Bin, > > > > record_transfer_result():comp_code: 1 csw org fbb4ec90, > > > adjusted to fbb4fffe > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 csw org fbb4ec90, > > > adjusted to fbb4fffe > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 csw org fbb4ec90, > > > adjusted to fbb4fffe > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 > > > record_transfer_result():comp_code: 1 > > > > In the earlier log you sent, you have the following: > > > > dev=fbb4f3c0, pipe=c0010283, buffer=fbb4fd80, > > length=2048buffer would cross 64KB > > boundary, so we will send request in more than 1 TRB, this is the key > > issue trigger condition > > xhci_bulk_tx()#0.1.running_total:0x280 > > xhci_bulk_tx()#0.2.trb_buff_len:0x280 > > xhci_bulk_tx()#0.3.running_total:0x280 > > xhci_bulk_tx()#0.4.num_trbs:0x2--2 Transfer > > TRB > > xhci_bulk_tx()#0.5.running_total:0x10280 > > xhci_bulk_tx()#0.start_trb:0xfbb47140 > > --xhci_bulk_tx()#0.>enqueue->generic:0xfbb47140- > > > > -Assemble > > 1st Transfer TRB > > xhci_bulk_tx()#0.addr:0xfbb4fd80 > > xhci_bulk_tx()#0.trb_buff_len:0x280 > > xhci_bulk_tx()#0.running_total:0x280 > > xhci_bulk_tx()#0.length:0x800 > > xhci_bulk_tx()#0.TRB_MAX_BUFF_SIZE:0x1 > > --xhci_bulk_tx()#0.>enqueue->generic:0xfbb47150- > > > > -Assemble > > 2nd Transfer TRB > > xhci_bulk_tx()#0.addr:0xfbb5 > > xhci_bulk_tx()#0.trb_buff_len:0x580 > > xhci_bulk_tx()#0.running_total:0x800 > > xhci_bulk_tx()#0.length:0x800 > > xhci_bulk_tx()#0.TRB_MAX_BUFF_SIZE:0x1 > > > > Could you please specify where this output is for? > > All this prints is within function xhci_bulk_tx(), > > Let me open those verbose output and check if that crossing 64KB case has been > triggered or not. > But I have to get back to you later (tomorrow). With your test commit and my verbose print code, issue cannot be reproduced in mass storage usage (although crossing 64KB boundary condition has met), so looks like the only different to ethernet device is that if xHC would receive short packet: => pri t t=usb read 0x8200 0 30 => run t usb read: device 0 block # 0, count 3145728 ... usb_stor_BBB_transport()csw org fbb4ac90, adjusted to fbb4fffe xhci_bulk_tx()dev=fbb49950, pipe=c0010203, buffer=fbb1a9c0, length=0x1f xhci_bulk_tx()#0.4.num_trbs:0x1 xhci_bulk_tx()#0.>enqueue->generic:0xfbb49570 xhci_bulk_tx()#0.last_transfer_trb_addr:0xfbb49570 --xhci_bulk_tx()#0.event->trans_event.buffer:0xfbb49570 xhci_bulk_tx()#0.event->complete_code:1 xhci_bulk_tx()#3.ep_index:0x3 xhci_bulk_tx()dev=fbb49950, pipe=c0008283, buffer=fbb1ab00, length=0x200 xhci_bulk_tx()#0.4.num_trbs:0x1 xhci_bulk_tx()#set TRB_ISP xhci_bulk_tx()#0.>enqueue->generic:0xfbb49180 xhci_bulk_tx()#0.last_transfer_trb_addr:0xfbb49180 --xhci_bulk_tx()#0.event->trans_event.buffer:0xfbb49180 xhci_bulk_tx()#0.event->complete_code:1 xhci_bulk_tx()#3.ep_index:0x2 xhci_bulk_tx()dev=fbb49950, pipe=c0008283, buffer=fbb4fffe, length=0xd-buffer cross 64KB boundary xhci_bulk_tx()#0.4.num_trbs:0x2 xhci_bulk_tx()#set TRB_ISP---Set ISP xhci_bulk_tx()#0.>enqueue->generic:0xfbb491901st transfer TRB queued xhci_bulk_tx()#0.last_transfer_trb_addr:0xfbb49190 xhci_bulk_tx()#set TRB_ISP---Set ISP xhci_bulk_tx()#0.>enqueue->generic:0xfbb491a02nd transfer TRB queued xhci_bulk_tx()#0.last_transfer_trb_addr:0xfbb491a0 --xhci_bulk_tx()#0.event->trans_event.buffer:0xfbb491a0-event TRB that pointing to 2nd transfer TRB xhci_bulk_tx()#0.event->complete_code:1-not short packet xhci_bulk_tx()#3.ep_index:0x2 num_trbs_count > 1! ... usb_stor_BBB_transport()csw org fbb4aea0, adjusted to fbb4fffe xhci_bulk_tx()dev=fbb49950, pipe=c0010203, buffer=fbb1ad00, length=0x1f xhci_bulk_tx()#0.4.num_trbs:0x1 xhci_bulk_tx()#0.>enqueue->generic:0xfbb49790 xhci_bulk_tx()#0.last_transfer_trb_addr:0xfbb49790 --xhci_bulk_tx()#0.event->trans_event.buffer:0xfbb49790 xhci_bulk_tx()#0.event->complete_code:1
