All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Remove PCIE root bridge LSI on powernv
@ 2022-03-21 15:33 Frederic Barrat
  2022-03-21 15:33 ` [PATCH 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Frederic Barrat @ 2022-03-21 15:33 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.


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          | 8 ++++++--
 hw/pci/pcie_aer.c      | 4 +++-
 4 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.35.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] pcie: Don't try triggering a LSI when not defined
  2022-03-21 15:33 [PATCH 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat
@ 2022-03-21 15:33 ` Frederic Barrat
  2022-03-24 13:07   ` Daniel Henrique Barboza
  2022-03-21 15:33 ` [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat
  2022-04-06  7:52 ` [PATCH 0/2] Remove PCIE root bridge LSI on powernv Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Frederic Barrat @ 2022-03-21 15:33 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     | 8 ++++++--
 hw/pci/pcie_aer.c | 4 +++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 67a5d67372..71c5194b80 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -354,7 +354,9 @@ static void hotplug_event_notify(PCIDevice *dev)
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_cap_flags_get_vector(dev));
     } else {
-        pci_set_irq(dev, dev->exp.hpev_notified);
+        if (pci_intx(dev) != -1) {
+            pci_set_irq(dev, dev->exp.hpev_notified);
+        }
     }
 }
 
@@ -362,7 +364,9 @@ static void hotplug_event_clear(PCIDevice *dev)
 {
     hotplug_event_update_event_status(dev);
     if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
-        pci_irq_deassert(dev);
+        if (pci_intx(dev) != -1) {
+            pci_irq_deassert(dev);
+        }
     }
 }
 
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index e1a8a88c8c..d936bfca20 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -291,7 +291,9 @@ static void pcie_aer_root_notify(PCIDevice *dev)
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_aer_root_get_vector(dev));
     } else {
-        pci_irq_assert(dev);
+        if (pci_intx(dev) != -1) {
+            pci_irq_assert(dev);
+        }
     }
 }
 
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge
  2022-03-21 15:33 [PATCH 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat
  2022-03-21 15:33 ` [PATCH 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat
@ 2022-03-21 15:33 ` Frederic Barrat
  2022-03-24 13:10   ` Daniel Henrique Barboza
  2022-04-06  7:52 ` [PATCH 0/2] Remove PCIE root bridge LSI on powernv Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Frederic Barrat @ 2022-03-21 15:33 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 ac801ac835..0d18c96117 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 b301762093..b66b75d4d7 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] 9+ messages in thread

* Re: [PATCH 1/2] pcie: Don't try triggering a LSI when not defined
  2022-03-21 15:33 ` [PATCH 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat
@ 2022-03-24 13:07   ` Daniel Henrique Barboza
  2022-03-24 13:47     ` Frederic Barrat
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Henrique Barboza @ 2022-03-24 13:07 UTC (permalink / raw)
  To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel



On 3/21/22 12:33, 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>
> ---

I assume that it's easier to handle just the codepaths that powernv PHBs uses
rather than handling all instances where pci_irq_handler() would be asserting
without LSIs.


Patch LGTM. Small nits below:

>   hw/pci/pcie.c     | 8 ++++++--
>   hw/pci/pcie_aer.c | 4 +++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 67a5d67372..71c5194b80 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -354,7 +354,9 @@ static void hotplug_event_notify(PCIDevice *dev)
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_cap_flags_get_vector(dev));
>       } else {
> -        pci_set_irq(dev, dev->exp.hpev_notified);
> +        if (pci_intx(dev) != -1) {
> +            pci_set_irq(dev, dev->exp.hpev_notified);
> +        }


Since you're not doing anything unless the condition is met, you can use 'else if'
like it's done in the other conditionals:


     if (msix_enabled(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 if (pci_intx(dev) != -1) {
         pci_set_irq(dev, dev->exp.hpev_notified);
     }



>       }
>   }
>   
> @@ -362,7 +364,9 @@ static void hotplug_event_clear(PCIDevice *dev)
>   {
>       hotplug_event_update_event_status(dev);
>       if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
> -        pci_irq_deassert(dev);
> +        if (pci_intx(dev) != -1) {
> +            pci_irq_deassert(dev);
> +        }
>       }

Similar comment here:

     if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified &&
         pci_intx(dev) != -1) {
         pci_irq_deassert(dev);
     }



>   }
>   
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index e1a8a88c8c..d936bfca20 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -291,7 +291,9 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_aer_root_get_vector(dev));
>       } else {
> -        pci_irq_assert(dev);
> +        if (pci_intx(dev) != -1) {
> +            pci_irq_assert(dev);
> +        }


And here:

     if (msix_enabled(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 if (pci_intx(dev) != -1) {
         pci_irq_assert(dev);
     }



Thanks,


Daniel

>       }
>   }
>   


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge
  2022-03-21 15:33 ` [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat
@ 2022-03-24 13:10   ` Daniel Henrique Barboza
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Henrique Barboza @ 2022-03-24 13:10 UTC (permalink / raw)
  To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel



On 3/21/22 12:33, 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 ac801ac835..0d18c96117 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 b301762093..b66b75d4d7 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] 9+ messages in thread

