On Mon, Sep 30, 2024 at 12:52:24PM -0600, Simon Glass wrote:
WARNING: This email originated from outside of GE HealthCare. Please validate the sender's email address before clicking on links or attachments as they may not be safe.
Hi Brian,
On Mon, 30 Sept 2024 at 10:10, Brian Ruley brian.ruley@gehealthcare.com wrote:
Conform to the style guide used in the project by making the following changes:
- Use single quotes for multiline strings (except docstrings)
- Fix line width to 79 cols
- Use f-string instead of formatting a regular string
Signed-off-by: Brian Ruley brian.ruley@gehealthcare.com
tools/binman/etype/nxp_imx8mcst.py | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Thanks for doing this.
Some of my comments on the other patch could be applied here, if you prefer.
diff --git a/tools/binman/etype/nxp_imx8mcst.py b/tools/binman/etype/nxp_imx8mcst.py index 8221517b0c..0c744a00d7 100644 --- a/tools/binman/etype/nxp_imx8mcst.py +++ b/tools/binman/etype/nxp_imx8mcst.py @@ -23,7 +23,7 @@ from u_boot_pylib import tools MAGIC_NXP_IMX_IVT = 0x412000d1 MAGIC_FITIMAGE = 0xedfe0dd0
-csf_config_template = """ +csf_config_template = ''' [Header] Version = 4.3 Hash Algorithm = sha256 @@ -53,7 +53,7 @@ csf_config_template = """ [Authenticate Data] Verification index = 2 Blocks = 0x1234 0x78 0xabcd "data.bin" -""" +'''
class Entry_nxp_imx8mcst(Entry_mkimage): """NXP i.MX8M CST .cfg file generator and cst invoker @@ -69,9 +69,21 @@ class Entry_nxp_imx8mcst(Entry_mkimage): def ReadNode(self): super().ReadNode() self.loader_address = fdt_util.GetInt(self._node, 'nxp,loader-address')
self.srk_table = os.getenv('SRK_TABLE', fdt_util.GetString(self._node, 'nxp,srk-table', 'SRK_1_2_3_4_table.bin'))self.csf_crt = os.getenv('CSF_KEY', fdt_util.GetString(self._node, 'nxp,csf-crt', 'CSF1_1_sha256_4096_65537_v3_usr_crt.pem'))self.img_crt = os.getenv('IMG_KEY', fdt_util.GetString(self._node, 'nxp,img-crt', 'IMG1_1_sha256_4096_65537_v3_usr_crt.pem'))
self.srk_table = os.getenv('SRK_TABLE', fdt_util.GetString(self._node, 'nxp,srk-table','SRK_1_2_3_4_table.bin'))self.csf_crt = os.getenv('CSF_KEY', fdt_util.GetString(self._node, 'nxp,csf-crt','CSF1_1_sha256_4096_65537_v3_usr_crt.pem'))self.img_crt = os.getenv('IMG_KEY', fdt_util.GetString(self._node, 'nxp,img-crt','IMG1_1_sha256_4096_65537_v3_usr_crt.pem')) self.unlock = fdt_util.GetBool(self._node, 'nxp,unlock') self.ReadEntries()@@ -118,7 +130,7 @@ class Entry_nxp_imx8mcst(Entry_mkimage): tools.write_file(output_dname, data)
# Generate CST configuration file used to sign payload
cfg_fname = tools.get_output_filename('nxp.csf-config-txt.%s' % uniq)
cfg_fname = tools.get_output_filename(f'nxp.csf-config-txt.{uniq}') config = configparser.ConfigParser() # Do not make key names lowercase config.optionxform = str@@ -127,7 +139,9 @@ class Entry_nxp_imx8mcst(Entry_mkimage): config['Install SRK']['File'] = '"' + self.srk_table + '"' config['Install CSFK']['File'] = '"' + self.csf_crt + '"' config['Install Key']['File'] = '"' + self.img_crt + '"'
config['Authenticate Data']['Blocks'] = hex(signbase) + ' 0 ' + hex(len(data)) + ' "' + str(output_dname) + '"'
config['Authenticate Data']['Blocks'] = (hex(signbase) + ' 0 '+ hex(len(data)) + ' "'+ str(output_dname) + '"') if not self.unlock: config.remove_section('Unlock') with open(cfg_fname, 'w') as cfgf:-- 2.39.5
Regards, Simon
Hi Simon,
Sure thing, I'm glad to contribute. I'm quite new to upstreaming so this is a good learning experience for me, and I thank you for your patience.
I didn't have time to respond yesterday, but I sent a new revision based on your comments.
Only one I didn't quite get was
All three options seem to read the 'nxp,srk-crt' property, so you can do that once the if() to reduce the amount of duplicated code.
Each is reading a different property "img-crt", "csf-crt", and "srk-crt", with the latter read only if "fast-auth" is set.
As for the testing part, after I got `binman test` working, I experimented with creating a test for the `nxp-imx8mcst` etype, using what you had done as a starting point. FYI, it doesn't depend on the `nxp-imx8mimage` because it's for invoking `mkimage` when building SPL, for example. So, what I did is just create a fake FIT image inside the nxp-imx8mcst node, and run the test. I can send the patch if you wish.
The test, however, was unsuccessful because a PKI tree is needed to sign the assets. I thought maybe you would have a comment on that?
Best, Brian