2023-01-10 18:12:23

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 0/2] PCI: Fix extended config space regression

From: Bjorn Helgaas <[email protected]>

07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") appeared
in v6.2-rc1 and broke extended config space on several machines.

This broke drivers that use things in extended config space, e.g., perf,
VSEC telemetry, EDAC, QAT, etc.

This happened because mmconfig-shared.c checks to see that ECAM space is
reserved in E820 or ACPI motherboard resources. If it's not, it assumes
ECAM doesn't work. 07eab0901ede removed some E820 entries, so it looked
like ECAM was no longer reserved, so we stopped using it.

The machines where this broke don't reserve the ECAM in ACPI PNP0C02
devices (which seems to be what the PCI Firmware spec requires), but they
do mention it in the EFI memory map as EfiMemoryMappedIO.

Bjorn Helgaas (2):
x86/pci: Simplify is_mmconf_reserved() messages
x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space

arch/x86/pci/mmconfig-shared.c | 44 +++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)

--
2.25.1


2023-01-10 18:26:33

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 1/2] x86/pci: Simplify is_mmconf_reserved() messages

From: Bjorn Helgaas <[email protected]>

is_mmconf_reserved() takes a "with_e820" parameter that only determines the
message logged if it finds the MMCONFIG region is reserved. Pass the
message directly, which will simplify a future patch that adds a new way of
looking for that reservation. No functional change intended.

Signed-off-by: Bjorn Helgaas <[email protected]>
---
arch/x86/pci/mmconfig-shared.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 758cbfe55daa..cd16bef5f2d9 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -446,13 +446,12 @@ typedef bool (*check_reserved_t)(u64 start, u64 end, enum e820_type type);

