[PATCH 0/5] i.MX habv4: decode error/warning events and support new CST version

[PATCH 0/5] i.MX habv4: decode error/warning events and support new CST version

[Marc Kleine-Budde]

Hello,

this patch series add support to device error and warning events generated by
the ROM code during HAB. It further adds support newer versions of the CST
tools.

regards,
Marc



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

[PATCH 1/5] i.MX habv4: properly indent defines and enums

[Marc Kleine-Budde]

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/hab/habv4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index ae43bdf0e36b..9ccec7847d9f 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -27,13 +27,13 @@
 #define HABV4_RVT_IMX28 0xffff8af8
 #define HABV4_RVT_IMX6_OLD 0x00000094
 #define HABV4_RVT_IMX6_NEW 0x00000098
-#define HABV4_RVT_IMX6UL   0x00000100
+#define HABV4_RVT_IMX6UL 0x00000100
 
 enum hab_tag {
 	HAB_TAG_IVT = 0xd1,		/* Image Vector Table */
 	HAB_TAG_DCD = 0xd2,		/* Device Configuration Data */
 	HAB_TAG_CSF = 0xd4,		/* Command Sequence File */
-	HAB_TAG_CRT = 0xd7, 		/* Certificate */
+	HAB_TAG_CRT = 0xd7,		/* Certificate */
 	HAB_TAG_SIG = 0xd8,		/* Signature */
 	HAB_TAG_EVT = 0xdb,		/* Event */
 	HAB_TAG_RVT = 0xdd,		/* ROM Vector Table */
-- 
2.18.0


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

[PATCH 2/5] i.MX habv4: convert habv4_get_status() and habv4_display_event() from printf to pr_err()

[Marc Kleine-Budde]

This way the events will be printed into the dmesg buffer,

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/hab/habv4.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index 9ccec7847d9f..4d2d377d084a 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -163,17 +163,17 @@ static void habv4_display_event(uint8_t *data, uint32_t len)
 
 	if (data && len) {
 		for (i = 0; i < len; i++) {
-			if (i == 0)
-				printf(" %02x", data[i]);
-			else if ((i % 8) == 0)
-				printf("\n %02x", data[i]);
+			if ((i % 8) == 0)
+				pr_err(" %02x", data[i]);
 			else if ((i % 4) == 0)
-				printf("  %02x", data[i]);
+				pr_cont("  %02x", data[i]);
+			else if ((i % 8) == 7)
+				pr_cont(" %02x\n", data[i]);
 			else
-				printf(" %02x", data[i]);
+				pr_cont(" %02x", data[i]);
 		}
+		pr_cont("\n");
 	}
-	printf("\n\n");
 }
 
 static int habv4_get_status(const struct habv4_rvt *rvt)
@@ -201,8 +201,8 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
 	}
 
 	while (rvt->report_event(HAB_STATUS_FAILURE, index, data, &len) == HAB_STATUS_SUCCESS) {
-		printf("-------- HAB Event %d --------\n"
-		       "event data:\n", index);
+		pr_err("-------- HAB Event %d --------\n", index);
+		pr_err("event data:\n");
 
 		habv4_display_event(data, len);
 		len = sizeof(data);
-- 
2.18.0


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

[PATCH 3/5] i.MX habv4: habv4_get_status(): display warning events, too

[Marc Kleine-Budde]

The current code only retrieves the HAB errors from the ROM. If there are HAB
warnings during startup, the code throws this wrong error message.

> HABv4: ERROR: Recompile with larger event data buffer (at least 36 bytes)

The correct solution is to retrieve the warnings from the ROM, too.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/hab/habv4.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index 4d2d377d084a..74b5af2229c3 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -179,7 +179,7 @@ static void habv4_display_event(uint8_t *data, uint32_t len)
 static int habv4_get_status(const struct habv4_rvt *rvt)
 {
 	uint8_t data[256];
-	uint32_t len = sizeof(data);
+	uint32_t len;
 	uint32_t index = 0;
 	enum hab_status status;
 	enum hab_config config = 0x0;
@@ -200,8 +200,19 @@ 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) {
+		pr_err("-------- HAB warning Event %d --------\n", index);
+		pr_err("event data:\n");
+
+		habv4_display_event(data, len);
+		len = sizeof(data);
+		index++;
+	}
+
+	len = sizeof(data);
 	while (rvt->report_event(HAB_STATUS_FAILURE, index, data, &len) == HAB_STATUS_SUCCESS) {
-		pr_err("-------- HAB Event %d --------\n", index);
+		pr_err("-------- HAB failure Event %d --------\n", index);
 		pr_err("event data:\n");
 
 		habv4_display_event(data, len);
@@ -210,6 +221,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
 	}
 
 	/* Check reason for stopping */
+	len = sizeof(data);
 	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);
 
