[PATCH] arm64: zynqmp: enable the PMUFW watchdog

[PATCH] arm64: zynqmp: enable the PMUFW watchdog

[Jorge Ramirez-Ortiz]

From 528b3117a36b7b4eea1839afbea7191d60638b0c Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <jorge@foundries.io>
Date: Wed, 23 Mar 2022 14:41:15 +0100
Subject: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

If the PMUFW was built with support for the CSU watchdog we must let the
firmware know that it can be started.

We do this by faking the FSBL done state.

Reference code in PMUFW:
https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_pmufw/src/xpfw_mod_common.c#L36

It needs this fix on the PMUFW
https://github.com/Xilinx/embeddedsw/pull/190

Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
 arch/arm/mach-zynqmp/include/mach/hardware.h | 5 ++++-
 board/xilinx/zynqmp/zynqmp.c                 | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
index a70d6d611b..24c3ef3d7d 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -174,8 +174,11 @@ struct csu_regs {
 
 #define ZYNQMP_PMU_BASEADDR	0xFFD80000
 
+#define FSBL_COMPLETION	0x1
+
 struct pmu_regs {
-	u32 reserved[18];
+	u32 reserved[17];
+	u32 gen_storage5; /* 0x44 */
 	u32 gen_storage6; /* 0x48 */
 };
 
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 70b3c81f12..c88c5fb8ea 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -419,6 +419,10 @@ int board_init(void)
 						zynqmp_pm_cfg_obj_size);
 	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
 
+	/* fake FSBL completion to start the PMUFW watchdog when enabled */
+	writel(readl(&pmu_base->gen_storage5) | FSBL_COMPLETION,
+	       &pmu_base->gen_storage5);
+
 	/* the CSU disables the JTAG interface when secure boot is enabled */
 	if (CONFIG_IS_ENABLED(ZYNQMP_RESTORE_JTAG))
 		restore_jtag();
-- 
2.34.1

Re: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

[Jorge Ramirez-Ortiz, Foundries]

On 23/03/22, Jorge Ramirez-Ortiz wrote:
> From 528b3117a36b7b4eea1839afbea7191d60638b0c Mon Sep 17 00:00:00 2001
> From: Jorge Ramirez-Ortiz <jorge@foundries.io>
> Date: Wed, 23 Mar 2022 14:41:15 +0100
> Subject: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

um, seems my sendpatch script is a bit off.
please let me know if I should send a v2.

> 
> If the PMUFW was built with support for the CSU watchdog we must let the
> firmware know that it can be started.
> 
> We do this by faking the FSBL done state.
> 
> Reference code in PMUFW:
> https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_pmufw/src/xpfw_mod_common.c#L36
> 
> It needs this fix on the PMUFW
> https://github.com/Xilinx/embeddedsw/pull/190
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h | 5 ++++-
>  board/xilinx/zynqmp/zynqmp.c                 | 4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index a70d6d611b..24c3ef3d7d 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -174,8 +174,11 @@ struct csu_regs {
>  
>  #define ZYNQMP_PMU_BASEADDR	0xFFD80000
>  
> +#define FSBL_COMPLETION	0x1
> +
>  struct pmu_regs {
> -	u32 reserved[18];
> +	u32 reserved[17];
> +	u32 gen_storage5; /* 0x44 */
>  	u32 gen_storage6; /* 0x48 */
>  };
>  
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 70b3c81f12..c88c5fb8ea 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -419,6 +419,10 @@ int board_init(void)
>  						zynqmp_pm_cfg_obj_size);
>  	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
>  
> +	/* fake FSBL completion to start the PMUFW watchdog when enabled */
> +	writel(readl(&pmu_base->gen_storage5) | FSBL_COMPLETION,
> +	       &pmu_base->gen_storage5);
> +
>  	/* the CSU disables the JTAG interface when secure boot is enabled */
>  	if (CONFIG_IS_ENABLED(ZYNQMP_RESTORE_JTAG))
>  		restore_jtag();
> -- 
> 2.34.1
> 

Re: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

[Michal Simek]

