Re: [PATCH 2/6] x86: Break struct x86_cpu_id out of processor.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.07.2025 19:31, Andrew Cooper wrote:
> Only 5 files use struct x86_cpu_id, so it should not be in processor.h.  This
> is in preparation to extend it with VFM support.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two nits:

> --- /dev/null
> +++ b/xen/arch/x86/include/asm/match-cpu.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef X86_MATCH_CPU_H
> +#define X86_MATCH_CPU_H
> +
> +#include <xen/stdint.h>
> +
> +struct x86_cpu_id {
> +    uint16_t vendor;
> +    uint16_t family;
> +    uint16_t model;
> +    uint16_t feature;
> +    const void *driver_data;
> +};
> +
> +/*
> + * x86_match_cpu() - match the CPU against an array of x86_cpu_ids[]
> + *
> + * @table: Array of x86_cpu_ids. Last entry terminated with {}.

This reads slightly odd to me: I'd have expected "Last entry needs to be ..."
or "Table terminated with ...".

> + * Returns the matching entry, otherwise NULL.

Perhaps add "first"? Due to use of wildcards, multiple entries could in
principle match.

Jan