| Message ID | 20230605170817.9913-2-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Prevent attempting updates known to fail | expand |
On 05.06.2023 19:08, Alejandro Vallejo wrote: > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -854,8 +854,14 @@ int __init early_microcode_init(unsigned long *module_map, > break; > > case X86_VENDOR_INTEL: > - if ( c->x86 >= 6 ) > - ucode_ops = intel_ucode_ops; > + /* > + * Intel introduced microcode loading with family 6. Because we > + * don't support compiling Xen for 32bit machines we're guaranteed > + * that at this point we're either in family 15 (Pentium 4) or 6 > + * (everything since then), so microcode facilities are always > + * present. > + */ > + ucode_ops = intel_ucode_ops; > break; > } There are many places where we make such connections / assumptions without long comments. I'd be okay with a brief one, but I'm not convinced we need one at all. Jan
On 12/06/2023 4:16 pm, Jan Beulich wrote: > On 05.06.2023 19:08, Alejandro Vallejo wrote: > >> --- a/xen/arch/x86/cpu/microcode/core.c >> +++ b/xen/arch/x86/cpu/microcode/core.c >> @@ -854,8 +854,14 @@ int __init early_microcode_init(unsigned long *module_map, >> break; >> >> case X86_VENDOR_INTEL: >> - if ( c->x86 >= 6 ) >> - ucode_ops = intel_ucode_ops; >> + /* >> + * Intel introduced microcode loading with family 6. Because we >> + * don't support compiling Xen for 32bit machines we're guaranteed >> + * that at this point we're either in family 15 (Pentium 4) or 6 >> + * (everything since then), so microcode facilities are always >> + * present. >> + */ >> + ucode_ops = intel_ucode_ops; >> break; >> } > There are many places where we make such connections / assumptions without > long comments. I'd be okay with a brief one, but I'm not convinced we need > one at all. I agree. I don't think we need a comment here. I'd also tweak the commit message to say "All 64bit-capable Intel CPUs are supported as far as microcode loading goes" or similar. It's subtly different IMO. The Intel microcode driver already relies on 64bit-ness to exclude and early case (on 32bit CPUs only) which lack Platform Flags. I'm happy to fix both of these up on commit. ~Andrew
On Mon, Jun 12, 2023 at 06:31:13PM +0100, Andrew Cooper wrote: > On 12/06/2023 4:16 pm, Jan Beulich wrote: > > There are many places where we make such connections / assumptions without > > long comments. I'd be okay with a brief one, but I'm not convinced we need > > one at all. > > I agree. I don't think we need a comment here. > > I'd also tweak the commit message to say "All 64bit-capable Intel CPUs > are supported as far as microcode loading goes" or similar. It's subtly > different IMO. > > The Intel microcode driver already relies on 64bit-ness to exclude and > early case (on 32bit CPUs only) which lack Platform Flags. > > I'm happy to fix both of these up on commit. > > ~Andrew Sure, I don't have a strong preference either way. I'll remove that for the new series. Alejandro
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index c1033f3bc2..29ff38f35c 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -854,8 +854,14 @@ int __init early_microcode_init(unsigned long *module_map, break; case X86_VENDOR_INTEL: - if ( c->x86 >= 6 ) - ucode_ops = intel_ucode_ops; + /* + * Intel introduced microcode loading with family 6. Because we + * don't support compiling Xen for 32bit machines we're guaranteed + * that at this point we're either in family 15 (Pentium 4) or 6 + * (everything since then), so microcode facilities are always + * present. + */ + ucode_ops = intel_ucode_ops; break; }
Intel only suuports 64bits on families 6 and 15, so we can always assume microcode loading facilities are present and skip the check. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- v2: * New addition --- xen/arch/x86/cpu/microcode/core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)