[PATCH] implement policy_pcr commands to lock NV-indexes behind a PCR

Dan Carpenter dan.carpenter at linaro.org
Tue Feb 20 18:56:13 CET 2024


I'm kind of new to u-boot and I'm not really able to review this code
as well as I should.

But also I can't apply the patch.  It seems white space damaged?  The
kernel has a good document on how to do this.  I'm pretty sure u-boot
does as well but I'm new.
https://www.kernel.org/doc/Documentation/process/email-clients.rst

Please run your patch through the scripts/checkpatch.pl script.  Stuff
like this triggers a warning:

> +static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag,
> +                        int argc, char *const argv[]) //TODO: session handle from auth session!
> +{
> +     struct udevice *dev;
> +     u32 nv_addr, nv_size, rc;
> +     void *session_addr = NULL;
> +     int ret;
> +
> +     ret = get_tpm(&dev);
> +           if (ret)
> +                 return ret;
> +
> +           if (argc < 4)
> +                 return CMD_RET_USAGE;


WARNING: suspect code indent for conditional statements (0, 0)
#250: FILE: cmd/tpm-v2.c:437:
+     if (ret)
+           return ret;

WARNING: suspect code indent for conditional statements (0, 0)
#253: FILE: cmd/tpm-v2.c:440:
+     if (argc < 4)
+           return CMD_RET_USAGE;

Also the subject should have a subsystem prefix and the information from
the email should be moved into the commit message.  Currently the commit
message is empty.

> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index 33dd103767..5b60883777 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -301,7 +301,8 @@ enum tpm2_startup_types {
>   */
>  enum tpm2_handles {
>       TPM2_RH_OWNER           = 0x40000001,
> -     TPM2_RS_PW        = 0x40000009,
> +     TPM2_RH_NULL            = 0x40000007,
> +     TPM2_RS_PW              = 0x40000009,

Changing TPM2_RS_PW is an unrelated whitespace change.  Do that as a
separate patch.  But I don't get it at all because the TPM2_RS_PW enum
has always been indented correctly as far as I can see.  So it's a
puzzle.

I mean there are a lot of TODOs and I understand that you just wanted a
high level review but I kept getting distracted and lost and I couldn't
apply the patch so it was just really hard to figure out what was going
on.  :(

regards,
dan carpenter



More information about the U-Boot mailing list