On 15.03.2025 02:00, dm...@proton.me wrote: > Add new symbol APIC_VECTOR_VALID to replace open-coded value 16 in > LAPIC and virtual LAPIC code.
First a good name is needed to make such a change. APIC_VECTOR_VALID could imo be the name of a predicate macro, but it can't be a mere number. Then ... > --- a/xen/arch/x86/cpu/mcheck/mce_intel.c > +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c > @@ -136,7 +136,7 @@ static void intel_init_thermal(struct cpuinfo_x86 *c) > * is required to set the same value for all threads/cores). > */ > if ( (val & APIC_DM_MASK) != APIC_DM_FIXED > - || (val & APIC_VECTOR_MASK) > 0xf ) > + || (val & APIC_VECTOR_MASK) > APIC_VECTOR_VALID ) ... care needs to be taken that replacements are done such that the "no functional change" claim is actually correct. (The 0xf, i.e. 15, is replaced by 16 here. I didn't check if there are other similar issues.) > --- a/xen/arch/x86/include/asm/apicdef.h > +++ b/xen/arch/x86/include/asm/apicdef.h > @@ -78,6 +78,7 @@ > #define APIC_DM_STARTUP 0x00600 > #define APIC_DM_EXTINT 0x00700 > #define APIC_VECTOR_MASK 0x000FF > +#define APIC_VECTOR_VALID (16) > #define APIC_ICR2 0x310 Nit: No real need for parentheses here; adjacent #define-s don't have any, so it's a little hard to see why you added them. Jan
