| Message ID | 5CC6DF710200007800229EC5@prv1-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: IRQ management adjustments | expand |
On Mon, Apr 29, 2019 at 05:26:41AM -0600, Jan Beulich wrote: > Since the "Cannot set affinity ..." warning is a one time one, avoid > triggering it already at boot time when parking secondary threads and > the serial console uses a (still unconnected at that time) PCI IRQ. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/irq.c > +++ b/xen/arch/x86/irq.c > @@ -2412,8 +2412,20 @@ void fixup_irqs(const cpumask_t *mask, b > vector = irq_to_vector(irq); > if ( vector >= FIRST_HIPRIORITY_VECTOR && > vector <= LAST_HIPRIORITY_VECTOR ) > + { > cpumask_and(desc->arch.cpu_mask, desc->arch.cpu_mask, mask); > > + /* > + * This can in particular happen when parking secondary threads > + * during boot and when the serial console wants to use a PCI IRQ. > + */ > + if ( desc->handler == &no_irq_type ) I found it weird that a irq has a vector assigned (in this case a high-priority vector) but no irq type set. Shouldn't the vector be assigned when the type is set? > + { > + spin_unlock(&desc->lock); > + continue; > + } > + } > + > if ( desc->arch.move_cleanup_count ) > { > /* The cleanup IPI may have got sent while we were still online. */ Thanks, Roger.
>>> On 06.05.19 at 15:52, <roger.pau@citrix.com> wrote: > On Mon, Apr 29, 2019 at 05:26:41AM -0600, Jan Beulich wrote: >> --- a/xen/arch/x86/irq.c >> +++ b/xen/arch/x86/irq.c >> @@ -2412,8 +2412,20 @@ void fixup_irqs(const cpumask_t *mask, b >> vector = irq_to_vector(irq); >> if ( vector >= FIRST_HIPRIORITY_VECTOR && >> vector <= LAST_HIPRIORITY_VECTOR ) >> + { >> cpumask_and(desc->arch.cpu_mask, desc->arch.cpu_mask, mask); >> >> + /* >> + * This can in particular happen when parking secondary threads >> + * during boot and when the serial console wants to use a PCI IRQ. >> + */ >> + if ( desc->handler == &no_irq_type ) > > I found it weird that a irq has a vector assigned (in this case a > high-priority vector) but no irq type set. > > Shouldn't the vector be assigned when the type is set? In general I would agree, but the way the serial console IRQ gets set up is different - see smp_intr_init(). When it's a PCI IRQ (IO-APIC pin 16 or above), we'll know how to program the IO-APIC RTE (edge/level, activity high/low) only when Dom0 boots, and hence we don't set ->handler early. Jan
On Mon, May 06, 2019 at 08:25:51AM -0600, Jan Beulich wrote: > >>> On 06.05.19 at 15:52, <roger.pau@citrix.com> wrote: > > On Mon, Apr 29, 2019 at 05:26:41AM -0600, Jan Beulich wrote: > >> --- a/xen/arch/x86/irq.c > >> +++ b/xen/arch/x86/irq.c > >> @@ -2412,8 +2412,20 @@ void fixup_irqs(const cpumask_t *mask, b > >> vector = irq_to_vector(irq); > >> if ( vector >= FIRST_HIPRIORITY_VECTOR && > >> vector <= LAST_HIPRIORITY_VECTOR ) > >> + { > >> cpumask_and(desc->arch.cpu_mask, desc->arch.cpu_mask, mask); > >> > >> + /* > >> + * This can in particular happen when parking secondary threads > >> + * during boot and when the serial console wants to use a PCI IRQ. > >> + */ > >> + if ( desc->handler == &no_irq_type ) > > > > I found it weird that a irq has a vector assigned (in this case a > > high-priority vector) but no irq type set. > > > > Shouldn't the vector be assigned when the type is set? > > In general I would agree, but the way the serial console IRQ > gets set up is different - see smp_intr_init(). When it's a PCI > IRQ (IO-APIC pin 16 or above), we'll know how to program > the IO-APIC RTE (edge/level, activity high/low) only when > Dom0 boots, and hence we don't set ->handler early. Oh, OK. I guess assuming level triggered active low unless dom0 provides a different configuration is not safe. Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
--- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -2412,8 +2412,20 @@ void fixup_irqs(const cpumask_t *mask, b vector = irq_to_vector(irq); if ( vector >= FIRST_HIPRIORITY_VECTOR && vector <= LAST_HIPRIORITY_VECTOR ) + { cpumask_and(desc->arch.cpu_mask, desc->arch.cpu_mask, mask); + /* + * This can in particular happen when parking secondary threads + * during boot and when the serial console wants to use a PCI IRQ. + */ + if ( desc->handler == &no_irq_type ) + { + spin_unlock(&desc->lock); + continue; + } + } + if ( desc->arch.move_cleanup_count ) { /* The cleanup IPI may have got sent while we were still online. */
Since the "Cannot set affinity ..." warning is a one time one, avoid triggering it already at boot time when parking secondary threads and the serial console uses a (still unconnected at that time) PCI IRQ. Signed-off-by: Jan Beulich <jbeulich@suse.com>