linux-kernel - Re: [PATCH 1/5] xen: events: use irq_alloc_desc(_at) instead of open-coding an IRQ allocator.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4CC60CB7.3070005@goop.org>
Date:	Mon, 25 Oct 2010 16:03:19 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	Ian Campbell <ian.campbell@...rix.com>,
	Stefano Stabellini <Stefano.Stabellini@...citrix.com>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
	mingo@...e.hu, xen-devel@...ts.xensource.com, tglx@...utronix.de
Subject: Re: [PATCH 1/5] xen: events: use irq_alloc_desc(_at) instead of open-coding
 an IRQ allocator.

 On 10/25/2010 10:35 AM, Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 25, 2010 at 05:23:29PM +0100, Ian Campbell wrote:
>> Encapsulate allocate and free in xen_irq_alloc and xen_irq_free.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@...rix.com>
>> ---
>>  drivers/xen/events.c |   68 ++++++++++++++++++++-----------------------------
>>  1 files changed, 28 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index 97612f5..c8f3e43 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -394,41 +394,29 @@ static int find_unbound_pirq(void)
>>  	return -1;
>>  }
>>  
>> -static int find_unbound_irq(void)
>> +static int xen_irq_alloc(void)
>>  {
>> -	struct irq_data *data;
>> -	int irq, res;
>> -	int start = get_nr_hw_irqs();
>> +	int irq = irq_alloc_desc(0);
>>  
>> -	if (start == nr_irqs)
>> -		goto no_irqs;
>> -
>> -	/* nr_irqs is a magic value. Must not use it.*/
>> -	for (irq = nr_irqs-1; irq > start; irq--) {
>> -		data = irq_get_irq_data(irq);
>> -		/* only 0->15 have init'd desc; handle irq > 16 */
>> -		if (!data)
>> -			break;
>> -		if (data->chip == &no_irq_chip)
>> -			break;
>> -		if (data->chip != &xen_dynamic_chip)
>> -			continue;
>> -		if (irq_info[irq].type == IRQT_UNBOUND)
>> -			return irq;
>> -	}
>> -
>> -	if (irq == start)
>> -		goto no_irqs;
>> +	if (irq < 0)
>> +		panic("No available IRQ to bind to: increase nr_irqs!\n");
>>  
>> -	res = irq_alloc_desc_at(irq, 0);
>> +	return irq;
>> +}
> So I am curious what the /proc/interrupts looks?The issue (and the reason
> for this implementation above) was that under PV with PCI devices we would
> overlap PCI devices IRQs with Xen event channels. So we could have a USB device
> at IRQ 16 _and_ also a xen_spinlock4 handler. That would throw off the system
> since the xen_spinlock4 was an edge type handler while the USB device was an
> level (at least on my box).

What?  Why?  How?  Surely if we're asking the irq subsystem to allocate
us an irq, it will return a fresh never-before-used (and certainly not
shared) irq?  Shared irqs only make sense if multiple devices are
actually sharing, say, a wire on the board.

Or am I missing something?

    J

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/