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

[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.

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.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]