[U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
i.MX6SX ROM implements unified table sections. The HAB function table is at offset 0x100. Update the HAB function pointers accordingly.
Signed-off-by: Nitin Garg nitin.garg@freescale.com Tested-by: Fabio Estevam fabio.estevam@freescale.com
---
arch/arm/include/asm/arch-mx6/hab.h | 16 +++++++++++----- include/configs/mx6_common.h | 4 ++++ 2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6/hab.h index 1f12695..c53709b 100644 --- a/arch/arm/include/asm/arch-mx6/hab.h +++ b/arch/arm/include/asm/arch-mx6/hab.h @@ -53,11 +53,17 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_t, void **, size_t *, hab_loader_callback_f_t); typedef void hapi_clock_init_t(void);
-#define HAB_RVT_REPORT_EVENT (*(uint32_t *)0x000000B4) -#define HAB_RVT_REPORT_STATUS (*(uint32_t *)0x000000B8) -#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)0x000000A4) -#define HAB_RVT_ENTRY (*(uint32_t *)0x00000098) -#define HAB_RVT_EXIT (*(uint32_t *)0x0000009C) +#ifdef CONFIG_ROM_UNIFIED_SECTIONS +#define HAB_RVT_BASE 0x00000100 +#else +#define HAB_RVT_BASE 0x00000094 +#endif + +#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE + 0x04)) +#define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08)) +#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10)) +#define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20)) +#define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE + 0x24))
#define HAB_RVT_REPORT_EVENT_NEW (*(uint32_t *)0x000000B8) #define HAB_RVT_REPORT_STATUS_NEW (*(uint32_t *)0x000000BC) diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h index 135a3f5..824e73f 100644 --- a/include/configs/mx6_common.h +++ b/include/configs/mx6_common.h @@ -30,4 +30,8 @@
#define CONFIG_MP
+#ifdef CONFIG_MX6SX +#define CONFIG_ROM_UNIFIED_SECTIONS +#endif + #endif
On Tue, Sep 30, 2014 at 12:40 PM, Nitin Garg nitin.garg@freescale.com wrote:
i.MX6SX ROM implements unified table sections. The HAB function table is at offset 0x100. Update the HAB function pointers accordingly.
Signed-off-by: Nitin Garg nitin.garg@freescale.com Tested-by: Fabio Estevam fabio.estevam@freescale.com
You should have marked this patch as v2 and described the changes below the --- line.
Thanks
Hi Nitin, hi Fabio,
thanks for immediately fix the issue - I have some comments.
On 30/09/2014 17:40, Nitin Garg wrote:
i.MX6SX ROM implements unified table sections. The HAB function table is at offset 0x100. Update the HAB function pointers accordingly.
Signed-off-by: Nitin Garg nitin.garg@freescale.com Tested-by: Fabio Estevam fabio.estevam@freescale.com
arch/arm/include/asm/arch-mx6/hab.h | 16 +++++++++++----- include/configs/mx6_common.h | 4 ++++ 2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6/hab.h index 1f12695..c53709b 100644 --- a/arch/arm/include/asm/arch-mx6/hab.h +++ b/arch/arm/include/asm/arch-mx6/hab.h @@ -53,11 +53,17 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_t, void **, size_t *, hab_loader_callback_f_t); typedef void hapi_clock_init_t(void);
-#define HAB_RVT_REPORT_EVENT (*(uint32_t *)0x000000B4) -#define HAB_RVT_REPORT_STATUS (*(uint32_t *)0x000000B8) -#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)0x000000A4) -#define HAB_RVT_ENTRY (*(uint32_t *)0x00000098) -#define HAB_RVT_EXIT (*(uint32_t *)0x0000009C) +#ifdef CONFIG_ROM_UNIFIED_SECTIONS +#define HAB_RVT_BASE 0x00000100 +#else +#define HAB_RVT_BASE 0x00000094 +#endif
It would be nice to understand how the choice is done. In my understanding, even HAB_RVT_REPORT_EVENT_NEW and HAB_RVT_REPORT_STATUS_NEW can be dropped and substituted by an offset.
I see then three cases:
mx6sx HAB_RVT_BASE = 0x00000100
mx6q with soc_rev() > 1.5 HAB_RVT_BASE = 0x0000098 or mx6dl with soc_rev > 1.2
mx6q with soc_rev() < 1.5 HAB_RVT_BASE = 0x0000094 or mx6d with soc_rev() < 1.2
then the single entries can be calculated from the base address.
+#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE + 0x04)) +#define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08)) +#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10)) +#define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20)) +#define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE + 0x24))
#define HAB_RVT_REPORT_EVENT_NEW (*(uint32_t *)0x000000B8) #define HAB_RVT_REPORT_STATUS_NEW (*(uint32_t *)0x000000BC) diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h index 135a3f5..824e73f 100644 --- a/include/configs/mx6_common.h +++ b/include/configs/mx6_common.h @@ -30,4 +30,8 @@
#define CONFIG_MP
+#ifdef CONFIG_MX6SX +#define CONFIG_ROM_UNIFIED_SECTIONS
I do not like this approach because we do not need an additional CONFIG_, that remains undocumented. If CONFIG_MX6SX, HAB_RVT_BASE is always 0x100 - CONFIG_ROM_UNIFIED_SECTIONS is like a redundant information, because it is not possible (or is it ?) to have a sx processor with a different base address.
Best regards, Stefano Babic
Hi Stefano,
On Tue, Sep 30, 2014 at 1:08 PM, Stefano Babic sbabic@denx.de wrote:
I do not like this approach because we do not need an additional CONFIG_, that remains undocumented. If CONFIG_MX6SX, HAB_RVT_BASE is always 0x100 - CONFIG_ROM_UNIFIED_SECTIONS is like a redundant information, because it is not possible (or is it ?) to have a sx processor with a different base address.
Yes, I agree that we should avoid introducing a new config like CONFIG_ROM_UNIFIED_SECTIONS.
What about this?
diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6 index 1f12695..c9e5318 100644 --- a/arch/arm/include/asm/arch-mx6/hab.h +++ b/arch/arm/include/asm/arch-mx6/hab.h @@ -53,11 +53,17 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_ void **, size_t *, hab_loader_callback_f_t); typedef void hapi_clock_init_t(void);
-#define HAB_RVT_REPORT_EVENT (*(uint32_t *)0x000000B4) -#define HAB_RVT_REPORT_STATUS (*(uint32_t *)0x000000B8) -#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)0x000000A4) -#define HAB_RVT_ENTRY (*(uint32_t *)0x00000098) -#define HAB_RVT_EXIT (*(uint32_t *)0x0000009C) +#ifdef CONFIG_MX6SX +#define HAB_RVT_BASE 0x00000100 +#else +#define HAB_RVT_BASE 0x00000094 +#endif + +#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE + 0x04)) +#define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08)) +#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10)) +#define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20)) +#define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE + 0x24))
#define HAB_RVT_REPORT_EVENT_NEW (*(uint32_t *)0x000000B8) #define HAB_RVT_REPORT_STATUS_NEW (*(uint32_t *)0x000000BC)
Hi Fabio,
On 30/09/2014 18:31, Fabio Estevam wrote:
Hi Stefano,
On Tue, Sep 30, 2014 at 1:08 PM, Stefano Babic sbabic@denx.de wrote:
I do not like this approach because we do not need an additional CONFIG_, that remains undocumented. If CONFIG_MX6SX, HAB_RVT_BASE is always 0x100 - CONFIG_ROM_UNIFIED_SECTIONS is like a redundant information, because it is not possible (or is it ?) to have a sx processor with a different base address.
Yes, I agree that we should avoid introducing a new config like CONFIG_ROM_UNIFIED_SECTIONS.
What about this?
diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6 index 1f12695..c9e5318 100644 --- a/arch/arm/include/asm/arch-mx6/hab.h +++ b/arch/arm/include/asm/arch-mx6/hab.h @@ -53,11 +53,17 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t, ptrdiff_ void **, size_t *, hab_loader_callback_f_t); typedef void hapi_clock_init_t(void);
-#define HAB_RVT_REPORT_EVENT (*(uint32_t *)0x000000B4) -#define HAB_RVT_REPORT_STATUS (*(uint32_t *)0x000000B8) -#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)0x000000A4) -#define HAB_RVT_ENTRY (*(uint32_t *)0x00000098) -#define HAB_RVT_EXIT (*(uint32_t *)0x0000009C) +#ifdef CONFIG_MX6SX +#define HAB_RVT_BASE 0x00000100 +#else +#define HAB_RVT_BASE 0x00000094 +#endif
+#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE + 0x04)) +#define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08)) +#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10)) +#define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20)) +#define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE + 0x24))
#define HAB_RVT_REPORT_EVENT_NEW (*(uint32_t *)0x000000B8) #define HAB_RVT_REPORT_STATUS_NEW (*(uint32_t *)0x000000BC)
ok - I have seen V3, I will apply it.
Regards, Stefano
On Tue, Sep 30, 2014 at 2:43 PM, Stefano Babic sbabic@denx.de wrote:
ok - I have seen V3, I will apply it.
Thanks, Stefano!
participants (3)
-
Fabio Estevam -
Nitin Garg -
Stefano Babic