Re: platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

[Shinichiro Kawasaki]

On Nov 13, 2024 / 19:34, Hans de Goede wrote:
> Hi,
> 
> On 13-Nov-24 6:41 PM, Daniel Walker (danielwa) wrote:
> > On Wed, Nov 13, 2024 at 06:04:44PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 13-Nov-24 5:33 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 13-Nov-24 5:24 PM, Hans de Goede wrote:
[...]
> >>> It probably has something to do with these 2 messages:
> >>>
> >>> pci 0000:00:1f.1: BAR 0 [mem 0xfd000000-0xfdffffff 64bit]: can't claim; no compatible bridge window
> >>> pci 0000:00:1f.1: BAR 0 [mem 0x280000000-0x280ffffff 64bit]: assigned
> >>>
> >>> I'm guessing that this re-assignment is messing up
> >>> the p2sb BAR caching, after which things go wrong.
> >>
> >> Hmm, but that should be fixed by 2c6370e66076 ("platform/x86: p2sb: Don't init until unassigned resources have been assigned")
> >>
> >> and you are seeing this with 6.12, which has that.
> >>
> >> Can you try adding a pr_info() to the top of p2sb_cache_resources()
> >> with 6.12 and then collec a new dmesg ?
> >>
> >> If that pr_info() is done after the:
> >>
> >> pci 0000:00:1f.1: BAR 0 [mem 0x280000000-0x280ffffff 64bit]: assigned
> >>
> >> message then that does not explain things.
> >>
> > 
> > I haven't testing adding a pr_info() but the messages seem to happen in the same
> > order in both working and non-working cases.
> > 
> > Does that matter?
> 
> The working case does not do the bar caching, we want to know if the
> bar caching in the non working case happens before or after the assignment:
> 
> pci 0000:00:1f.1: BAR 0 [mem 0x280000000-0x280ffffff 64bit]: assigned
> 
> It should happen after the assignment.

Hello Daniel,

It's my sorrow that the change cause this trouble. I have created a debug patch
for the kernel and attached to this e-mail. It adds some pr_info() to answer
the question from Hans. It will also show us a bit more things. Could you try it
on your system? It should apply to v6.12-rcX kernels without conflicts.

diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index 31f38309b389..e9038e8ebc87 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -89,6 +89,12 @@ static void p2sb_scan_and_cache_devfn(struct pci_bus *bus, unsigned int devfn)
 		return;
 
 	p2sb_read_bar0(pdev, &cache->res);
+
+	pr_info("%s: devfn=%x.%x\n", __func__,
+		PCI_SLOT(devfn), PCI_FUNC(devfn));
+	pr_info("%s: %llx-%llx: %lx\n", __func__,
+		cache->res.start, cache->res.end, cache->res.flags);
+
 	cache->bus_dev_id = bus->dev.id;
 
 	pci_stop_and_remove_bus_device(pdev);
@@ -130,6 +136,8 @@ static int p2sb_cache_resources(void)
 	u16 class;
 	int ret;
 
+	pr_info("%s\n", __func__);
+
 	/* Get devfn for P2SB device itself */
 	p2sb_get_devfn(&devfn_p2sb);
 
@@ -157,6 +165,9 @@ static int p2sb_cache_resources(void)
 	 * Unhide the P2SB device here, if needed.
 	 */
 	pci_bus_read_config_dword(bus, devfn_p2sb, P2SBC, &value);
+
+	pr_info("%s: P2SBC_HIDE=%lu\n", __func__, value & P2SBC_HIDE);
+
 	if (value & P2SBC_HIDE)
 		pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0);
 
@@ -189,6 +200,8 @@ int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
 {
 	struct p2sb_res_cache *cache;
 
+	pr_info("%s\n", __func__);
+
 	bus = p2sb_get_bus(bus);
 	if (!bus)
 		return -ENODEV;
@@ -204,6 +217,12 @@ int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
 		return -ENOENT;
 
 	memcpy(mem, &cache->res, sizeof(*mem));
+
+	pr_info("%s: devfn=%x.%x\n", __func__,
+		PCI_SLOT(devfn), PCI_FUNC(devfn));
+	pr_info("%s: %llx-%llx: %lx\n", __func__,
+		cache->res.start, cache->res.end, cache->res.flags);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(p2sb_bar);