Messages in this thread | | | Date | Tue, 18 Jan 2022 13:03:19 -0800 | From | "Luck, Tony" <> | Subject | Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions |
| |
On Tue, Jan 18, 2022 at 09:02:41PM +0100, Borislav Petkov wrote: > On Fri, Jan 07, 2022 at 02:54:38PM -0800, Tony Luck wrote: > > +} ppin_info[] = { > > + [X86_VENDOR_INTEL] = { > > + .feature = X86_FEATURE_INTEL_PPIN, > > + .msr_ppin_ctl = MSR_PPIN_CTL, > > + .msr_ppin = MSR_PPIN > > + }, > > + [X86_VENDOR_AMD] = { > > X86_VENDOR_AMD is index 2 not 1 so ppin_info[1] is empty. > > I wouldn't mind swapping the numbers here in a pre-patch: > > #define X86_VENDOR_CYRIX 1 > #define X86_VENDOR_AMD 2 > > nothing would depend on those naked numbers, right? :)
You are much braver than I :-) How confident are you that nobody implicitly depends on those? Is it worth the risk/churn just to save 12 bytes in the ppin_info[] array?
I'll fix up all the other stuff you found and post a V2 soon. Thanks for the review.
-Tony
| |