Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-10-05 Thread Igor Opaniuk
Hi Jorge,

On Tue, Oct 5, 2021 at 2:13 PM Jorge Ramirez-Ortiz  wrote:
>
> Confirm the secure boot configuration on the console.
>
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT   0
>
>  struct csu_regs {
> -   u32 reserved0[4];
> +   u32 status;
> +   u32 reserved0[3];
> u32 multi_boot;
> u32 reserved1[11];
> u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
> return 0;
>  }
>
> +static void secure_boot(void)
> +{
> +   u32 status;
> +
> +   status = readl(&csu_base->status);
> +   if (status & (BIT(0) | BIT(1))) {
> +   printf("Secure Boot:\t%s%s\n",
> +  status & BIT(0) ? "authenticated" : "not 
> authenticated",
> +  status & BIT(1) ? ", encrypted" : ", not encrypted");
> +   }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
>
> @@ -391,8 +403,10 @@ int board_init(void)
> fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>
> -   if (current_el() == 3)
> +   if (current_el() == 3) {
> multi_boot();
> +   secure_boot();
> +   }
>
> return 0;
>  }
> --
> 2.31.1
>

Reviewed-by: Igor Opaniuk 

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Embedded Software Engineer
T:  +380 938364067
E: igor.opan...@foundries.io
W: www.foundries.io


Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-10-05 Thread Oleksandr Suvorov
On Tue, Oct 5, 2021 at 2:13 PM Jorge Ramirez-Ortiz  wrote:
>
> Confirm the secure boot configuration on the console.
>
> Signed-off-by: Jorge Ramirez-Ortiz 

Acked-by: Oleksandr Suvorov 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT   0
>
>  struct csu_regs {
> -   u32 reserved0[4];
> +   u32 status;
> +   u32 reserved0[3];
> u32 multi_boot;
> u32 reserved1[11];
> u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
> return 0;
>  }
>
> +static void secure_boot(void)
> +{
> +   u32 status;
> +
> +   status = readl(&csu_base->status);
> +   if (status & (BIT(0) | BIT(1))) {
> +   printf("Secure Boot:\t%s%s\n",
> +  status & BIT(0) ? "authenticated" : "not 
> authenticated",
> +  status & BIT(1) ? ", encrypted" : ", not encrypted");
> +   }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
>
> @@ -391,8 +403,10 @@ int board_init(void)
> fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>
> -   if (current_el() == 3)
> +   if (current_el() == 3) {
> multi_boot();
> +   secure_boot();
> +   }
>
> return 0;
>  }
> --
> 2.31.1
>


--
Best regards
Oleksandr

Oleksandr Suvorov
cryo...@gmail.com


Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-10-05 Thread Michal Simek



On 10/5/21 1:13 PM, Jorge Ramirez-Ortiz wrote:
> Confirm the secure boot configuration on the console.
> 
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT 0
>  
>  struct csu_regs {
> - u32 reserved0[4];
> + u32 status;
> + u32 reserved0[3];
>   u32 multi_boot;
>   u32 reserved1[11];
>   u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
>   return 0;
>  }
>  
> +static void secure_boot(void)
> +{
> + u32 status;
> +
> + status = readl(&csu_base->status);

I would prefer to use zynqmp_mmio_read instead to make sure that we can
call this function also from regular u-boot not running in EL3.
For SPL it will be just readl what you have here too.


> + if (status & (BIT(0) | BIT(1))) {
> + printf("Secure Boot:\t%s%s\n",
> +status & BIT(0) ? "authenticated" : "not authenticated",
> +status & BIT(1) ? ", encrypted" : ", not encrypted");

It is pretty much visible that instead of BIT(X) you should use macros.

> + }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL 0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG 0xFFA50914
>  
> @@ -391,8 +403,10 @@ int board_init(void)
>   fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>  
> - if (current_el() == 3)
> + if (current_el() == 3) {
>   multi_boot();

This code has changed a little bit. Please use the latest u-boot version.

> + secure_boot();

Is it useful to get information only for SPL in EL3? Just asking.

> + }
>  
>   return 0;
>  }
> 

