Re: New TPM commands.

2023-12-28 Thread Ilias Apalodimas
dle */
>     tpm_u16(count),   /* Number of bytes */
>     tpm_u16(0), /* Offset */
>   };
> @@ -930,7 +964,7 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
> const void *data,
>   u32 count)
>  {
>   struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> - uint offset = 10 + 8 + 4 + 9 + 2;
> + uint offset = TPM2_HDR_LEN + 8 + 4 + 9 + 2;
>   uint len = offset + count + 2;
>   /* Use empty password auth if platform hierarchy is disabled */
>   u32 auth = priv->plat_hier_disabled ? HR_NV_INDEX + index :
> @@ -943,18 +977,21 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
> const void *data,
> 
>     /* handles 8 bytes */
>     tpm_u32(auth),/* Primary platform seed */
> -   tpm_u32(HR_NV_INDEX + index), /* Password authorisation */
> +   tpm_u32(index),   /*nv index*/
> 
>     /* AUTH_SESSION */
> -   tpm_u32(9), /* Authorization size */
> -   tpm_u32(TPM2_RS_PW),  /* Session handle */
> +   tpm_u32(9), /* Authorization size - 4 bytes */
> +   /*auth handle - 9 bytes */
> +   tpm_u32(TPM2_RS_PW),/* Password authorisation */  /* Session 
> handle */
>     tpm_u16(0), /* Size of  */
>     /*  (if any) */
>     0,  /* Attributes: Cont/Excl/Rst */
>     tpm_u16(0), /* Size of  */
>     /*  (if any) */
> -
> -   tpm_u16(count),
> +   /*end auth handle */
> +   tpm_u16(count),/*size of buffer - 2 bytes*/
> +   /*data (buffer)*/
> +   /*offset -> the octet offset into the NV Area*/
>   };
>   size_t response_len = COMMAND_BUFFER_SIZE;
>   u8 response[COMMAND_BUFFER_SIZE];
> --
> 2.34.1
> ===END PATCH===
> 
> 
> 
> 差出人: Ilias Apalodimas 
> 送信日時: 2023年12月20日 17:17
> 宛先: Niek Nooijens / OC-IAB PBD-C DEVEL 1-1 
> CC: u-boot@lists.denx.de 
> 件名: Re: New TPM commands.
> 
> [ilias.apalodi...@linaro.org 
> からのメールを受け取る頻度は高くありません。これが問題である可能性の理由については、https://aka.ms/LearnAboutSenderIdentification
>  をご覧ください。]
> 
> Hi Niek
> 
> On Wed, 20 Dec 2023 at 09:11, niek.nooij...@omron.com
>  wrote:
> >
> > Hi There
> >
> > I added some new commands to the TPM2 command to allow read/writes to 
> > nv_memory. I also implemented the nv_define and nv_undefine commands so 
> > spaces can be created/deleted.
> > Still need to test with PCR policies, but at least for now we can store 
> > values in the TPM.
> 
> Thanks for the patch. There's a standard process for sending patches
> [0]. Try to follow the guidelines on your next revision, it makes
> things substantially easier for me. Also, spend some time writing a
> clear patch description on *why* you need this merged.
> 
> >
> > Here's the patch:
> >
> > Signed-off-by: Niek Nooijens 
> > BEGIN OF PATCH==
> > diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> > index d93b83ada9..d2a06b9f65 100644
> > --- a/cmd/tpm-v2.c
> > +++ b/cmd/tpm-v2.c
> > @@ -356,6 +356,133 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl 
> > *cmdtp, int flag,
> >   key, key_sz));
> >  }
> >
> > +static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
> > +      int argc, char *const argv[])
> > +{
> > + struct udevice *dev;
> > + struct tpm_chip_priv *priv;
> > + u32 nv_addr, nv_size,nv_attributes, rc;
> > + void *policy_addr = NULL;
> > + size_t policy_size = 0;
> > + int ret;
> > +
> > + nv_attributes = 0;
> > +
> > + if ((argc < 3 && argc > 6) || argc == 4)
> > +   return CMD_RET_USAGE;
> > +
> > + ret = get_tpm();
> > + if (ret)
> > +   return ret;
> > +
> > + priv = dev_get_uclass_priv(dev);
> > + if (!priv)
> > +   return -EINVAL;
> > +
> > + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> 
> Ditch the '//' comments throughout the patch. It's pretty clear what's 
> happening
> 
> > +
> > + nv_size = simple_strtoul(argv[2], NULL, 0); //size
> 
> ditto
> 
> > +
> > + if(argc > 3) { //attributes
> > +   nv_attributes = simple_strtoul(argv[3], NULL, 0);
> > + } else {
> > +   nv_att

Re: New TPM commands.

2023-12-22 Thread Ilias Apalodimas
fset */
>   };
> @@ -930,7 +964,7 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
> const void *data,
>   u32 count)
>  {
>   struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> - uint offset = 10 + 8 + 4 + 9 + 2;
> + uint offset = TPM2_HDR_LEN + 8 + 4 + 9 + 2;
>   uint len = offset + count + 2;
>   /* Use empty password auth if platform hierarchy is disabled */
>   u32 auth = priv->plat_hier_disabled ? HR_NV_INDEX + index :
> @@ -943,18 +977,21 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
> const void *data,
>
>     /* handles 8 bytes */
>     tpm_u32(auth),/* Primary platform seed */
> -   tpm_u32(HR_NV_INDEX + index), /* Password authorisation */
> +   tpm_u32(index),   /*nv index*/
>
>     /* AUTH_SESSION */
> -   tpm_u32(9), /* Authorization size */
> -   tpm_u32(TPM2_RS_PW),  /* Session handle */
> +   tpm_u32(9), /* Authorization size - 4 bytes */
> +   /*auth handle - 9 bytes */
> +   tpm_u32(TPM2_RS_PW),/* Password authorisation */  /* Session 
> handle */
>     tpm_u16(0), /* Size of  */
>     /*  (if any) */
>     0,  /* Attributes: Cont/Excl/Rst */
>     tpm_u16(0), /* Size of  */
>     /*  (if any) */
> -
> -   tpm_u16(count),
> +   /*end auth handle */
> +   tpm_u16(count),/*size of buffer - 2 bytes*/
> +   /*data (buffer)*/
> +   /*offset -> the octet offset into the NV Area*/
>   };
>   size_t response_len = COMMAND_BUFFER_SIZE;
>   u8 response[COMMAND_BUFFER_SIZE];
> --
> 2.34.1
> ===END PATCH===
>
>
> 
> 差出人: Ilias Apalodimas 
> 送信日時: 2023年12月20日 17:17
> 宛先: Niek Nooijens / OC-IAB PBD-C DEVEL 1-1 
> CC: u-boot@lists.denx.de 
> 件名: Re: New TPM commands.
>
> [ilias.apalodi...@linaro.org 
> からのメールを受け取る頻度は高くありません。これが問題である可能性の理由については、https://aka.ms/LearnAboutSenderIdentification
>  をご覧ください。]
>
> Hi Niek
>
> On Wed, 20 Dec 2023 at 09:11, niek.nooij...@omron.com
>  wrote:
> >
> > Hi There
> >
> > I added some new commands to the TPM2 command to allow read/writes to 
> > nv_memory. I also implemented the nv_define and nv_undefine commands so 
> > spaces can be created/deleted.
> > Still need to test with PCR policies, but at least for now we can store 
> > values in the TPM.
>
> Thanks for the patch. There's a standard process for sending patches
> [0]. Try to follow the guidelines on your next revision, it makes
> things substantially easier for me. Also, spend some time writing a
> clear patch description on *why* you need this merged.
>
> >
> > Here's the patch:
> >
> > Signed-off-by: Niek Nooijens 
> > BEGIN OF PATCH==
> > diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> > index d93b83ada9..d2a06b9f65 100644
> > --- a/cmd/tpm-v2.c
> > +++ b/cmd/tpm-v2.c
> > @@ -356,6 +356,133 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl 
> > *cmdtp, int flag,
> >   key, key_sz));
> >  }
> >
> > +static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
> > +      int argc, char *const argv[])
> > +{
> > + struct udevice *dev;
> > + struct tpm_chip_priv *priv;
> > + u32 nv_addr, nv_size,nv_attributes, rc;
> > + void *policy_addr = NULL;
> > + size_t policy_size = 0;
> > + int ret;
> > +
> > + nv_attributes = 0;
> > +
> > + if ((argc < 3 && argc > 6) || argc == 4)
> > +   return CMD_RET_USAGE;
> > +
> > + ret = get_tpm();
> > + if (ret)
> > +   return ret;
> > +
> > + priv = dev_get_uclass_priv(dev);
> > + if (!priv)
> > +   return -EINVAL;
> > +
> > + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
>
> Ditch the '//' comments throughout the patch. It's pretty clear what's 
> happening
>
> > +
> > + nv_size = simple_strtoul(argv[2], NULL, 0); //size
>
> ditto
>
> > +
> > + if(argc > 3) { //attributes
> > +   nv_attributes = simple_strtoul(argv[3], NULL, 0);
> > + } else {
> > +   nv_attributes = 
> > TPMA_NV_PLATFORMCREATE|TPMA_NV_OWNERWRITE|TPMA_NV_OWNERREAD|TPMA_NV_PPWRITE|TPMA_NV_PPREAD;
> > +

Re: New TPM commands.

2023-12-21 Thread niek.nooij...@omron.com
size */
+   /*end auth area*/
    tpm_u16(0), /* auth_size */

    /* message 14 bytes + policy */
@@ -842,6 +846,35 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 
space_index,
  return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
 }

+u32 tpm2_nv_undefine_space(struct udevice *dev, u32 space_index)
+{
+ const int platform_len = sizeof(u32);
+ const int session_hdr_len = 13;
+ const int message_len = 4;
+ u8 command_v2[COMMAND_BUFFER_SIZE] = {
+   /* header 10 bytes */
+   tpm_u16(TPM2_ST_SESSIONS),/* TAG */
+   tpm_u32(TPM2_HDR_LEN + platform_len + session_hdr_len +
+   message_len),/* Length - header + provision + index + 
auth area*/
+   tpm_u32(TPM2_CC_NV_UNDEFINE_SPACE),/* Command code */
+
+   /* handles 4 bytes */
+   tpm_u32(TPM2_RH_PLATFORM),/* Primary platform seed */
+   /* nv_index */
+   tpm_u32(space_index),
+
+   /*null auth session*/
+   tpm_u32(9), /* Header size */
+   tpm_u32(TPM2_RS_PW),  /* Password authorisation FIXME: 
allow PCR authorization */
+   tpm_u16(0), /* nonce_size */
+   0,  /* session_attrs */
+   tpm_u16(0), /* HMAC size */
+   /*end auth area*/
+
+ };
+ return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
+}
+
 u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm,
    const u8 *digest, u32 digest_len)
 {
@@ -890,22 +923,23 @@ u32 tpm2_nv_read_value(struct udevice *dev, u32 index, 
void *data, u32 count)
  u8 command_v2[COMMAND_BUFFER_SIZE] = {
    /* header 10 bytes */
    tpm_u16(TPM2_ST_SESSIONS),/* TAG */
-   tpm_u32(10 + 8 + 4 + 9 + 4),  /* Length */
+   tpm_u32(TPM2_HDR_LEN + 8 + 4 + 9 + 4),/* Length */
    tpm_u32(TPM2_CC_NV_READ), /* Command code */

    /* handles 8 bytes */
    tpm_u32(TPM2_RH_PLATFORM),/* Primary platform seed */
-   tpm_u32(HR_NV_INDEX + index), /* Password authorisation */
+   tpm_u32(index),   /*nv index*/

    /* AUTH_SESSION */
-   tpm_u32(9), /* Authorization size */
-   tpm_u32(TPM2_RS_PW),  /* Session handle */
+   tpm_u32(9), /* Authorization size - 4 bytes*/
+   /*auth handle - 9 bytes */
+   tpm_u32(TPM2_RS_PW),  /* Password authorisation */
    tpm_u16(0), /* Size of  */
    /*  (if any) */
    0,  /* Attributes: Cont/Excl/Rst */
    tpm_u16(0), /* Size of  */
    /*  (if any) */
-
+   /*end auth handle */
    tpm_u16(count),   /* Number of bytes */
    tpm_u16(0), /* Offset */
  };
@@ -930,7 +964,7 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
const void *data,
  u32 count)
 {
  struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
- uint offset = 10 + 8 + 4 + 9 + 2;
+ uint offset = TPM2_HDR_LEN + 8 + 4 + 9 + 2;
  uint len = offset + count + 2;
  /* Use empty password auth if platform hierarchy is disabled */
  u32 auth = priv->plat_hier_disabled ? HR_NV_INDEX + index :
@@ -943,18 +977,21 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, 
const void *data,

    /* handles 8 bytes */
    tpm_u32(auth),/* Primary platform seed */
-   tpm_u32(HR_NV_INDEX + index), /* Password authorisation */
+   tpm_u32(index),   /*nv index*/

    /* AUTH_SESSION */
-   tpm_u32(9), /* Authorization size */
-   tpm_u32(TPM2_RS_PW),  /* Session handle */
+   tpm_u32(9), /* Authorization size - 4 bytes */
+   /*auth handle - 9 bytes */
+   tpm_u32(TPM2_RS_PW),/* Password authorisation */  /* Session 
handle */
    tpm_u16(0), /* Size of  */
    /*  (if any) */
    0,  /* Attributes: Cont/Excl/Rst */
    tpm_u16(0), /* Size of  */
    /*  (if any) */
-
-   tpm_u16(count),
+   /*end auth handle */
+   tpm_u16(count),/*size of buffer - 2 bytes*/
+   /*data (buffer)*/
+   /*offset -> the octet offset into the NV Area*/
  };
  size_t response_len = COMMAND_BUFFER_SIZE;
  u8 response[COMMAND_BUFFER_SIZE];
