在2025年1月14日一月 下午6:42，Mateusz Jończyk写道： [...]

Hello,

Thank you for this patch, I'm testing it right now.

Shouldn't this be #ifndef-ed CONFIG_MACH_LOONGSON64 and perhaps CONFIG_RALINK?

Hi Mateusz,

Those platforms are not using PCI_DRIVERS_LEGACY, so won't be affected by this patch.

PCI_DRIVERS_GENERIC systems are handling logic_pio properly, so no need for this workaround.

Thanks

Loongson64 explicitly calls logic_pio_register_range(), so seems not to need this. Ralink also defined PCI_IOBASE a long time ago.

Greetings,

Mateusz

On Tue, Jan 14, 2025, at 20:20, Jiaxun Yang wrote:

在2025年1月14日一月 下午7:03，Arnd Bergmann写道：

...

On Tue, Jan 14, 2025, at 19:11, Jiaxun Yang wrote:

...

+unsigned long pci_address_to_pio(phys_addr_t address) +{

• if (address > IO_SPACE_LIMIT)

• return (unsigned long)-1;
  

• return (unsigned long) address;

+}

/*

Isn't the argument to this function a CPU physical address? I don't think there is a point comparing it to IO_SPACE_LIMIT on architectures where I/O space is memory mapped.

Actually not. It seems like the argument here is just raw PIO offset, without applying mips_io_port_base.

We should validate it to ensure it's within the range specified by mips_io_port_base (which is sized by IO_SPACE_LIMIT).

I don't know what you mean with "raw PIO offset", but I don't think that's how it's supopsed to be used. Here is how this gets called in of_pci_range_to_resource()

if (res->flags & IORESOURCE_IO) { unsigned long port; err = pci_register_io_range(&np->fwnode, range->cpu_addr, range->size); if (err) goto invalid_range; port = pci_address_to_pio(range->cpu_addr); if (port == (unsigned long)-1) { err = -EINVAL; goto invalid_range; } start = port; }

Where "range->cpu_addr" is the phys_addr_t location of the MMIO window that maps the host controllers port ranges, i.e. the fully translated address from the "ranges" property.

The point of the pci_address_to_pio() function is to convert this into the Linux-internal virtual port number that gets used as an argument to inb()/outb() and reported in /proc/ioports and that may or may not be the same as the address on the bus itself, depending on the how the translation gets set up.

On loongson, we seem to have two port ranges that are set up like

isa@18000000 { compatible = "isa"; ranges = <1 0x0 0x0 0x18000000 0x4000>; };

pci@1a000000 { ranges = <0x01000000 0x0 0x00020000 0x0 0x18020000 0x0 0x00020000>, <0x02000000 0x0 0x40000000 0x0 0x40000000 0x0 0x40000000>; }

Here, the cpu_addr is 0x18000000 for the isa bus and 0x18020000 for the PCI bus, apparently the intention being that these are consecutive in physical space, though Linux is free to rearrange the logical port numbers in a different way, e.g. to ensure that the PCI bus can use port numbers below 0x10000.

On Malta, I see a very strange

isa { compatible = "isa"; ranges = <1 0 0 0x1000>; };

which maps the first 4096 port numbers into cpu_addr=0x0. The actual port window appears to be at a board specific location

#define MALTA_GT_PORT_BASE get_gt_port_base(GT_PCI0IOLD_OFS) #define MALTA_BONITO_PORT_BASE ((unsigned long)ioremap (0x1fd00000, 0x10000)) #define MALTA_MSC_PORT_BASE get_msc_port_base(MSC01_PCI_SC2PIOBASL)

So e.g. on Bonito, the ranges property would have to be

ranges = <1 0 0x1fd00000 0x1000>;

Not sure if this is patched in by the bootloader, or where the corresponding window for PCI gets defined, but I suspect that the reason for the regression is that the caller of pci_address_to_pio() accidentally passed in '0' instead of the physical address, and it happened to work because of the missing PCI_IOBASE definition but broke after that got defined.

Arnd

在2025年1月15日一月 上午7:38，Arnd Bergmann写道： [...]

Where "range->cpu_addr" is the phys_addr_t location of the MMIO window that maps the host controllers port ranges, i.e. the fully translated address from the "ranges" property.

This is valid for MIPS's PCI_DRIVERS_GENERIC, but not for PCI_DRIVERS_LEGACY, which have it's own implementation of various functions. See arch/mips/pci/pci-legacy.c, pci_load_of_ranges. The address translation is handled by io_map_base, which usually comes from mips_io_port_base.

The point of the pci_address_to_pio() function is to convert this into the Linux-internal virtual port number that gets used as an argument to inb()/outb() and reported in /proc/ioports and that may or may not be the same as the address on the bus itself, depending on the how the translation gets set up.

On loongson, we seem to have two port ranges that are set up like

            isa@18000000 {
                    compatible = "isa";
                    ranges = <1 0x0 0x0 0x18000000 0x4000>;
            };

            pci@1a000000 {
                    ranges = <0x01000000 0x0 0x00020000 0x0 

0x18020000 0x0 0x00020000>, <0x02000000 0x0 0x40000000 0x0 0x40000000 0x0 0x40000000>; }

Here, the cpu_addr is 0x18000000 for the isa bus and 0x18020000 for the PCI bus, apparently the intention being that these are consecutive in physical space, though Linux is free to rearrange the logical port numbers in a different way, e.g. to ensure that the PCI bus can use port numbers below 0x10000.

On Malta, I see a very strange

    isa {
            compatible = "isa";
            ranges = <1 0 0 0x1000>;
    };

which maps the first 4096 port numbers into cpu_addr=0x0. The actual port window appears to be at a board specific location

#define MALTA_GT_PORT_BASE get_gt_port_base(GT_PCI0IOLD_OFS) #define MALTA_BONITO_PORT_BASE ((unsigned long)ioremap (0x1fd00000, 0x10000)) #define MALTA_MSC_PORT_BASE get_msc_port_base(MSC01_PCI_SC2PIOBASL)

So e.g. on Bonito, the ranges property would have to be

  ranges = <1 0 0x1fd00000 0x1000>;

Not sure if this is patched in by the bootloader, or where the corresponding window for PCI gets defined, but I suspect that the reason for the regression is that the caller of pci_address_to_pio() accidentally passed in '0' instead of the physical address, and it happened to work because of the missing PCI_IOBASE definition but broke after that got defined.

Many other pci-legacy drivers are also working this way, PCI core is not aware of virtual port to physical MMIO address mapping. PCI core just see it as x86 style port.

Thanks

   Arnd