On Fri, Jan 12, 2024 at 05:29:58PM +0100, Stefan Kerkmann wrote:
> On 12.01.24 16:21, Stefan Kerkmann wrote:
> > The existing habv4 rom vector table had some mismatches in the API of
> > the function pointers which broke calling into the HAB rom - mainly
> > observed with the `report_event` function. The suspected culprit here is
> > the `bytes` pointer which was `uint32_t*` vs. the documented `size_t*`.
> >
> > When compiled using the ILP32 data model e.g. for 32-Bit systems both
> > referrenced values have the same width, but once compiled for (I)LP64
> > they differ as `size_t` is 64-Bit wide there. This seems to trigger a
> > memory corruption once that pointer is passed to the HAB boot rom code
> > and dereferenced there, the root cause wasn't investigated further
> > though.
> >
> > As this implementation has only been tested on imx8mm, imx8nm and imx8mp
> > boards I'm beeing defensive and only enable it for these targets. Once
> > all SOCs of the family have been verified to work correctly the OCRAM
> > readout workaround can be removed.
> >
> > Signed-off-by: Stefan Kerkmann <s.kerkmann@xxxxxxxxxxxxxx>
> > ---
> > drivers/hab/habv4.c | 48 ++++++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 38 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
> > index 92bee8399d..4e401ca9d3 100644
> > --- a/drivers/hab/habv4.c
> > +++ b/drivers/hab/habv4.c
> > @@ -184,6 +184,33 @@ enum hab_sip_cmd {
> > FSL_SIP_HAB_AUTH_IMG_NO_DCD = 0x08,
> > };
> > +static enum hab_status hab_sip_report_event(enum hab_status status,
> > + uint32_t index, uint8_t *event,
> > + size_t *bytes)
> > +{
> > + struct arm_smccc_res res;
> > +
> > + v8_flush_dcache_range((unsigned long)bytes,
> > + (unsigned long)bytes + sizeof(*bytes));
> > +
> > + if (event)
> > + v8_flush_dcache_range((unsigned long)event,
> > + (unsigned long)event + *bytes);
> > +
> > + arm_smccc_smc(FSL_SIP_HAB, FSL_SIP_HAB_REPORT_EVENT,
> > + (unsigned long)index, (unsigned long)event,
> > + (unsigned long)bytes, 0, 0, 0, &res);
> > +
> > + v8_inv_dcache_range((unsigned long)bytes,
> > + (unsigned long)bytes + sizeof(*bytes));
> > +
> > + if (event)
> > + v8_inv_dcache_range((unsigned long)event,
> > + (unsigned long)event + *bytes);
> > +
> > + return (enum hab_status)res.a0;
> > +}
> > +
> > static enum hab_status hab_sip_report_status(enum hab_config *config,
> > enum habv4_state *state)
> > {
> > @@ -220,9 +247,6 @@ static uint32_t hab_sip_get_version(void)
> > #define HABV4_EVENT_MAX_LEN 0x80
> > #define IMX8MQ_ROM_OCRAM_ADDRESS 0x9061C0
> > -#define IMX8MM_ROM_OCRAM_ADDRESS 0x908040
> > -#define IMX8MN_ROM_OCRAM_ADDRESS 0x908040
> > -#define IMX8MP_ROM_OCRAM_ADDRESS 0x90D040
> > static enum hab_status imx8m_read_sram_events(enum hab_status status,
> > uint32_t index, uint8_t *event,
> > @@ -239,12 +263,6 @@ static enum hab_status imx8m_read_sram_events(enum hab_status status,
> > if (cpu_is_mx8mq())
> > sram = (char *)IMX8MQ_ROM_OCRAM_ADDRESS;
> > - else if (cpu_is_mx8mm())
> > - sram = (char *)IMX8MM_ROM_OCRAM_ADDRESS;
> > - else if (cpu_is_mx8mn())
> > - sram = (char *)IMX8MN_ROM_OCRAM_ADDRESS;
> > - else if (cpu_is_mx8mp())
> > - sram = (char *)IMX8MP_ROM_OCRAM_ADDRESS;
> > else
> > return HAB_STATUS_FAILURE;
> > @@ -296,9 +314,19 @@ static enum hab_status imx8m_read_sram_events(enum hab_status status,
> > return HAB_STATUS_FAILURE;
> > }
> > +static enum hab_status imx8m_report_event(enum hab_status status,
> > + uint32_t index, uint8_t *event,
> > + size_t *bytes)
> > +{
> > + if (cpu_is_mx8mm() || cpu_is_mx8mn() || cpu_is_imx8mp())
>
> There is a typo in the condition, that I somehow introduced wrangling
> testing branches. It should be `cpu_is_mx8mp()` not `cpu_is_imx8mp()`
Fixed while applying.
> I'll
> send another series with fix with the upcoming review feedback (which is to
> come as I was told).
I haven't read this part carefully enough. Ok, let's see what happens, I
can replace this series with another one should there be more review
feedback.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
