On Tuesday, September 06, 2011 01:43:33 PM Albert ARIBAUD wrote:
(splitting this discussion between the patch question and longer term RFC, here I follow the RFC and update the subject line accordingly)
Seems like we're having two problems there:
- at least some cpus or cores (xscale) do not implement all of what was
thought to be armv5 cache operations, which suggests we should indeed keep this patch in arm926ejs;
- that many cpus and cores implement a good subset of armv5 cache
operations, which warrants having an armv5 directory where we would define them once only for all to use.
I think the Right Way is to create an armv5 subtree with armv5-common operations, but with the capability of overruling these with cpu-level variants, themselves being possibly overridden by core-level variants.
All right, but what about the CPUs that don't adapt so fast?
Adapt to what?
Adapt to this new cache stuff. Some CPUs have no cache functions implemented at all and therefore with the new stuff, they will use the generic ones -- which might be not good.
There will be CPUs with broken cache-ops. How do you intend to solve that ?
If a cpu has partly broken cache ops, but its cache is still useable by not using the armv5-stock cache ops implementation, then the cpu should override the broken armv5 ops with its own version.
For instance, say some armv5 based cpu badly implements full flush but correctly implements cache line flush. The cpu code should then provide its own version of the full cache flush op, based on looping through the cache lines instead of the armv5 single coprocessor instruction.
If, OTOH, a cpu has broken cache ops, to the point that cache is unuseable, then it should have its cache disabled, which can be done through configuration options or by providing cpu-specific cache ops implementations that will emit appropriate messages.
Ack, agree.
Now as to where to put this:
As ARMv5 is an ISA, not a cpu or core, I'm uncomfortable with creating arch/arm/cpu/armv5 anyway. I'd prefer an ISA subtree, either in parallel with the cpu subtree: arch/arm/isa [/armv5], or, and I would slightly prefer that even though some paths would become rather long, a hierarchy withe cpus stand below their isa: arch/arm/isa [/armv5/cpu/arm926ejs]. To reduce path length, we could remove the 'cpu' part and consider that anything below the 'isa' is a cpu, just like currently anything below the cpu is a core.
This is completely out of scope for this patch. My proposal would be to merge this, then start mucking with this moving files around.
Indeed -- I was not suggestion a rework of the patch, only sending out an RFC of sorts.
To sum it up, we would have
arch/arm/isa/armv5 (where ARMv5t ISA common code would reside, including cache ops)
arch/arm/isa/armv5/arm926ejs (where ARM926EJ-S cpu common code would reside, including cache ops)
arch/arm/isa/armv5/arm926ejs/orion5x (a personal favorite :) where Orion5x core code would reside, including cache ops)
Maybe we could even make do without the .../isa/... level and put ISAs directly under ARM -- I don't think any ARM ISA will ever be named 'include' or 'lib' or 'Makefile'. :)
Comments?
I'll have to think about it,
By all means do. :)
Amicalement,
Cheers