static bool __ref is_mmconf_reserved(check_reserved_t is_reserved,
struct pci_mmcfg_region *cfg,
- struct device *dev, int with_e820)
+ struct device *dev, char *method)
{
u64 addr = cfg->res.start;
u64 size = resource_size(&cfg->res);
u64 old_size = size;
int num_buses;
- char *method = with_e820 ? "E820" : "ACPI motherboard resources";

while (!is_reserved(addr, addr + size, E820_TYPE_RESERVED)) {
size >>= 1;
@@ -464,10 +463,10 @@ static bool __ref is_mmconf_reserved(check_reserved_t is_reserved,
return false;

if (dev)
- dev_info(dev, "MMCONFIG at %pR reserved in %s\n",
+ dev_info(dev, "MMCONFIG at %pR reserved as %s\n",
&cfg->res, method);
else
- pr_info(PREFIX "MMCONFIG at %pR reserved in %s\n",
+ pr_info(PREFIX "MMCONFIG at %pR reserved as %s\n",
&cfg->res, method);

if (old_size != size) {
@@ -500,7 +499,8 @@ static bool __ref
pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int early)
{
if (!early && !acpi_disabled) {
- if (is_mmconf_reserved(is_acpi_reserved, cfg, dev, 0))
+ if (is_mmconf_reserved(is_acpi_reserved, cfg, dev,
+ "ACPI motherboard resource"))
return true;

if (dev)
@@ -527,7 +527,8 @@ pci_mmcfg_check_reserved(struct device *dev, struct pci_mmcfg_region *cfg, int e
/* Don't try to do this check unless configuration
type 1 is available. how about type 2 ?*/
if (raw_pci_ops)
- return is_mmconf_reserved(e820__mapped_all, cfg, dev, 1);
+ return is_mmconf_reserved(e820__mapped_all, cfg, dev,
+ "E820 entry");

return false;
}
--
2.25.1

2023-01-10 21:21:26

by Liang, Kan

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: Fix extended config space regression



On 2023-01-10 1:02 p.m., Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") appeared
> in v6.2-rc1 and broke extended config space on several machines.
>
> This broke drivers that use things in extended config space, e.g., perf,
> VSEC telemetry, EDAC, QAT, etc.
>
> This happened because mmconfig-shared.c checks to see that ECAM space is
> reserved in E820 or ACPI motherboard resources. If it's not, it assumes
> ECAM doesn't work. 07eab0901ede removed some E820 entries, so it looked
> like ECAM was no longer reserved, so we stopped using it.
>
> The machines where this broke don't reserve the ECAM in ACPI PNP0C02
> devices (which seems to be what the PCI Firmware spec requires), but they
> do mention it in the EFI memory map as EfiMemoryMappedIO.
>
> Bjorn Helgaas (2):
> x86/pci: Simplify is_mmconf_reserved() messages
> x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space
>

This patch series fixes the issue on my machine.

Tested-by: Kan Liang <[email protected]>

Thanks,
Kan

> arch/x86/pci/mmconfig-shared.c | 44 +++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 6 deletions(-)
>

2023-01-10 23:19:30

by Tony Luck

[permalink] [raw]
Subject: RE: [PATCH 0/2] PCI: Fix extended config space regression

> This broke drivers that use things in extended config space, e.g., perf,
> VSEC telemetry, EDAC, QAT, etc.

For the EDAC on Broadwell case I reported:

Tested-by: Tony Luck <[email protected]>

2023-01-11 12:34:00

by Cabiddu, Giovanni

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: Fix extended config space regression

On Tue, Jan 10, 2023 at 12:02:41PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") appeared
> in v6.2-rc1 and broke extended config space on several machines.
>
> This broke drivers that use things in extended config space, e.g., perf,
> VSEC telemetry, EDAC, QAT, etc.
For QAT:

Tested-by: Giovanni Cabiddu <[email protected]>

--
Giovanni

2023-01-11 23:08:29

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: Fix extended config space regression

On Tue, Jan 10, 2023 at 12:02:41PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") appeared
> in v6.2-rc1 and broke extended config space on several machines.
>
> This broke drivers that use things in extended config space, e.g., perf,
> VSEC telemetry, EDAC, QAT, etc.
>
> This happened because mmconfig-shared.c checks to see that ECAM space is
> reserved in E820 or ACPI motherboard resources. If it's not, it assumes
> ECAM doesn't work. 07eab0901ede removed some E820 entries, so it looked
> like ECAM was no longer reserved, so we stopped using it.
>
> The machines where this broke don't reserve the ECAM in ACPI PNP0C02
> devices (which seems to be what the PCI Firmware spec requires), but they
> do mention it in the EFI memory map as EfiMemoryMappedIO.
>
> Bjorn Helgaas (2):
> x86/pci: Simplify is_mmconf_reserved() messages
> x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space
>
> arch/x86/pci/mmconfig-shared.c | 44 +++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 6 deletions(-)

Applied to for-linus for v6.2. Sorry for the breakage and thank you
very much for the debugging and testing help!

2023-01-12 06:18:49

by Sun, Yunying

[permalink] [raw]
Subject: RE: [PATCH 0/2] PCI: Fix extended config space regression

With this updated patches, perf uncore works fine on SPR(tested on both DNP and MCC).

Tested-by: Yunying Sun <[email protected]>

-----Original Message-----
From: Bjorn Helgaas <[email protected]>
Sent: Wednesday, 11 January, 2023 02:03
To: [email protected]
Cc: Williams, Dan J <[email protected]>; Kan Liang <[email protected]>; Luck, Tony <[email protected]>; Box, David E <[email protected]>; Sun, Yunying <[email protected]>; Jiang, Dave <[email protected]>; Mika Westerberg <[email protected]>; Cabiddu, Giovanni <[email protected]>; Herbert Xu <[email protected]>; Hans de Goede <[email protected]>; Florent DELAHAYE <[email protected]>; Konrad J Hambrick <[email protected]>; Matt Hansen <[email protected]>; Nicholas Johnson <[email protected]>; Benoit Gr?goire <[email protected]>; Werner Sembach <[email protected]>; [email protected]; [email protected]; Bjorn Helgaas <[email protected]>
Subject: [PATCH 0/2] PCI: Fix extended config space regression

From: Bjorn Helgaas <[email protected]>

07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") appeared in v6.2-rc1 and broke extended config space on several machines.

This broke drivers that use things in extended config space, e.g., perf, VSEC telemetry, EDAC, QAT, etc.

This happened because mmconfig-shared.c checks to see that ECAM space is reserved in E820 or ACPI motherboard resources. If it's not, it assumes ECAM doesn't work. 07eab0901ede removed some E820 entries, so it looked like ECAM was no longer reserved, so we stopped using it.

The machines where this broke don't reserve the ECAM in ACPI PNP0C02 devices (which seems to be what the PCI Firmware spec requires), but they do mention it in the EFI memory map as EfiMemoryMappedIO.

Bjorn Helgaas (2):
x86/pci: Simplify is_mmconf_reserved() messages
x86/pci: Treat EfiMemoryMappedIO as reservation of ECAM space

arch/x86/pci/mmconfig-shared.c | 44 +++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)

--
2.25.1