mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] i.MX: HABv4: Improve HAB event printing
@ 2020-11-23 16:38 Sascha Hauer
  2020-11-24  7:57 ` robin
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2020-11-23 16:38 UTC (permalink / raw)
  To: Barebox List; +Cc: Robin van der Gracht

Instead of using a fixed sized buffer for the report_event function,
let's call it two times, once with a NULL pointer to get the size of the
event and a second time with a buffer of that size.
Also, instead of separating the events into warning and error type,
iterate over all events in one single loop. This helps to get the events
in the order they occured which probably helps the reader to make more
sense of them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/hab/habv4.c | 55 +++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index e94f827549..c2acb81369 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -500,11 +500,34 @@ static bool is_known_rng_fail_event(const uint8_t *data, size_t len)
 	return false;
 }
 
+static uint8_t *hab_get_event(const struct habv4_rvt *rvt, int index, int *len)
+{
+	enum hab_status err;
+	uint8_t *buf;
+
+	err = rvt->report_event(HAB_STATUS_ANY, index, NULL, len);
+	if (err != HAB_STATUS_SUCCESS)
+		return NULL;
+
+	buf = malloc(*len);
+	if (!buf)
+		return NULL;
+
+	err = rvt->report_event(HAB_STATUS_ANY, index, buf, len);
+	if (err != HAB_STATUS_SUCCESS) {
+		pr_err("Unexpected HAB return code\n");
+		free(buf);
+		return NULL;
+	}
+
+	return buf;
+}
+
 static int habv4_get_status(const struct habv4_rvt *rvt)
 {
-	uint8_t data[256];
+	uint8_t *data;
 	uint32_t len;
-	uint32_t index = 0;
+	int i;
 	enum hab_status status;
 	enum hab_config config = 0x0;
 	enum hab_state state = 0x0;
@@ -524,40 +547,24 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
 		return 0;
 	}
 
-	len = sizeof(data);
-	while (rvt->report_event(HAB_STATUS_WARNING, index, data, &len) == HAB_STATUS_SUCCESS) {
+	for (i = 0;; i++) {
+		data = hab_get_event(rvt, i, &len);
+		if (!data)
+			break;
 
 		/* suppress RNG self-test fail events if they can be handled in software */
 		if (IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_SELF_TEST) &&
 		    is_known_rng_fail_event(data, len)) {
 			pr_debug("RNG self-test failure detected, will run software self-test\n");
 		} else {
-			pr_err("-------- HAB warning Event %d --------\n", index);
+			pr_err("-------- HAB Event %d --------\n", i);
 			pr_err("event data:\n");
 			habv4_display_event(data, len);
 		}
 
-		len = sizeof(data);
-		index++;
+		free(data);
 	}
 
-	len = sizeof(data);
-	index = 0;
-	while (rvt->report_event(HAB_STATUS_FAILURE, index, data, &len) == HAB_STATUS_SUCCESS) {
-		pr_err("-------- HAB failure Event %d --------\n", index);
-		pr_err("event data:\n");
-
-		habv4_display_event(data, len);
-		len = sizeof(data);
-		index++;
-	}
-
-	/* Check reason for stopping */
-	len = sizeof(data);
-	index = 0;
-	if (rvt->report_event(HAB_STATUS_ANY, index, NULL, &len) == HAB_STATUS_SUCCESS)
-		pr_err("ERROR: Recompile with larger event data buffer (at least %d bytes)\n\n", len);
-
 	return -EPERM;
 }
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX: HABv4: Improve HAB event printing
  2020-11-23 16:38 [PATCH] i.MX: HABv4: Improve HAB event printing Sascha Hauer
@ 2020-11-24  7:57 ` robin
  2020-11-24  8:32   ` robin
  0 siblings, 1 reply; 5+ messages in thread
From: robin @ 2020-11-24  7:57 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

Hi Sascha,

On 2020-11-23 17:38, Sascha Hauer wrote:
> Instead of using a fixed sized buffer for the report_event function,
> let's call it two times, once with a NULL pointer to get the size of 
> the
> event and a second time with a buffer of that size.
> Also, instead of separating the events into warning and error type,
> iterate over all events in one single loop. This helps to get the 
> events
> in the order they occured which probably helps the reader to make more
> sense of them.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Even better!

Is it worth mentioning that this also fixes the unjustified
'Recompile with larger event data buffer' error?

Acked-by: Robin van der Gracht

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX: HABv4: Improve HAB event printing
  2020-11-24  7:57 ` robin