-- 
2.18.0


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

[PATCH 4/5] i.MX habv4: habv4_display_event_record(): Add function to decode event record

[Marc Kleine-Budde]

The code was leveraged from u-boot commit:

    29067abfaf39 iMX: adding parsing to hab_status command

by: Ulises Cardenas <Ulises.Cardenas@freescale.com>

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/hab/habv4.c | 211 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 211 insertions(+)

diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index 74b5af2229c3..28fd42ecd7a8 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -29,6 +29,21 @@
 #define HABV4_RVT_IMX6_NEW 0x00000098
 #define HABV4_RVT_IMX6UL 0x00000100
 
+struct __packed hab_hdr {
+	uint8_t tag;			/* Tag field */
+	__be16 len;			/* Length field in bytes (big-endian) */
+	uint8_t par;			/* Parameters field */
+};
+
+struct __packed hab_event_record {
+	struct hab_hdr hdr;
+	uint8_t status;			/* Status -> enum hab_status*/
+	uint8_t reason;			/* Reason -> enum hab_reason */
+	uint8_t context;		/* Context -> enum hab_context */
+	uint8_t engine;			/* Engine -> enum hab_engine */
+	uint8_t data[0];		/* Record Data */
+};
+
 enum hab_tag {
 	HAB_TAG_IVT = 0xd1,		/* Image Vector Table */
 	HAB_TAG_DCD = 0xd2,		/* Device Configuration Data */
@@ -69,6 +84,66 @@ enum hab_state {
 	HAB_STATE_NONE = 0xf0,		/* No security state machine */
 };
 
+enum hab_reason {
+	HAB_REASON_RSN_ANY = 0x00,		/* Match any reason */
+	HAB_REASON_UNS_COMMAND = 0x03,		/* Unsupported command */
+	HAB_REASON_INV_IVT = 0x05,		/* Invalid ivt */
+	HAB_REASON_INV_COMMAND = 0x06,		/* Invalid command: command malformed */
+	HAB_REASON_UNS_STATE = 0x09,		/* Unsuitable state */
+	HAB_REASON_UNS_ENGINE = 0x0a,		/* Unsupported engine */
+	HAB_REASON_INV_ASSERTION = 0x0c,	/* Invalid assertion */
+	HAB_REASON_INV_INDEX = 0x0f,		/* Invalid index: access denied */
+	HAB_REASON_INV_CSF = 0x11,		/* Invalid csf */
+	HAB_REASON_UNS_ALGORITHM = 0x12,	/* Unsupported algorithm */
+	HAB_REASON_UNS_PROTOCOL = 0x14,		/* Unsupported protocol */
+	HAB_REASON_INV_SIZE = 0x17,		/* Invalid data size */
+	HAB_REASON_INV_SIGNATURE = 0x18,	/* Invalid signature */
+	HAB_REASON_UNS_KEY = 0x1b,		/* Unsupported key type/parameters */
+	HAB_REASON_INV_KEY = 0x1d,		/* Invalid key */
+	HAB_REASON_INV_RETURN = 0x1e,		/* Failed callback function */
+	HAB_REASON_INV_CERTIFICATE = 0x21,	/* Invalid certificate */
+	HAB_REASON_INV_ADDRESS = 0x22,		/* Invalid address: access denied */
+	HAB_REASON_UNS_ITEM = 0x24,		/* Unsupported configuration item */
+	HAB_REASON_INV_DCD = 0x27,		/* Invalid dcd */
+	HAB_REASON_INV_CALL = 0x28,		/* Function called out of sequence */
+	HAB_REASON_OVR_COUNT = 0x2b,		/* Expired poll count */
+	HAB_REASON_OVR_STORAGE = 0x2d,		/* Exhausted storage region */
+	HAB_REASON_MEM_FAIL = 0x2e,		/* Memory failure */
+	HAB_REASON_ENG_FAIL = 0x30,		/* Engine failure */
+};
+
+enum hab_context {
+	HAB_CONTEXT_ANY = 0x00,			/* Match any context */
+	HAB_CONTEXT_AUTHENTICATE = 0x0a,	/* Logged in hab_rvt.authenticate_image() */
+	HAB_CONTEXT_TARGET = 0x33,		/* Event logged in hab_rvt.check_target() */
+	HAB_CONTEXT_ASSERT = 0xa0,		/* Event logged in hab_rvt.assert() */
+	HAB_CONTEXT_COMMAND = 0xc0,		/* Event logged executing csf/dcd command */
+	HAB_CONTEXT_CSF = 0xcf,			/* Event logged in hab_rvt.run_csf() */
+	HAB_CONTEXT_AUT_DAT = 0xdb,		/* Authenticated data block */
+	HAB_CONTEXT_DCD = 0xdd,			/* Event logged in hab_rvt.run_dcd() */
+	HAB_CONTEXT_ENTRY = 0xe1,		/* Event logged in hab_rvt.entry() */
+	HAB_CONTEXT_EXIT = 0xee,		/* Event logged in hab_rvt.exit() */
+	HAB_CONTEXT_FAB = 0xff,			/* Event logged in hab_fab_test() */
+};
+
+enum hab_engine {
+	HAB_ENGINE_ANY = 0x00,		/* Select first compatible engine */
+	HAB_ENGINE_SCC = 0x03,		/* Security controller */
+	HAB_ENGINE_RTIC = 0x05,		/* Run-time integrity checker */
+	HAB_ENGINE_SAHARA = 0x06,	/* Crypto accelerator */
+	HAB_ENGINE_CSU = 0x0a,		/* Central Security Unit */
+	HAB_ENGINE_SRTC = 0x0c,		/* Secure clock */
+	HAB_ENGINE_DCP = 0x1b,		/* Data Co-Processor */
+	HAB_ENGINE_CAAM = 0x1d,		/* CAAM */
+	HAB_ENGINE_SNVS = 0x1e,		/* Secure Non-Volatile Storage */
+	HAB_ENGINE_OCOTP = 0x21,	/* Fuse controller */
+	HAB_ENGINE_DTCP = 0x22,		/* DTCP co-processor */
+	HAB_ENGINE_HDCP = 0x24,		/* HDCP co-processor */
+	HAB_ENGINE_ROM = 0x36,		/* Protected ROM area */
+	HAB_ENGINE_RTL = 0x77,		/* RTL simulation engine */
+	HAB_ENGINE_SW = 0xff,		/* Software engine */
+};
+
 enum hab_target {
 	HAB_TARGET_MEMORY = 0x0f,	/* Check memory white list */
 	HAB_TARGET_PERIPHERAL = 0xf0,	/* Check peripheral white list*/
@@ -157,6 +232,140 @@ static const char *habv4_get_state_str(enum hab_state state)
 	return "<unknown>";
 }
 
+static const char *habv4_get_reason_str(enum hab_reason reason)
+{
+	switch (reason) {
+	case HAB_REASON_RSN_ANY:
+		return "Match any reason"; break;
+	case HAB_REASON_UNS_COMMAND:
+		return "Unsupported command"; break;
+	case HAB_REASON_INV_IVT:
+		return "Invalid ivt"; break;
+	case HAB_REASON_INV_COMMAND:
+		return "Invalid command: command malformed"; break;
+	case HAB_REASON_UNS_STATE:
+		return "Unsuitable state"; break;
+	case HAB_REASON_UNS_ENGINE:
+		return "Unsupported engine"; break;
+	case HAB_REASON_INV_ASSERTION:
+		return "Invalid assertion"; break;
+	case HAB_REASON_INV_INDEX:
+		return "Invalid index: access denied"; break;
+	case HAB_REASON_INV_CSF:
+		return "Invalid csf"; break;
+	case HAB_REASON_UNS_ALGORITHM:
+		return "Unsupported algorithm"; break;
+	case HAB_REASON_UNS_PROTOCOL:
+		return "Unsupported protocol"; break;
+	case HAB_REASON_INV_SIZE:
+		return "Invalid data size"; break;
+	case HAB_REASON_INV_SIGNATURE:
+		return "Invalid signature"; break;
+	case HAB_REASON_UNS_KEY:
+		return "Unsupported key type/parameters"; break;
+	case HAB_REASON_INV_KEY:
+		return "Invalid key"; break;
+	case HAB_REASON_INV_RETURN:
+		return "Failed callback function"; break;
+	case HAB_REASON_INV_CERTIFICATE:
+		return "Invalid certificate"; break;
+	case HAB_REASON_INV_ADDRESS:
+		return "Invalid address: access denied"; break;
+	case HAB_REASON_UNS_ITEM:
+		return "Unsupported configuration item"; break;
+	case HAB_REASON_INV_DCD:
+		return "Invalid dcd"; break;
+	case HAB_REASON_INV_CALL:
+		return "Function called out of sequence"; break;
+	case HAB_REASON_OVR_COUNT:
+		return "Expired poll count"; break;
+	case HAB_REASON_OVR_STORAGE:
+		return "Exhausted storage region"; break;
+	case HAB_REASON_MEM_FAIL:
+		return "Memory failure"; break;
+	case HAB_REASON_ENG_FAIL:
+		return "Engine failure"; break;
+	}
+
+	return "<unknown>";
+}
+
+static const char *habv4_get_context_str(enum hab_context context)
+{
+	switch (context){
+	case HAB_CONTEXT_ANY:
+		return "Match any context"; break;
+	case HAB_CONTEXT_AUTHENTICATE:
+		return "Logged in hab_rvt.authenticate_image()"; break;
+	case HAB_CONTEXT_TARGET:
+		return "Event logged in hab_rvt.check_target()"; break;
+	case HAB_CONTEXT_ASSERT:
+		return "Event logged in hab_rvt.assert()"; break;
+	case HAB_CONTEXT_COMMAND:
+		return "Event logged executing csf/dcd command"; break;
+	case HAB_CONTEXT_CSF:
+		return "Event logged in hab_rvt.run_csf()"; break;
+	case HAB_CONTEXT_AUT_DAT:
+		return "Authenticated data block"; break;
+	case HAB_CONTEXT_DCD:
+		return "Event logged in hab_rvt.run_dcd()"; break;
+	case HAB_CONTEXT_ENTRY:
+		return "Event logged in hab_rvt.entry()"; break;
+	case HAB_CONTEXT_EXIT:
+		return "Event logged in hab_rvt.exit()"; break;
+	case HAB_CONTEXT_FAB:
+		return "Event logged in hab_fab_test()"; break;
+	}
+
+	return "<unknown>";
+}
+
+static const char *habv4_get_engine_str(enum hab_engine engine)
+{
+	switch (engine){
+	case HAB_ENGINE_ANY:
+		return "Select first compatible engine"; break;
+	case HAB_ENGINE_SCC:
+		return "Security controller"; break;
+	case HAB_ENGINE_RTIC:
+		return "Run-time integrity checker"; break;
+	case HAB_ENGINE_SAHARA:
+		return "Crypto accelerator"; break;
+	case HAB_ENGINE_CSU:
+		return "Central Security Unit"; break;
+	case HAB_ENGINE_SRTC:
+		return "Secure clock"; break;
+	case HAB_ENGINE_DCP:
+		return "Data Co-Processor"; break;
+	case HAB_ENGINE_CAAM:
+		return "CAAM"; break;
+	case HAB_ENGINE_SNVS:
+		return "Secure Non-Volatile Storage"; break;
+	case HAB_ENGINE_OCOTP:
+		return "Fuse controller"; break;
+	case HAB_ENGINE_DTCP:
+		return "DTCP co-processor"; break;
+	case HAB_ENGINE_HDCP:
+		return "HDCP co-processor"; break;
+	case HAB_ENGINE_ROM:
+		return "Protected ROM area"; break;
+	case HAB_ENGINE_RTL:
+		return "RTL simulation engine"; break;
+	case HAB_ENGINE_SW:
+		return "Software engine"; break;
+	}
+
+	return "<unknown>";
+}
+
+static void habv4_display_event_record(struct hab_event_record *record)
+{
+	pr_err("Status: %s (0x%02x)\n", habv4_get_status_str(record->status), record->status);
+	pr_err("Reason: %s (0x%02x)\n", habv4_get_reason_str(record->reason), record->reason);
+	pr_err("Context: %s (0x%02x)\n", habv4_get_context_str(record->context), record->context);
+	pr_err("Engine: %s (0x%02x)\n", habv4_get_engine_str(record->engine), record->engine);
+}
+
 static void habv4_display_event(uint8_t *data, uint32_t len)
 {
 	unsigned int i;
@@ -174,6 +383,8 @@ static void habv4_display_event(uint8_t *data, uint32_t len)
 		}
 		pr_cont("\n");
 	}
