[PATCH] zynqmp: restore the jtag interface

2021-10-13 Thread Jorge Ramirez-Ortiz
When boot.bin is configured for secure boot the CSU will disable the
JTAG interface on all cases.

Some boards might rely on this interface for flashing to QSPI in which
case those systems might end up bricked during development.

This commit will restore the interface under CSU control

Signed-off-by: Jorge Ramirez-Ortiz 
---
 arch/arm/mach-zynqmp/Kconfig |  8 +
 arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++-
 board/xilinx/zynqmp/zynqmp.c | 20 -
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index f7b08db355..ee0895d9a2 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
  Overwrite bootmode selected via boot mode pins to tell SPL what should
  be the next boot device.
 
+config SPL_ZYNQMP_RESTORE_JTAG
+   bool "Restore JTAG"
+   depends on SPL
+   help
+Booting SPL in secure mode causes the CSU to disable the JTAG interface
+even if no eFuses were burnt. This option restores the interface if
+possible.
+
 config ZYNQ_SDHCI_MAX_FREQ
default 2
 
diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index eebf38551c..e6a3ee4a57 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -39,20 +39,26 @@
 #define RESET_REASON_INTERNAL  BIT(1)
 #define RESET_REASON_EXTERNAL  BIT(0)
 
+#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK  0x01002002
+#define CRLAPB_RST_LPD_DBG_RESET   0
+
 struct crlapb_regs {
u32 reserved0[36];
u32 cpu_r5_ctrl; /* 0x90 */
-   u32 reserved1[37];
+   u32 reserved1[7];
+   u32 dbg_lpd_ctrl; /* 0xB0 */
+   u32 reserved2[29];
u32 timestamp_ref_ctrl; /* 0x128 */
-   u32 reserved2[53];
+   u32 reserved3[53];
u32 boot_mode; /* 0x200 */
-   u32 reserved3_0[7];
+   u32 reserved4_0[7];
u32 reset_reason; /* 0x220 */
-   u32 reserved3_1[6];
+   u32 reserved4_1[6];
u32 rst_lpd_top; /* 0x23C */
-   u32 reserved4[4];
+   u32 rst_lpd_dbg; /* 0x240 */
+   u32 reserved5[3];
u32 boot_pin_ctrl; /* 0x250 */
-   u32 reserved5[21];
+   u32 reserved6[21];
 };
 
 #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
@@ -141,12 +147,23 @@ struct apu_regs {
 #define ZYNQMP_SILICON_VER_MASK0xF
 #define ZYNQMP_SILICON_VER_SHIFT   0
 
+#define CSU_JTAG_SEC_GATE_DISABLE  GENMASK(7, 0)
+#define CSU_JTAG_DAP_ENABLE_DEBUG  GENMASK(7, 0)
+#define CSU_JTAG_CHAIN_WR_SETUPGENMASK(1, 0)
+#define CSU_PCAP_PROG_RELEASE_PL   BIT(0)
+
 struct csu_regs {
u32 reserved0[4];
u32 multi_boot;
-   u32 reserved1[11];
+   u32 reserved1[7];
+   u32 jtag_chain_status_wr;
+   u32 jtag_chain_status;
+   u32 jtag_sec;
+   u32 jtag_dap_cfg;
u32 idcode;
u32 version;
+   u32 reserved2[3055];
+   u32 pcap_prog;
 };
 
 #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 000a7cde8d..483f0b9ab2 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -358,6 +358,21 @@ static int multi_boot(void)
return multiboot;
 }
 
+#if defined(CONFIG_SPL_BUILD)
+static void restore_jtag(void)
+{
+   if (current_el() != 3)
+   return;
+
+   writel(CSU_JTAG_SEC_GATE_DISABLE, _base->jtag_sec);
+   writel(CSU_JTAG_DAP_ENABLE_DEBUG, _base->jtag_dap_cfg);
+   writel(CSU_JTAG_CHAIN_WR_SETUP, _base->jtag_chain_status_wr);
+   writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, _base->dbg_lpd_ctrl);
+   writel(CRLAPB_RST_LPD_DBG_RESET, _base->rst_lpd_dbg);
+   writel(CSU_PCAP_PROG_RELEASE_PL, _base->pcap_prog);
+}
+#endif
+
 #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
 #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
 
