[Skiboot] [PATCH 02/19] hdata/spira: add ibm, secureboot node in P9

[Vasant Hegde]

On 11/11/2017 10:58 PM, Claudio Carvalho wrote:
> In P9, skiboot builds the device tree from the HDAT. These are the
> "ibm,secureboot" node changes compared to P8:
>
> - The Container-Verification-Code (CVC), a.k.a. ROM code, is no longer
>   stored in a secure ROM with static address. In P9, it is stored in a
>   hostboot reserved memory and each service provided also has a version,
>   not only an offset.
>
> - The hash-algo property is not provided via HDAT, instead it provides
>   the hw-key-hash-size, which is indeed the information required by the
>   CVC to verify containers.
>
> Although the CVC location has changed and the hash-algo is not provided
> in the HDAT, the "ibm,secureboot" compatible property doesn't need to be
> bumped to "ibm,secureboot-v2" in P9. For now we can initialize the CVC
> early during the HDAT parsing and emulate the hash-algo from the
> hw-key-hash-size. The "ibm,secureboot" node update can be postponed,
> even because currently only skiboot consumes it.
>
> This parses the iplparams_sysparams HDAT structure and creates the
> "ibm,secureboot" in P9.
>
> Signed-off-by: Claudio Carvalho <cclaudio at linux.vnet.ibm.com>
> ---
>  hdata/spira.c | 39 +++++++++++++++++++++++++++++++++++++++
>  hdata/spira.h | 14 ++++++++------
>  2 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/hdata/spira.c b/hdata/spira.c
> index 936fadc..33926ed 100644
> --- a/hdata/spira.c
> +++ b/hdata/spira.c
> @@ -933,6 +933,42 @@ static void add_nmmu(void)
>  	}
>  }
>
> +static void dt_init_secureboot_node(const struct iplparams_sysparams *sysparams)
> +{
> +	struct dt_node *node;
> +	u16 sys_sec_setting;
> +	u16 hw_key_hash_size;
> +
> +	node = dt_new(dt_root, "ibm,secureboot");
> +	assert(node);
> +
> +	dt_add_property_string(node, "compatible", "ibm,secureboot-v1");
> +
> +	sys_sec_setting = be16_to_cpu(sysparams->sys_sec_setting);
> +	if (sys_sec_setting & SEC_CONTAINER_SIG_CHECKING)
> +		dt_add_property(node, "secure-enabled", NULL, 0);
> +	if (sys_sec_setting & SEC_HASHES_EXTENDED_TO_TPM)
> +		dt_add_property(node, "trusted-enabled", NULL, 0);
> +
> +	hw_key_hash_size = be16_to_cpu(sysparams->hw_key_hash_size);
> +
> +	/*
> +	 * FIXME: replace hash-algo by the hw-key-hash-size property when
> +	 * compatible is bumped to ibm,secureboot-v2
> +	 */
> +	if (hw_key_hash_size == 64) {
> +		dt_add_property_string(node, "hash-algo", "sha512");
> +		dt_add_property(node, "hw-key-hash", sysparams->hw_key_hash,
> +				hw_key_hash_size);
> +	} else {
> +		prlog(PR_ERR, "TPMREL: hw_key_hash_size=%d not supported\n",
> +		      hw_key_hash_size);
> +	}
> +
> +	if (be16_to_cpu(sysparams->sys_attributes) & SYS_ATTR_MULTIPLE_TPM)
> +		prlog(PR_WARNING, "Multiple TPM set, but not supported\n");
> +}
> +
>  static void add_iplparams_sys_params(const void *iplp, struct dt_node *node)
>  {
>  	const struct iplparams_sysparams *p;
> @@ -1019,6 +1055,9 @@ static void add_iplparams_sys_params(const void *iplp, struct dt_node *node)
>  	sys_attributes = be32_to_cpu(p->sys_attributes);
>  	if (sys_attributes & SYS_ATTR_RISK_LEVEL)
>  		dt_add_property(node, "elevated-risk-level", NULL, 0);
> +
> +	if (version >= 0x60)

IMO its not safe to just rely on the version number. We have had number of such 
issues earlier.

May be better add P9 check here and then check whether values are actually 
populated or not ?

Otherwise patch looks good to me.

Reviewed-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>

-Vasant