On 3/23/22 15:04, Jorge Ramirez-Ortiz wrote:
>  From 528b3117a36b7b4eea1839afbea7191d60638b0c Mon Sep 17 00:00:00 2001
> From: Jorge Ramirez-Ortiz <jorge@foundries.io>
> Date: Wed, 23 Mar 2022 14:41:15 +0100
> Subject: [PATCH] arm64: zynqmp: enable the PMUFW watchdog
> 
> If the PMUFW was built with support for the CSU watchdog we must let the
> firmware know that it can be started.
> 
> We do this by faking the FSBL done state.
> 
> Reference code in PMUFW:
> https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_pmufw/src/xpfw_mod_common.c#L36
> 
> It needs this fix on the PMUFW
> https://github.com/Xilinx/embeddedsw/pull/190


/* FSBL might not be running anywhere - i.e: SPL boots PMUFW */

SPL doesn't boot PMUFW. It is the first firmware in BOOT.BIN which is loaded by 
bootrom to PMU. It means comment is not correct.
FSBL or SPL just sending PMU configuration object to PMUFW.




> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
>   arch/arm/mach-zynqmp/include/mach/hardware.h | 5 ++++-
>   board/xilinx/zynqmp/zynqmp.c                 | 4 ++++
>   2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index a70d6d611b..24c3ef3d7d 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -174,8 +174,11 @@ struct csu_regs {
>   
>   #define ZYNQMP_PMU_BASEADDR	0xFFD80000
>   
> +#define FSBL_COMPLETION	0x1
> +
>   struct pmu_regs {
> -	u32 reserved[18];
> +	u32 reserved[17];
> +	u32 gen_storage5; /* 0x44 */
>   	u32 gen_storage6; /* 0x48 */
>   };
>   
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 70b3c81f12..c88c5fb8ea 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -419,6 +419,10 @@ int board_init(void)
>   						zynqmp_pm_cfg_obj_size);
>   	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
>   
> +	/* fake FSBL completion to start the PMUFW watchdog when enabled */
> +	writel(readl(&pmu_base->gen_storage5) | FSBL_COMPLETION,
> +	       &pmu_base->gen_storage5);

I look at the code in FSBL and it set it up XFsbl_HandoffExit which is at the 
end of FSBL. It means this should be setup in SPL not in U-Boot proper.


There is also any XFSBL_FSBL_ENCRYPTED_MASK which maybe you should take a look 
in connection your other patches.

> +
>   	/* the CSU disables the JTAG interface when secure boot is enabled */
>   	if (CONFIG_IS_ENABLED(ZYNQMP_RESTORE_JTAG))
>   		restore_jtag();

M

Re: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

[Jorge Ramirez-Ortiz, Foundries]

On 24/03/22, Michal Simek wrote:
> 
> 
> On 3/23/22 15:04, Jorge Ramirez-Ortiz wrote:
> >  From 528b3117a36b7b4eea1839afbea7191d60638b0c Mon Sep 17 00:00:00 2001
> > From: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > Date: Wed, 23 Mar 2022 14:41:15 +0100
> > Subject: [PATCH] arm64: zynqmp: enable the PMUFW watchdog
> > 
> > If the PMUFW was built with support for the CSU watchdog we must let the
> > firmware know that it can be started.
> > 
> > We do this by faking the FSBL done state.
> > 
> > Reference code in PMUFW:
> > https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_pmufw/src/xpfw_mod_common.c#L36
> > 
> > It needs this fix on the PMUFW
> > https://github.com/Xilinx/embeddedsw/pull/190
> 
> 
> /* FSBL might not be running anywhere - i.e: SPL boots PMUFW */
> 
> SPL doesn't boot PMUFW. It is the first firmware in BOOT.BIN which is loaded
> by bootrom to PMU. It means comment is not correct.
> FSBL or SPL just sending PMU configuration object to PMUFW.

yeah I guess it depends what you mean by boot - unless SPL configures it, it
wont really boot to a functional state.

but anyway ok, will change.