@@ -377,11 +392,14 @@ int board_init(void)
zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
zynqmp_pm_cfg_obj_size);
printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
+
+   /* the CSU disables the JTAG interface when secure boot is enabled */
+   if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
+   restore_jtag();
 #else
if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
xilinx_read_eeprom();
 #endif
-
printf("EL Level:\tEL%d\n", current_el());
 
/* Bug in ROM sets wrong value in this register */
-- 
2.31.1



Re: [PATCH] zynqmp: restore the jtag interface

2021-08-27 Thread Michal Simek



On 8/27/21 11:17 AM, Jorge Ramirez-Ortiz, Foundries wrote:
> On 12/08/21, Michal Simek wrote:
>>
>>
>> On 7/16/21 7:16 PM, Jorge Ramirez-Ortiz wrote:
>>> As a security feature, if boot.bin was configured for secure boot the
>>> CSU will disable the JTAG interface on all cases.
>>>
>>> Some boards might rely on this interface for flashing to QSPI in which
>>> case those systems might end up bricked during development.
>>>
>>> This commit attempts to restore the interface - if the CSU allows for it.
>>
>> sorry I missed this.
>>
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz 
>>> ---
>>>  arch/arm/mach-zynqmp/Kconfig |  9 
>>>  arch/arm/mach-zynqmp/include/mach/hardware.h | 23 +--
>>>  board/xilinx/zynqmp/zynqmp.c | 24 
>>>  3 files changed, 49 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
>>> index 39144d654e..1e551c0020 100644
>>> --- a/arch/arm/mach-zynqmp/Kconfig
>>> +++ b/arch/arm/mach-zynqmp/Kconfig
>>> @@ -149,6 +149,15 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
>>>   Overwrite bootmode selected via boot mode pins to tell SPL what should
>>>   be the next boot device.
>>>  
>>> +config SPL_ZYNQMP_RESTORE_JTAG
>>> +   bool "Restore JTAG"
>>> +   depends on SPL
>>> +   default n
>>
>> no need for default n - that's default option anyway.
> 
> ok
> 
>>
>>> +   help
>>> +Booting SPL in secure mode causes the CSU to disable the JTAG interface
>>> +even if no eFuses were burnt. This option restores the interface if
>>> +possible.
>>
>> Indentation is weird. Please look at tabs/spaces.
> 
> ok
> 
>>
>>> +
>>>  config ZYNQ_SDHCI_MAX_FREQ
>>> default 2
>>>  
>>> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
>>> b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> index 3776499070..58822e3f25 100644
>>> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> @@ -42,17 +42,20 @@
>>>  struct crlapb_regs {
>>> u32 reserved0[36];
>>> u32 cpu_r5_ctrl; /* 0x90 */
>>> -   u32 reserved1[37];
>>> +   u32 reserved1[7];
>>> +   u32 dbg_lpd_ctrl; /* 0xB0 */
>>> +   u32 reserved2[29];
>>> u32 timestamp_ref_ctrl; /* 0x128 */
>>> -   u32 reserved2[53];
>>> +   u32 reserved3[53];
>>> u32 boot_mode; /* 0x200 */
>>> -   u32 reserved3_0[7];
>>> +   u32 reserved4_0[7];
>>> u32 reset_reason; /* 0x220 */
>>> -   u32 reserved3_1[6];
>>> +   u32 reserved4_1[6];
>>> u32 rst_lpd_top; /* 0x23C */
>>> -   u32 reserved4[4];
>>> +   u32 rst_lpd_dbg; /* 0x240 */
>>> +   u32 reserved5[3];
>>> u32 boot_pin_ctrl; /* 0x250 */
>>> -   u32 reserved5[21];
>>> +   u32 reserved6[21];
>>>  };
>>>  
>>>  #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
>>> @@ -141,9 +144,15 @@ struct apu_regs {
>>>  struct csu_regs {
>>> u32 reserved0[4];
>>> u32 multi_boot;
>>> -   u32 reserved1[11];
>>> +   u32 reserved1[7];
>>> +   u32 jtag_chain_status_wr;
>>> +   u32 jtag_chain_status;
>>> +   u32 jtag_sec;
>>> +   u32 jtag_dap_cfg;
>>> u32 idcode;
>>> u32 version;
>>> +   u32 reserved2[3055];
>>> +   u32 pcap_prog;
>>>  };
>>>  
>>>  #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index 1748fec2e4..feffda54e7 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -355,6 +355,25 @@ static int multi_boot(void)
>>> return 0;
>>>  }
>>>  
>>> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
>>> +static void restore_jtag(void)
>>> +{
>>> +   const u32 disable_security_gate = 0xff;
>>> +   const u32 setup_clock = 0x01002002;
>>> +   const u32 setup_jtag = 0x3;
>>> +   const u32 release_pl = 0x1;
>>> +   const u32 enable_debug = 0xff;
>>> +   const u32 do_reset = 0x0;
>>
>> All above should be just macros. Please put them to hardware.h
> 
> you mean something like #define CSU_SETUP_CLOCK 0x01002002?
> I dont really agree with that because we'll lose the context where it
> is being used (and the context is given by the restore jtag function)
> unless we do CSU_RESTORE_JTAG_SETUP_CLOCK 
> is this what you want? 

Macro name is up to you but should register description.

For example yours disable_security_gate
is based on reg database _pmu_sec + _pltap_sec + _dap_sec
together.

#define CSU_JTAG_SEC_GATE_DISABLE   0x1ff

And then you can use directly:

writel(CSU_JTAG_SEC_GATE_DISABLE, _base->jtag_sec);

Thanks,
Michal


Re: [PATCH] zynqmp: restore the jtag interface

2021-08-27 Thread Jorge Ramirez-Ortiz, Foundries
On 12/08/21, Michal Simek wrote:
> 
> 
> On 7/16/21 7:16 PM, Jorge Ramirez-Ortiz wrote:
> > As a security feature, if boot.bin was configured for secure boot the
> > CSU will disable the JTAG interface on all cases.
> > 
> > Some boards might rely on this interface for flashing to QSPI in which
> > case those systems might end up bricked during development.
> > 
> > This commit attempts to restore the interface - if the CSU allows for it.
> 
> sorry I missed this.
> 
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz 
> > ---
> >  arch/arm/mach-zynqmp/Kconfig |  9 
> >  arch/arm/mach-zynqmp/include/mach/hardware.h | 23 +--
> >  board/xilinx/zynqmp/zynqmp.c | 24 
> >  3 files changed, 49 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> > index 39144d654e..1e551c0020 100644
> > --- a/arch/arm/mach-zynqmp/Kconfig
> > +++ b/arch/arm/mach-zynqmp/Kconfig
> > @@ -149,6 +149,15 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
> >   Overwrite bootmode selected via boot mode pins to tell SPL what should
> >   be the next boot device.
> >  
> > +config SPL_ZYNQMP_RESTORE_JTAG
> > +   bool "Restore JTAG"
> > +   depends on SPL
> > +   default n
> 
> no need for default n - that's default option anyway.

ok

> 
> > +   help
> > +Booting SPL in secure mode causes the CSU to disable the JTAG interface
> > +even if no eFuses were burnt. This option restores the interface if
> > +possible.
> 
> Indentation is weird. Please look at tabs/spaces.

ok

> 
> > +
> >  config ZYNQ_SDHCI_MAX_FREQ
> > default 2
> >  
> > diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> > b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > index 3776499070..58822e3f25 100644
> > --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> > +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > @@ -42,17 +42,20 @@
> >  struct crlapb_regs {
> > u32 reserved0[36];
> > u32 cpu_r5_ctrl; /* 0x90 */
> > -   u32 reserved1[37];
> > +   u32 reserved1[7];
> > +   u32 dbg_lpd_ctrl; /* 0xB0 */
> > +   u32 reserved2[29];
> > u32 timestamp_ref_ctrl; /* 0x128 */
> > -   u32 reserved2[53];
> > +   u32 reserved3[53];
> > u32 boot_mode; /* 0x200 */
> > -   u32 reserved3_0[7];
> > +   u32 reserved4_0[7];
> > u32 reset_reason; /* 0x220 */
> > -   u32 reserved3_1[6];
> > +   u32 reserved4_1[6];
> > u32 rst_lpd_top; /* 0x23C */
> > -   u32 reserved4[4];
> > +   u32 rst_lpd_dbg; /* 0x240 */
> > +   u32 reserved5[3];
> > u32 boot_pin_ctrl; /* 0x250 */
> > -   u32 reserved5[21];
> > +   u32 reserved6[21];
> >  };
> >  
> >  #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> > @@ -141,9 +144,15 @@ struct apu_regs {
> >  struct csu_regs {
> > u32 reserved0[4];
> > u32 multi_boot;
> > -   u32 reserved1[11];
> > +   u32 reserved1[7];
> > +   u32 jtag_chain_status_wr;
> > +   u32 jtag_chain_status;
> > +   u32 jtag_sec;
> > +   u32 jtag_dap_cfg;
> > u32 idcode;
> > u32 version;
> > +   u32 reserved2[3055];
> > +   u32 pcap_prog;
> >  };
> >  
> >  #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > index 1748fec2e4..feffda54e7 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -355,6 +355,25 @@ static int multi_boot(void)
> > return 0;
> >  }
> >  
> > +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> > +static void restore_jtag(void)
> > +{
> > +   const u32 disable_security_gate = 0xff;
> > +   const u32 setup_clock = 0x01002002;
> > +   const u32 setup_jtag = 0x3;
> > +   const u32 release_pl = 0x1;
> > +   const u32 enable_debug = 0xff;
> > +   const u32 do_reset = 0x0;
> 
> All above should be just macros. Please put them to hardware.h

you mean something like #define CSU_SETUP_CLOCK 0x01002002?
I dont really agree with that because we'll lose the context where it
is being used (and the context is given by the restore jtag function)
unless we do CSU_RESTORE_JTAG_SETUP_CLOCK 
is this what you want? 

> 
> > +
> > +   writel(disable_security_gate, _base->jtag_sec);
> > +   writel(enable_debug, _base->jtag_dap_cfg);
> > +   writel(setup_jtag, _base->jtag_chain_status_wr);
> > +   writel(setup_clock, _base->dbg_lpd_ctrl);
> > +   writel(do_reset, _base->rst_lpd_dbg);
> > +   writel(release_pl, _base->pcap_prog);
> > +}
> > +#endif
> > +
> >  #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
> >  #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
> >  
> > @@ -374,6 +393,11 @@ int board_init(void)
> > zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
> > zynqmp_pm_cfg_obj_size);
> > printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> > +
> > +#if defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> > +   /* the CSU disables the JTAG 

