Hi Simon

On 31/03/23 02:01, Simon Glass wrote:
Hi Neha,

On Fri, 24 Mar 2023 at 22:28, Neha Malcom Francis <n-fran...@ti.com> wrote:

Hi Simon,

Before I roll out the entire series that works for packaging K3
bootloader images, wanted to get some reviews and comments regarding the
implementation of the signing etype [1] . I believe I've taken into
consideration what we've discussed earlier [2].

Let me know your thoughts. The tree is also WIP, and was mainly designed
for testing the signing etype on two of the devices. I will add the
remaining and refine the series based on the next comments.

Yes this looks reasonable to me. For the openssl method, can you
create a new 'real' method and put the cert stuff in there instead of
using a 'custom' one? It seems to have a lot in common with what is
there. We should really have an internal cert-generator method in that
class, with other functions in that class doing appropriate things. I
am looking for code reuse here, as well as clear indication of what
the cert is for or does.

So you're suggesting to include the config creation within the openssl btool, am I right? For example methods to generate a config, add section to a config, add extension to a section of a config... etc? I can take a look at implementing this if this is what you have suggested.

If most of the cert is the same, you could
pass a dict for the CN stuff, perhaps?

What is the taml for? It is hard to tell, from the example provided.
Did you mean YAML? If so in the patch I linked, I don't think I had a example for that. Could you point out what exactly you're asking about?


Do you have a .dts which shows the full image for a board? I think the
cert stuff looks right, but it's a bit hard to tell.

Yes, in the same github link I have the whole tree [3] that contains DTBs for couple of the devices [4] and [5], please have a look and let me know (I might force push a few changes in the next couple of days, so better to look from the tree)


When sending the patches please do do follow the function-commenting
style and make sure that it is clear what each arg means. E.g. I saw a
hash integer which I assume is used to pass 256 or 384 or 512 for sha
hashing. It should indicate the possible values / meaning in the arg.
In fact, in that case, it might be better to pass a string like
'sha256'.

Yes, you're right. Will address that when sending the patch series completely.


Anyway, apart from my questions it seems good.

Regards,
Simon


[1]
https://github.com/nehamalcom/u-boot/commit/ea7413ed5864340bd6f01e704e8bdcc073a7896b#diff-efb03d61a324724c4f86bf42b45c4e4e614cab18e1b3184f63721d62280a11b5

[2]
https://patchwork.ozlabs.org/project/uboot/patch/20230224120340.587786-1-n-fran...@ti.com/

--
Thanking You
Neha Malcom Francis

[3] https://github.com/nehamalcom/u-boot/commits/ti-secure-functionality
[4] https://github.com/nehamalcom/u-boot/commit/dda1f9caf436df59c576466f35262df90aa1c0af


--
Thanking You
Neha Malcom Francis

Reply via email to