Thanks,
Michal


Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-07-22 Thread Igor Opaniuk
Hi Jorge,

On Thu, Jul 22, 2021 at 2:19 PM Jorge Ramirez-Ortiz  wrote:
>
> Confirm the secure boot configuration on the console.
>
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT   0
>
>  struct csu_regs {
> -   u32 reserved0[4];
> +   u32 status;
> +   u32 reserved0[3];
> u32 multi_boot;
> u32 reserved1[11];
> u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
> return 0;
>  }
>
> +static void secure_boot(void)
> +{
> +   u32 status;
> +
> +   status = readl(&csu_base->status);
> +   if (status & (BIT(0) | BIT(1))) {
> +   printf("Secure Boot:\t%s%s\n",
> +  status & BIT(0) ? "authenticated" : "not 
> authenticated",
> +  status & BIT(1) ? ", encrypted" : ", not encrypted");
> +   }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
>
> @@ -391,8 +403,10 @@ int board_init(void)
> fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>
> -   if (current_el() == 3)
> +   if (current_el() == 3) {
> multi_boot();
> +   secure_boot();
> +   }
>
> return 0;
>  }
> --
> 2.31.1
>

Reviewed-by: Igor Opaniuk 

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opan...@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk


Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-08-11 Thread Jorge Ramirez-Ortiz, Foundries
On 22/07/21, Jorge Ramirez-Ortiz wrote:

reminder

> Confirm the secure boot configuration on the console.
> 
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT 0
>  
>  struct csu_regs {
> - u32 reserved0[4];
> + u32 status;
> + u32 reserved0[3];
>   u32 multi_boot;
>   u32 reserved1[11];
>   u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
>   return 0;
>  }
>  
> +static void secure_boot(void)
> +{
> + u32 status;
> +
> + status = readl(&csu_base->status);
> + if (status & (BIT(0) | BIT(1))) {
> + printf("Secure Boot:\t%s%s\n",
> +status & BIT(0) ? "authenticated" : "not authenticated",
> +status & BIT(1) ? ", encrypted" : ", not encrypted");
> + }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL 0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG 0xFFA50914
>  
> @@ -391,8 +403,10 @@ int board_init(void)
>   fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>  
> - if (current_el() == 3)
> + if (current_el() == 3) {
>   multi_boot();
> + secure_boot();
> + }
>  
>   return 0;
>  }
> -- 
> 2.31.1
> 


Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-08-11 Thread Michal Simek



