On 27.10.2025 12:53, Jan Beulich wrote: > On 27.10.2025 12:33, Roger Pau Monné wrote: >> On Mon, Oct 27, 2025 at 11:23:58AM +0100, Jan Beulich wrote: >>> On 24.10.2025 15:24, Roger Pau Monné wrote: >>>> On Thu, Oct 23, 2025 at 05:50:17PM +0200, Jan Beulich wrote: >>>>> @@ -343,6 +347,12 @@ static int __init hpet_setup_msi_irq(str >>>>> u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx)); >>>>> irq_desc_t *desc = irq_to_desc(ch->msi.irq); >>>>> >>>>> + clear_irq_vector(ch->msi.irq); >>>>> + ret = bind_irq_vector(ch->msi.irq, HPET_BROADCAST_VECTOR, >>>>> &cpu_online_map); >>>> >>>> By passing cpu_online_map here, it leads to _bind_irq_vector() doing: >>>> >>>> cpumask_copy(desc->arch.cpu_mask, &cpu_online_map); >>>> >>>> Which strictly speaking is wrong. However this is just a cosmetic >>>> issue until the irq is used for the first time, at which point it will >>>> be assigned to a concrete CPU. >>>> >>>> You could do: >>>> >>>> cpumask_clear(desc->arch.cpu_mask); >>>> cpumask_set_cpu(cpumask_any(&cpu_online_map), desc->arch.cpu_mask); >>>> >>>> (Or equivalent) >>>> >>>> To assign the interrupt to a concrete CPU and reflex it on the >>>> cpu_mask after the bind_irq_vector() call, but I can live with it >>>> being like this. I have patches to adjust _bind_irq_vector() myself, >>>> which I hope I will be able to post soon. >>> >>> Hmm, I wrongly memorized hpet_broadcast_init() as being pre-SMP-init only. >>> It has three call sites: >>> - mwait_idle_init(), called from cpuidle_presmp_init(), >>> - amd_cpuidle_init(), calling in only when invoked the very first time, >>> which is again from cpuidle_presmp_init(), >>> - _disable_pit_irq(), called from the regular initcall disable_pit_irq(). >>> I.e. for the latter you're right that the CPU mask is too broad (in only a >>> cosmetic way though). Would be you okay if I used cpumask_of(0) in place >>> of &cpu_online_map? >> >> Using cpumask_of(0) would be OK, as the per-cpu vector_irq array will >> be updated ahead of assigning the interrupt to a CPU, and hence it >> doesn't need to be done for all possible online CPUs in >> _bind_irq_vector(). >> >> In the context here it would be more accurate to provide an empty CPU >> mask, as the interrupt is not yet targeting any CPU. Using CPU 0 >> would be a placeholder, which seems fine for the purpose. > > Putting an empty mask there, while indeed logically correct, would (I fear) > again put us at risk with other code making various assumptions.
And indeed: _bind_irq_vector() would reject an empty mask. Jan
