On 20.11.2025 12:51, Andrew Cooper wrote:
> On 19/11/2025 10:50 am, Jan Beulich wrote:
>> --- a/xen/arch/x86/cpu/mcheck/mce.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce.c
>> @@ -110,13 +110,13 @@ static void __init mcheck_intel_therm_in
>> }
>>
>> /* P4/Xeon Thermal regulation detect and init */
>> -static void intel_init_thermal(struct cpuinfo_x86 *c)
>> +static void intel_init_thermal(const struct cpuinfo_x86 *c, bool bsp)
>> {
>> uint64_t msr_content;
>> uint32_t val;
>> int tm2 = 0;
>> unsigned int cpu = smp_processor_id();
>> - static uint8_t thermal_apic_vector;
>> + static uint8_t __ro_after_init thermal_apic_vector;
>>
>> if ( !intel_thermal_supported(c) )
>> return; /* -ENODEV */
>> @@ -160,7 +160,8 @@ static void intel_init_thermal(struct cp
>> return; /* -EBUSY */
>> }
>>
>> - alloc_direct_apic_vector(&thermal_apic_vector, intel_thermal_interrupt);
>> + if ( bsp )
>> + alloc_direct_apic_vector(&thermal_apic_vector,
>> intel_thermal_interrupt);
>
> We really don't want both c and bsp passed in. That can only go wrong.
>
> Furthermore, this function has 2 other examples generating bsp locally.
>
> The function is in desperate need of cleanup (MSRs, variable and
> constant names), but right now this makes it worse.
>
> Please either use c == &boot_cpu_data, and I'll do some cleanup later,
> or generate bsp = c == &boot_cpu_data and fix up all users in the function.
No, throughout mce/ this won't work as long as acpi/power.c:enter_state() has
mcheck_init(&boot_cpu_data, false);
Jan