Re: [PATCH] zynqmp: restore the jtag interface

2021-08-11 Thread Michal Simek



On 7/16/21 7:16 PM, Jorge Ramirez-Ortiz wrote:
> As a security feature, if boot.bin was configured for secure boot the
> CSU will disable the JTAG interface on all cases.
> 
> Some boards might rely on this interface for flashing to QSPI in which
> case those systems might end up bricked during development.
> 
> This commit attempts to restore the interface - if the CSU allows for it.

sorry I missed this.

> 
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/Kconfig |  9 
>  arch/arm/mach-zynqmp/include/mach/hardware.h | 23 +--
>  board/xilinx/zynqmp/zynqmp.c | 24 
>  3 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index 39144d654e..1e551c0020 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -149,6 +149,15 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
> Overwrite bootmode selected via boot mode pins to tell SPL what should
> be the next boot device.
>  
> +config SPL_ZYNQMP_RESTORE_JTAG
> +   bool "Restore JTAG"
> +   depends on SPL
> +   default n

no need for default n - that's default option anyway.

> +   help
> +  Booting SPL in secure mode causes the CSU to disable the JTAG interface
> +  even if no eFuses were burnt. This option restores the interface if
> +  possible.

Indentation is weird. Please look at tabs/spaces.

> +
>  config ZYNQ_SDHCI_MAX_FREQ
>   default 2
>  
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..58822e3f25 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -42,17 +42,20 @@
>  struct crlapb_regs {
>   u32 reserved0[36];
>   u32 cpu_r5_ctrl; /* 0x90 */
> - u32 reserved1[37];
> + u32 reserved1[7];
> + u32 dbg_lpd_ctrl; /* 0xB0 */
> + u32 reserved2[29];
>   u32 timestamp_ref_ctrl; /* 0x128 */
> - u32 reserved2[53];
> + u32 reserved3[53];
>   u32 boot_mode; /* 0x200 */
> - u32 reserved3_0[7];
> + u32 reserved4_0[7];
>   u32 reset_reason; /* 0x220 */
> - u32 reserved3_1[6];
> + u32 reserved4_1[6];
>   u32 rst_lpd_top; /* 0x23C */
> - u32 reserved4[4];
> + u32 rst_lpd_dbg; /* 0x240 */
> + u32 reserved5[3];
>   u32 boot_pin_ctrl; /* 0x250 */
> - u32 reserved5[21];
> + u32 reserved6[21];
>  };
>  
>  #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> @@ -141,9 +144,15 @@ struct apu_regs {
>  struct csu_regs {
>   u32 reserved0[4];
>   u32 multi_boot;
> - u32 reserved1[11];
> + u32 reserved1[7];
> + u32 jtag_chain_status_wr;
> + u32 jtag_chain_status;
> + u32 jtag_sec;
> + u32 jtag_dap_cfg;
>   u32 idcode;
>   u32 version;
> + u32 reserved2[3055];
> + u32 pcap_prog;
>  };
>  
>  #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..feffda54e7 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,25 @@ static int multi_boot(void)
>   return 0;
>  }
>  
> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> +static void restore_jtag(void)
> +{
> + const u32 disable_security_gate = 0xff;
> + const u32 setup_clock = 0x01002002;
> + const u32 setup_jtag = 0x3;
> + const u32 release_pl = 0x1;
> + const u32 enable_debug = 0xff;
> + const u32 do_reset = 0x0;

All above should be just macros. Please put them to hardware.h

> +
> + writel(disable_security_gate, _base->jtag_sec);
> + writel(enable_debug, _base->jtag_dap_cfg);
> + writel(setup_jtag, _base->jtag_chain_status_wr);
> + writel(setup_clock, _base->dbg_lpd_ctrl);
> + writel(do_reset, _base->rst_lpd_dbg);
> + writel(release_pl, _base->pcap_prog);
> +}
> +#endif
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL 0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG 0xFFA50914
>  
> @@ -374,6 +393,11 @@ int board_init(void)
>   zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
>   zynqmp_pm_cfg_obj_size);
>   printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> +
> +#if defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> + /* the CSU disables the JTAG interface when secure boot is enabled */
> + restore_jtag();
> +#endif

