Re: Big Fix for 2.2.1

Petr Vandrovec Ing. VTEI (VANDROVE@vc.cvut.cz)
Tue, 26 Jan 1999 22:46:39 MET-1

Hello Leonard,
>On Tue, 26 Jan 1999, Leonard N. Zubkoff wrote:
>> = Linus Torvalds
>>> = Leonard N. Zubkoff
>>> (1) The identity phys_to_virt(virt_to_phys(x)) == x is violated.
>> Not for the default setup. Guess why the default setup is the default
>> setup?
> If the default setup is only correct for PAGE_OFFSET == 0xC0000000, then why
Also 0x80000000, 0xE0000000, 0xF0000000.... There is not space for
improvements on 4GB machines, but there is really room for improvements
on 64MB machines.
> did you permit the comment in page.h telling precisely how to change it to a
> value that violated this assumption?
>> We also currently accept things like
>> readb(0xA0000);
>> ie the "legacy ISA region" is currently (for backwards compatibility
>> reasons) not required to be io-remapped.
We should disallow it when vremap() was invented. But after battle, everyone
is general (marshal?).
>> Despite the fact that it happens to work with the current PAGE_OFFSET, it still
>> feels conceptually wrong that one takes the result of ioremap, a kernel virtual
>> address, and then applies __io_virt to it. It may be clever that __io_virt
>> works for both the legacy ISA I/O and ioremap cases with the default
>> PAGE_OFFSET, but it's certainly confusing combined with the comments about
>> changing PAGE_OFFSET. It wouldn't surprise me if other driver authors spend
>> time researching bug reports when people change PAGE_OFFSET and the driver
>> breaks.
> changes. The only other value I know of that works correctly is 0x80000000.
I was thinking about it when writting matroxfb, but after I found what
is hidding in other architectures asm-*/io.h, it was (by me and others)
considered as 2.3 thing...

But only for your info, I'm running kernels patched by Jakub Jelinek's
btfixup patch and my change which adds

extern inline void * __io_virt2(void *addr) {
if (addr < (void*)PAGE_OFFSET) {
printk("Erm, access to %p at %p\n", addr, __builtin_return_address(0));
(int)addr += PAGE_OFFSET;
}
return addr;
}

and calls __io_virt2() instead of __io_virt() in read[bwl]/write[bwl] and
I find only one thing, access to 'EISA', as pointed in your patch too.
It is on my 'standard' machine with aic7870, sb, mad16, bttv, tulip, vgacon
(not so much used), matroxfb and atyfb. So these drivers conforms to 2.3
ioremap, I think :-) (at least on non-Alpha, they use ioremap(), but I did
not check whether they are using read[bwl] to access ioremap()ped memory).
Best regards,
Petr Vandrovec
vandrove@vc.cvut.cz

P.S.: If someone is interested in my detection code, patch for 2.2.0-pre9
is at ftp://platan.vc.cvut.cz/pub/linux/iovirt2.
P.P.S.: Parameter for __io_virt2 should be unsigned long int instead of
void*, if you want really right solution.
P.P.P.S.: Also, %eip is better than __builtin_return_address(0) for
inlined function.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/