--
2.34.1
===END PATCH===



差出人: Ilias Apalodimas 
送信日時: 2023年12月20日 17:17
宛先: Niek Nooijens / OC-IAB PBD-C DEVEL 1-1 
CC: u-boot@lists.denx.de 
件名: Re: New TPM commands.

[ilias.apalodi...@linaro.org 
からのメールを受け取

New TPM commands.

2023-12-20 Thread niek.nooij...@omron.com
Hi There

I added some new commands to the TPM2 command to allow read/writes to 
nv_memory. I also implemented the nv_define and nv_undefine commands so spaces 
can be created/deleted.
Still need to test with PCR policies, but at least for now we can store values 
in the TPM.

Here's the patch:

Signed-off-by: Niek Nooijens 
BEGIN OF PATCH==
diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
index d93b83ada9..d2a06b9f65 100644
--- a/cmd/tpm-v2.c
+++ b/cmd/tpm-v2.c
@@ -356,6 +356,133 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl *cmdtp, 
int flag,
  key, key_sz));
 }

+static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
+      int argc, char *const argv[])
+{
+ struct udevice *dev;
+ struct tpm_chip_priv *priv;
+ u32 nv_addr, nv_size,nv_attributes, rc;
+ void *policy_addr = NULL;
+ size_t policy_size = 0;
+ int ret;
+
+ nv_attributes = 0;
+
+ if ((argc < 3 && argc > 6) || argc == 4)
+   return CMD_RET_USAGE;
+
+ ret = get_tpm();
+ if (ret)
+   return ret;
+
+ priv = dev_get_uclass_priv(dev);
+ if (!priv)
+   return -EINVAL;
+
+ nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
+
+ nv_size = simple_strtoul(argv[2], NULL, 0); //size
+
+ if(argc > 3) { //attributes
+   nv_attributes = simple_strtoul(argv[3], NULL, 0);
+ } else {
+   nv_attributes = 
TPMA_NV_PLATFORMCREATE|TPMA_NV_OWNERWRITE|TPMA_NV_OWNERREAD|TPMA_NV_PPWRITE|TPMA_NV_PPREAD;
+ }
+
+ if(argc > 4) {//policy
+   policy_addr = map_sysmem(simple_strtoul(argv[4], NULL, 0), 0);
+   if((nv_attributes & (TPMA_NV_POLICYREAD|TPMA_NV_POLICYWRITE)) == 0) 
{ //not sure if I should enforce this or just warn the user?
+ printf("Warning: policy provided, but TPMA_NV_POLICYREAD and 
TPMA_NV_POLICYWRITE are NOT set!\n");
+   }
+   policy_size = simple_strtoul(argv[5], NULL, 0);
+ }
+
+ rc = tpm2_nv_define_space(dev, nv_addr, nv_size, 
nv_attributes,policy_addr, policy_size);
+
+ if (rc) {
+   printf("ERROR: nv_define #%u returns: 0x%x\n", nv_addr, rc);
+ }
+ if(argc > 4) {
+   unmap_sysmem(policy_addr);
+ }
+ return report_return_code(rc);
+}
+
+static int do_tpm_nv_undefine(struct cmd_tbl *cmdtp, int flag,
+      int argc, char *const argv[])
+{
+ struct udevice *dev;
+ u32 nv_addr,ret, rc;
+
+ ret = get_tpm();
+ if (ret)
+   return ret;
+
+ if (argc !=2)
+ return CMD_RET_USAGE;
+ nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
+ rc = tpm2_nv_undefine_space(dev, nv_addr);
+
+ return report_return_code(rc);
+}
+
+static int do_tpm_nv_read_value(struct cmd_tbl *cmdtp, int flag,
+      int argc, char *const argv[])
+{
+ struct udevice *dev;
+ u32 nv_addr, nv_size, rc;
+ int ret;
+ void *out_data;
+ ret = get_tpm();
+   if (ret)
+ return ret;
+
+   if (argc != 4)
+ return CMD_RET_USAGE;
+
+ nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
+
+ nv_size = simple_strtoul(argv[2], NULL, 0); //size
+
+ out_data = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
+
+ rc = tpm2_nv_read_value(dev,nv_addr, out_data, nv_size);
+
+ if (rc) {
+   printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
+ }
+ unmap_sysmem(out_data);
+ return report_return_code(rc);
+}
+
+static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag,
+      int argc, char *const argv[])
+{
+ struct udevice *dev;
+ u32 nv_addr, nv_size, rc;
+ int ret;
+ ret = get_tpm();
+   if (ret)
+ return ret;
+
+   if (argc != 4)
+ return CMD_RET_USAGE;
+
+ nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
+
+ nv_size = simple_strtoul(argv[2], NULL, 0); //size
+
+ void *data_to_write = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
+
+ rc = tpm2_nv_write_value(dev,nv_addr, data_to_write, nv_size);
+
+ if (rc) {
+   printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
+ }
+ unmap_sysmem(data_to_write);
+ return report_return_code(rc);
+}
+
 static struct cmd_tbl tpm2_commands[] = {
  U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""),
  U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""),