As checkpatch suggests
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
where possible


>  #else
>   if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
>   xilinx_read_eeprom();
> 

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze

Re: [PATCH] zynqmp: restore the jtag interface

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

reminder


> As a security feature, if boot.bin was configured for secure boot the
> CSU will disable the JTAG interface on all cases.
> 
> Some boards might rely on this interface for flashing to QSPI in which
> case those systems might end up bricked during development.
> 
> This commit attempts to restore the interface - if the CSU allows for it.
> 
> Signed-off-by: Jorge Ramirez-Ortiz 
> ---
>  arch/arm/mach-zynqmp/Kconfig |  9 
>  arch/arm/mach-zynqmp/include/mach/hardware.h | 23 +--
>  board/xilinx/zynqmp/zynqmp.c | 24 
>  3 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index 39144d654e..1e551c0020 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -149,6 +149,15 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
> Overwrite bootmode selected via boot mode pins to tell SPL what should
> be the next boot device.
>  
> +config SPL_ZYNQMP_RESTORE_JTAG
> +   bool "Restore JTAG"
> +   depends on SPL
> +   default n
> +   help
> +  Booting SPL in secure mode causes the CSU to disable the JTAG interface
> +  even if no eFuses were burnt. This option restores the interface if
> +  possible.
> +
>  config ZYNQ_SDHCI_MAX_FREQ
>   default 2
>  
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
> b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index 3776499070..58822e3f25 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -42,17 +42,20 @@
>  struct crlapb_regs {
>   u32 reserved0[36];
>   u32 cpu_r5_ctrl; /* 0x90 */
> - u32 reserved1[37];
> + u32 reserved1[7];
> + u32 dbg_lpd_ctrl; /* 0xB0 */
> + u32 reserved2[29];
>   u32 timestamp_ref_ctrl; /* 0x128 */
> - u32 reserved2[53];
> + u32 reserved3[53];
>   u32 boot_mode; /* 0x200 */
> - u32 reserved3_0[7];
> + u32 reserved4_0[7];
>   u32 reset_reason; /* 0x220 */
> - u32 reserved3_1[6];
> + u32 reserved4_1[6];
>   u32 rst_lpd_top; /* 0x23C */
> - u32 reserved4[4];
> + u32 rst_lpd_dbg; /* 0x240 */
> + u32 reserved5[3];
>   u32 boot_pin_ctrl; /* 0x250 */
> - u32 reserved5[21];
> + u32 reserved6[21];
>  };
>  
>  #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> @@ -141,9 +144,15 @@ struct apu_regs {
>  struct csu_regs {
>   u32 reserved0[4];
>   u32 multi_boot;
> - u32 reserved1[11];
> + u32 reserved1[7];
> + u32 jtag_chain_status_wr;
> + u32 jtag_chain_status;
> + u32 jtag_sec;
> + u32 jtag_dap_cfg;
>   u32 idcode;
>   u32 version;
> + u32 reserved2[3055];
> + u32 pcap_prog;
>  };
>  
>  #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 1748fec2e4..feffda54e7 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -355,6 +355,25 @@ static int multi_boot(void)
>   return 0;
>  }
>  
> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> +static void restore_jtag(void)
> +{
> + const u32 disable_security_gate = 0xff;
> + const u32 setup_clock = 0x01002002;
> + const u32 setup_jtag = 0x3;
> + const u32 release_pl = 0x1;
> + const u32 enable_debug = 0xff;
> + const u32 do_reset = 0x0;
> +
> + writel(disable_security_gate, _base->jtag_sec);
> + writel(enable_debug, _base->jtag_dap_cfg);
> + writel(setup_jtag, _base->jtag_chain_status_wr);
> + writel(setup_clock, _base->dbg_lpd_ctrl);
> + writel(do_reset, _base->rst_lpd_dbg);
> + writel(release_pl, _base->pcap_prog);
> +}
> +#endif
> +
>  #define PS_SYSMON_ANALOG_BUS_VAL 0x3210
>  #define PS_SYSMON_ANALOG_BUS_REG 0xFFA50914
>  
> @@ -374,6 +393,11 @@ int board_init(void)
>   zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
>   zynqmp_pm_cfg_obj_size);
>   printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> +
> +#if defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
> + /* the CSU disables the JTAG interface when secure boot is enabled */
> + restore_jtag();
> +#endif
>  #else
>   if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
>   xilinx_read_eeprom();
> -- 
> 2.31.1
> 


