Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6328 bytes --]

On Tue, 14 Oct 2025, Maciej W. Rozycki wrote:

> On Mon, 13 Oct 2025, Thomas Bogendoerfer wrote:
> 
> > > This patch causes boot failures when trying to boot mips images from
> > > ide drive in qemu. As far as I can see the interface no longer instantiates.
> > > 
> > > Reverting this patch fixes the problem. Bisect log attached for reference.
> > 
> > Patch below fixes my qemu malta setup. Now I'm wondering, why this is
> > needed. It was added with commit
> > 
> > aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")
> > 
> > Maciej, do you remember why this is needed ?
> 
>  I do.  The reason is preventing PCI port I/O mappings below 0x100, which 
> interferes badly with how the PIIX4 decodes port I/O cycles.  That did 
> happen in the field, wreaking havoc and prompting my change.
> 
>  By the look of the code it would definitely trigger for the Bonito64 
> system controller, which has a fixed port I/O target address range and, 
> depending on the settings left by the firmware, it might also trigger for 
> the Galileo GT64120A and SOC-it 101 system controllers, which have 
> variable port I/O target address ranges.
> 
>  Here's an example map of Malta port I/O resources (SOC-it 101 variant):
> 
> 00000000-0000001f : dma1
> 00000020-00000021 : pic1
> 00000040-0000005f : timer
> 00000060-0000006f : keyboard
> 00000070-00000077 : rtc0
> 00000080-0000008f : dma page reg
> 000000a0-000000a1 : pic2
> 000000c0-000000df : dma2
> 00000170-00000177 : ata_piix
> 000001f0-000001f7 : ata_piix
> 000002f8-000002ff : serial
> 00000376-00000376 : ata_piix
> 00000378-0000037a : parport0
> 0000037b-0000037f : parport0
> 000003f6-000003f6 : ata_piix
> 000003f8-000003ff : serial
> 00001000-00ffffff : MSC PCI I/O
>   00001000-0000103f : 0000:00:0a.3
>   00001040-0000105f : 0000:00:0a.2
>     00001040-0000105f : uhci_hcd
>   00001060-0000107f : 0000:00:0b.0
>     00001060-0000107f : pcnet32_probe_pci
>   00001080-000010ff : 0000:00:12.0
>     00001080-000010ff : defxx
>   00001100-0000110f : 0000:00:0a.3
>   00001400-000014ff : 0000:00:13.0
>   00001800-0000180f : 0000:00:0a.1
>     00001800-0000180f : ata_piix
> 
> As you can see there are holes in the map below 0x100, so e.g. if the bus 
> master IDE I/O space registers (claimed last in the list by `ata_piix') 
> were assigned to 00000030-0000003f, then all hell would break loose.  It 
> is exactly the mapping that happened in the absence of the code piece in 
> question IIRC.
> 
>  The choice of 0x1000 as the lower boundary IIRC has something to do with 
> alignment; I think the decoding base has to be a multiple of 0x1000 and 
> given that the ACPI resource is decoded by a non-standard BAR at 0x40 in 
> the configuration space (set up by `malta_piix_func3_base_fixup' BTW) we 
> just need to match its setting.
> 
>  Can you please check what the port I/O map looks like with your setup 
> with and without your patch applied?
> 
>  NB there is still something fishy with the setup of SOC-it 101's PCI 
> decoding windows, which is why I have forced `defxx' with a patch to use 
> port I/O, as reported above.  The driver uses MMIO unconditionally on PCI 
> systems nowadays, but using MMIO prevents it from working with the SOC-it 
> 101 system controller and I yet need to debug it.  Conversely MMIO used to 
> work just fine with the Galileo GT64120A system controller while I still 
> had one operational.

Are you sure pci-malta.c has to do anything like this as 
pcibios_align_resource() does lower bound IO resource start addresses if 
PCIBIOS_MIN_IO is set?

How about this patch below?

(I'm not sure if it should actually be
	PCIBIOS_MIN_IO = 0x1000 - hose->io_resource->start;
to allow resources starting from 0x1000 if ->start is not at 0.)

--
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 14 Oct 2025 13:47:49 +0300
Subject: [PATCH 1/1] MIPS: Malta: Use pcibios_align_resource() to block io range
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

According to Maciej W. Rozycki <macro@orcam.me.uk>, the
mips_pcibios_init() for malta adjusts root bus IO resource start
address to prevent interfering with PIIX4 I/O cycle decoding. Adjusting
lower bound leaves PIIX4 IO resources outside of the root bus resource
and assign_fixed_resource_on_bus() does not put the resources into the
resource tree.

Prior to commit ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")
the arch specific pcibios_enable_resources() did not check if the
resources were assigned which diverges from what PCI core checks,
effectively hiding the PIIX4 IO resources were not properly within the
resource tree. After starting to use pcibios_enable_resources() from
PCI core, enabling PIIX4 fails:

ata_piix 0000:00:0a.1: BAR 0 [io  0x01f0-0x01f7]: not claimed; can't enable device
ata_piix 0000:00:0a.1: probe with driver ata_piix failed with error -22

MIPS PCI code already has support for enforcing lower bounds using
PCIBIOS_MIN_IO in pcibios_align_resource(). Make malta PCI code too to
use PCIBIOS_MIN_IO.

Fixes: ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")
Fixes: aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")
Link: https://lore.kernel.org/linux-pci/9085ab12-1559-4462-9b18-f03dcb9a4088@roeck-us.net/
Link: https://lore.kernel.org/linux-pci/alpine.DEB.2.21.2510132229120.39634@angie.orcam.me.uk/
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/mips/pci/pci-malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/pci/pci-malta.c b/arch/mips/pci/pci-malta.c
index 6aefdf20ca05..f4ea1c99852f 100644
--- a/arch/mips/pci/pci-malta.c
+++ b/arch/mips/pci/pci-malta.c
@@ -231,7 +231,7 @@ void __init mips_pcibios_init(void)
 
 	/* PIIX4 ACPI starts at 0x1000 */
 	if (controller->io_resource->start < 0x00001000UL)
-		controller->io_resource->start = 0x00001000UL;
+		PCIBIOS_MIN_IO = 0x1000;
 
 	iomem_resource.end &= 0xfffffffffULL;			/* 64 GB */
 	ioport_resource.end = controller->io_resource->end;

base-commit: 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
-- 
2.39.5