Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 12 Oct 2022 11:52:15 +0300
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     Pankaj Gupta <pankaj.gupta@nxp.com>
Cc:     a.fatoum@pengutronix.de, gilad@benyossef.com, Jason@zx2c4.com,
        jejb@linux.ibm.com, zohar@linux.ibm.com, dhowells@redhat.com,
        sumit.garg@linaro.org, david@sigma-star.at, michael@walle.cc,
        john.ernberg@actia.se, jmorris@namei.org, serge@hallyn.com,
        herbert@gondor.apana.org.au, davem@davemloft.net,
        j.luebbe@pengutronix.de, ebiggers@kernel.org, richard@nod.at,
        keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
        linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org, sahil.malhotra@nxp.com,
        kshitiz.varshney@nxp.com, horia.geanta@nxp.com, V.Sethi@nxp.com
Subject: Re: [PATCH v0 1/8] hw-bound-key: introducing the generic structure
Message-ID: <Y0aAP/NvH9mgAi8n@kernel.org>
References: <20221006130837.17587-1-pankaj.gupta@nxp.com>
 <20221006130837.17587-2-pankaj.gupta@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221006130837.17587-2-pankaj.gupta@nxp.com>
Precedence: bulk

On Thu, Oct 06, 2022 at 06:38:30PM +0530, Pankaj Gupta wrote:
> Hardware bound keys buffer has additional information,
> that will be accessed using this new structure.

I don't really understand what I should get from this.

It lacks motivation and function of this structure, even
the name of the structure.

Hardware bound key does not mean anything at all without
a context. I don't know what it is.

> 
> structure members are:
> - flags, flags for hardware specific information.
> - key_sz, size of the plain key.

Who cares listing member names?

> 
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---
>  include/linux/hw_bound_key.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 include/linux/hw_bound_key.h
> 
> diff --git a/include/linux/hw_bound_key.h b/include/linux/hw_bound_key.h
> new file mode 100644
> index 000000000000..e7f152410438
> --- /dev/null
> +++ b/include/linux/hw_bound_key.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only
> + *
> + * Copyright 2022 NXP
> + * Author: Pankaj Gupta <pankaj.gupta@nxp.com>

Formatting here is incorrect and there is no such license in
existence as "GPL-2.0-only".

Should probably be:

/* SPDX-License-Identifier: GPL-2.0+ */
/*
 * Copyright (C) 2022 NXP Semiconductors N.V.
 */

Author-field is redundant as it is part of the git metadata.
Also it is inaccurate description of authorship, as a file
can have multiple contributors over time.

This all is documented in 

https://www.kernel.org/doc/html/latest/process/license-rules.html

> + */
> +
> +#ifndef _HW_BOUND_KEY_H
> +#define _HW_BOUND_KEY_H
> +
> +#include "types.h"
> +
> +struct hw_bound_key_info {
> +	/* Key types specific to the hw. [Implementation Defined]
> +	 */
> +	uint8_t flags;
> +	uint8_t reserved;
> +	/* Plain key size.
> +	 */
> +	uint16_t key_sz;
> +};
> +
> +#define set_hbk_info(hbk_info, hw_flags, key_len) do {\
> +	hbk_info->flags = hw_flags;\
> +	hbk_info->key_sz = key_len;\
> +} while (0)
> +
> +#endif /* _HW_BOUND_KEY_H */
> -- 
> 2.17.1
> 

BR, Jarkko