[03/19] hdata/tpmrel.c: add firmware event log info to the tpm node

This parses the firmware event log information from the
secureboot_tpm_info HDAT structure and add it to the tpm device tree
node.

There can be multiple secureboot_tpm_info entries with each entry
corresponding to a master processor that has a tpm device, however,
multiple tpm is not supported.

Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
---
 hdata/Makefile.inc       |  2 +-
 hdata/hdata.h            |  1 +
 hdata/spira.c            |  3 ++
 hdata/spira.h            | 33 ++++++++++++++++++
 hdata/test/hdata_to_dt.c |  1 +
 hdata/tpmrel.c           | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 hdata/tpmrel.c

On Sun, Nov 12, 2017 at 4:28 AM, Claudio Carvalho
<cclaudio@linux.vnet.ibm.com> wrote:
> This parses the firmware event log information from the
> secureboot_tpm_info HDAT structure and add it to the tpm device tree
> node.
>
> There can be multiple secureboot_tpm_info entries with each entry
> corresponding to a master processor that has a tpm device, however,
> multiple tpm is not supported.
>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
> ---
>  hdata/Makefile.inc       |  2 +-
>  hdata/hdata.h            |  1 +
>  hdata/spira.c            |  3 ++
>  hdata/spira.h            | 33 ++++++++++++++++++
>  hdata/test/hdata_to_dt.c |  1 +
>  hdata/tpmrel.c           | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644 hdata/tpmrel.c
>
> diff --git a/hdata/Makefile.inc b/hdata/Makefile.inc
> index 5b79dfe..c17b04f 100644
> --- a/hdata/Makefile.inc
> +++ b/hdata/Makefile.inc
> @@ -2,7 +2,7 @@
>
>  SUBDIRS += hdata
>  HDATA_OBJS = spira.o paca.o pcia.o hdif.o memory.o fsp.o iohub.o vpd.o slca.o
> -HDATA_OBJS += cpu-common.o vpd-common.o hostservices.o i2c.o
> +HDATA_OBJS += cpu-common.o vpd-common.o hostservices.o i2c.o tpmrel.o
>  DEVSRC_OBJ = hdata/built-in.o
>
>  $(DEVSRC_OBJ): $(HDATA_OBJS:%=hdata/%)
> diff --git a/hdata/hdata.h b/hdata/hdata.h
> index ce3719a..981affd 100644
> --- a/hdata/hdata.h
> +++ b/hdata/hdata.h
> @@ -54,6 +54,7 @@ extern void slca_dt_add_sai_node(void);
>  extern bool hservices_from_hdat(const void *fdt, size_t size);
>  int parse_i2c_devs(const struct HDIF_common_hdr *hdr, int idata_index,
>         struct dt_node *xscom);
> +extern void node_stb_parse(void);
>
>  /* used to look up the device-tree node representing a slot */
>  struct dt_node *find_slot_entry_node(struct dt_node *root, u32 entry_id);
> diff --git a/hdata/spira.c b/hdata/spira.c
> index 33926ed..576b5c5 100644
> --- a/hdata/spira.c
> +++ b/hdata/spira.c
> @@ -1664,6 +1664,9 @@ int parse_hdat(bool is_opal)
>
>         add_stop_levels();
>
> +       /* Parse node secure and trusted boot data */
> +       node_stb_parse();
> +
>         prlog(PR_DEBUG, "Parsing HDAT...done\n");
>
>         return 0;
> diff --git a/hdata/spira.h b/hdata/spira.h
> index a9f1313..88fd2bf 100644
> --- a/hdata/spira.h
> +++ b/hdata/spira.h
> @@ -1225,6 +1225,39 @@ struct ipmi_sensors {
>  /* Idata index 1 : LED - sensors ID mapping data */
>  #define IPMI_SENSORS_IDATA_LED         1
>
> +/*
> + * Node Secure and Trusted Boot Related Data
> + */
> +#define STB_HDIF_SIG   "TPMREL"
> +
> +/*
> + * Idata index 0 : Secure Boot and TPM Instance Info
> + *
> + * There can be multiple entries with each entry corresponding to
> + * a master processor that has a TPM device
> + */
> +#define TPMREL_IDATA_SECUREBOOT_TPM_INFO       0
> +
> +struct secureboot_tpm_info {
> +       __be32 chip_id;
> +       __be32 dbob_id;
> +       uint8_t locality1;
> +       uint8_t locality2;
> +       uint8_t locality3;
> +       uint8_t locality4;
> +#define TPM_PRESENT_AND_FUNCTIONAL     0x01
> +#define TPM_PRESENT_AND_NOT_FUNCTIONAL 0x02
> +#define TPM_NOT_PRESENT                        0x03
> +       uint8_t tpm_status;
> +       uint8_t reserved[3];
> +       /* zero indicates no tpm log data */
> +       __be32 srtm_log_offset;
> +       __be32 srtm_log_size;
> +       /* zero indicates no tpm log data */
> +       __be32 drtm_log_offset;
> +       __be32 drtm_log_size;
> +} __packed;
> +
>  static inline const char *cpu_state(u32 flags)
>  {
>         switch ((flags & CPU_ID_VERIFY_MASK) >> CPU_ID_VERIFY_SHIFT) {
> diff --git a/hdata/test/hdata_to_dt.c b/hdata/test/hdata_to_dt.c
> index f597914..09e192e 100644
> --- a/hdata/test/hdata_to_dt.c
> +++ b/hdata/test/hdata_to_dt.c
> @@ -139,6 +139,7 @@ static bool spira_check_ptr(const void *ptr, const char *file, unsigned int line
>  #include "../slca.c"
>  #include "../hostservices.c"
>  #include "../i2c.c"
> +#include "../tpmrel.c"
>  #include "../../core/vpd.c"
>  #include "../../core/device.c"
>  #include "../../core/chip.c"
> diff --git a/hdata/tpmrel.c b/hdata/tpmrel.c
> new file mode 100644
> index 0000000..0aaa70b
> --- /dev/null
> +++ b/hdata/tpmrel.c
> @@ -0,0 +1,90 @@
> +/* Copyright 2013-2017 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "TPMREL: " fmt
> +#endif
> +
> +#include <skiboot.h>
> +#include <device.h>
> +
> +#include "spira.h"
> +#include "hdata.h"
> +#include "hdif.h"
> +
> +static void tpmrel_add_firmware_event_log(const struct HDIF_common_hdr *hdif_hdr)
> +{
> +       const struct secureboot_tpm_info *stinfo;
> +       struct dt_node *xscom, *node;
> +       uint64_t addr;
> +       bool tpmfound = false;
> +       int count, i;
> +
> +       count = HDIF_get_iarray_size(hdif_hdr, TPMREL_IDATA_SECUREBOOT_TPM_INFO);
> +       if (count > 1) {
> +               prlog(PR_ERR, "multinode not supported, count=%d\n", count);
> +               return;
> +       }
> +
> +       for (i = 0; i < count; i++) {
> +
> +               stinfo = HDIF_get_iarray_item(hdif_hdr,
> +                                             TPMREL_IDATA_SECUREBOOT_TPM_INFO,
> +                                             i, NULL);
> +

> +               xscom = find_xscom_for_chip(be32_to_cpu(stinfo->chip_id));
> +               if (xscom) {
> +                       dt_for_each_node(xscom, node) {
> +                               if (dt_has_node_property(node, "label", "tpm")) {
> +                                       tpmfound=true;

I usually write this sort of thing so that it breaks out of the loop
when it finds the target node. Then you can just check if !node to see
if the scan failed/succeeded rather than having a separate variable.
Up to you though.

> +                                       addr = (uint64_t) stinfo +
> +                                               be32_to_cpu(stinfo->srtm_log_offset);
> +                                       dt_add_property_u64s(node, "linux,sml-base", addr);
> +                                       dt_add_property_cells(node, "linux,sml-size",
> +                                                             be32_to_cpu(stinfo->srtm_log_size));
> +                                       break;
> +                               }
> +                       }
> +                       if (!tpmfound &&

Also, can you add a comment explaining what you're doing here. It took
me a minute to work out it was looking up the I2C device node. It
might be worth adding the I2C link ID as a DT_PRIVATE property to the
I2C device node and looking for that instead.

> +                           stinfo->tpm_status == TPM_PRESENT_AND_FUNCTIONAL) {

Hmm, won't the sml-base and size properties that you just added be
invalid if the TPM is non-functional? You might want to move this
check to the start of the loop.

> +                               prlog(PR_ERR, "TPM functional but not found "
> +                                     "for chip_id=%d.\n", stinfo->chip_id);
> +                               continue;
> +                       }
> +               } else {
> +                       prlog(PR_ERR, "could not add TPM device, chip_id=%d "
> +                             "invalid\n", stinfo->chip_id);
> +                       continue;
> +               }
> +       }
> +}
> +
> +void node_stb_parse(void)
> +{
> +       struct HDIF_common_hdr *hdif_hdr;
> +
> +       /* TPMREL exists only on POWER9 and above */
> +       if (proc_gen < proc_gen_p9)
> +               return;

Is the secure boot information always present on P9s? You might need
to check the structure version too or do some sanity checking of the
data itself.

> +
> +       hdif_hdr = get_hdif(&spira.ntuples.node_stb_data, "TPMREL");
> +       if (!hdif_hdr) {
> +               prlog(PR_DEBUG, "could not find TPMREL data\n");
> +                       return;
> +       }
> +
> +       tpmrel_add_firmware_event_log(hdif_hdr);
> +}
> --
> 2.7.4
>

Oliver <oohall@gmail.com> writes:
>> --- /dev/null
>> +++ b/hdata/tpmrel.c
>> @@ -0,0 +1,90 @@
>> +/* Copyright 2013-2017 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#ifndef pr_fmt
>> +#define pr_fmt(fmt) "TPMREL: " fmt
>> +#endif
>> +
>> +#include <skiboot.h>
>> +#include <device.h>
>> +
>> +#include "spira.h"
>> +#include "hdata.h"
>> +#include "hdif.h"
>> +
>> +static void tpmrel_add_firmware_event_log(const struct HDIF_common_hdr *hdif_hdr)
>> +{
>> +       const struct secureboot_tpm_info *stinfo;
>> +       struct dt_node *xscom, *node;
>> +       uint64_t addr;
>> +       bool tpmfound = false;
>> +       int count, i;
>> +
>> +       count = HDIF_get_iarray_size(hdif_hdr, TPMREL_IDATA_SECUREBOOT_TPM_INFO);
>> +       if (count > 1) {
>> +               prlog(PR_ERR, "multinode not supported, count=%d\n", count);
>> +               return;
>> +       }
>> +
>> +       for (i = 0; i < count; i++) {
>> +
>> +               stinfo = HDIF_get_iarray_item(hdif_hdr,
>> +                                             TPMREL_IDATA_SECUREBOOT_TPM_INFO,
>> +                                             i, NULL);
>> +
>
>> +               xscom = find_xscom_for_chip(be32_to_cpu(stinfo->chip_id));
>> +               if (xscom) {
>> +                       dt_for_each_node(xscom, node) {
>> +                               if (dt_has_node_property(node, "label", "tpm")) {
>> +                                       tpmfound=true;
>
> I usually write this sort of thing so that it breaks out of the loop
> when it finds the target node. Then you can just check if !node to see
> if the scan failed/succeeded rather than having a separate variable.
> Up to you though.

I'm inclined to agree, I think it ends up being a bit neater.

>
>> +                                       addr = (uint64_t) stinfo +
>> +                                               be32_to_cpu(stinfo->srtm_log_offset);
>> +                                       dt_add_property_u64s(node, "linux,sml-base", addr);
>> +                                       dt_add_property_cells(node, "linux,sml-size",
>> +                                                             be32_to_cpu(stinfo->srtm_log_size));
>> +                                       break;
>> +                               }
>> +                       }
>> +                       if (!tpmfound &&
>
> Also, can you add a comment explaining what you're doing here. It took
> me a minute to work out it was looking up the I2C device node. It
> might be worth adding the I2C link ID as a DT_PRIVATE property to the
> I2C device node and looking for that instead.
>
>> +                           stinfo->tpm_status == TPM_PRESENT_AND_FUNCTIONAL) {
>
> Hmm, won't the sml-base and size properties that you just added be
> invalid if the TPM is non-functional? You might want to move this
> check to the start of the loop.

+1

On 11/11/2017 10:58 PM, Claudio Carvalho wrote:
> This parses the firmware event log information from the
> secureboot_tpm_info HDAT structure and add it to the tpm device tree
> node.
>
> There can be multiple secureboot_tpm_info entries with each entry
> corresponding to a master processor that has a tpm device, however,
> multiple tpm is not supported.
>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.vnet.ibm.com>
> ---
>  hdata/Makefile.inc       |  2 +-
>  hdata/hdata.h            |  1 +
>  hdata/spira.c            |  3 ++
>  hdata/spira.h            | 33 ++++++++++++++++++
>  hdata/test/hdata_to_dt.c |  1 +
>  hdata/tpmrel.c           | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644 hdata/tpmrel.c
>


.../...

> +
> +static void tpmrel_add_firmware_event_log(const struct HDIF_common_hdr *hdif_hdr)
> +{
> +	const struct secureboot_tpm_info *stinfo;
> +	struct dt_node *xscom, *node;
> +	uint64_t addr;
> +	bool tpmfound = false;
> +	int count, i;
> +
> +	count = HDIF_get_iarray_size(hdif_hdr, TPMREL_IDATA_SECUREBOOT_TPM_INFO);
> +	if (count > 1) {
> +		prlog(PR_ERR, "multinode not supported, count=%d\n", count);
> +		return;
> +	}
> +
> +	for (i = 0; i < count; i++) {
> +
> +		stinfo = HDIF_get_iarray_item(hdif_hdr,
> +					      TPMREL_IDATA_SECUREBOOT_TPM_INFO,
> +					      i, NULL);

Again don't trust HDAT too much. Please check whether stinfo got valid data or not.

> +
> +		xscom = find_xscom_for_chip(be32_to_cpu(stinfo->chip_id));
> +		if (xscom) {
> +			dt_for_each_node(xscom, node) {
> +				if (dt_has_node_property(node, "label", "tpm")) {
> +					tpmfound=true;
> +					addr = (uint64_t) stinfo +
> +						be32_to_cpu(stinfo->srtm_log_offset);
> +					dt_add_property_u64s(node, "linux,sml-base", addr);
> +					dt_add_property_cells(node, "linux,sml-size",
> +							      be32_to_cpu(stinfo->srtm_log_size));
> +					break;
> +				}
> +			}
> +			if (!tpmfound &&
> +			    stinfo->tpm_status == TPM_PRESENT_AND_FUNCTIONAL) {
> +				prlog(PR_ERR, "TPM functional but not found "
> +				      "for chip_id=%d.\n", stinfo->chip_id);
> +				continue;
> +			}
> +		} else {
> +			prlog(PR_ERR, "could not add TPM device, chip_id=%d "
> +			      "invalid\n", stinfo->chip_id);
> +			continue;
> +		}
> +	}
> +}
> +
> +void node_stb_parse(void)
> +{
> +	struct HDIF_common_hdr *hdif_hdr;
> +
> +	/* TPMREL exists only on POWER9 and above */
> +	if (proc_gen < proc_gen_p9)
> +		return;
> +
> +	hdif_hdr = get_hdif(&spira.ntuples.node_stb_data, "TPMREL");
> +	if (!hdif_hdr) {
> +		prlog(PR_DEBUG, "could not find TPMREL data\n");
> +			return;

Coding style issue.

-Vasant