+
+	habv4_display_event_record((struct hab_event_record *)data);
 }
 
 static int habv4_get_status(const struct habv4_rvt *rvt)
-- 
2.18.0


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

[PATCH 5/5] scripts imx-image: add support for newer versions of "cst"

[Marc Kleine-Budde]

Older versions of "cst" want to read the CSF frm STDIN, while newer versions
want to read the CSF from a file. Sadly, the "-i" option doesn't understand
"-i -" to read from STDIN, so we give it "/dev/stdin" instead.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 scripts/imx/imx-image.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
index 452a544bc3eb..17d504586967 100644
--- a/scripts/imx/imx-image.c
+++ b/scripts/imx/imx-image.c
@@ -558,7 +558,21 @@ static int hab_sign(struct config_data *data)
 		}
 	}
 
-	ret = asprintf(&command, "%s -o %s", cst, csffile);
+	/*
+	 * Older versions of "cst" want to read the CSF frm STDIN,
+	 * while newer versions want to read the CSF from a
+	 * file. Sadly, the "-i" option doesn't understand "-i -" to
+	 * read from STDIN, so we give it "/dev/stdin" instead.
+	 */
+	ret = asprintf(&command,
+		       "if %s | grep 'Input CSF text filename'; then"
+		       "	%s -o %s -i /dev/stdin;"
+		       "else"
+		       "	%s -o %s;"
+		       "fi",
+		       cst,
+		       cst, csffile,
+		       cst, csffile);
 	if (ret < 0)
 		return -ENOMEM;
 