@ 2020-11-24  8:32   ` robin
  2020-11-24  9:12     ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: robin @ 2020-11-24  8:32 UTC (permalink / raw)
  To: robin; +Cc: Barebox List

Hi Sascha,

On 2020-11-24 08:57, robin wrote:
> Hi Sascha,
> 
> On 2020-11-23 17:38, Sascha Hauer wrote:
>> Instead of using a fixed sized buffer for the report_event function,
>> let's call it two times, once with a NULL pointer to get the size of 
>> the
>> event and a second time with a buffer of that size.
>> Also, instead of separating the events into warning and error type,
>> iterate over all events in one single loop. This helps to get the 
>> events
>> in the order they occured which probably helps the reader to make more
>> sense of them.
>> 
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Even better!
> 
> Is it worth mentioning that this also fixes the unjustified
> 'Recompile with larger event data buffer' error?
> 
> Acked-by: Robin van der Gracht

I just noticed your previous email. I'll test this today.

Regards Robin

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX: HABv4: Improve HAB event printing
  2020-11-24  8:32   ` robin
@ 2020-11-24  9:12     ` Sascha Hauer
  2020-11-24  9:34       ` robin
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2020-11-24  9:12 UTC (permalink / raw)
  To: robin; +Cc: Barebox List

On Tue, Nov 24, 2020 at 09:32:14AM +0100, robin wrote:
> Hi Sascha,
> 
> On 2020-11-24 08:57, robin wrote:
> > Hi Sascha,
> > 
> > On 2020-11-23 17:38, Sascha Hauer wrote:
> > > Instead of using a fixed sized buffer for the report_event function,
> > > let's call it two times, once with a NULL pointer to get the size of
> > > the
> > > event and a second time with a buffer of that size.
> > > Also, instead of separating the events into warning and error type,
> > > iterate over all events in one single loop. This helps to get the
> > > events
> > > in the order they occured which probably helps the reader to make more
> > > sense of them.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > Even better!
> > 
> > Is it worth mentioning that this also fixes the unjustified
> > 'Recompile with larger event data buffer' error?
> > 
> > Acked-by: Robin van der Gracht
> 
> I just noticed your previous email. I'll test this today.

Rouven just told me that testing this is actually very simple. I was was
afraid I had to bring up a full HAB stack, but actually all I had to do
is to enable HAB in barebox, start it on a non HAB enabled board and be
done.

Here's the output which looks sane to me:

HABv4: Status: Operation failed (0x33)
HABv4: Config: Non-secure IC (0xf0)
HABv4: State: Non-secure state (0x66)
HABv4: -------- HAB Event 0 --------
HABv4: event data:
HABv4:  db 00 08 41  33 22 0a 00

HABv4: Status: Operation failed (0x33)
HABv4: Reason: Invalid address: access denied (0x22)
HABv4: Context: Logged in hab_rvt.authenticate_image() (0x0a)
HABv4: Engine: Select first compatible engine (0x00)
HABv4: -------- HAB Event 1 --------
HABv4: event data:
HABv4:  db 00 14 41  33 0c a0 00
HABv4:  00 00 00 00  10 00 04 00
HABv4:  00 00 00 20
HABv4: Status: Operation failed (0x33)
HABv4: Reason: Invalid assertion (0x0c)
HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
HABv4: Engine: Select first compatible engine (0x00)
HABv4: -------- HAB Event 2 --------
HABv4: event data:
HABv4:  db 00 14 41  33 0c a0 00
HABv4:  00 00 00 00  10 00 04 2c
HABv4:  00 00 03 00
HABv4: Status: Operation failed (0x33)
HABv4: Reason: Invalid assertion (0x0c)
HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
HABv4: Engine: Select first compatible engine (0x00)
HABv4: -------- HAB Event 3 --------
HABv4: event data:
HABv4:  db 00 14 41  33 0c a0 00
HABv4:  00 00 00 00  10 00 04 20
HABv4:  00 00 00 01
HABv4: Status: Operation failed (0x33)
HABv4: Reason: Invalid assertion (0x0c)
HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
HABv4: Engine: Select first compatible engine (0x00)
HABv4: -------- HAB Event 4 --------
HABv4: event data:
HABv4:  db 00 14 41  33 0c a0 00
HABv4:  00 00 00 00  10 00 10 00
HABv4:  00 00 00 04
HABv4: Status: Operation failed (0x33)
HABv4: Reason: Invalid assertion (0x0c)
HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
HABv4: Engine: Select first compatible engine (0x00)

