Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Mario Limonciello <mario.limonciello@amd.com>]

+Boris

Please include him on the next version too.

On 10/2/26 06:33, Ilpo Järvinen wrote:
> While testing the resource placement changes, tests hit a case where
> igb fails to probe when BAR 0 is placed at 0x9c000000:
> 
>     90000000-9cffffff : PCI Bus 0000:a0
>    -  90000000-902fffff : PCI Bus 0000:a1
>    -    90000000-900fffff : 0000:a1:00.0
>    -      90000000-900fffff : igb
>    -    90100000-901fffff : 0000:a1:00.0
>    -    90200000-90203fff : 0000:a1:00.0
>    -      90200000-90203fff : igb
>    +  9be00000-9c0fffff : PCI Bus 0000:a1
>    +    9be00000-9befffff : 0000:a1:00.0
>    +    9bf00000-9bf03fff : 0000:a1:00.0
>    +    9c000000-9c0fffff : 0000:a1:00.0
>       9c100000-9c17ffff : amd_iommu
>       9c180000-9c1803ff : IOAPIC 8
> 
>    -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
>    -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
>    -       Expansion ROM at 90100000 [disabled] [size=1M]
>    +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
>    +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
>    +       Expansion ROM at 9be00000 [disabled] [size=1M]
> 
> igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
> ...
>   igb_get_invariants_82575+0xff/0xf00 [igb]
>   igb_probe+0x3c8/0x1190 [igb]
>   local_pci_probe+0x3b/0x80
> 
> It turns out there is a 64kB iomem black hole at 9c000000 that returns
> ~0 and this is where igb's BAR 0 resides. If another BAR of the same
> card is placed into that address, it is similarly black holed.
> 
> Mark the problematic 64kB range reserved using a quirk bound to the
> bridge found in the problematic system.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> Mario suggested the quirk to be based on the bridge instead of the
> endpoint device which certainly looks better and cleaner than the
> approach used in v1.
> 
> The current plan is to try a different card in the same slot but it is
> a bit hard for me to predictable on what timescale that can be done.
> 

FWIW - I do think this approach is pragmatic for the given situation.

> ---
>   drivers/pci/quirks.c | 35 +++++++++++++++++++++++++++++++++++

I think a better location will be arch/x86/pci/fixup.c as these quirks 
will only matter for x86.

>   1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21f..5483b47d8d54 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6288,6 +6288,41 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
>   
> +/*
> + * TODO: Remove when/if root cause is found.
> + *
> + * Genoa appears to have a 64kB iomem black hole starting at 0x9c000000
> + * address for which all reads return ~0 blocking an overlapping BAR from
> + * working.
> + *
> + * Work around the problem by reserving the space prior to making any iomem
> + * allocations that could overlap with the black hole.
> + */
> +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> +							     "reserved");

I'd give it more detail for anyone looking at reservations to get a hint 
back to this function/commit.  Maybe "genoa_black_hole"?

> +
> +static void genoa_iomem_black_hole(struct pci_dev *dev)
> +{
> +	struct resource *r;
> +
> +	pci_bus_for_each_resource(dev->bus, r) {
> +		if (!r || !r->flags || !resource_assigned(r))
> +			continue;
> +
> +		if (!__resource_contains_unbound(r, &black_hole_res))
> +			continue;
> +
> +		if (resource_assigned(&black_hole_res))
> +			pci_dbg(dev, "iomem black hole workaround already applied\n");
> +		else if (!insert_resource(r, &black_hole_res))
> +			pci_info(dev, "iomem black hole workaround enabled\n");
> +		else
> +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> +		return;
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
> +
>   #ifdef CONFIG_PCIEASPM
>   /*
>    * Several Intel DG2 graphics devices advertise that they can only tolerate