On 7/22/21 1:19 PM, Jorge Ramirez-Ortiz wrote:
> Confirm the secure boot configuration on the console.
> 
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
>  board/xilinx/zynqmp/zynqmp.c | 16 +++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..3d3ffa086e 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -139,7 +139,8 @@ struct apu_regs {
>  #define ZYNQMP_SILICON_VER_SHIFT 0
>  
>  struct csu_regs {
> - u32 reserved0[4];
> + u32 status;
> + u32 reserved0[3];
>   u32 multi_boot;
>   u32 reserved1[11];
>   u32 idcode;
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..b7d11630d1 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,18 @@ static int multi_boot(void)
>   return 0;
>  }
>  
> +static void secure_boot(void)
> +{
> + u32 status;
> +
> + status = readl(&csu_base->status);
> + if (status & (BIT(0) | BIT(1))) {

please create macros for these bits.

{} around are not needed.


> + printf("Secure Boot:\t%s%s\n",
> +status & BIT(0) ? "authenticated" : "not authenticated",
> +status & BIT(1) ? ", encrypted" : ", not encrypted");

Isn't this more space efficient?
printf("Secure Boot:\t%sauthenticated, %sencrypted\n",
   status & BIT(0) ? "" : "not ",
   status & BIT(1) ? "" : "not ");

And as I see it is.
aarch64: (for 1/1 boards) all -33.0 rodata -17.0 spl/u-boot-spl:all
-33.0 spl/u-boot-spl:rodata -17.0 spl/u-boot-spl:text -16.0 text -16.0
xilinx_zynqmp_virt: all -33 rodata -17 spl/u-boot-spl:all
-33 spl/u-boot-spl:rodata -17 spl/u-boot-spl:text -16 text -16
   spl-u-boot-spl: add: 0/0, grow: 0/-1 bytes: 0/-16 (-16)


> + }
> +}
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL 0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG 0xFFA50914
>  
> @@ -391,8 +403,10 @@ int board_init(void)
>   fpga_add(fpga_xilinx, &zynqmppl);
>  #endif
>  
> - if (current_el() == 3)
> + if (current_el() == 3) {
>   multi_boot();
> + secure_boot();
> + }

Please take a look at
https://lists.denx.de/pipermail/u-boot/2021-July/456382.html
I have changed multi_boot function a little bit.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



Re: [PATCH] arm64: zynqmp: Print the secure boot status information in EL3

2021-08-27 Thread Jorge Ramirez-Ortiz, Foundries
On 12/08/21, Michal Simek wrote:
> 
> 
> On 7/22/21 1:19 PM, Jorge Ramirez-Ortiz wrote:
> > Confirm the secure boot configuration on the console.
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz 
> > ---
> >  arch/arm/mach-zynqmp/include/mach/hardware.h |  3 ++-
> >  board/xilinx/zynqmp/zynqmp.c | 16 +++-
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> > b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > index 3776499070..3d3ffa086e 100644
> > --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> > +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > @@ -139,7 +139,8 @@ struct apu_regs {
> >  #define ZYNQMP_SILICON_VER_SHIFT   0
> >  
> >  struct csu_regs {
> > -   u32 reserved0[4];
> > +   u32 status;
> > +   u32 reserved0[3];
> > u32 multi_boot;
> > u32 reserved1[11];
> > u32 idcode;
> > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > index 1748fec2e4..b7d11630d1 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -355,6 +355,18 @@ static int multi_boot(void)
> > return 0;
> >  }
> >  
> > +static void secure_boot(void)
> > +{
> > +   u32 status;
> > +
> > +   status = readl(&csu_base->status);
> > +   if (status & (BIT(0) | BIT(1))) {
> 
> please create macros for these bits.

ok
> 
> {} around are not needed.

yep

> 
> 
> > +   printf("Secure Boot:\t%s%s\n",
> > +  status & BIT(0) ? "authenticated" : "not authenticated",
> > +  status & BIT(1) ? ", encrypted" : ", not encrypted");
> 
> Isn't this more space efficient?
> printf("Secure Boot:\t%sauthenticated, %sencrypted\n",
>  status & BIT(0) ? "" : "not ",
>  status & BIT(1) ? "" : "not ");
> 
> And as I see it is.
> aarch64: (for 1/1 boards) all -33.0 rodata -17.0 spl/u-boot-spl:all
> -33.0 spl/u-boot-spl:rodata -17.0 spl/u-boot-spl:text -16.0 text -16.0
> xilinx_zynqmp_virt: all -33 rodata -17 spl/u-boot-spl:all
> -33 spl/u-boot-spl:rodata -17 spl/u-boot-spl:text -16 text -16
>spl-u-boot-spl: add: 0/0, grow: 0/-1 bytes: 0/-16 (-16)
> 
>

ok with me

> > +   }
> > +}
> > +
> >  #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
> >  #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
> >  
> > @@ -391,8 +403,10 @@ int board_init(void)
> > fpga_add(fpga_xilinx, &zynqmppl);
> >  #endif
> >  
> > -   if (current_el() == 3)
> > +   if (current_el() == 3) {
> > multi_boot();
> > +   secure_boot();
> > +   }
> 
> Please take a look at
> https://lists.denx.de/pipermail/u-boot/2021-July/456382.html
> I have changed multi_boot function a little bit.

ok

> 
> Thanks,
> Michal
> 
> -- 
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
>