linux-kernel - Re: [PATCH] PCI : Fix pcie_flag_reg in set_pcie_port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241104235931.GA1447239@bhelgaas>
Date: Mon, 4 Nov 2024 17:59:31 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Gowthami Thiagarajan <gthiagarajan@...vell.com>
Cc: bhelgaas@...gle.com, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI : Fix pcie_flag_reg in set_pcie_port_type

On Mon, Nov 04, 2024 at 02:46:27PM +0530, Gowthami Thiagarajan wrote:
> The port type in set_pcie_port_type is not set proper when an invalid
> topology is detected. Since the port type was not set proper, the child's
> extended config space becomes inaccessible.

Please describe how the topology is invalid.

> [   70.440438] pci 0002:00:00.0: [177d:a002] type 01 class 0x060401
> [   70.463056] pci 0002:00:00.0: reg 0x38: [mem 0x600000000000-0x6000000007ff pref]
> [   71.806936] pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> [   71.906688] pci (null): claims to be downstream port but is acting as upstream port, correcting type
> [   71.916138] pci 0002:01:00.0: [177d:a002] type 01 class 0x060401
> [   71.935982] pci 0002:01:00.0: reg 0x38: [mem 0x600000000000-0x6000000007ff pref]
> [   72.134703] pci 0002:01:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> [   72.198956] pci_bus 0002:02: extended config space not accessible
> .......
> [   83.762956] pci_bus 0002:03: extended config space not accessible
> [   83.792530] pci 0002:03:00.0: [177d:a065] type 00 class 0x020000
> [   83.813188] pci 0002:03:00.0: reg 0x10: [mem 0x600000000000-0x6003ffffffff 64bit pref]
> [   83.832490] pci 0002:03:00.0: reg 0x18: [mem 0x600000000000-0x600003ffffff 64bit pref]
> [   83.848507] pci 0002:03:00.0: reg 0x20: [mem 0x600000000000-0x60000000ffff 64bit pref]
> [   83.935564] pci_bus 0002:03: busn_res: [bus 03-ff] end is updated to 03
> [   83.998804] pci_bus 0002:04: extended config space not accessible
> [   84.025026] pci 0002:04:00.0: [177d:a063] type 00 class 0x020000
> [   84.055298] pci 0002:04:00.0: reg 0x18: [mem 0x600000000000-0x600003ffffff 64bit pref]
> [   84.147582] pci_bus 0002:04: busn_res: [bus 04-ff] end is updated to 04
> [   84.202778] pci_bus 0002:05: extended config space not accessible
> [   84.228684] pci 0002:05:00.0: [177d:a063] type 00 class 0x020000
> [   84.258887] pci 0002:05:00.0: reg 0x18: [mem 0x600000000000-0x600003ffffff 64bit pref]

Remove timestamps since they don't help to understand the problem.

Also, if this only has to do with the port type, the BAR information
probably doesn't help understand this either, so we can remove it.

I think if you use a current kernel, the dmesg logging should include
the port type (Root Port, Upstream Port, etc), which might make this
more readable.

> Signed-off-by: Gowthami Thiagarajan <gthiagarajan@...vell.com>
> ---
>  drivers/pci/probe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4f68414c3086..263ec21451d9 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1596,7 +1596,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
>  		if (pcie_downstream_port(parent)) {
>  			pci_info(pdev, "claims to be downstream port but is acting as upstream port, correcting type\n");
>  			pdev->pcie_flags_reg &= ~PCI_EXP_FLAGS_TYPE;
> -			pdev->pcie_flags_reg |= PCI_EXP_TYPE_UPSTREAM;
> +			pdev->pcie_flags_reg |= PCI_EXP_TYPE_UPSTREAM << 4;
>  		}
>  	} else if (type == PCI_EXP_TYPE_UPSTREAM) {
>  		/*
> @@ -1607,7 +1607,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
>  		if (pci_pcie_type(parent) == PCI_EXP_TYPE_UPSTREAM) {
>  			pci_info(pdev, "claims to be upstream port but is acting as downstream port, correcting type\n");
>  			pdev->pcie_flags_reg &= ~PCI_EXP_FLAGS_TYPE;
> -			pdev->pcie_flags_reg |= PCI_EXP_TYPE_DOWNSTREAM;
> +			pdev->pcie_flags_reg |= PCI_EXP_TYPE_DOWNSTREAM << 4;
>  		}
>  	}
>  }
> -- 
> 2.25.1
>