Anyway, it won't hurt when you give it a test as well.

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i.MX: HABv4: Improve HAB event printing
  2020-11-24  9:12     ` Sascha Hauer
@ 2020-11-24  9:34       ` robin
  0 siblings, 0 replies; 5+ messages in thread
From: robin @ 2020-11-24  9:34 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On 2020-11-24 10:12, Sascha Hauer wrote:
> On Tue, Nov 24, 2020 at 09:32:14AM +0100, robin wrote:
>> Hi Sascha,
>> 
>> On 2020-11-24 08:57, robin wrote:
>> > Hi Sascha,
>> >
>> > On 2020-11-23 17:38, Sascha Hauer wrote:
>> > > Instead of using a fixed sized buffer for the report_event function,
>> > > let's call it two times, once with a NULL pointer to get the size of
>> > > the
>> > > event and a second time with a buffer of that size.
>> > > Also, instead of separating the events into warning and error type,
>> > > iterate over all events in one single loop. This helps to get the
>> > > events
>> > > in the order they occured which probably helps the reader to make more
>> > > sense of them.
>> > >
>> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> >
>> > Even better!
>> >
>> > Is it worth mentioning that this also fixes the unjustified
>> > 'Recompile with larger event data buffer' error?
>> >
>> > Acked-by: Robin van der Gracht
>> 
>> I just noticed your previous email. I'll test this today.
> 
> Rouven just told me that testing this is actually very simple. I was 
> was
> afraid I had to bring up a full HAB stack, but actually all I had to do
> is to enable HAB in barebox, start it on a non HAB enabled board and be
> done.
> 
> Here's the output which looks sane to me:
> 
> HABv4: Status: Operation failed (0x33)
> HABv4: Config: Non-secure IC (0xf0)
> HABv4: State: Non-secure state (0x66)
> HABv4: -------- HAB Event 0 --------
> HABv4: event data:
> HABv4:  db 00 08 41  33 22 0a 00
> 
> HABv4: Status: Operation failed (0x33)
> HABv4: Reason: Invalid address: access denied (0x22)
> HABv4: Context: Logged in hab_rvt.authenticate_image() (0x0a)
> HABv4: Engine: Select first compatible engine (0x00)
> HABv4: -------- HAB Event 1 --------
> HABv4: event data:
> HABv4:  db 00 14 41  33 0c a0 00
> HABv4:  00 00 00 00  10 00 04 00
> HABv4:  00 00 00 20
> HABv4: Status: Operation failed (0x33)
> HABv4: Reason: Invalid assertion (0x0c)
> HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
> HABv4: Engine: Select first compatible engine (0x00)
> HABv4: -------- HAB Event 2 --------
> HABv4: event data:
> HABv4:  db 00 14 41  33 0c a0 00
> HABv4:  00 00 00 00  10 00 04 2c
> HABv4:  00 00 03 00
> HABv4: Status: Operation failed (0x33)
> HABv4: Reason: Invalid assertion (0x0c)
> HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
> HABv4: Engine: Select first compatible engine (0x00)
> HABv4: -------- HAB Event 3 --------
> HABv4: event data:
> HABv4:  db 00 14 41  33 0c a0 00
> HABv4:  00 00 00 00  10 00 04 20
> HABv4:  00 00 00 01
> HABv4: Status: Operation failed (0x33)
> HABv4: Reason: Invalid assertion (0x0c)
> HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
> HABv4: Engine: Select first compatible engine (0x00)
> HABv4: -------- HAB Event 4 --------
> HABv4: event data:
> HABv4:  db 00 14 41  33 0c a0 00
> HABv4:  00 00 00 00  10 00 10 00
> HABv4:  00 00 00 04
> HABv4: Status: Operation failed (0x33)
> HABv4: Reason: Invalid assertion (0x0c)
> HABv4: Context: Event logged in hab_rvt.assert() (0xa0)
> HABv4: Engine: Select first compatible engine (0x00)
> 
> Anyway, it won't hurt when you give it a test as well.

I would have done the same test. So I think its OK like this. I just
realized I only have access to locked chips atm. so you testing this
is great :P.

I did run this on a locked unit (just to be sure) but no issues there.

Robin

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-11-24  9:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 16:38 [PATCH] i.MX: HABv4: Improve HAB event printing Sascha Hauer
2020-11-24  7:57 ` robin
2020-11-24  8:32   ` robin
2020-11-24  9:12     ` Sascha Hauer
2020-11-24  9:34       ` robin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox