Need advices about PCI IRQ reference count tracking

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.

[+cc Yinghai]

Quoted 15 lines · Jiang Liu

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?

Quoted 3 lines

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.

Quoted 18 lines · Bjorn Helgaas

[+cc Yinghai]

Quoted 15 lines · Jiang Liu

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.

Quoted 4 lines

Quoted 4 lines

Another possible workaround is to disable IOAPIC hotplug when pci_routeirq is set, but that sounds not the best solution.

That is not good.

Quoted 1 line

Quoted 3 lines

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.

Quoted 6 lines

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

Quoted 20 lines · Yinghai Lu

Quoted 18 lines · Bjorn Helgaas

[+cc Yinghai]

Quoted 15 lines · Jiang Liu

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?

Quoted 5 lines

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.

Quoted 25 lines · Jiang Liu

Quoted 22 lines · Yinghai Lu

Quoted 18 lines · Bjorn Helgaas

[+cc Yinghai]

Quoted 15 lines · Jiang Liu

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?

Quoted 6 lines

Quoted 3 lines

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