[PATCH] zynqmp: restore the jtag interface

2021-07-16 Thread Jorge Ramirez-Ortiz
As a security feature, if boot.bin was configured for secure boot the
CSU will disable the JTAG interface on all cases.

Some boards might rely on this interface for flashing to QSPI in which
case those systems might end up bricked during development.

This commit attempts to restore the interface - if the CSU allows for it.

Signed-off-by: Jorge Ramirez-Ortiz 
---
 arch/arm/mach-zynqmp/Kconfig |  9 
 arch/arm/mach-zynqmp/include/mach/hardware.h | 23 +--
 board/xilinx/zynqmp/zynqmp.c | 24 
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index 39144d654e..1e551c0020 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -149,6 +149,15 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
  Overwrite bootmode selected via boot mode pins to tell SPL what should
  be the next boot device.
 
+config SPL_ZYNQMP_RESTORE_JTAG
+   bool "Restore JTAG"
+   depends on SPL
+   default n
+   help
+Booting SPL in secure mode causes the CSU to disable the JTAG interface
+even if no eFuses were burnt. This option restores the interface if
+possible.
+
 config ZYNQ_SDHCI_MAX_FREQ
default 2
 
diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h 
b/arch/arm/mach-zynqmp/include/mach/hardware.h
index 3776499070..58822e3f25 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -42,17 +42,20 @@
 struct crlapb_regs {
u32 reserved0[36];
u32 cpu_r5_ctrl; /* 0x90 */
-   u32 reserved1[37];
+   u32 reserved1[7];
+   u32 dbg_lpd_ctrl; /* 0xB0 */
+   u32 reserved2[29];
u32 timestamp_ref_ctrl; /* 0x128 */
-   u32 reserved2[53];
+   u32 reserved3[53];
u32 boot_mode; /* 0x200 */
-   u32 reserved3_0[7];
+   u32 reserved4_0[7];
u32 reset_reason; /* 0x220 */
-   u32 reserved3_1[6];
+   u32 reserved4_1[6];
u32 rst_lpd_top; /* 0x23C */
-   u32 reserved4[4];
+   u32 rst_lpd_dbg; /* 0x240 */
+   u32 reserved5[3];
u32 boot_pin_ctrl; /* 0x250 */
-   u32 reserved5[21];
+   u32 reserved6[21];
 };
 
 #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
