Re: [PATCH RESEND v3 2/3] MIPS: Introduce PCI_IO_VMMAP

[Jiaxun Yang <jiaxun.yang@flygoat.com>]

于 2020年5月14日 GMT+08:00 下午10:21:49, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>On Sat, May 09, 2020 at 01:09:07AM +0800, Jiaxun Yang wrote:
>> 
>> 
>> 于 2020年5月9日 GMT+08:00 上午12:52:35, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>> >On Sat, May 09, 2020 at 12:22:25AM +0800, Jiaxun Yang wrote:
>> >> 
>> >> 
>> >> 于 2020年5月9日 GMT+08:00 上午12:11:02, Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写到:
>> >> >On Fri, May 08, 2020 at 07:44:37PM +0800, Jiaxun Yang wrote:
>> >> >> Define PCI_IOBASE for MIPS at the strat of kernel mapping segment.
>> >> >> That would allow virt address of I/O ports to be dynamicly mapped.
>> >> >> So we'll be able to combine multiple MMIO ranges into I/O ports
>> >> >> and thus we can take advantage of logic_pio mechanism.
>> >> >
>> >> >What is the advantage ?
>> >> >
>> >> >From my point of view this will be slower because of TLB faults for
>> >> >PCI IO space accesses.
>> >> 
>> >> Advantage is we can use logic_pio to manage multiple I/O Port ranges.
>> >
>> >and what exactly does this buy us ? I looked at lib/logic_pio.c and
>> >there didn't appear anything in my mind other than yet another
>> >interface for doing the same thing...
>> 
>> With Logic_PIO subsystem, each I/O region will be registered
>> in logic_pio and being mapped in TLB, that means I/O regions can l
>> be sparsely layouted in MMIO.
>
>this isn't an advantage. You will get TLB miss exceptions, which you
>don't get when using XPHYS addresses, which the current code does.
>IMHO it you make more sense to use normal iomap/iounmap functions
>in logic_pio.c. With that you would get TLB less access for memory
>mapped PCI IO regions.
>
>> And device drivers are accessing in*/out* functions via ioport allocated by
>> logic_pio instead of the physical I/O ports.
>
>your current patches don't select INDIRECT_PIO, so you are still
>using the MIPS access methods just via the mapping from logic_pio. This
>works as long as you are using already mapped ranges. This is at least
>inconsistent.

Not exactly. I'm emulating the behaviour of Aarch64's PCI ioport handling.

And the mapping will done by pci_remap_iospace when adding resources
to PCI bridge.
PCI subsystem will ensure all the ioports they used is mapped so there won't be any inconsistent.
Except some legacy driver need to be converted.

Yeah, I was confused by logic_pio at very start and holded the same opinion
with you that it's totally misuse of MMIO ioports until John Garry
kindly explained [1] to me what it exactly did.

>
>If you really want to ignore the TLB miss problem, please drop
>patch 1 and 2 and use something like
>
>#define PCI_IOBASE	_AC(0xc000000000000000, UL)
>#define PCI_IOSIZE	SZ_16SM
>#define MAP_BASE	(PCI_IOBASE + PCI_IOSIZE)
>
>in arch/mips/include/asm/mach-loongson64/spaces.h
>
>That should do the trick without moving stuff around and it will
>keep it out of the normal MIPS stuff.

I do believe that similar to Aarch64 world, device with multiple PCIe
host bridge and sparsely layouted MMIO IOport range will appear very soon.

So I'm creating a general method for MIPS.
If that's inappropriate for you
I'll make it as Loongson only.

Thanks.

>
>Thomas.
>
[1]: https://lkml.org/lkml/2020/2/20/1207
-- 
Jiaxun Yang