> 
> 
> 
> 
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > ---
> >   arch/arm/mach-zynqmp/include/mach/hardware.h | 5 ++++-
> >   board/xilinx/zynqmp/zynqmp.c                 | 4 ++++
> >   2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > index a70d6d611b..24c3ef3d7d 100644
> > --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> > +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > @@ -174,8 +174,11 @@ struct csu_regs {
> >   #define ZYNQMP_PMU_BASEADDR	0xFFD80000
> > +#define FSBL_COMPLETION	0x1
> > +
> >   struct pmu_regs {
> > -	u32 reserved[18];
> > +	u32 reserved[17];
> > +	u32 gen_storage5; /* 0x44 */
> >   	u32 gen_storage6; /* 0x48 */
> >   };
> > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > index 70b3c81f12..c88c5fb8ea 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -419,6 +419,10 @@ int board_init(void)
> >   						zynqmp_pm_cfg_obj_size);
> >   	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> > +	/* fake FSBL completion to start the PMUFW watchdog when enabled */
> > +	writel(readl(&pmu_base->gen_storage5) | FSBL_COMPLETION,
> > +	       &pmu_base->gen_storage5);
> 
> I look at the code in FSBL and it set it up XFsbl_HandoffExit which is at
> the end of FSBL. It means this should be setup in SPL not in U-Boot proper.

not sure what you mean. This piece of code I am posting only executes in SPL,
doesn it?

> 
> 
> There is also any XFSBL_FSBL_ENCRYPTED_MASK which maybe you should take a
> look in connection your other patches.

um, ok thanks for the info!

> 
> > +
> >   	/* the CSU disables the JTAG interface when secure boot is enabled */
> >   	if (CONFIG_IS_ENABLED(ZYNQMP_RESTORE_JTAG))
> >   		restore_jtag();
> 
> M

Re: [PATCH] arm64: zynqmp: enable the PMUFW watchdog

[Michal Simek]

On 3/24/22 22:03, Jorge Ramirez-Ortiz, Foundries wrote:
> On 24/03/22, Michal Simek wrote:
>>
>>
>> On 3/23/22 15:04, Jorge Ramirez-Ortiz wrote:
>>>   From 528b3117a36b7b4eea1839afbea7191d60638b0c Mon Sep 17 00:00:00 2001
>>> From: Jorge Ramirez-Ortiz <jorge@foundries.io>
>>> Date: Wed, 23 Mar 2022 14:41:15 +0100
>>> Subject: [PATCH] arm64: zynqmp: enable the PMUFW watchdog
>>>
>>> If the PMUFW was built with support for the CSU watchdog we must let the
>>> firmware know that it can be started.
>>>
>>> We do this by faking the FSBL done state.
>>>
>>> Reference code in PMUFW:
>>> https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_apps/zynqmp_pmufw/src/xpfw_mod_common.c#L36
>>>
>>> It needs this fix on the PMUFW
>>> https://github.com/Xilinx/embeddedsw/pull/190
>>
>>
>> /* FSBL might not be running anywhere - i.e: SPL boots PMUFW */
>>
>> SPL doesn't boot PMUFW. It is the first firmware in BOOT.BIN which is loaded
>> by bootrom to PMU. It means comment is not correct.
>> FSBL or SPL just sending PMU configuration object to PMUFW.
> 
> yeah I guess it depends what you mean by boot - unless SPL configures it, it
> wont really boot to a functional state.
> 
> but anyway ok, will change.
> 
> 
>>
>>
>>
>>
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
>>> ---
>>>    arch/arm/mach-zynqmp/include/mach/hardware.h | 5 ++++-
>>>    board/xilinx/zynqmp/zynqmp.c                 | 4 ++++
>>>    2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> index a70d6d611b..24c3ef3d7d 100644
>>> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> @@ -174,8 +174,11 @@ struct csu_regs {
>>>    #define ZYNQMP_PMU_BASEADDR	0xFFD80000
>>> +#define FSBL_COMPLETION	0x1
>>> +
>>>    struct pmu_regs {
>>> -	u32 reserved[18];
>>> +	u32 reserved[17];
>>> +	u32 gen_storage5; /* 0x44 */
>>>    	u32 gen_storage6; /* 0x48 */
>>>    };
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index 70b3c81f12..c88c5fb8ea 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -419,6 +419,10 @@ int board_init(void)
>>>    						zynqmp_pm_cfg_obj_size);
>>>    	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
>>> +	/* fake FSBL completion to start the PMUFW watchdog when enabled */
>>> +	writel(readl(&pmu_base->gen_storage5) | FSBL_COMPLETION,
>>> +	       &pmu_base->gen_storage5);
>>
>> I look at the code in FSBL and it set it up XFsbl_HandoffExit which is at
>> the end of FSBL. It means this should be setup in SPL not in U-Boot proper.
> 
> not sure what you mean. This piece of code I am posting only executes in SPL,
> doesn it?

ah. ok. Then fine.

M