@@ -374,6 +501,10 @@ static struct cmd_tbl tpm2_commands[] = {
   do_tpm_pcr_setauthpolicy, "", ""),
  U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1,
   do_tpm_pcr_setauthvalue, "", ""),
+ U_BOOT_CMD_MKENT(nv_define, 0, 1, do_tpm_nv_define, "", ""),
+ U_BOOT_CMD_MKENT(nv_undefine, 0, 1, do_tpm_nv_undefine, "", ""),
+ U_BOOT_CMD_MKENT(nv_read, 0, 1, do_tpm_nv_read_value, "", ""),
+ U_BOOT_CMD_MKENT(nv_write, 0, 1, do_tpm_nv_write_value, "", ""),
 };

 struct cmd_tbl 

Re: New TPM commands.

2023-12-20 Thread Ilias Apalodimas
Hi Niek

On Wed, 20 Dec 2023 at 09:11, niek.nooij...@omron.com
 wrote:
>
> Hi There
>
> I added some new commands to the TPM2 command to allow read/writes to 
> nv_memory. I also implemented the nv_define and nv_undefine commands so 
> spaces can be created/deleted.
> Still need to test with PCR policies, but at least for now we can store 
> values in the TPM.

Thanks for the patch. There's a standard process for sending patches
[0]. Try to follow the guidelines on your next revision, it makes
things substantially easier for me. Also, spend some time writing a
clear patch description on *why* you need this merged.