* Re: [PATCH 1/2] pcie: Don't try triggering a LSI when not defined
  2022-03-24 13:07   ` Daniel Henrique Barboza
@ 2022-03-24 13:47     ` Frederic Barrat
  2022-03-24 14:02       ` Daniel Henrique Barboza
  0 siblings, 1 reply; 9+ messages in thread
From: Frederic Barrat @ 2022-03-24 13:47 UTC (permalink / raw)
  To: Daniel Henrique Barboza, clg, mst, marcel.apfelbaum, qemu-ppc,
	qemu-devel



On 24/03/2022 14:07, Daniel Henrique Barboza wrote:
> 
> 
> On 3/21/22 12:33, 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>
>> ---
> 
> I assume that it's easier to handle just the codepaths that powernv PHBs 
> uses
> rather than handling all instances where pci_irq_handler() would be 
> asserting
> without LSIs.


The real reason is that the LSI is added when we realize the 
TYPE_PCIE_ROOT_PORT object. See rp_realize(). So I'm only trying to fix 
the code paths that can be called from a subclass of TYPE_PCIE_ROOT_PORT 
which would choose to override the "interrupt pin" setting in the config 
space. I believe they are all covered by this patch.
The assert() in pci_irq_handler() is there for a reason and I don't want 
to mess with that.

   Fred


> 
> 
> Patch LGTM. Small nits below:
> 
>>   hw/pci/pcie.c     | 8 ++++++--
>>   hw/pci/pcie_aer.c | 4 +++-
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index 67a5d67372..71c5194b80 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -354,7 +354,9 @@ static void hotplug_event_notify(PCIDevice *dev)
>>       } else if (msi_enabled(dev)) {
>>           msi_notify(dev, pcie_cap_flags_get_vector(dev));
>>       } else {
>> -        pci_set_irq(dev, dev->exp.hpev_notified);
>> +        if (pci_intx(dev) != -1) {
>> +            pci_set_irq(dev, dev->exp.hpev_notified);
>> +        }
> 
> 
> Since you're not doing anything unless the condition is met, you can use 
> 'else if'
> like it's done in the other conditionals:
> 
> 
>      if (msix_enabled(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 if (pci_intx(dev) != -1) {
>          pci_set_irq(dev, dev->exp.hpev_notified);
>      }
> 
> 
> 
>>       }
>>   }
>> @@ -362,7 +364,9 @@ static void hotplug_event_clear(PCIDevice *dev)
>>   {
>>       hotplug_event_update_event_status(dev);
>>       if (!msix_enabled(dev) && !msi_enabled(dev) && 
>> !dev->exp.hpev_notified) {
>> -        pci_irq_deassert(dev);
>> +        if (pci_intx(dev) != -1) {
>> +            pci_irq_deassert(dev);
>> +        }
>>       }
> 
> Similar comment here:
> 
>      if (!msix_enabled(dev) && !msi_enabled(dev) && 
> !dev->exp.hpev_notified &&
>          pci_intx(dev) != -1) {
>          pci_irq_deassert(dev);
>      }
> 
> 
> 
>>   }
>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>> index e1a8a88c8c..d936bfca20 100644
>> --- a/hw/pci/pcie_aer.c
>> +++ b/hw/pci/pcie_aer.c
>> @@ -291,7 +291,9 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>>       } else if (msi_enabled(dev)) {
>>           msi_notify(dev, pcie_aer_root_get_vector(dev));
>>       } else {
>> -        pci_irq_assert(dev);
>> +        if (pci_intx(dev) != -1) {
>> +            pci_irq_assert(dev);
>> +        }
> 
> 
> And here:
> 
>      if (msix_enabled(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 if (pci_intx(dev) != -1) {
>          pci_irq_assert(dev);
>      }
> 
> 
> 
> Thanks,
> 
> 
> Daniel
> 
>>       }
>>   }


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] pcie: Don't try triggering a LSI when not defined
  2022-03-24 13:47     ` Frederic Barrat