-- 
2.18.0


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

Re: [PATCH 5/5] scripts imx-image: add support for newer versions of "cst"

[Sascha Hauer]

On Mon, Aug 27, 2018 at 05:14:28PM +0200, Marc Kleine-Budde wrote:
> Older versions of "cst" want to read the CSF frm STDIN, while newer versions
> want to read the CSF from a file. Sadly, the "-i" option doesn't understand
> "-i -" to read from STDIN, so we give it "/dev/stdin" instead.
> 
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  scripts/imx/imx-image.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
> index 452a544bc3eb..17d504586967 100644
> --- a/scripts/imx/imx-image.c
> +++ b/scripts/imx/imx-image.c
> @@ -558,7 +558,21 @@ static int hab_sign(struct config_data *data)
>  		}
>  	}
>  
> -	ret = asprintf(&command, "%s -o %s", cst, csffile);
> +	/*
> +	 * Older versions of "cst" want to read the CSF frm STDIN,
> +	 * while newer versions want to read the CSF from a
> +	 * file. Sadly, the "-i" option doesn't understand "-i -" to
> +	 * read from STDIN, so we give it "/dev/stdin" instead.
> +	 */
> +	ret = asprintf(&command,
> +		       "if %s | grep 'Input CSF text filename'; then"
> +		       "	%s -o %s -i /dev/stdin;"
> +		       "else"
> +		       "	%s -o %s;"
> +		       "fi",
> +		       cst,
> +		       cst, csffile,
> +		       cst, csffile);

