1. Patchew
  2. QEMU
  3. View series

[PATCH v2] pci: ensure valid link status bits for downstream ports

Sebastian Ott posted 1 patch 5 months, 1 week ago 
hw/pci/pcie.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
[PATCH v2] pci: ensure valid link status bits for downstream ports
Posted by Sebastian Ott 5 months, 1 week ago 
PCI hotplug for downstream endpoints on arm fails because Linux'
PCIe hotplug driver doesn't like the QEMU provided LNKSTA:

  pcieport 0000:08:01.0: pciehp: Slot(2): Card present
  pcieport 0000:08:01.0: pciehp: Slot(2): Link Up
  pcieport 0000:08:01.0: pciehp: Slot(2): Cannot train link: status 0x2000

There's 2 cases where LNKSTA isn't setup properly:
* the downstream device has no express capability
* max link width of the bridge is 0

Move the sanity checks added via 88c869198aa63
("pci: Sanity test minimum downstream LNKSTA") outside of the
branch to make sure downstream ports always have a valid LNKSTA.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Tested-by: Zhenyu Zhang <zhenyzha@redhat.com>
---
 hw/pci/pcie.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 0b455c8654..1b12db6fa2 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -1113,18 +1113,22 @@ void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
         if ((lnksta & PCI_EXP_LNKSTA_NLW) > (lnkcap & PCI_EXP_LNKCAP_MLW)) {
             lnksta &= ~PCI_EXP_LNKSTA_NLW;
             lnksta |= lnkcap & PCI_EXP_LNKCAP_MLW;
-        } else if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
-            lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
         }
 
         if ((lnksta & PCI_EXP_LNKSTA_CLS) > (lnkcap & PCI_EXP_LNKCAP_SLS)) {
             lnksta &= ~PCI_EXP_LNKSTA_CLS;
             lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
-        } else if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
-            lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
         }
     }
 
+    if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
+        lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
+    }
+
+    if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
+        lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
+    }
+
     pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
                                  PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
     pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, lnksta &
-- 
2.42.0
Re: [PATCH v2] pci: ensure valid link status bits for downstream ports
Posted by Michael Tokarev 3 months, 3 weeks ago 
03.12.2024 15:19, Sebastian Ott wrote:
> PCI hotplug for downstream endpoints on arm fails because Linux'
> PCIe hotplug driver doesn't like the QEMU provided LNKSTA:
> 
>    pcieport 0000:08:01.0: pciehp: Slot(2): Card present
>    pcieport 0000:08:01.0: pciehp: Slot(2): Link Up
>    pcieport 0000:08:01.0: pciehp: Slot(2): Cannot train link: status 0x2000
> 
> There's 2 cases where LNKSTA isn't setup properly:
> * the downstream device has no express capability
> * max link width of the bridge is 0
> 
> Move the sanity checks added via 88c869198aa63
> ("pci: Sanity test minimum downstream LNKSTA") outside of the
> branch to make sure downstream ports always have a valid LNKSTA.

This seems to be a qemu-stable material.  Please let me know it it
is not.

Thanks,

/mjt

> Signed-off-by: Sebastian Ott <sebott@redhat.com>
> Tested-by: Zhenyu Zhang <zhenyzha@redhat.com>
> ---
>   hw/pci/pcie.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 0b455c8654..1b12db6fa2 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -1113,18 +1113,22 @@ void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
>           if ((lnksta & PCI_EXP_LNKSTA_NLW) > (lnkcap & PCI_EXP_LNKCAP_MLW)) {
>               lnksta &= ~PCI_EXP_LNKSTA_NLW;
>               lnksta |= lnkcap & PCI_EXP_LNKCAP_MLW;
> -        } else if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
> -            lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
>           }
>   
>           if ((lnksta & PCI_EXP_LNKSTA_CLS) > (lnkcap & PCI_EXP_LNKCAP_SLS)) {
>               lnksta &= ~PCI_EXP_LNKSTA_CLS;
>               lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
> -        } else if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
> -            lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
>           }
>       }
>   
> +    if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
> +        lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
> +    }
> +
> +    if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
> +        lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
> +    }
> +
>       pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
>                                    PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
>       pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, lnksta &
Re: [PATCH v2] pci: ensure valid link status bits for downstream ports
Posted by Alex Williamson 5 months, 1 week ago 
On Tue,  3 Dec 2024 13:19:28 +0100
Sebastian Ott <sebott@redhat.com> wrote:

