* Need advices about PCI IRQ reference count tracking @ 2014-07-16 3:34 Jiang Liu 2014-07-16 18:04 ` Bjorn Helgaas 0 siblings, 1 reply; 6+ messages in thread From: Jiang Liu @ 2014-07-16 3:34 UTC (permalink / raw) To: Bjorn Helgaas, linux-pci@vger.kernel.org Hi Bjorn, When enabling ACPI based IOAPIC hotplug, we encountered an issue with PCI IRQ reference count tracking and need your advices. In order to hot-remove an IOAPIC, we need to track and keep balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may be called twice for a PCI device, 1) pci_acpi_init() if pci_routeirq is true 2) pci_enable_device() So in function acpi_pci_irq_enable(), we need a way to track whether an IOAPIC IRQ has been assigned to the PCI device. Previously we check "if (dev->irq > 0)" for that, but that's wrong because dev->irq may be set to non-zero in pci_read_irq() if BIOS has already assigned a legacy IRQ for the device. So is it OK to add a flag into pci_dev to track this information? Another possible workaround is to disable IOAPIC hotplug when pci_routeirq is set, but that sounds not the best solution. Regards! Gerry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Need advices about PCI IRQ reference count tracking 2014-07-16 3:34 Need advices about PCI IRQ reference count tracking Jiang Liu @ 2014-07-16 18:04 ` Bjorn Helgaas 2014-07-16 18:37 ` Yinghai Lu 0 siblings, 1 reply; 6+ messages in thread From: Bjorn Helgaas @ 2014-07-16 18:04 UTC (permalink / raw) To: Jiang Liu; +Cc: linux-pci@vger.kernel.org, Yinghai Lu [+cc Yinghai] On Tue, Jul 15, 2014 at 9:34 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: > Hi Bjorn, > When enabling ACPI based IOAPIC hotplug, we encountered an > issue with PCI IRQ reference count tracking and need your advices. > In order to hot-remove an IOAPIC, we need to track and keep > balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may > be called twice for a PCI device, > 1) pci_acpi_init() if pci_routeirq is true > 2) pci_enable_device() > > So in function acpi_pci_irq_enable(), we need a way to track whether > an IOAPIC IRQ has been assigned to the PCI device. Previously we check > "if (dev->irq > 0)" for that, but that's wrong because dev->irq may > be set to non-zero in pci_read_irq() if BIOS has already assigned > a legacy IRQ for the device. So is it OK to add a flag into pci_dev > to track this information? > > Another possible workaround is to disable IOAPIC hotplug when > pci_routeirq is set, but that sounds not the best solution. My first choice would be to get rid of pci_routeirq. It was added in 2004 as a workaround for broken drivers. I don't know any reason why we need the "pci=routeirq" option any more. However, 629e15d245f4 ("x86, irq: update_mptable needs pci_routeirq") added another use of pci_routeirq. This is only used when booting with the undocumented "update_mptable" option, and I can't tell whether it's useful or not. Bjorn P.S. I'm going on vacation for a few weeks, so I won't be able to follow this for a while. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Need advices about PCI IRQ reference count tracking 2014-07-16 18:04 ` Bjorn Helgaas @ 2014-07-16 18:37 ` Yinghai Lu 2014-07-17 3:02 ` Jiang Liu 0 siblings, 1 reply; 6+ messages in thread From: Yinghai Lu @ 2014-07-16 18:37 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: Jiang Liu, linux-pci@vger.kernel.org On Wed, Jul 16, 2014 at 11:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: > [+cc Yinghai] > > On Tue, Jul 15, 2014 at 9:34 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> Hi Bjorn, >> When enabling ACPI based IOAPIC hotplug, we encountered an >> issue with PCI IRQ reference count tracking and need your advices. >> In order to hot-remove an IOAPIC, we need to track and keep >> balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may >> be called twice for a PCI device, >> 1) pci_acpi_init() if pci_routeirq is true >> 2) pci_enable_device() >> >> So in function acpi_pci_irq_enable(), we need a way to track whether >> an IOAPIC IRQ has been assigned to the PCI device. Previously we check >> "if (dev->irq > 0)" for that, but that's wrong because dev->irq may >> be set to non-zero in pci_read_irq() if BIOS has already assigned >> a legacy IRQ for the device. So is it OK to add a flag into pci_dev >> to track this information? Why do you need to track it? When do you hot-remove ioapic with pci root bus, all the drivers for devices on that pci root bus should be stopped at first, so no one will use the ioapic before ioapic is being removed. >> >> Another possible workaround is to disable IOAPIC hotplug when >> pci_routeirq is set, but that sounds not the best solution. That is not good. > > My first choice would be to get rid of pci_routeirq. It was added in > 2004 as a workaround for broken drivers. I don't know any reason why > we need the "pci=routeirq" option any more. I always use "pci=routeirq apic=debug" to check the print out if BIOS routing irq correctly. > > However, 629e15d245f4 ("x86, irq: update_mptable needs pci_routeirq") > added another use of pci_routeirq. This is only used when booting > with the undocumented "update_mptable" option, and I can't tell > whether it's useful or not. Yes, we could drop update_mptable now. Will produce one patch to do that. Thanks Yinghai ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Need advices about PCI IRQ reference count tracking 2014-07-16 18:37 ` Yinghai Lu @ 2014-07-17 3:02 ` Jiang Liu 2014-07-17 3:06 ` Yinghai Lu 0 siblings, 1 reply; 6+ messages in thread From: Jiang Liu @ 2014-07-17 3:02 UTC (permalink / raw) To: Yinghai Lu, Bjorn Helgaas; +Cc: linux-pci@vger.kernel.org On 2014/7/17 2:37, Yinghai Lu wrote: > On Wed, Jul 16, 2014 at 11:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> [+cc Yinghai] >> >> On Tue, Jul 15, 2014 at 9:34 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >>> Hi Bjorn, >>> When enabling ACPI based IOAPIC hotplug, we encountered an >>> issue with PCI IRQ reference count tracking and need your advices. >>> In order to hot-remove an IOAPIC, we need to track and keep >>> balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may >>> be called twice for a PCI device, >>> 1) pci_acpi_init() if pci_routeirq is true >>> 2) pci_enable_device() >>> >>> So in function acpi_pci_irq_enable(), we need a way to track whether >>> an IOAPIC IRQ has been assigned to the PCI device. Previously we check >>> "if (dev->irq > 0)" for that, but that's wrong because dev->irq may >>> be set to non-zero in pci_read_irq() if BIOS has already assigned >>> a legacy IRQ for the device. So is it OK to add a flag into pci_dev >>> to track this information? > > Why do you need to track it? > > When do you hot-remove ioapic with pci root bus, all the drivers for > devices on that pci root bus should be stopped at first, so no one > will use the ioapic before ioapic is being removed. Hi Yinghai, By tracking IOAPIC pin usage count, we could release the assigned IRQ number when there are no device uses an IOAPIC pin anymore. > >>> >>> Another possible workaround is to disable IOAPIC hotplug when >>> pci_routeirq is set, but that sounds not the best solution. > > That is not good. > >> >> My first choice would be to get rid of pci_routeirq. It was added in >> 2004 as a workaround for broken drivers. I don't know any reason why >> we need the "pci=routeirq" option any more. > > I always use "pci=routeirq apic=debug" to check the print out > if BIOS routing irq correctly. So seems the solution is to add a flag to struct pci_dev, right? Regards! Gerry > >> >> However, 629e15d245f4 ("x86, irq: update_mptable needs pci_routeirq") >> added another use of pci_routeirq. This is only used when booting >> with the undocumented "update_mptable" option, and I can't tell >> whether it's useful or not. > > Yes, we could drop update_mptable now. Will produce one patch to do that. > > Thanks > > Yinghai > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Need advices about PCI IRQ reference count tracking 2014-07-17 3:02 ` Jiang Liu @ 2014-07-17 3:06 ` Yinghai Lu 2014-07-17 3:14 ` Jiang Liu 0 siblings, 1 reply; 6+ messages in thread From: Yinghai Lu @ 2014-07-17 3:06 UTC (permalink / raw) To: Jiang Liu; +Cc: Bjorn Helgaas, linux-pci@vger.kernel.org On Wed, Jul 16, 2014 at 8:02 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: > > > On 2014/7/17 2:37, Yinghai Lu wrote: >> On Wed, Jul 16, 2014 at 11:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> [+cc Yinghai] >>> >>> On Tue, Jul 15, 2014 at 9:34 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >>>> Hi Bjorn, >>>> When enabling ACPI based IOAPIC hotplug, we encountered an >>>> issue with PCI IRQ reference count tracking and need your advices. >>>> In order to hot-remove an IOAPIC, we need to track and keep >>>> balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may >>>> be called twice for a PCI device, >>>> 1) pci_acpi_init() if pci_routeirq is true >>>> 2) pci_enable_device() >>>> >>>> So in function acpi_pci_irq_enable(), we need a way to track whether >>>> an IOAPIC IRQ has been assigned to the PCI device. Previously we check >>>> "if (dev->irq > 0)" for that, but that's wrong because dev->irq may >>>> be set to non-zero in pci_read_irq() if BIOS has already assigned >>>> a legacy IRQ for the device. So is it OK to add a flag into pci_dev >>>> to track this information? >> >> Why do you need to track it? >> >> When do you hot-remove ioapic with pci root bus, all the drivers for >> devices on that pci root bus should be stopped at first, so no one >> will use the ioapic before ioapic is being removed. > Hi Yinghai, > By tracking IOAPIC pin usage count, we could release the > assigned IRQ number when there are no device uses an IOAPIC pin anymore. Delay releasing IRQ number later when destroying irq domain? Yinghai ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Need advices about PCI IRQ reference count tracking 2014-07-17 3:06 ` Yinghai Lu @ 2014-07-17 3:14 ` Jiang Liu 0 siblings, 0 replies; 6+ messages in thread From: Jiang Liu @ 2014-07-17 3:14 UTC (permalink / raw) To: Yinghai Lu; +Cc: Bjorn Helgaas, linux-pci@vger.kernel.org On 2014/7/17 11:06, Yinghai Lu wrote: > On Wed, Jul 16, 2014 at 8:02 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> >> >> On 2014/7/17 2:37, Yinghai Lu wrote: >>> On Wed, Jul 16, 2014 at 11:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>>> [+cc Yinghai] >>>> >>>> On Tue, Jul 15, 2014 at 9:34 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >>>>> Hi Bjorn, >>>>> When enabling ACPI based IOAPIC hotplug, we encountered an >>>>> issue with PCI IRQ reference count tracking and need your advices. >>>>> In order to hot-remove an IOAPIC, we need to track and keep >>>>> balance of IOAPIC pin usage count. But acpi_pci_irq_enable() may >>>>> be called twice for a PCI device, >>>>> 1) pci_acpi_init() if pci_routeirq is true >>>>> 2) pci_enable_device() >>>>> >>>>> So in function acpi_pci_irq_enable(), we need a way to track whether >>>>> an IOAPIC IRQ has been assigned to the PCI device. Previously we check >>>>> "if (dev->irq > 0)" for that, but that's wrong because dev->irq may >>>>> be set to non-zero in pci_read_irq() if BIOS has already assigned >>>>> a legacy IRQ for the device. So is it OK to add a flag into pci_dev >>>>> to track this information? >>> >>> Why do you need to track it? >>> >>> When do you hot-remove ioapic with pci root bus, all the drivers for >>> devices on that pci root bus should be stopped at first, so no one >>> will use the ioapic before ioapic is being removed. >> Hi Yinghai, >> By tracking IOAPIC pin usage count, we could release the >> assigned IRQ number when there are no device uses an IOAPIC pin anymore. > > Delay releasing IRQ number later when destroying irq domain? Hi Yinghai, We could release IRQ number at runtime not only for IOAPIC hotplug, but also for PCI device hotplug. For example, release IRQ number if an IOAPIC pin is not used anymore after hot-removal of a PCI device. Regards! Gerry > > Yinghai > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-17 3:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-16 3:34 Need advices about PCI IRQ reference count tracking Jiang Liu 2014-07-16 18:04 ` Bjorn Helgaas 2014-07-16 18:37 ` Yinghai Lu 2014-07-17 3:02 ` Jiang Liu 2014-07-17 3:06 ` Yinghai Lu 2014-07-17 3:14 ` Jiang Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).