Could you separate the cst calling convention detection from the actual
call, so effectively do the if/else in C rather than shell? That would
give us a place to add a debug printf and say which way we actually call
the cst tool and it will be slightly less magical when something goes
wrong.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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

Re: [PATCH 0/5] i.MX habv4: decode error/warning events and support new CST version

[Sascha Hauer]

On Mon, Aug 27, 2018 at 05:14:23PM +0200, Marc Kleine-Budde wrote:
> Hello,
> 
> this patch series add support to device error and warning events generated by
> the ROM code during HAB. It further adds support newer versions of the CST
> tools.

Applied 1-4 for now.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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

Re: [PATCH 5/5] scripts imx-image: add support for newer versions of "cst"

[Marc Kleine-Budde]

[-- Attachment #1.1.1: Type: text/plain, Size: 2133 bytes --]

On 08/29/2018 09:15 AM, Sascha Hauer wrote:
> On Mon, Aug 27, 2018 at 05:14:28PM +0200, Marc Kleine-Budde wrote:
>> Older versions of "cst" want to read the CSF frm STDIN, while newer versions
>> want to read the CSF from a file. Sadly, the "-i" option doesn't understand
>> "-i -" to read from STDIN, so we give it "/dev/stdin" instead.
>>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>  scripts/imx/imx-image.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
>> index 452a544bc3eb..17d504586967 100644
>> --- a/scripts/imx/imx-image.c
>> +++ b/scripts/imx/imx-image.c
>> @@ -558,7 +558,21 @@ static int hab_sign(struct config_data *data)
>>  		}
>>  	}
>>  
>> -	ret = asprintf(&command, "%s -o %s", cst, csffile);
>> +	/*
>> +	 * Older versions of "cst" want to read the CSF frm STDIN,
>> +	 * while newer versions want to read the CSF from a
>> +	 * file. Sadly, the "-i" option doesn't understand "-i -" to
>> +	 * read from STDIN, so we give it "/dev/stdin" instead.
>> +	 */
>> +	ret = asprintf(&command,
>> +		       "if %s | grep 'Input CSF text filename'; then"
>> +		       "	%s -o %s -i /dev/stdin;"
>> +		       "else"
>> +		       "	%s -o %s;"
>> +		       "fi",
>> +		       cst,
>> +		       cst, csffile,
>> +		       cst, csffile);
> 
> Could you separate the cst calling convention detection from the actual
> call, so effectively do the if/else in C rather than shell? That would
> give us a place to add a debug printf and say which way we actually call
> the cst tool and it will be slightly less magical when something goes
> wrong.

