mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Jeremy Fitzhardinge <jeremy@goop.org>,
	Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>
Subject: Re: [PATCH 09/22] xen: Find an unbound irq number in reverse order (high to low).
Date: Tue, 5 Oct 2010 17:46:13 +0100	[thread overview]
Message-ID: <alpine.DEB.2.00.1010051617160.2440@kaball-desktop> (raw)
In-Reply-To: <1286216015-9710-10-git-send-email-konrad.wilk@oracle.com>

On Mon, 4 Oct 2010, Konrad Rzeszutek Wilk wrote:
> In earlier Xen Linux kernels, the IRQ mapping was a straight 1:1 and the
> find_unbound_irq started looking around 256 for open IRQs and up. IRQs
> from 0 to 255 were reserved for PCI devices.  Previous to this patch,
> the 'find_unbound_irq'  started looking at get_nr_hw_irqs() number.
> For privileged  domain where the ACPI information is available that
> returns the upper-bound of what the GSIs. For non-privileged PV domains,
> where ACPI is no-existent the get_nr_hw_irqs() reports the IRQ_LEGACY (16).
> With PCI passthrough enabled, and with PCI cards that have IRQs pinned
> to a higher number than 16 we collide with previously allocated IRQs.
> Specifically the PCI IRQs collide with the IPI's for Xen functions
> (as they are allocated earlier).
> For example:
> 
> 00:00.11 USB Controller: ATI Technologies Inc SB700 USB OHCI1 Controller (prog-if 10 [OHCI])
> 	...
> 	Interrupt: pin A routed to IRQ 18
> 
> [root@localhost ~]# cat /proc/interrupts | head
>            CPU0       CPU1       CPU2
>  16:      38186          0          0   xen-dyn-virq      timer0
>  17:        149          0          0   xen-dyn-ipi       spinlock0
>  18:        962          0          0   xen-dyn-ipi       resched0
> 
> and when the USB controller is loaded, the kernel reports:
> IRQ handler type mismatch for IRQ 18
> current handler: resched0
> 
> One way to fix this is to reverse the logic when looking for un-used
> IRQ numbers and start with the highest available number. With that,
> we would get:
> 
>            CPU0       CPU1       CPU2
> ... snip ..
> 292:         35          0          0   xen-dyn-ipi       callfunc0
> 293:       3992          0          0   xen-dyn-ipi       resched0
> 294:        224          0          0   xen-dyn-ipi       spinlock0
> 295:      57183          0          0   xen-dyn-virq      timer0
> NMI:          0          0          0   Non-maskable interrupts
> .. snip ..
> 
> And interrupts for PCI cards are now accessible.
> 

Unfortunately this is the wrong way to fix the issue: Xen has a range of
allowed pirq for each domain and we don't know exactly what is the
maximum pirq (see my patch "xen: get the maximum number of pirqs from
xen" [1]).
Considering that we might use the irq number returned by
find_unbound_irq through xen_allocate_pirq as pirq number in some cases,
starting from the highest value could be unsafe.
In practice it should be impossible to see this issue because it can
only happen if the irq returned by xen_allocate_pirq is higher than the
max pirq in xen. However AFAIK when we call xen_allocate_pirq with the
intention of using the return value as pirq we always fall in the if
(identity_mapped_irq(gsi) || !xen_initial_domain()) that avoid calling
find_unbound_irq.
In any case I still think that it is a good idea to try to avoid the problem.

In order to solve this issue you can:

- add a static offset to get_nr_hw_irqs in case of pv domains;

- use the patch "xen: get the maximum number of pirqs from
xen" [1] to get the highest possible pirq number from xen so that we can be
sure we are using an allowed value;

- if the xen interface allows it, disjoin the concept of pirq from the
concept of linux irq; then you can let xen choose any pirq number it
wants (see my patch "xen: support pirq != irq" [2] and how you can exploit
it in the patch "xen: implement xen_hvm_register_pirq" [3]).
Of course this doesn't help if it must be linux the one that chooses the
pirq number.

Feel free to pick up any of the patches I sent as part of the "PV on
HVM: receive interrupts as xen events" series.


References:
[1] http://lkml.org/lkml/2010/8/30/177
[2] http://lkml.org/lkml/2010/8/30/171
[3] http://lkml.org/lkml/2010/8/30/172


  reply	other threads:[~2010-10-05 16:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-04 18:13 [PATCH v7] Xen PCI + Xen PCI frontend driver Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 01/22] xen: Don't disable the I/O space Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 02/22] xen: define BIOVEC_PHYS_MERGEABLE() Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 03/22] xen: implement pirq type event channels Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 04/22] x86/io_apic: add get_nr_irqs_gsi() Konrad Rzeszutek Wilk
2010-10-05  9:13   ` Thomas Gleixner
2010-10-04 18:13 ` [PATCH 05/22] xen: identity map gsi->irqs Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 07/22] xen: set pirq name to something useful Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 08/22] xen: statically initialize cpu_evtchn_mask_p Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 09/22] xen: Find an unbound irq number in reverse order (high to low) Konrad Rzeszutek Wilk
2010-10-05 16:46   ` Stefano Stabellini [this message]
2010-10-06 22:59     ` Konrad Rzeszutek Wilk
2010-10-07 10:38       ` Stefano Stabellini
2010-10-07 14:05         ` [Xen-devel] " Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 10/22] xen: Provide a variant of xen_poll_irq with timeout Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 12/22] x86/PCI: Clean up pci_cache_line_size Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 13/22] x86/PCI: make sure _PAGE_IOMAP it set on pci mappings Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 14/22] x86/PCI: Export pci_walk_bus function Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 15/22] x86: Copy-n-paste arch_teardown_msi_irqs from msi.c to io_apic.c Konrad Rzeszutek Wilk
2010-10-05  9:07   ` Thomas Gleixner
2010-10-07  1:09     ` Konrad Rzeszutek Wilk
2010-10-07  2:57       ` Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 16/22] x86: Introduce x86_msi_ops Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 17/22] xen/x86/PCI: Add support for the Xen PCI subsystem Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 21/22] xen/pci: Request ACS when Xen-SWIOTLB is activated Konrad Rzeszutek Wilk
2010-10-04 18:13 ` [PATCH 22/22] MAINTAINERS: Add myself for Xen PCI and Xen SWIOTLB maintainer Konrad Rzeszutek Wilk
2010-10-05  7:47 ` [Xen-devel] [PATCH v7] Xen PCI + Xen PCI frontend driver Jan Beulich
2010-10-06 22:50   ` Konrad Rzeszutek Wilk
2010-10-07 19:44     ` Konrad Rzeszutek Wilk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1010051617160.2440@kaball-desktop \
    --to=stefano.stabellini@eu.citrix.com \
    --cc=Jeremy.Fitzhardinge@citrix.com \
    --cc=jeremy@goop.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®