[PATCH] ARM: KVM: iterate over all CPUs for CPU compatibility check

[Andre Przywara]

On 04/19/2013 06:13 PM, Christoffer Dall wrote:
> On Fri, Apr 19, 2013 at 5:58 AM, Andre Przywara
> <andre.przywara at linaro.org> wrote:
>> On 04/17/2013 11:12 AM, Christoffer Dall wrote:
>>>
>>>  ...
>>>
>>> You could also try installing a vector handler early and detect faults,
>>> and add an alternative return path from the init function with some
>>> error reporting value in r0 or something like that, just for debugging,
>>> naturally, but that could be a way to detect if we really are taking
>>> recursive faults here.
>>
>>
>> OK, I added code to return earlier on CPUs not from cluster 0.
>> Indeed it hangs in the HSCR write. The two A15s pass this instruction,
>> writing 0x30c5187F into the register.
>> This means all the fixed bits for A15 correctly, C,A,M and I set and WXN,
>> EE, TE cleared. FI was also cleared
>> The A7 wanted to write the very same value. I tried to set bit 21, which
>> kind of the A7 TRM hints to do: but no change.
>> Before the HSCLTR write, the register reads 0x30c50878, with SCTLR being
>> 0x30c5387d.
>> So the code wants to set M, A, C and I in HSCLTR. Interestingly SCTLR has
>> the V bits set, could that be an issue?
>>
> Can you try writing 0x30c50879 into the register instead? Basically
> check to see if enabling caches or alignment checks causes the issue,
> or if it is indeed enabling the MMU that's the issue... If that works,
> start a bisect on the remaining bits. Also, just for fun, could you
> try flushing the entire I-cache before writing into the HSCLTR?

OK, both clearing the I-bit and doing an "isb; ICIALLU" before the "isb; 
write HSCLTR; isb" worked, the kernel boots on and KVM is enabled.

I could easily make a patch, but I am not sure how to proceed from here:

1.) At least I don't have an understanding why this is only a problem on 
A7 and not on A15. I would feel better if we have an explanation for 
this. Mark, Will, Peter: any ideas?

2.) We still do not claim to support A7s. ARM_CPU_PART_CORTEX_A15 is the 
only MIDR content not returning -EINVAL. And there is no coproc_a7.c 
yet. From browsing through coproc_a15.c I sense it looks compatible to 
the A7, only handling L2CTLR[25:24] and ACTLR[SMP] explicitly. On pure 
A7 systems this should work, but in mixed (aka. b.L) systems guests 
would see inconsistent input depending on the current scheduling. This 
looks like we need a real solution, which needs some time and work 
(read: proper b.L support).

3.) Even if we would understand this fix and take it, the kernel 
currently still randomly enables or disables KVM - with only A15 
supported and the check routine running on one of the cores only.
And even if we add A7 to the list, I think we still need to iterate 
through all cores to catch unsupported cores in a different b.L 
configuration.

I suppose it is to late for 3.9 in any case right? Or should we try to 
push at least the ICIALLU fix?

> Why wouldn't the V bit be set in the SCTLR, Linus uses high vectors
> (at 0xffff0000) for exception handling on ARM.

Shame on me, didn't know this. I assumed that those high vectors are 
there for legacy reasons only. Good to know and thanks for telling me.


Regards,
Andre.