> PCI hotplug for downstream endpoints on arm fails because Linux'
> PCIe hotplug driver doesn't like the QEMU provided LNKSTA:
> 
>   pcieport 0000:08:01.0: pciehp: Slot(2): Card present
>   pcieport 0000:08:01.0: pciehp: Slot(2): Link Up
>   pcieport 0000:08:01.0: pciehp: Slot(2): Cannot train link: status 0x2000
> 
> There's 2 cases where LNKSTA isn't setup properly:
> * the downstream device has no express capability
> * max link width of the bridge is 0
> 
> Move the sanity checks added via 88c869198aa63
> ("pci: Sanity test minimum downstream LNKSTA") outside of the
> branch to make sure downstream ports always have a valid LNKSTA.
> 
> Signed-off-by: Sebastian Ott <sebott@redhat.com>
> Tested-by: Zhenyu Zhang <zhenyzha@redhat.com>
> ---
>  hw/pci/pcie.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>


> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 0b455c8654..1b12db6fa2 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -1113,18 +1113,22 @@ void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
>          if ((lnksta & PCI_EXP_LNKSTA_NLW) > (lnkcap & PCI_EXP_LNKCAP_MLW)) {
>              lnksta &= ~PCI_EXP_LNKSTA_NLW;
>              lnksta |= lnkcap & PCI_EXP_LNKCAP_MLW;
> -        } else if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
> -            lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
>          }
>  
>          if ((lnksta & PCI_EXP_LNKSTA_CLS) > (lnkcap & PCI_EXP_LNKCAP_SLS)) {
>              lnksta &= ~PCI_EXP_LNKSTA_CLS;
>              lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
> -        } else if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
> -            lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
>          }
>      }
>  
> +    if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
> +        lnksta |= QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1);
> +    }
> +
> +    if (!(lnksta & PCI_EXP_LNKSTA_CLS)) {
> +        lnksta |= QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT);
> +    }
> +
>      pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
>                                   PCI_EXP_LNKSTA_CLS | PCI_EXP_LNKSTA_NLW);
>      pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA, lnksta &
Re: [PATCH v2] pci: ensure valid link status bits for downstream ports
Posted by Sebastian Ott 4 months ago 
Hello Michael, Marcel,

On Tue, 3 Dec 2024, Alex Williamson wrote:

> On Tue,  3 Dec 2024 13:19:28 +0100
> Sebastian Ott <sebott@redhat.com> wrote:
>
>> PCI hotplug for downstream endpoints on arm fails because Linux'
>> PCIe hotplug driver doesn't like the QEMU provided LNKSTA:
>>
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Card present
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Link Up
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Cannot train link: status 0x2000
>>
>> There's 2 cases where LNKSTA isn't setup properly:
>> * the downstream device has no express capability
>> * max link width of the bridge is 0
>>
>> Move the sanity checks added via 88c869198aa63
>> ("pci: Sanity test minimum downstream LNKSTA") outside of the
>> branch to make sure downstream ports always have a valid LNKSTA.
>>
>> Signed-off-by: Sebastian Ott <sebott@redhat.com>
>> Tested-by: Zhenyu Zhang <zhenyzha@redhat.com>
>> ---
>>  hw/pci/pcie.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

Can you pick this up for the next qemu release, please? It fixes PCI
hotplug for downstream endpoints on arm.

Thanks,
Sebastian
Re: [PATCH v2] pci: ensure valid link status bits for downstream ports
Posted by Sebastian Ott 4 months, 3 weeks ago 
Hi,

On Tue, 3 Dec 2024, Alex Williamson wrote:
> On Tue,  3 Dec 2024 13:19:28 +0100
> Sebastian Ott <sebott@redhat.com> wrote:
>
>> PCI hotplug for downstream endpoints on arm fails because Linux'
>> PCIe hotplug driver doesn't like the QEMU provided LNKSTA:
>>
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Card present
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Link Up
>>   pcieport 0000:08:01.0: pciehp: Slot(2): Cannot train link: status 0x2000
>>
>> There's 2 cases where LNKSTA isn't setup properly:
>> * the downstream device has no express capability
>> * max link width of the bridge is 0
>>
>> Move the sanity checks added via 88c869198aa63
>> ("pci: Sanity test minimum downstream LNKSTA") outside of the
>> branch to make sure downstream ports always have a valid LNKSTA.
>>
>> Signed-off-by: Sebastian Ott <sebott@redhat.com>
>> Tested-by: Zhenyu Zhang <zhenyzha@redhat.com>
>> ---
>>  hw/pci/pcie.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

Can someone pick this up for the next qemu release?

Thanks,
Sebastian

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.