@@ -141,9 +144,15 @@ struct apu_regs {
 struct csu_regs {
u32 reserved0[4];
u32 multi_boot;
-   u32 reserved1[11];
+   u32 reserved1[7];
+   u32 jtag_chain_status_wr;
+   u32 jtag_chain_status;
+   u32 jtag_sec;
+   u32 jtag_dap_cfg;
u32 idcode;
u32 version;
+   u32 reserved2[3055];
+   u32 pcap_prog;
 };
 
 #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 1748fec2e4..feffda54e7 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -355,6 +355,25 @@ static int multi_boot(void)
return 0;
 }
 
+#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
+static void restore_jtag(void)
+{
+   const u32 disable_security_gate = 0xff;
+   const u32 setup_clock = 0x01002002;
+   const u32 setup_jtag = 0x3;
+   const u32 release_pl = 0x1;
+   const u32 enable_debug = 0xff;
+   const u32 do_reset = 0x0;
+
+   writel(disable_security_gate, _base->jtag_sec);
+   writel(enable_debug, _base->jtag_dap_cfg);
+   writel(setup_jtag, _base->jtag_chain_status_wr);
+   writel(setup_clock, _base->dbg_lpd_ctrl);
+   writel(do_reset, _base->rst_lpd_dbg);
+   writel(release_pl, _base->pcap_prog);
+}
+#endif
+
 #define PS_SYSMON_ANALOG_BUS_VAL   0x3210
 #define PS_SYSMON_ANALOG_BUS_REG   0xFFA50914
 
@@ -374,6 +393,11 @@ int board_init(void)
zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
zynqmp_pm_cfg_obj_size);
printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
+
+#if defined(CONFIG_SPL_ZYNQMP_RESTORE_JTAG)
+   /* the CSU disables the JTAG interface when secure boot is enabled */
+   restore_jtag();
+#endif
 #else
if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
xilinx_read_eeprom();
-- 
2.31.1