@ 2022-03-24 14:02       ` Daniel Henrique Barboza
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Henrique Barboza @ 2022-03-24 14:02 UTC (permalink / raw)
  To: Frederic Barrat, clg, mst, marcel.apfelbaum, qemu-ppc, qemu-devel



On 3/24/22 10:47, Frederic Barrat wrote:
> 
> 
> On 24/03/2022 14:07, Daniel Henrique Barboza wrote:
>>
>>
>> On 3/21/22 12:33, 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>
>>> ---
>>
>> I assume that it's easier to handle just the codepaths that powernv PHBs uses
>> rather than handling all instances where pci_irq_handler() would be asserting
>> without LSIs.
> 
> 
> The real reason is that the LSI is added when we realize the TYPE_PCIE_ROOT_PORT object. See rp_realize(). So I'm only trying to fix the code paths that can be called from a subclass of TYPE_PCIE_ROOT_PORT which would choose to override the "interrupt pin" setting in the config space. I believe they are all covered by this patch.
> The assert() in pci_irq_handler() is there for a reason and I don't want to mess with that.


Yes, handling this situation inside pci_irq_handler() would require changing the
"assert(0 <= irq_num && irq_num < PCI_NUM_PINS)" assert or doing some sanity before
it to avoid the trigger.

Since this is a common code used everywhere we're better of doing minimalist changes
as you're doing. We can reevaluate this design if more machines/devices start to get
in the same situation we have now with powernv PHBs.


Daniel

> 
>    Fred
> 
> 
>>
>>
>> Patch LGTM. Small nits below:
>>
>>>   hw/pci/pcie.c     | 8 ++++++--
>>>   hw/pci/pcie_aer.c | 4 +++-
>>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>>> index 67a5d67372..71c5194b80 100644
>>> --- a/hw/pci/pcie.c
>>> +++ b/hw/pci/pcie.c
>>> @@ -354,7 +354,9 @@ static void hotplug_event_notify(PCIDevice *dev)
>>>       } else if (msi_enabled(dev)) {
>>>           msi_notify(dev, pcie_cap_flags_get_vector(dev));
>>>       } else {
>>> -        pci_set_irq(dev, dev->exp.hpev_notified);
>>> +        if (pci_intx(dev) != -1) {
>>> +            pci_set_irq(dev, dev->exp.hpev_notified);
>>> +        }
>>
>>
>> Since you're not doing anything unless the condition is met, you can use 'else if'
>> like it's done in the other conditionals:
>>
>>
>>      if (msix_enabled(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 if (pci_intx(dev) != -1) {
>>          pci_set_irq(dev, dev->exp.hpev_notified);
>>      }
>>
>>
>>
>>>       }
>>>   }
>>> @@ -362,7 +364,9 @@ static void hotplug_event_clear(PCIDevice *dev)
>>>   {
>>>       hotplug_event_update_event_status(dev);
>>>       if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
>>> -        pci_irq_deassert(dev);
>>> +        if (pci_intx(dev) != -1) {
>>> +            pci_irq_deassert(dev);
>>> +        }
>>>       }
>>
>> Similar comment here:
>>
>>      if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified &&
>>          pci_intx(dev) != -1) {
>>          pci_irq_deassert(dev);
>>      }
>>
>>
>>
>>>   }
>>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>>> index e1a8a88c8c..d936bfca20 100644
>>> --- a/hw/pci/pcie_aer.c
>>> +++ b/hw/pci/pcie_aer.c
>>> @@ -291,7 +291,9 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>>>       } else if (msi_enabled(dev)) {
>>>           msi_notify(dev, pcie_aer_root_get_vector(dev));
>>>       } else {
>>> -        pci_irq_assert(dev);
>>> +        if (pci_intx(dev) != -1) {
>>> +            pci_irq_assert(dev);
>>> +        }
>>
>>
>> And here:
>>
>>      if (msix_enabled(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 if (pci_intx(dev) != -1) {
>>          pci_irq_assert(dev);
>>      }
>>
>>
>>
>> Thanks,
>>
>>
>> Daniel
>>
>>>       }
>>>   }


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] Remove PCIE root bridge LSI on powernv
  2022-03-21 15:33 [PATCH 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat
  2022-03-21 15:33 ` [PATCH 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat
  2022-03-21 15:33 ` [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat
@ 2022-04-06  7:52 ` Michael S. Tsirkin
  2022-04-06  8:57   ` Frederic Barrat
  2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2022-04-06  7:52 UTC (permalink / raw)
  To: Frederic Barrat; +Cc: danielhb413, qemu-ppc, clg, qemu-devel

On Mon, Mar 21, 2022 at 04:33:55PM +0100, 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.


Hi Frederic, thanks for the patch!
I assume you will address Daniel's comments and post a new version,
right?

> 
> 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          | 8 ++++++--
>  hw/pci/pcie_aer.c      | 4 +++-
>  4 files changed, 11 insertions(+), 3 deletions(-)
> 
> -- 
> 2.35.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] Remove PCIE root bridge LSI on powernv
  2022-04-06  7:52 ` [PATCH 0/2] Remove PCIE root bridge LSI on powernv Michael S. Tsirkin
@ 2022-04-06  8:57   ` Frederic Barrat
  0 siblings, 0 replies; 9+ messages in thread
From: Frederic Barrat @ 2022-04-06  8:57 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: danielhb413, qemu-ppc, clg, qemu-devel



On 06/04/2022 09:52, Michael S. Tsirkin wrote:
> On Mon, Mar 21, 2022 at 04:33:55PM +0100, 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.
> 
> 
> Hi Frederic, thanks for the patch!
> I assume you will address Daniel's comments and post a new version,
> right?


Yes, I will. I'm obviously only targeting 7.1

   Fred

> 
>>
>> 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          | 8 ++++++--
>>   hw/pci/pcie_aer.c      | 4 +++-
>>   4 files changed, 11 insertions(+), 3 deletions(-)
>>
>> -- 
>> 2.35.1
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-04-06  8:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 15:33 [PATCH 0/2] Remove PCIE root bridge LSI on powernv Frederic Barrat
2022-03-21 15:33 ` [PATCH 1/2] pcie: Don't try triggering a LSI when not defined Frederic Barrat
2022-03-24 13:07   ` Daniel Henrique Barboza
2022-03-24 13:47     ` Frederic Barrat
2022-03-24 14:02       ` Daniel Henrique Barboza
2022-03-21 15:33 ` [PATCH 2/2] ppc/pnv: Remove LSI on the PCIE host bridge Frederic Barrat
2022-03-24 13:10   ` Daniel Henrique Barboza
2022-04-06  7:52 ` [PATCH 0/2] Remove PCIE root bridge LSI on powernv Michael S. Tsirkin
2022-04-06  8:57   ` Frederic Barrat

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.