Makes sense. Do the "cst | grep" in one shell and evaluate the return
value, do the if else in C and then do the proper shell call?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

Re: [PATCH 5/5] scripts imx-image: add support for newer versions of "cst"

[Sascha Hauer]

On Wed, Aug 29, 2018 at 09:29:25AM +0200, Marc Kleine-Budde wrote:
> On 08/29/2018 09:15 AM, Sascha Hauer wrote:
> > On Mon, Aug 27, 2018 at 05:14:28PM +0200, Marc Kleine-Budde wrote:
> >> Older versions of "cst" want to read the CSF frm STDIN, while newer versions
> >> want to read the CSF from a file. Sadly, the "-i" option doesn't understand
> >> "-i -" to read from STDIN, so we give it "/dev/stdin" instead.
> >>
> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >> ---
> >>  scripts/imx/imx-image.c | 16 +++++++++++++++-
> >>  1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/imx/imx-image.c b/scripts/imx/imx-image.c
> >> index 452a544bc3eb..17d504586967 100644
> >> --- a/scripts/imx/imx-image.c
> >> +++ b/scripts/imx/imx-image.c
> >> @@ -558,7 +558,21 @@ static int hab_sign(struct config_data *data)
> >>  		}
> >>  	}
> >>  
> >> -	ret = asprintf(&command, "%s -o %s", cst, csffile);
> >> +	/*
> >> +	 * Older versions of "cst" want to read the CSF frm STDIN,
> >> +	 * while newer versions want to read the CSF from a
> >> +	 * file. Sadly, the "-i" option doesn't understand "-i -" to
> >> +	 * read from STDIN, so we give it "/dev/stdin" instead.
> >> +	 */
> >> +	ret = asprintf(&command,
> >> +		       "if %s | grep 'Input CSF text filename'; then"
> >> +		       "	%s -o %s -i /dev/stdin;"
> >> +		       "else"
> >> +		       "	%s -o %s;"
> >> +		       "fi",
> >> +		       cst,
> >> +		       cst, csffile,
> >> +		       cst, csffile);
> > 
> > Could you separate the cst calling convention detection from the actual
> > call, so effectively do the if/else in C rather than shell? That would
> > give us a place to add a debug printf and say which way we actually call
> > the cst tool and it will be slightly less magical when something goes
> > wrong.
> 
> Makes sense. Do the "cst | grep" in one shell and evaluate the return
> value, do the if else in C and then do the proper shell call?

Yes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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