RE: [PATCH v3 04/10] x86/microcode_core_early.c: Define interfacesfor early loading ucode

[Yu, Fenghua]

> -----Original Message-----
> From: Borislav Petkov [mailto:bp@xxxxxxxxx]
> Sent: Sunday, December 16, 2012 9:57 AM
> To: Yu, Fenghua
> Cc: H Peter Anvin; Ingo Molnar; Thomas Gleixner; Mallick, Asit K;
> Tigran Aivazian; Andreas Herrmann; Borislav Petkov; Yinghai Lu; linux-
> kernel; x86
> Subject: Re: [PATCH v3 04/10] x86/microcode_core_early.c: Define
> interfaces for early loading ucode
> 
> On Sun, Dec 16, 2012 at 02:43:23AM -0800, Fenghua Yu wrote:
> > +#define QCHAR(a, b, c, d) ((a) + ((b) << 8) + ((c) << 16) + ((d) <<
> 24))
> > +#define CPUID_INTEL1 QCHAR('G', 'e', 'n', 'u')
> > +#define CPUID_INTEL2 QCHAR('i', 'n', 'e', 'I')
> > +#define CPUID_INTEL3 QCHAR('n', 't', 'e', 'l')
> > +#define CPUID_AMD1 QCHAR('A', 'u', 't', 'h')
> > +#define CPUID_AMD2 QCHAR('e', 'n', 't', 'i')
> > +#define CPUID_AMD3 QCHAR('c', 'A', 'M', 'D')
> > +
> > +#define CPUID_IS(a, b, c) (!((ebx ^ (a))|(edx ^ (b))|(ecx ^ (c))))
> 
> What, this macro is relying on external variable names and doesn't have
> them as its own macro arguments? Why? This is really fragile and very
> much prone to errors.
> 
> What's wrong with doing:
> 
> #define CPUID_IS(a, b, c, ebx, ecx, edx) (!(((ebx) ^ (a))|((edx) ^
> (b))|((ecx) ^ (c))))
> 
> ?

Nothing wrong with taking the parameters. But CPUID_IS is only limited in this file. There is no other places to use it. I think either way is ok.

Thanks.

-Fenghua
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶ŸÿÃ-»+ƒùdš_