On 20.11.2025 10:58, Roger Pau Monne wrote:
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -115,7 +115,7 @@ static void release_old_vec(struct irq_desc *desc)
> unsigned int vector = desc->arch.old_vector;
>
> desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED;
> - cpumask_clear(desc->arch.old_cpu_mask);
> + desc->arch.old_cpu = CPU_INVALID;
With this, ...
> @@ -221,10 +220,10 @@ static void _clear_irq_vector(struct irq_desc *desc)
> {
> /* If we were in motion, also clear desc->arch.old_vector */
> old_vector = desc->arch.old_vector;
> - cpumask_and(tmp_mask, desc->arch.old_cpu_mask, &cpu_online_map);
>
> - for_each_cpu(cpu, tmp_mask)
> + if ( cpu_online(desc->arch.old_cpu) )
... you pretty certainly want to guard against the value making it here (even
if just accidentally), and thus triggering the assertion in cpumask_check().
(Again possibly elsewhere as well.)
> @@ -581,16 +575,16 @@ static int _assign_irq_vector(struct irq_desc *desc,
> const cpumask_t *mask)
> * in the 'mask' parameter.
> */
> desc->arch.vector = desc->arch.old_vector;
> - desc->arch.cpu = cpumask_any(desc->arch.old_cpu_mask);
> + desc->arch.cpu = desc->arch.old_cpu;
>
> /* Undo any possibly done cleanup. */
> per_cpu(vector_irq, desc->arch.cpu)[desc->arch.vector] = irq;
>
> /* Cancel the pending move and release the current vector. */
> desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED;
> - cpumask_clear(desc->arch.old_cpu_mask);
> + desc->arch.old_cpu = CPU_INVALID;
> desc->arch.move_in_progress = 0;
> - desc->arch.move_cleanup_count = 0;
> + desc->arch.move_cleanup = false;
Nit: Excess blank.
> @@ -2003,7 +1994,7 @@ void do_IRQ(struct cpu_user_regs *regs)
> ~irq, CPUMASK_PR(desc->affinity),
> /* TODO: handle hipri vectors nicely. */
> CPUMASK_PR(get_cpumask(desc->arch.cpu)),
> - CPUMASK_PR(desc->arch.old_cpu_mask),
> + CPUMASK_PR(get_cpumask(desc->arch.old_cpu)),
I should have asked on the previous patch already: Does it actually make sense
to still print these in mask form? Without that you wouldn't need get_cpumask(),
and as a result you also wouldn't need cpumask_none.
> @@ -2685,12 +2664,9 @@ void fixup_irqs(void)
> * per-cpu vector table will no longer have ->arch.old_vector
> * setup, and hence ->arch.old_cpu_mask would be stale.
> */
> - cpumask_clear_cpu(cpu, desc->arch.old_cpu_mask);
> - if ( cpumask_empty(desc->arch.old_cpu_mask) )
> - {
> - desc->arch.move_in_progress = 0;
> - release_old_vec(desc);
> - }
> + desc->arch.old_cpu = CPU_INVALID;
> + desc->arch.move_in_progress = 0;
As you touch the line anyway, switch to using "false"?
Jan
