[PATCH] make c/s 21089 work again with c/s 21092

[PATCH] make c/s 21089 work again with c/s 21092

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

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  




[-- Attachment #2: x86-startup-irq-from-setup-gsi-fix2.patch --]
[-- Type: text/plain, Size: 1229 bytes --]

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  

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] make c/s 21089 work again with c/s 21092

[Bastian Blank]

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

Re: [PATCH] make c/s 21089 work again with c/s 21092

[Jan Beulich]

>>> 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

Re: Re: [PATCH] make c/s 21089 work again with c/s 21092

[Konrad Rzeszutek Wilk]

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

Re: Re: [PATCH] make c/s 21089 work again with c/s 21092

[Jan Beulich]

>>> 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?

Jan

Re: [PATCH] make c/s 21089 work again with c/s 21092

[Bastian Blank]

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

Re: Re: [PATCH] make c/s 21089 work again with c/s 21092

[Konrad Rzeszutek Wilk]

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