On Mon May 13, 2024 at 4:11 AM EEST, Joachim Vandersmissen wrote: > On 5/12/24 6:11 PM, Jarkko Sakkinen wrote: > > On Sat May 11, 2024 at 9:23 AM EEST, Joachim Vandersmissen wrote: > >> v4: FIPS_SIGNATURE_SELFTEST_RSA is no longer user-configurable and will > >> be set when the dependencies are fulfilled. > >> > >> ---8<--- > > This is in wrong place. If the patch is applied it will be included to > > the kernel git log. Please put your log before diffstat. > I will keep it in mind for the next round. > > > >> In preparation of adding new ECDSA self-tests, the existing data is > >> moved to a separate file. A new configuration option is added to > >> control the compilation of the separate file. This configuration option > >> also enforces dependencies that were missing from the existing > >> CONFIG_FIPS_SIGNATURE_SELFTEST option. > > 1. Please just call the thing by its name instead of building tension > > with "the new configuration option". > > 2. Lacks the motivation of adding a new configuration option. > The configuration option is there to ensure that the RSA (or ECDSA) > self-tests only get compiled in when RSA (or ECDSA) is actually enabled. > Otherwise, the self-test will panic on boot. I can make this more > explicit in the commit message. > > > >> The old fips_signature_selftest is no longer an init function, but now > >> a helper function called from fips_signature_selftest_rsa. > > This is confusing, please remove. > Fair enough, I'll remove it from the commit message. Yeah, I mean it is good to enough to have a code change no need to document it here :-) > > > > So why just send this and not this plus the selftest? Feels incomplete > > to me. > > Do you mean the ECDSA self-test? I didn't include that one here because > I didn't want to make the commit too big. So, I'd suggest to make a patch set with the second patch containing the tests. BR, Jarkko
