* [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv @ 2022-04-08 13:13 Frederic Barrat 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Frederic Barrat @ 2022-04-08 13:13 UTC (permalink / raw) To: clg, danielhb413, mst, marcel.apfelbaum, qemu-ppc, qemu-devel The powernv8/powernv9/powernv10 machines allocate a LSI for their root port bridge, which is not the case on real hardware. The default root port implementation in qemu requests a LSI. Since the powernv implementation derives from it, that's where the LSI is coming from. This series fixes it, so that the model matches the hardware. However, the code in hw/pci to handle AER and hotplug events assume a LSI is defined. It tends to assert/deassert a LSI if MSI or MSIX is not enabled. Since we have hardware where that is not true, this patch also fixes a few code paths to check if a LSI is configured before trying to trigger it. Changes from v1: - addressed comments from Daniel Frederic Barrat (2): pcie: Don't try triggering a LSI when not defined ppc/pnv: Remove LSI on the PCIE host bridge hw/pci-host/pnv_phb3.c | 1 + hw/pci-host/pnv_phb4.c | 1 + hw/pci/pcie.c | 5 +++-- hw/pci/pcie_aer.c | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined 2022-04-08 13:13 [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat @ 2022-04-08 13:13 ` Frederic Barrat 2022-04-08 21:12 ` Daniel Henrique Barboza ` (2 more replies) 2022-04-08 13:13 ` [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat 2022-04-20 19:17 ` [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Daniel Henrique Barboza 2 siblings, 3 replies; 8+ messages in thread From: Frederic Barrat @ 2022-04-08 13:13 UTC (permalink / raw) To: clg, danielhb413, mst, marcel.apfelbaum, qemu-ppc, qemu-devel This patch skips [de]asserting a LSI interrupt if the device doesn't have any LSI defined. Doing so would trigger an assert in pci_irq_handler(). The PCIE root port implementation in qemu requests a LSI (INTA), but a subclass may want to change that behavior since it's a valid configuration. For example on the POWER8/POWER9/POWER10 systems, the root bridge doesn't request any LSI. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- hw/pci/pcie.c | 5 +++-- hw/pci/pcie_aer.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 67a5d67372..68a62da0b5 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev) msix_notify(dev, pcie_cap_flags_get_vector(dev)); } else if (msi_enabled(dev)) { msi_notify(dev, pcie_cap_flags_get_vector(dev)); - } else { + } else if (pci_intx(dev) != -1) { pci_set_irq(dev, dev->exp.hpev_notified); } } @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev) static void hotplug_event_clear(PCIDevice *dev) { hotplug_event_update_event_status(dev); - if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { + if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 && + !dev->exp.hpev_notified) { pci_irq_deassert(dev); } } diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index e1a8a88c8c..92bd0530dd 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev) msix_notify(dev, pcie_aer_root_get_vector(dev)); } else if (msi_enabled(dev)) { msi_notify(dev, pcie_aer_root_get_vector(dev)); - } else { + } else if (pci_intx(dev) != -1) { pci_irq_assert(dev); } } -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat @ 2022-04-08 21:12 ` Daniel Henrique Barboza 2022-04-20 17:09 ` Daniel Henrique Barboza 2022-04-20 17:18 ` Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: Daniel Henrique Barboza @ 2022-04-08 21:12 UTC (permalink / raw) To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel On 4/8/22 10:13, Frederic Barrat wrote: > This patch skips [de]asserting a LSI interrupt if the device doesn't > have any LSI defined. Doing so would trigger an assert in > pci_irq_handler(). > > The PCIE root port implementation in qemu requests a LSI (INTA), but a > subclass may want to change that behavior since it's a valid > configuration. For example on the POWER8/POWER9/POWER10 systems, the > root bridge doesn't request any LSI. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > hw/pci/pcie.c | 5 +++-- > hw/pci/pcie_aer.c | 2 +- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 67a5d67372..68a62da0b5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev) > msix_notify(dev, pcie_cap_flags_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_cap_flags_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_set_irq(dev, dev->exp.hpev_notified); > } > } > @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev) > static void hotplug_event_clear(PCIDevice *dev) > { > hotplug_event_update_event_status(dev); > - if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { > + if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 && > + !dev->exp.hpev_notified) { > pci_irq_deassert(dev); > } > } > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index e1a8a88c8c..92bd0530dd 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev) > msix_notify(dev, pcie_aer_root_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_aer_root_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_irq_assert(dev); > } > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat 2022-04-08 21:12 ` Daniel Henrique Barboza @ 2022-04-20 17:09 ` Daniel Henrique Barboza 2022-04-20 17:18 ` Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: Daniel Henrique Barboza @ 2022-04-20 17:09 UTC (permalink / raw) To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel Michael, Let me know if you want me to grab this patch from the ppc tree. I'm going to send a PR in the next few days. Thanks, Daniel On 4/8/22 10:13, Frederic Barrat wrote: > This patch skips [de]asserting a LSI interrupt if the device doesn't > have any LSI defined. Doing so would trigger an assert in > pci_irq_handler(). > > The PCIE root port implementation in qemu requests a LSI (INTA), but a > subclass may want to change that behavior since it's a valid > configuration. For example on the POWER8/POWER9/POWER10 systems, the > root bridge doesn't request any LSI. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- > hw/pci/pcie.c | 5 +++-- > hw/pci/pcie_aer.c | 2 +- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 67a5d67372..68a62da0b5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev) > msix_notify(dev, pcie_cap_flags_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_cap_flags_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_set_irq(dev, dev->exp.hpev_notified); > } > } > @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev) > static void hotplug_event_clear(PCIDevice *dev) > { > hotplug_event_update_event_status(dev); > - if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { > + if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 && > + !dev->exp.hpev_notified) { > pci_irq_deassert(dev); > } > } > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index e1a8a88c8c..92bd0530dd 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev) > msix_notify(dev, pcie_aer_root_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_aer_root_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_irq_assert(dev); > } > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat 2022-04-08 21:12 ` Daniel Henrique Barboza 2022-04-20 17:09 ` Daniel Henrique Barboza @ 2022-04-20 17:18 ` Michael S. Tsirkin 2 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2022-04-20 17:18 UTC (permalink / raw) To: Frederic Barrat; +Cc: danielhb413, qemu-ppc, clg, qemu-devel On Fri, Apr 08, 2022 at 03:13:02PM +0200, Frederic Barrat wrote: > This patch skips [de]asserting a LSI interrupt if the device doesn't > have any LSI defined. Doing so would trigger an assert in > pci_irq_handler(). > > The PCIE root port implementation in qemu requests a LSI (INTA), but a > subclass may want to change that behavior since it's a valid > configuration. For example on the POWER8/POWER9/POWER10 systems, the > root bridge doesn't request any LSI. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Feel free to merge with the second patch. Thanks! > --- > hw/pci/pcie.c | 5 +++-- > hw/pci/pcie_aer.c | 2 +- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index 67a5d67372..68a62da0b5 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev) > msix_notify(dev, pcie_cap_flags_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_cap_flags_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_set_irq(dev, dev->exp.hpev_notified); > } > } > @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev) > static void hotplug_event_clear(PCIDevice *dev) > { > hotplug_event_update_event_status(dev); > - if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { > + if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 && > + !dev->exp.hpev_notified) { > pci_irq_deassert(dev); > } > } > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index e1a8a88c8c..92bd0530dd 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev) > msix_notify(dev, pcie_aer_root_get_vector(dev)); > } else if (msi_enabled(dev)) { > msi_notify(dev, pcie_aer_root_get_vector(dev)); > - } else { > + } else if (pci_intx(dev) != -1) { > pci_irq_assert(dev); > } > } > -- > 2.35.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge 2022-04-08 13:13 [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat @ 2022-04-08 13:13 ` Frederic Barrat 2022-04-08 21:13 ` Daniel Henrique Barboza 2022-04-20 19:17 ` [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Daniel Henrique Barboza 2 siblings, 1 reply; 8+ messages in thread From: Frederic Barrat @ 2022-04-08 13:13 UTC (permalink / raw) To: clg, danielhb413, mst, marcel.apfelbaum, qemu-ppc, qemu-devel The phb3/phb4/phb5 root ports inherit from the default PCIE root port implementation, which requests a LSI interrupt (#INTA). On real hardware (POWER8/POWER9/POWER10), there is no such LSI. This patch corrects it so that it matches the hardware. As a consequence, the device tree previously generated was bogus, as the root bridge LSI was not properly mapped. On some implementation (powernv9), it was leading to inconsistent interrupt controller (xive) data. With this patch, it is now clean. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- hw/pci-host/pnv_phb3.c | 1 + hw/pci-host/pnv_phb4.c | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 6e9aa9d6ac..6a884833a8 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -1162,6 +1162,7 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp) error_propagate(errp, local_err); return; } + pci_config_set_interrupt_pin(pci->config, 0); } static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index 11c97e27eb..dd81e940b7 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1772,6 +1772,7 @@ static void pnv_phb4_root_port_reset(DeviceState *dev) pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1); pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */ pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff); + pci_config_set_interrupt_pin(conf, 0); } static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp) -- 2.35.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge 2022-04-08 13:13 ` [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat @ 2022-04-08 21:13 ` Daniel Henrique Barboza 0 siblings, 0 replies; 8+ messages in thread From: Daniel Henrique Barboza @ 2022-04-08 21:13 UTC (permalink / raw) To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel On 4/8/22 10:13, Frederic Barrat wrote: > The phb3/phb4/phb5 root ports inherit from the default PCIE root port > implementation, which requests a LSI interrupt (#INTA). On real > hardware (POWER8/POWER9/POWER10), there is no such LSI. This patch > corrects it so that it matches the hardware. > > As a consequence, the device tree previously generated was bogus, as > the root bridge LSI was not properly mapped. On some > implementation (powernv9), it was leading to inconsistent interrupt > controller (xive) data. With this patch, it is now clean. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > hw/pci-host/pnv_phb3.c | 1 + > hw/pci-host/pnv_phb4.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c > index 6e9aa9d6ac..6a884833a8 100644 > --- a/hw/pci-host/pnv_phb3.c > +++ b/hw/pci-host/pnv_phb3.c > @@ -1162,6 +1162,7 @@ static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp) > error_propagate(errp, local_err); > return; > } > + pci_config_set_interrupt_pin(pci->config, 0); > } > > static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data) > diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c > index 11c97e27eb..dd81e940b7 100644 > --- a/hw/pci-host/pnv_phb4.c > +++ b/hw/pci-host/pnv_phb4.c > @@ -1772,6 +1772,7 @@ static void pnv_phb4_root_port_reset(DeviceState *dev) > pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1); > pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */ > pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff); > + pci_config_set_interrupt_pin(conf, 0); > } > > static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv 2022-04-08 13:13 [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat 2022-04-08 13:13 ` [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat @ 2022-04-20 19:17 ` Daniel Henrique Barboza 2 siblings, 0 replies; 8+ messages in thread From: Daniel Henrique Barboza @ 2022-04-20 19:17 UTC (permalink / raw) To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks, Daniel On 4/8/22 10:13, Frederic Barrat wrote: > The powernv8/powernv9/powernv10 machines allocate a LSI for their root > port bridge, which is not the case on real hardware. The default root > port implementation in qemu requests a LSI. Since the powernv > implementation derives from it, that's where the LSI is coming > from. This series fixes it, so that the model matches the hardware. > > However, the code in hw/pci to handle AER and hotplug events assume a > LSI is defined. It tends to assert/deassert a LSI if MSI or MSIX is > not enabled. Since we have hardware where that is not true, this patch > also fixes a few code paths to check if a LSI is configured before > trying to trigger it. > > > Changes from v1: > - addressed comments from Daniel > > > Frederic Barrat (2): > pcie: Don't try triggering a LSI when not defined > ppc/pnv: Remove LSI on the PCIE host bridge > > hw/pci-host/pnv_phb3.c | 1 + > hw/pci-host/pnv_phb4.c | 1 + > hw/pci/pcie.c | 5 +++-- > hw/pci/pcie_aer.c | 2 +- > 4 files changed, 6 insertions(+), 3 deletions(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-04-20 20:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-08 13:13 [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat 2022-04-08 13:13 ` [PATCH v2 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat 2022-04-08 21:12 ` Daniel Henrique Barboza 2022-04-20 17:09 ` Daniel Henrique Barboza 2022-04-20 17:18 ` Michael S. Tsirkin 2022-04-08 13:13 ` [PATCH v2 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat 2022-04-08 21:13 ` Daniel Henrique Barboza 2022-04-20 19:17 ` [PATCH v2 0/2] Remove PCIE root bridge LSI on powernv Daniel Henrique Barboza
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.