Jan Beulich
2010-Apr-13 13:36 UTC
[Xen-devel] [PATCH] make c/s 21089 work again with c/s 21092
Unfortunately the latter c/s'' change to mpparse.c yielded the former
patch non-functional - Xen''s serial port IRQ is not in IQR_DISABLED
state, yet must be allowed to get its trigger mode and polarity set
up in order for it to be usable.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- 2010-04-13.orig/xen/arch/x86/mpparse.c 2010-04-13 15:25:22.000000000 +0200
+++ 2010-04-13/xen/arch/x86/mpparse.c 2010-04-13 15:26:38.000000000 +0200
@@ -1103,6 +1103,8 @@ int mp_register_gsi (u32 gsi, int trigge
int ioapic = -1;
int ioapic_pin = 0;
int idx, bit = 0;
+ struct irq_desc * desc;
+ unsigned long flags;
/*
* Mapping between Global System Interrups, which
@@ -1127,8 +1129,13 @@ int mp_register_gsi (u32 gsi, int trigge
if (ioapic_renumber_irq)
gsi = ioapic_renumber_irq(ioapic, gsi);
- if (!(irq_to_desc(gsi)->status & IRQ_DISABLED))
+ desc = irq_to_desc(gsi);
+ spin_lock_irqsave(&desc->lock, flags);
+ if (!(desc->status & IRQ_DISABLED) && desc->handler !=
&no_irq_type) {
+ spin_unlock_irqrestore(&desc->lock, flags);
return -EEXIST;
+ }
+ spin_unlock_irqrestore(&desc->lock, flags);
/*
* Avoid pin reprogramming. PRTs typically include entries
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Bastian Blank
2010-Apr-13 15:10 UTC
[Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
On Tue, Apr 13, 2010 at 02:36:55PM +0100, Jan Beulich wrote:> Unfortunately the latter c/s'' change to mpparse.c yielded the former > patch non-functional - Xen''s serial port IRQ is not in IQR_DISABLED > state, yet must be allowed to get its trigger mode and polarity set > up in order for it to be usable.This looks not right. The original patch was designed to keep this function away from the irqs of the serial port. This patch adds another check for disallowing access but does not touch the IQR_DISABLED check. Please explain which machines have non-legacy configured interrupts for the first two serial ports. Bastian -- There are certain things men must do to remain men. -- Kirk, "The Ultimate Computer", stardate 4929.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Apr-13 15:53 UTC
[Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
>>> Bastian Blank <waldi@debian.org> 13.04.10 17:10 >>> >On Tue, Apr 13, 2010 at 02:36:55PM +0100, Jan Beulich wrote: >> Unfortunately the latter c/s'' change to mpparse.c yielded the former >> patch non-functional - Xen''s serial port IRQ is not in IQR_DISABLED >> state, yet must be allowed to get its trigger mode and polarity set >> up in order for it to be usable. > >This looks not right. The original patch was designed to keep this >function away from the irqs of the serial port. This patch adds another >check for disallowing access but does not touch the IQR_DISABLED check. > >Please explain which machines have non-legacy configured interrupts for >the first two serial ports.I''m talking about PCI serial cards - in the one case I''m working with, it sits on IRQ 17 (level, low) and hence Xen can''t enable it until Dom0 retrieved sufficient information from ACPI. Since desc->status doesn''t have IRQ_DISABLED for such an interrupt anymore, your patch basically made it impossible for the needed information to be processed, and hence the IRQ could (again, i.e. as before my patch was applied) not be enabled. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Apr-13 16:11 UTC
Re: [Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
On Tue, Apr 13, 2010 at 04:53:35PM +0100, Jan Beulich wrote:> >>> Bastian Blank <waldi@debian.org> 13.04.10 17:10 >>> > >On Tue, Apr 13, 2010 at 02:36:55PM +0100, Jan Beulich wrote: > >> Unfortunately the latter c/s'' change to mpparse.c yielded the former > >> patch non-functional - Xen''s serial port IRQ is not in IQR_DISABLED > >> state, yet must be allowed to get its trigger mode and polarity set > >> up in order for it to be usable. > > > >This looks not right. The original patch was designed to keep this > >function away from the irqs of the serial port. This patch adds another > >check for disallowing access but does not touch the IQR_DISABLED check. > > > >Please explain which machines have non-legacy configured interrupts for > >the first two serial ports. > > I''m talking about PCI serial cards - in the one case I''m working with,Can''t you use polling for it? For example this is what I do for my PCI serial card: com1=115200,8n1,0xd800,0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Apr-14 06:41 UTC
Re: [Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
>>> Konrad Rzeszutek Wilk 04/13/10 6:12 PM >>> >On Tue, Apr 13, 2010 at 04:53:35PM +0100, Jan Beulich wrote: >> I''m talking about PCI serial cards - in the one case I''m working with, > >Can''t you use polling for it? For example this is what I do for my PCI >serial card: > >com1=115200,8n1,0xd800,0It works with low rates of output, but in order to not use sync_console and still be able to reliably get through high rate bursts of output, interrupt driven communication seems to be the only reliable way (other than using unreasonably high values with "serial_tx_buffer="). And also from a more general perspective - why should legacy IRQs work, but not PCI ones? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Bastian Blank
2010-Apr-14 07:15 UTC
[Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
On Tue, Apr 13, 2010 at 02:36:55PM +0100, Jan Beulich wrote:> + spin_lock_irqsave(&desc->lock, flags);Why do you lock here? The whole thing is racy anyway and this should only battle against concurent registrations from dom0. Bastian -- It is a human characteristic to love little animals, especially if they''re attractive in some way. -- McCoy, "The Trouble with Tribbles", stardate 4525.6 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Apr-14 07:41 UTC
[Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
>>> Bastian Blank 04/14/10 9:16 AM >>> >On Tue, Apr 13, 2010 at 02:36:55PM +0100, Jan Beulich wrote: >> + spin_lock_irqsave(&desc->lock, flags); > >Why do you lock here? The whole thing is racy anyway and this should only >battle against concurent registrations from dom0.It seems cleaner to do proper locking here, even though I too think it''s not strictly needed. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Apr-14 13:25 UTC
Re: [Xen-devel] Re: [PATCH] make c/s 21089 work again with c/s 21092
On Wed, Apr 14, 2010 at 07:41:30AM +0100, Jan Beulich wrote:> >>> Konrad Rzeszutek Wilk 04/13/10 6:12 PM >>> > >On Tue, Apr 13, 2010 at 04:53:35PM +0100, Jan Beulich wrote: > >> I''m talking about PCI serial cards - in the one case I''m working with, > > > >Can''t you use polling for it? For example this is what I do for my PCI > >serial card: > > > >com1=115200,8n1,0xd800,0 > > It works with low rates of output, but in order to not use sync_console and still be able to reliably get through high rate bursts of output, interrupt driven communication seems to be the only reliable way (other than using unreasonably high values with "serial_tx_buffer="). > > And also from a more general perspective - why should legacy IRQs work, but not PCI ones?You have a good point. I actually had the darnest time getting these cards to work with Linux so was quite happy to see Xen working so well with them. I will take a look at your patch.> > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel