On Mon, May 21, 2012 at 12:21 PM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> Not switching the fault registers seem like a bad idea: should
> the guest be preempted while handling a fault, we may end up with
> some odd behaviour if the host uses the same registers before
> coming back to the guest.
>
> Fix this by switching DFSR, IFSR, DFAR, IFAR, ADFSR and AIFSR.
>
> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
> ---
> ?arch/arm/include/asm/kvm_host.h | ? ?6 ++++++
> ?arch/arm/kernel/asm-offsets.c ? | ? ?6 ++++++
> ?arch/arm/kvm/interrupts.S ? ? ? | ? 29 +++++++++++++++++++++++++++--
> ?3 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 44abdc8..ab7f010 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -75,6 +75,12 @@ enum cp15_regs {
> ? ? ? ?c2_TTBR1_high, ? ? ? ? ?/* TTBR1 top 32 bits */
> ? ? ? ?c2_TTBCR, ? ? ? ? ? ? ? /* Translation Table Base Control R. */
> ? ? ? ?c3_DACR, ? ? ? ? ? ? ? ?/* Domain Access Control Register */
> + ? ? ? c5_DFSR, ? ? ? ? ? ? ? ?/* Data Fault Status Register */
> + ? ? ? c5_IFSR, ? ? ? ? ? ? ? ?/* Instruction Fault Status Register */
> + ? ? ? c5_ADFSR, ? ? ? ? ? ? ? /* Auxilary Data Fault Status Register */
> + ? ? ? c5_AIFSR, ? ? ? ? ? ? ? /* Auxilary Instruction Fault Status Register */
> + ? ? ? c6_DFAR, ? ? ? ? ? ? ? ?/* Data Fault Address Register */
> + ? ? ? c6_IFAR, ? ? ? ? ? ? ? ?/* Instruction Fault Address Register */
> ? ? ? ?c10_PRRR, ? ? ? ? ? ? ? /* Primary Region Remap Register */
> ? ? ? ?c10_NMRR, ? ? ? ? ? ? ? /* Normal Memory Remap Register */
> ? ? ? ?c13_CID, ? ? ? ? ? ? ? ?/* Context ID Register */
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index c8c1b91..c4c5928 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -155,6 +155,12 @@ int main(void)
> ? DEFINE(VCPU_TTBR1, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.cp15[c2_TTBR1]));
> ? DEFINE(VCPU_TTBCR, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.cp15[c2_TTBCR]));
> ? DEFINE(VCPU_DACR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c3_DACR]));
> + ?DEFINE(VCPU_DFSR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c5_DFSR]));
> + ?DEFINE(VCPU_IFSR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c5_IFSR]));
> + ?DEFINE(VCPU_ADFSR, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.cp15[c5_ADFSR]));
> + ?DEFINE(VCPU_AIFSR, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.cp15[c5_AIFSR]));
> + ?DEFINE(VCPU_DFAR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c6_DFAR]));
> + ?DEFINE(VCPU_IFAR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c6_IFAR]));
> ? DEFINE(VCPU_PRRR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c10_PRRR]));
> ? DEFINE(VCPU_NMRR, ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.cp15[c10_NMRR]));
> ? DEFINE(VCPU_CID, ? ? ? ? ? ? offsetof(struct kvm_vcpu, arch.cp15[c13_CID]));
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index 2c32d22..712462c 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -180,13 +180,26 @@ ENTRY(__kvm_flush_vm_context)
> ? ? ? ?mrc ? ? p15, 0, r3, c13, c0, 2 ?@ TID_URW
> ? ? ? ?mrc ? ? p15, 0, r4, c13, c0, 3 ?@ TID_URO
> ? ? ? ?mrc ? ? p15, 0, r5, c13, c0, 4 ?@ TID_PRIV
> + ? ? ? mrc ? ? p15, 0, r6, c5, c0, 0 ? @ DFSR
> + ? ? ? mrc ? ? p15, 0, r7, c5, c0, 1 ? @ IFSR
> + ? ? ? mrc ? ? p15, 0, r8, c5, c1, 0 ? @ ADFSR
> + ? ? ? mrc ? ? p15, 0, r9, c5, c1, 1 ? @ AIFSR
> + ? ? ? mrc ? ? p15, 0, r10, c6, c0, 0 ?@ DFAR
> + ? ? ? mrc ? ? p15, 0, r11, c6, c0, 2 ?@ IFAR
> +
> ? ? ? ?.if \vcpu == 0
> - ? ? ? push ? ?{r2-r5} ? ? ? ? ? ? ? ? @ Push CP15 registers
> + ? ? ? push ? ?{r2-r11} ? ? ? ? ? ? ? ?@ Push CP15 registers
> ? ? ? ?.else
> ? ? ? ?str ? ? r2, [\vcpup, #VCPU_CID]
> ? ? ? ?str ? ? r3, [\vcpup, #VCPU_TID_URW]
> ? ? ? ?str ? ? r4, [\vcpup, #VCPU_TID_URO]
> ? ? ? ?str ? ? r5, [\vcpup, #VCPU_TID_PRIV]
> + ? ? ? str ? ? r6, [\vcpup, #VCPU_DFSR]
> + ? ? ? str ? ? r7, [\vcpup, #VCPU_IFSR]
> + ? ? ? str ? ? r8, [\vcpup, #VCPU_ADFSR]
> + ? ? ? str ? ? r9, [\vcpup, #VCPU_AIFSR]
> + ? ? ? str ? ? r10, [\vcpup, #VCPU_DFAR]
> + ? ? ? str ? ? r11, [\vcpup, #VCPU_IFAR]
> ? ? ? ?.endif
> ?.endm
>
> @@ -197,18 +210,30 @@ ENTRY(__kvm_flush_vm_context)
> ?*/
> ?.macro write_cp15_state vcpu=0, vcpup
> ? ? ? ?.if \vcpu == 0
> - ? ? ? pop ? ? {r2-r5}
> + ? ? ? pop ? ? {r2-r11}
> ? ? ? ?.else
> ? ? ? ?ldr ? ? r2, [\vcpup, #VCPU_CID]
> ? ? ? ?ldr ? ? r3, [\vcpup, #VCPU_TID_URW]
> ? ? ? ?ldr ? ? r4, [\vcpup, #VCPU_TID_URO]
> ? ? ? ?ldr ? ? r5, [\vcpup, #VCPU_TID_PRIV]
> + ? ? ? ldr ? ? r6, [\vcpup, #VCPU_DFSR]
> + ? ? ? ldr ? ? r7, [\vcpup, #VCPU_IFSR]
> + ? ? ? ldr ? ? r8, [\vcpup, #VCPU_ADFSR]
> + ? ? ? ldr ? ? r9, [\vcpup, #VCPU_AIFSR]
> + ? ? ? ldr ? ? r10, [\vcpup, #VCPU_DFAR]
> + ? ? ? ldr ? ? r11, [\vcpup, #VCPU_IFAR]
> ? ? ? ?.endif
>
> ? ? ? ?mcr ? ? p15, 0, r2, c13, c0, 1 ?@ CID
> ? ? ? ?mcr ? ? p15, 0, r3, c13, c0, 2 ?@ TID_URW
> ? ? ? ?mcr ? ? p15, 0, r4, c13, c0, 3 ?@ TID_URO
> ? ? ? ?mcr ? ? p15, 0, r5, c13, c0, 4 ?@ TID_PRIV
> + ? ? ? mcr ? ? p15, 0, r6, c5, c0, 0 ? @ DFSR
> + ? ? ? mcr ? ? p15, 0, r7, c5, c0, 1 ? @ IFSR
> + ? ? ? mcr ? ? p15, 0, r8, c5, c1, 0 ? @ ADFSR
> + ? ? ? mcr ? ? p15, 0, r9, c5, c1, 1 ? @ AIFSR
> + ? ? ? mcr ? ? p15, 0, r10, c6, c0, 0 ?@ DFAR
> + ? ? ? mcr ? ? p15, 0, r11, c6, c0, 2 ?@ IFAR
>
> ? ? ? ?.if \vcpu == 0
> ? ? ? ?pop ? ? {r2-r11}
> --
ack (I actually thought I added this once already, but must have gone
lost somewhere).
Thanks,
Christoffer
