Re: Big Fix for 2.2.1

Linus Torvalds (torvalds@transmeta.com)
Tue, 26 Jan 1999 10:31:51 -0800 (PST)


On Tue, 26 Jan 1999, Leonard N. Zubkoff wrote:
>
> (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?

> (2) The readb/writeb/readw/writew/readl/writel macros are completely incorrect
> since they are supposed to be applied to memory mapped I/O regions
> allocated with ioremap, but ioremap returns a kernel virtual address
> and hence no further mapping is appropriate.

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.

Look at various network drivers for example - your patches probably break
a noticeable number of them.

> The BusLogic driver ran into (1) and the DAC960 driver ran into (2). With the
> patch enclosed below, both drivers work fine when PAGE_OFFSET is changed.

With the patch enclosed below, tons of random device drivers may break.
Not something to be done lightly.

> The
> change to traps.c is also necessary as readl was incorrect usage, even though
> the modified code is now uglier.

The modified code in your patch is completely wrong, and is MUCH more
incorrect than the old one.

The old one made the assumption that the legacy ISA region doesn't need to
be IO-remapped and can be directly accessed with read[bwl]/write[bwl] -
which is a painful assumption, but it's a legacy one, and breaking that
assumption means that _tons_ of drivers will have to be checked.

The new code makes the assumption that the legacy ISA region is mapped in
the kernel virtual address space, and that phys_to_virt() has any meaning
for IO addresses. That happens to be the case right now, but it's a
completely invalid assumption that does not work on many other
architectures.

So the old code was at least technically correct. The new one isn't.

In short, I will accept this kind of patch for 2.3 (fixed up properly),
but I hope you see why I would never consider it for 2.2.x.

The _proper_ fixup is to make sure that every access to IO memory is
preceded by a "ioremap()" to make sure the address is mapped. The
"phys_to_virt()" function has absolutely nothing at all to do with IO
memory, and anything that uses it is buggy.

NOW do you understand why I use binary and/or and force the power-of-two
rule?

Linus

-
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/