[RFC] arm : ALIGNMENT_TRAP and MTD

[RFC] arm : ALIGNMENT_TRAP and MTD

[Hyok S. Choi]

Dear MTD folks,

In ARM arch, we're merging my uClinux/ARM tree into mainline.
While merging a certain CPU support codes, we found a bit doubt on the
conditional including of MTD configuration file in arch/arm/Kconfig.
The CPU core has no H/W based alignment mechanism, thus can not handle unaligned access.
However as many uClinux archs do, many of the platforms had supported MTD drivers.

Currently, _only_ ARM arch includes MTD configuration file conditionally as follows:

if ALIGNMENT_TRAP
source "drivers/mtd/Kconfig"
endif

All the other archs includes the config w/o any condition.

To support the CPU core which has no H/W alignment,
the condition should be changed to somewhat strange :

if ALIGNMENT_TRAP || CPU_HAS_NO_CP15
source "drivers/mtd/Kconfig"
endif

Currently only one platform (ARCH_EBSA110) is the out of the condition
of above if condition.

What would be the best condition? or just no condition?
I think just including w/o any condition could be clear. :-)

On Wednesday 21 June 2006 20:32, Hyok S. Choi wrote:
> On Wednesday 21 June 2006 19:42, Uwe Zeisberger wrote:
> > >   However, the mtd drivers has been used for those CPUs.
> > >   the drivers/mtd/Kconfig could be included if CPU_HAS_NO_CP15 in
> > >   arch/arm/Kconfig.
> > > 
> > >   if ALIGNMENT_TRAP || CPU_HAS_NO_CP15
> > >   source "drivers/mtd/Kconfig"
> > >   endif
> > I disbelieve this is the right fix.  Either the mtd drivers have
> > alignment problems or not.  So in the first case it's not save if you
> > have no alignment trap, be it on a processor with or without cp15.  In
> > the latter case the complete if ALIGNMENT_TRAP should be removed.
>   See below.
> 
> > >   Anyway, is this really needed to be blocked if ALIGNMENT_TRAP is not
> > >   enabled?
> > There are (or may be) some drivers, that don't pay attention to only
> > make aligned accesses.  I'd prefer if the individual drivers would be
> > marked and not all mtd drivers together.  For me the AMDSTD driver seems
> > to work.
> I agree. That's why I commented as above. In general, every other
> architectures including uClinux archs just includes mtd configuration
> w/o any condition.
> Currently the ALIGNMENT_TRAP is disabled only for ARCH_EBSA110.

-- 
Hyok
ARM Linux 2.6 MPU/noMMU Project http://opensrc.sec.samsung.com/

Re: [RFC] arm : ALIGNMENT_TRAP and MTD

[Thomas Gleixner]

On Fri, 2006-06-23 at 16:35 +0900, Hyok S. Choi wrote:
> Dear MTD folks,
> 
> In ARM arch, we're merging my uClinux/ARM tree into mainline.
> While merging a certain CPU support codes, we found a bit doubt on the
> conditional including of MTD configuration file in arch/arm/Kconfig.
> The CPU core has no H/W based alignment mechanism, thus can not handle unaligned access.
> However as many uClinux archs do, many of the platforms had supported MTD drivers.
> 
> Currently, _only_ ARM arch includes MTD configuration file conditionally as follows:
> 
> if ALIGNMENT_TRAP
> source "drivers/mtd/Kconfig"
> endif

MTD does unaligned accesses. This dependency is there since the early
2.4 kernels.

I really do not want to meddle in the ARM dependency problems. Just my
$0.02: removing this will simply open another round of bug reports
related to this problem. Search the MTD archives, we had plenty of them.

Is there a damn good reason, why the NOMMMU stuff cannot have this
active ? If it enables code which is not feasible on NOMMU there are
ways to solve such problems quite elegant.

It's not a MTD problem, it's an ARM specific requirement.

	tglx

Re: [RFC] arm : ALIGNMENT_TRAP and MTD

[David Woodhouse]

On Fri, 2006-06-23 at 21:22 +0200, Thomas Gleixner wrote:
> 
> Is there a damn good reason, why the NOMMMU stuff cannot have this
> active ? If it enables code which is not feasible on NOMMU there are
> ways to solve such problems quite elegant.
> 
> It's not a MTD problem, it's an ARM specific requirement.

No, it isn't. ARM was just one of the first CPUs where we really _can't_
fix up alignment faults. FR-V is another. It's often done in the MMU, so
there's a strong correlation with uCLinux architectures -- and now we've
merged uCLinux, it's a problem we should really address properly.

-- 
dwmw2

Re: [RFC] arm : ALIGNMENT_TRAP and MTD

[Hyok S. Choi]

