[PATCH] arm64: head.S: Fix CNTHCTL_EL2 access on VHE system
Jintack Lim
jintack at cs.columbia.edu
Mon Nov 28 18:12:17 PST 2016
On Mon, Nov 28, 2016 at 11:56 AM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> On 28/11/16 16:43, Jintack Lim wrote:
>> From: Jintack <jintack at cs.columbia.edu>
>>
>> Bit positions of CNTHCTL_EL2 are changing depending on HCR_EL2.E2H bit.
>> EL1PCEN and EL1PCTEN are 1st and 0th bits when E2H is not set, but they
>> are 11th and 10th bits respectively when E2H is set. Current code is
>> unintentionally setting wrong bits to CNTHCTL_EL2 with E2H set.
>>
>> In fact, we don't need to set those two bits, which allow EL1 and EL0 to
>> access physical timer and counter respectively, if E2H and TGE are set
>> for the host kernel. They will be configured later as necessary. First,
>> we don't need to configure those bits for EL1, since the host kernel
>> runs in EL2. It is a hypervisor's responsibility to configure them
>> before entering a VM, which runs in EL0 and EL1. Second, EL0 accesses
>> are configured in the later stage of boot process.
>>
>> Signed-off-by: Jintack Lim <jintack at cs.columbia.edu>
>> ---
>> arch/arm64/kernel/head.S | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 332e331..bc3d2db 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -524,10 +524,16 @@ set_hcr:
>> msr hcr_el2, x0
>> isb
>>
>> - /* Generic timers. */
>> + /*
>> + * Allow Non-secure EL1 and EL0 to access physical timer and counter.
>> + * This is not necessary for VHE, since the host kernel runs in EL2,
>> + * and EL0 accesses are configured in the later stage of boot process.
>> + */
>> + cbnz x2, 1f
>> mrs x0, cnthctl_el2
>> orr x0, x0, #3 // Enable EL1 physical timers
>> msr cnthctl_el2, x0
>> +1:
>> msr cntvoff_el2, xzr // Clear virtual offset
>>
>> #ifdef CONFIG_ARM_GIC_V3
>>
>
> Nice catch. It may be worth documenting that when HCR_EL2.E2H == 1,
> CNTHCTL_EL2 has the same bit layout as CNTKCTL_EL1, allowing the kernel
> designed to run at EL1 to transparently mess with the EL0 bits (as
> CNTHCTL_EL2 and CNTKCTL_EL1 are the same register in this configuration).
Hi Marc, thanks for the review. I'll add this comment and send out v2.
>
> Acked-by: Marc Zyngier <marc.zyngier at arm.com>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
>
More information about the linux-arm-kernel
mailing list