Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault.
Going to 128-byte cache lines was fine, but they should have made the lower-power part of the chip match (which in this case they likely couldn't because they licensed that design), or they should have made sure 128-byte lines were reported in all circumstances (which requires a hack similar to the workaround done in the kernel because again they can't make the A53 report 64 bytes).
> Samsung's design breaks correctly written user-land code. Hence, they're 100% at fault.
This is similar to blaming OS/BIOS manufaturers for Y2K breaking "correctly written code" at the turn of the millenium.The code was incorrect in this instance because it assumed the cache size would be the same for all cores, Samsung simply manufactured a SOC that breaks that faulty assumption.
If we forget about the ARM spec explicitly enforcing this anyway, here's some food for thought: how exactly do you suppose the GCC people could write correct code, if they aren't allowed to make that assumption?
The CTR holds minimum line length values for:
- the instruction caches
...this value is the most efficient address stride to use to apply to a sequence of address-based maintenance operations to a range of addresses...
The documentation for the CTR_EL0 reg talks about "caches under this processors control" which you could argue don't include other cores, but if you allow migration between cores, then line size changes underneath running software break the above "most efficient address stride" assumption. So you can't do that.
It boils down to this: If you can't assume that cache line size doesn't change underneath you, then you can't invalidate line by line at all, and would have to go word by word. That's terrible for performance (and a huge waste), which is why the spec says to use the above value for those operations.
I appreciate you digging up the relevant text from the manual, but I don't think you should accuse Samsung of wrongdoing based on such far fetched assumptions.
That document does not explicitly forbids this. In addition, take a look at their sample code which indeed reads some cache configuration registers during each call. The same code is found verbatim in the linux kernel, if you now don't trust the ARM engineers :)
I don't think considering how to actually implement the basic operation without a terrible performance penalty is a "far fetched assumption".
> That document does not explicitly forbids this.
Yet it makes it clear software can be written to assume it doesn't happen, which is the same thing.
> In addition, take a look at their sample code which indeed reads some cache configuration registers during each call.
This doesn't prevent the problem at all, it just reduces the window for things to go wrong.
> The same code is found verbatim in the linux kernel, if you now don't trust the ARM engineers :)
I trust the ARM engineers. As I already said, their code is right, Samsung got it wrong. Note that the libgcc code that breaks was ALSO written by ARM.
Going to 128-byte cache lines was fine, but they should have made the lower-power part of the chip match (which in this case they likely couldn't because they licensed that design), or they should have made sure 128-byte lines were reported in all circumstances (which requires a hack similar to the workaround done in the kernel because again they can't make the A53 report 64 bytes).