>
> Here's the patch:
>
> Signed-off-by: Niek Nooijens 
> BEGIN OF PATCH==
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index d93b83ada9..d2a06b9f65 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -356,6 +356,133 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl 
> *cmdtp, int flag,
>   key, key_sz));
>  }
>
> +static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
> +      int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + struct tpm_chip_priv *priv;
> + u32 nv_addr, nv_size,nv_attributes, rc;
> + void *policy_addr = NULL;
> + size_t policy_size = 0;
> + int ret;
> +
> + nv_attributes = 0;
> +
> + if ((argc < 3 && argc > 6) || argc == 4)
> +   return CMD_RET_USAGE;
> +
> + ret = get_tpm();
> + if (ret)
> +   return ret;
> +
> + priv = dev_get_uclass_priv(dev);
> + if (!priv)
> +   return -EINVAL;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr

Ditch the '//' comments throughout the patch. It's pretty clear what's happening

> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size

ditto

> +
> + if(argc > 3) { //attributes
> +   nv_attributes = simple_strtoul(argv[3], NULL, 0);
> + } else {
> +   nv_attributes = 
> TPMA_NV_PLATFORMCREATE|TPMA_NV_OWNERWRITE|TPMA_NV_OWNERREAD|TPMA_NV_PPWRITE|TPMA_NV_PPREAD;
> + }