On Tuesday 27 June 2006 22:30, David Woodhouse wrote:
> On Fri, 2006-06-23 at 21:22 +0200, Thomas Gleixner wrote:
> > 
> > Is there a damn good reason, why the NOMMMU stuff cannot have this
> > active ? If it enables code which is not feasible on NOMMU there are
> > ways to solve such problems quite elegant.
> > 
> > It's not a MTD problem, it's an ARM specific requirement.
> 
> No, it isn't. ARM was just one of the first CPUs where we really _can't_
> fix up alignment faults. FR-V is another. It's often done in the MMU, so
> there's a strong correlation with uCLinux architectures -- and now we've
> merged uCLinux, it's a problem we should really address properly.

I agree on this. I'm doing the uClinux merging for ARM architecture.

How about to make an option for enabling accessing through the get_unaligned() in MTD?
Then ARM uClinux and FR-V could select/deselect the option and will have no problem.

Hyok

-- 
Hyok
ARM Linux 2.6 MPU/noMMU Project http://opensrc.sec.samsung.com/

Re: [RFC] arm : ALIGNMENT_TRAP and MTD

[David Woodhouse]

On Wed, 2006-06-28 at 14:04 +0900, Hyok S. Choi wrote:
> 
> > No, it isn't. ARM was just one of the first CPUs where we really _can't_
> > fix up alignment faults. FR-V is another. It's often done in the MMU, so
> > there's a strong correlation with uCLinux architectures -- and now we've
> > merged uCLinux, it's a problem we should really address properly.
> 
> I agree on this. I'm doing the uClinux merging for ARM architecture.
> 
> How about to make an option for enabling accessing through the get_unaligned() in MTD?
> Then ARM uClinux and FR-V could select/deselect the option and will have no problem.

I don't like that. I'd rather see you add a probability argument to
get_unaligned() instead, as I described in my other email.

-- 
dwmw2

Re: [RFC] arm : ALIGNMENT_TRAP and MTD

[David Woodhouse]

On Fri, 2006-06-23 at 16:35 +0900, Hyok S. Choi wrote:
> Dear MTD folks,
> 
> In ARM arch, we're merging my uClinux/ARM tree into mainline.
> While merging a certain CPU support codes, we found a bit doubt on the
> conditional including of MTD configuration file in arch/arm/Kconfig.
> The CPU core has no H/W based alignment mechanism, thus can not handle unaligned access.
> However as many uClinux archs do, many of the platforms had supported MTD drivers.
> 
> Currently, _only_ ARM arch includes MTD configuration file conditionally as follows:
> 
> if ALIGNMENT_TRAP
> source "drivers/mtd/Kconfig"
> endif

Historically, ARM was the only architecture which attempted to run Linux
without unaligned accesses "just working" -- either natively in the CPU
or through fixups. This situation was considered 'broken'. 

The problem manifests itself in the NOR chip drivers, but only if the
bus width is greater than 1 byte, and if a user of the MTD device writes
from a buffer which is not aligned in memory to that bus width.

JFFS2 does this, when garbage-collecting dirent nodes. Since the name
field is just an array of characters, it's stored in memory in the
'struct jffs2_full_dirent' without alignment constraints -- and it's
always an odd address because it comes after the single char used for
the 'type' field.

I did once add a couple of 'get_unaligned()' calls to the places in the
NOR flash chip drivers which matter, but Alan objected -- because
get_unaligned() should be reserved for the case where the datum is
_expected_ to be unaligned, not where it simply _might_ be unaligned.

These days, we support a number of architectures which just _cannot_ fix
up alignment errors. On FR-V without an MMU, for example, the alignment
trap is an imprecise exception. The networking code now works correctly
in that situation too, I believe -- I think the correct fix would be for
me to put back the get_unaligned() in a few places which I originally
added them, but then removed them again when Alan objected. 

However, the objection that get_unaligned() should be used only when the
datum is _expected_ to be unaligned is still a valid one. It'll lead to
suboptimal code on architectures where the CPU doesn't natively handle
misaligned loads, when the misaligned case is _not_ the common case.

I think what we should actually do, if we're going to accept that Linux
runs without alignment fixups, is introduce a new get_maybe_unaligned()
call which should be used if there is a low, but non-zero, probability
of the address being unaligned. It would still take the
dereference-and-fault path on most architectures, but those
architectures which _really_ cannot deal with an alignment trap can emit
the slow-path instead.

Actually, I think I'd rather see just a second argument to
get_unaligned() which gives an estimate of the probability that the
address is unaligned. Each architecture can make its own decision about
where the threshold should be between just dereferencing the pointer and
maybe taking a trap, and emitting the slow code to load it manually. It
depends on the relative costs of each, on any given architecture.

-- 
dwmw2