No need for this extra help.

I think there is since we removed something that should be there.

Sure, but it is more stuff to maintain and easier to have conflicts as a consequence.

Maybe create another option that could later be upstreamed?

but conflicts here are good. we don't want changes in this area to go unnoticed do we?

I mean, it could have conflicts because of our new text, not because of the option changes. And that sort of conflict is just noise.

Can we have a separated IMX_HAB option that can be used here? This is not IMX8M specific.

We can add a new option that is set by default if IMX_HAB and update this ifdef using the new option. Then we can easily disable the required option in our own config fragment.

we could, but that could be done when support for another board comes along no? why should we try to be generic when there is only one board to support yet? board number 2 is the one that should define the interface IMO...and it makes sense because the one bringing up board 2 could validate against imx8mm as well

Because I don't see how this wouldn't be an issue with other targets, and best to fix in a generic way first.

Any iMX target that requires IMX_HAB to be enabled will face this same issue.

well no, how would this be an issue for other targets? it hasnt been an issue so far so why now does it have to be an issue; of course it would be an issue if they try to use the functionality implemented for imx8mm since they dont have it yet.

I disagree with fixing in a generic way first. The first implementation should be specific and testable. the second implementation should be generic allowing the first implementer and the second implementer to tests separatelly.

well no, how would this be an issue for other targets? it hasnt been an issue so far so why now does it have to be an issue; of course it would be an issue if they try to use the functionality implemented for imx8mm since they dont have it yet.

What I mean is that it would not work for other targets as we enable a similar boot path, because of the same restrictions applied to imx8mm.

For example, CONFIG_IMX_HAB is required for mfgtools and this would cause an issue when loading the kernel fitimage as we do for imx7ulp (which is not an issue now as that target is using an older u-boot). Same will happen once we enable IMX_HAB on mfgtools for imx6ull (booting fit for installing at the linux level, and not u-boot). Same thing for imx7d.

Since we will have to fix this quite soon anyway, I would rather prefer to find a way to not imply HAB verified boot in bootm when IMX_HAB has to be enabled (otherwise we will have to come to the generic implementation in a few weeks).

I disagree with fixing in a generic way first. The first implementation should be specific and testable. the second implementation should be generic allowing the first implementer and the second implementer to tests separatelly.

Sure, that works, but since we will have to fix for the other targets over the next few weeks, I would rather fix the generic way now and not have to come back to this later.

@mike-scott what do you think?

Can you explain why we need these changes?

I would also prefer an option that is not bound to imx8, as this is actually generic stuff (and it surprises me this is not an issue with imx6/7).

the commit and the target of this work is imx8mm so this addresses imx8mm leaving other platforms unaffected
but sure, when we add support for other boards, this will have to be modified and a more generic definition done.

But why isn't this an issue with imx6/imx7?

we must be talking past each other :) imx6 and imx7 don't define (CONFIG_SPL_RSA && CONFIG_IMX8MM) so this leaves the previous implementation the way it was.

if you are saying the previous implementation is broken, sure lets fix it on a separate commit by extending the changes done in this one.

or if you are saying we want imx6 and imx7 to now work the same way that imx8mm will work after this commit sure, lets change it...but IMO should be on a separate commit, not pushing it all together...

What I'm asking is why do we need to change the generic rsa-verify function only for IMX8MM if this code path seems to be shared with imx6/imx7.

I didn't get why we need this change from your commit, it is not clear.

ah right, it is because those platforms somehow manage to define USE_HOSTCC and I wasnt able to enable such definition