You don't need {}

> +
> + if(argc > 4) {//policy
> +   policy_addr = map_sysmem(simple_strtoul(argv[4], NULL, 0), 0);
> +   if((nv_attributes & (TPMA_NV_POLICYREAD|TPMA_NV_POLICYWRITE)) == 
> 0) { //not sure if I should enforce this or just warn the user?
> + printf("Warning: policy provided, but TPMA_NV_POLICYREAD 
> and TPMA_NV_POLICYWRITE are NOT set!\n");

Just return an error here.

> +   }
> +   policy_size = simple_strtoul(argv[5], NULL, 0);
> + }
> +
> + rc = tpm2_nv_define_space(dev, nv_addr, nv_size, 
> nv_attributes,policy_addr, policy_size);
> +
> + if (rc) {
> +   printf("ERROR: nv_define #%u returns: 0x%x\n", nv_addr, rc);
> + }
> + if(argc > 4) {

policy_addr is initialized to NULL, just look at the ptr here instead of argc

> +   unmap_sysmem(policy_addr);
> + }

You don't need {} in any of the above

> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_undefine(struct cmd_tbl *cmdtp, int flag,
> +      int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr,ret, rc;
> +
> + ret = get_tpm();
> + if (ret)
> +   return ret;
> +
> + if (argc !=2)
> + return CMD_RET_USAGE;
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> + rc = tpm2_nv_undefine_space(dev, nv_addr);
> +
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_read_value(struct cmd_tbl *cmdtp, int flag,
> +      int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, nv_size, rc;
> + int ret;
> + void *out_data;
> + ret = get_tpm();
> +   if (ret)
> + return ret;
> +
> +   if (argc != 4)
> + return CMD_RET_USAGE;

Indentation is off

> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size
> +
> + out_data = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
> +
> + rc = tpm2_nv_read_value(dev,nv_addr, out_data, nv_size);
> +
> + if (rc) {
> +   printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
> + }
> + unmap_sysmem(out_data);
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag,
> +      int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, nv_size, rc;
> + int ret;
> + ret = get_tpm();
> +   if (ret)
> + return ret;
> +
> +   if (argc != 4)
> + return CMD_RET_USAGE;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size
> +
> + void *data_to_write = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
> +
> + rc =