[v2,08/14] platform/x86/intel/ifs: Add metadata support

  From: Ashok Raj <ashok.raj@intel.com>

One of the existing reserved fields in microcode header has been
allocated to indicate the size for metadata structures.

The metadata section within microcode header is as shown below:

    Microcode Format
   +----------------------+  Base
   |Header Version        |
   +----------------------+
   |Update revision       |
   +----------------------+
   |Date DDMMYYYY         |
   +----------------------+
   |Sig                   |
   +----------------------+
   |Checksum              |
   +----------------------+
   |Loader Version        |
   +----------------------+
   |Processor Flags       |
   +----------------------+
   |Data Size             |
   +----------------------+
   |Total Size            |
   +----------------------+
   |Meta Size             |
   +----------------------+
   |Reserved              |
   +----------------------+
   |Reserved              |
   +----------------------+  Base+48
   |                      |
   |                      |
   |                      |
   |                      |
   |    Microcode         |
   |                      |
   |     Data             |
   |                      |
   |                      |
   +----------------------+  Base+48+data_size-
   |                      |     meta_size
   |   Meta Data          |
   |   structure(s)       |
   |                      |
   +----------------------+  Base+48+data_size
   |   Extended Signature |
   |        Table         |
   |                      |
   |                      |
   |                      |
   |                      |
   |                      |
   +----------------------+  Base+total_size

Add an accessor function which will return a pointer to the
start of a specific meta_type being queried.

In subsequent patches, IFS test image file (which reuse microcode header
format) will make use of metadata section.

Reviewed-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
 drivers/platform/x86/intel/ifs/load.c | 32 +++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
  

On 11/7/2022 2:53 PM, Jithu Joseph wrote:
> In subsequent patches, IFS test image file (which reuse microcode header

s/In subsequent patches/Upcoming

s/reuse/reuses

> format) will make use of metadata section.
> 
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>

Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
  

Hi,

On 11/7/22 23:53, Jithu Joseph wrote:
> From: Ashok Raj <ashok.raj@intel.com>
> 
> One of the existing reserved fields in microcode header has been
> allocated to indicate the size for metadata structures.
> 
> The metadata section within microcode header is as shown below:
> 
>     Microcode Format
>    +----------------------+  Base
>    |Header Version        |
>    +----------------------+
>    |Update revision       |
>    +----------------------+
>    |Date DDMMYYYY         |
>    +----------------------+
>    |Sig                   |
>    +----------------------+
>    |Checksum              |
>    +----------------------+
>    |Loader Version        |
>    +----------------------+
>    |Processor Flags       |
>    +----------------------+
>    |Data Size             |
>    +----------------------+
>    |Total Size            |
>    +----------------------+
>    |Meta Size             |
>    +----------------------+
>    |Reserved              |
>    +----------------------+
>    |Reserved              |
>    +----------------------+  Base+48
>    |                      |
>    |                      |
>    |                      |
>    |                      |
>    |    Microcode         |
>    |                      |
>    |     Data             |
>    |                      |
>    |                      |
>    +----------------------+  Base+48+data_size-
>    |                      |     meta_size
>    |   Meta Data          |
>    |   structure(s)       |
>    |                      |
>    +----------------------+  Base+48+data_size
>    |   Extended Signature |
>    |        Table         |
>    |                      |
>    |                      |
>    |                      |
>    |                      |
>    |                      |
>    +----------------------+  Base+total_size
> 
> Add an accessor function which will return a pointer to the
> start of a specific meta_type being queried.
> 
> In subsequent patches, IFS test image file (which reuse microcode header
> format) will make use of metadata section.
> 
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/platform/x86/intel/ifs/load.c | 32 +++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index 89ce265887ea..60ba5a057f91 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -44,6 +44,38 @@ static const char * const scan_authentication_status[] = {
>  	[2] = "Chunk authentication error. The hash of chunk did not match expected value"
>  };
>  
> +#define META_TYPE_END		(0)
> +
> +struct metadata_header {
> +	unsigned int		type;
> +	unsigned int		blk_size;
> +};
> +
> +static struct metadata_header *ifs_find_meta_data(void *ucode, unsigned int meta_type)
> +{
> +	struct metadata_header *meta_header;
> +	unsigned long data_size, total_meta;
> +	unsigned long meta_size = 0;
> +
> +	data_size = get_datasize(ucode);
> +	total_meta = ((struct microcode_intel *)ucode)->hdr.metasize;
> +
> +	if (!total_meta)
> +		return NULL;
> +
> +	meta_header = (ucode + MC_HEADER_SIZE + data_size) - total_meta;
> +
> +	while ((meta_header->type != META_TYPE_END) && meta_header->blk_size &&
> +	       meta_size < total_meta) {
> +		meta_size += meta_header->blk_size;
> +		if (meta_header->type == meta_type)
> +			return meta_header;
> +
> +		meta_header = (void *)meta_header + meta_header->blk_size;
> +	}
> +	return NULL;
> +}
> +
>  /*
>   * To copy scan hashes and authenticate test chunks, the initiating cpu must point
>   * to the EDX:EAX to the test image in linear address.
  

On Mon, Nov 07, 2022 at 02:53:17PM -0800, Jithu Joseph wrote:
> diff --git a/drivers/platform/x86/intel/ifs/load.c b/drivers/platform/x86/intel/ifs/load.c
> index 89ce265887ea..60ba5a057f91 100644
> --- a/drivers/platform/x86/intel/ifs/load.c
> +++ b/drivers/platform/x86/intel/ifs/load.c
> @@ -44,6 +44,38 @@ static const char * const scan_authentication_status[] = {
>  	[2] = "Chunk authentication error. The hash of chunk did not match expected value"
>  };
>  
> +#define META_TYPE_END		(0)

MC_HEADER_META_TYPE_END

> +
> +struct metadata_header {
> +	unsigned int		type;
> +	unsigned int		blk_size;
> +};
> +
> +static struct metadata_header *ifs_find_meta_data(void *ucode, unsigned int meta_type)

It's a static function - no need for the ifs_ prefix.

> +{
> +	struct metadata_header *meta_header;
> +	unsigned long data_size, total_meta;
> +	unsigned long meta_size = 0;
> +
> +	data_size = get_datasize(ucode);
> +	total_meta = ((struct microcode_intel *)ucode)->hdr.metasize;
> +


^ Superfluous newline.

> +	if (!total_meta)
> +		return NULL;
> +
> +	meta_header = (ucode + MC_HEADER_SIZE + data_size) - total_meta;
> +
> +	while ((meta_header->type != META_TYPE_END) && meta_header->blk_size &&

You don't need the brackets.

> +	       meta_size < total_meta) {

And you can align all three conditions vertically for better readability:

        while (meta_header->type != META_TYPE_END && 
               meta_header->blk_size &&
               meta_size < total_meta) {

...