On 02/03/2026 7:19 pm, Kevin Lampis wrote:
> struct cpuinfo_x86
> .x86 => .family
> .x86_vendor => .vendor
> .x86_model => .model
> .x86_mask => .stepping
>
> No functional change.
>
> This work is part of making Xen safe for Intel family 18/19.
>
> Signed-off-by: Kevin Lampis <[email protected]>
> ---
>
> In xen/arch/x86/cpu/mcheck/mce.c: mcheck_init(...)
> Xen only calls `intel_mcheck_init(...)` if family is 6 or 15.
>
> Linux only calls `mce_intel_feature_init(...)` if family != 5.
>
> Do we need to do something like extend this switch statement in
> `mcheck_init(...)` to include family 18 and 19?
Yes, we will want to include 18/19.
The Machine Check infrastructure in the Pentium was very primitive, but
everything 6 and later should be fine. I think we'll want to switch to
the Linux form here.
>
>
> In xen/arch/x86/cpu/mcheck/mce.c: mce_firstbank(...)
> The check
> c->family == 6 && c->vendor == X86_VENDOR_INTEL && c->model < 0x1a
>
> could be re-written as
> c->vfm >= INTEL_PENTIUM_PRO && c->vfm < INTEL_NEHALEM_EP
>
> I don't know if that would be better.
There's a subtlety. This suggestion is for ...
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 9a91807cfb..10e826e3a6 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -564,8 +564,8 @@ bool mce_available(const struct cpuinfo_x86 *c)
> */
> unsigned int mce_firstbank(struct cpuinfo_x86 *c)
> {
> - return c->x86 == 6 &&
> - c->x86_vendor == X86_VENDOR_INTEL && c->x86_model < 0x1a;
> + return c->family == 6 &&
> + c->vendor == X86_VENDOR_INTEL && c->model < 0x1a;
... here, but you've made a related change ...
> diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c
> b/xen/arch/x86/cpu/mcheck/mce_intel.c
> index 839a0e5ba9..9100ce0f6c 100644
> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
> @@ -711,10 +711,7 @@ static bool mce_is_broadcast(struct cpuinfo_x86 *c)
> * DisplayFamily_DisplayModel encoding of 06H_EH and above,
> * a MCA signal is broadcast to all logical processors in the system
> */
> - if ( c->x86_vendor == X86_VENDOR_INTEL && c->x86 == 6 &&
> - c->x86_model >= 0xe )
> - return true;
> - return false;
> + return c->vfm >= INTEL_CORE_YONAH;
... here.
It's not safe to have a single inequality like this, unless you're
absolutely certain you're inside a c->vendor==$ME region.
In both cases here you're replacing a vendor check, suggesting that we
might not be within a suitably guarded region. mce_firstbank() does
seem to be used from common code. mce_is_broadcast() OTOH does look to
be fully within an Intel-only region.
Otherwise, The vendor encoding in the higher bits will make a false
positive/negative match on one side of the inequality.
It is safe to use c->vfm >= FOO && v->vfm <= BAR, because constants of
another vendor will not be captured.
~Andrew
