2022-11-07 22:58:18

by Joseph, Jithu

[permalink] [raw]
Subject: [PATCH v2 08/14] platform/x86/intel/ifs: Add metadata support

From: Ashok Raj <[email protected]>

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 <[email protected]>
Signed-off-by: Ashok Raj <[email protected]>
Signed-off-by: Jithu Joseph <[email protected]>
---
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.
--
2.25.1



2022-11-09 03:40:20

by Sohil Mehta

[permalink] [raw]
Subject: Re: [PATCH v2 08/14] platform/x86/intel/ifs: Add metadata support

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 <[email protected]>
> Signed-off-by: Ashok Raj <[email protected]>
> Signed-off-by: Jithu Joseph <[email protected]>

Reviewed-by: Sohil Mehta <[email protected]>

2022-11-10 21:25:18

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 08/14] platform/x86/intel/ifs: Add metadata support

Hi,

On 11/7/22 23:53, Jithu Joseph wrote:
> From: Ashok Raj <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Ashok Raj <[email protected]>
> Signed-off-by: Jithu Joseph <[email protected]>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

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.


2022-11-11 17:07:17

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2 08/14] platform/x86/intel/ifs: Add metadata support

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) {

...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette