On Fri, Jun 21, 2013 at 01:08:46PM +0100, Marc Zyngier wrote:
> Not saving PAR is an unfortunate oversight. If the guest performs
> an AT* operation and gets scheduled out before reading the result
> of the translation from PAR, it could become corrupted by another
> guest or the host.
>
> Saving this register is made slightly more complicated as KVM also
> uses it on the permission fault handling path, leading to an ugly
> "stash and restore" sequence. Fortunately, this is already a slow
> path so we don't really care. Also, Linux doesn't do any AT*
> operation, so Linux guests are not impacted by this bug.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++----------
> arch/arm/kvm/coproc.c | 4 ++++
> arch/arm/kvm/interrupts.S | 12 +++++++++++-
> arch/arm/kvm/interrupts_head.S | 10 ++++++++--
> 4 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 18d5032..4bb08e3 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -37,16 +37,18 @@
> #define c5_AIFSR 15 /* Auxilary Instrunction Fault Status R */
> #define c6_DFAR 16 /* Data Fault Address Register */
> #define c6_IFAR 17 /* Instruction Fault Address Register */
> -#define c9_L2CTLR 18 /* Cortex A15 L2 Control Register */
> -#define c10_PRRR 19 /* Primary Region Remap Register */
> -#define c10_NMRR 20 /* Normal Memory Remap Register */
> -#define c12_VBAR 21 /* Vector Base Address Register */
> -#define c13_CID 22 /* Context ID Register */
> -#define c13_TID_URW 23 /* Thread ID, User R/W */
> -#define c13_TID_URO 24 /* Thread ID, User R/O */
> -#define c13_TID_PRIV 25 /* Thread ID, Privileged */
> -#define c14_CNTKCTL 26 /* Timer Control Register (PL1) */
> -#define NR_CP15_REGS 27 /* Number of regs (incl. invalid) */
> +#define c7_PAR 18 /* Physical Address Register */
> +#define c7_PAR_high 19 /* PAR top 32 bits */
> +#define c9_L2CTLR 20 /* Cortex A15 L2 Control Register */
> +#define c10_PRRR 21 /* Primary Region Remap Register */
> +#define c10_NMRR 22 /* Normal Memory Remap Register */
> +#define c12_VBAR 23 /* Vector Base Address Register */
> +#define c13_CID 24 /* Context ID Register */
> +#define c13_TID_URW 25 /* Thread ID, User R/W */
> +#define c13_TID_URO 26 /* Thread ID, User R/O */
> +#define c13_TID_PRIV 27 /* Thread ID, Privileged */
> +#define c14_CNTKCTL 28 /* Timer Control Register (PL1) */
> +#define NR_CP15_REGS 29 /* Number of regs (incl. invalid) */
>
> #define ARM_EXCEPTION_RESET 0
> #define ARM_EXCEPTION_UNDEFINED 1
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 8eea97b..4a51990 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -180,6 +180,10 @@ static const struct coproc_reg cp15_regs[] = {
> NULL, reset_unknown, c6_DFAR },
> { CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
> NULL, reset_unknown, c6_IFAR },
> +
> + /* PAR swapped by interrupt.S */
> + { CRn( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
> +
> /*
> * DC{C,I,CI}SW operations:
> */
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index f7793df..d0a8fa3 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -414,6 +414,10 @@ guest_trap:
> mrcne p15, 4, r2, c6, c0, 4 @ HPFAR
> bne 3f
>
> + /* Preserve PAR */
> + mrrc p15, 0, r0, r1, c7 @ PAR
> + push {r0, r1}
> +
> /* Resolve IPA using the xFAR */
> mcr p15, 0, r2, c7, c8, 0 @ ATS1CPR
> isb
> @@ -424,13 +428,19 @@ guest_trap:
> lsl r2, r2, #4
> orr r2, r2, r1, lsl #24
>
> + /* Restore PAR */
> + pop {r0, r1}
> + mcrr p15, 0, r0, r1, c7 @ PAR
> +
> 3: load_vcpu @ Load VCPU pointer to r0
> str r2, [r0, #VCPU_HPFAR]
>
> 1: mov r1, #ARM_EXCEPTION_HVC
> b __kvm_vcpu_return
>
> -4: pop {r0, r1, r2} @ Failed translation, return to guest
> +4: pop {r0, r1} @ Failed translation, return to guest
> + mcrr p15, 0, r0, r1, c7 @ PAR
> + pop {r0, r1, r2}
> eret
>
> /*
> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> index 3c8f2f0..2478af1 100644
> --- a/arch/arm/kvm/interrupts_head.S
> +++ b/arch/arm/kvm/interrupts_head.S
> @@ -302,11 +302,14 @@ vcpu .req r0 @ vcpu pointer always in r0
> .endif
>
> mrc p15, 0, r2, c14, c1, 0 @ CNTKCTL
> + mrrc p15, 0, r3, r4, c7 @ PAR
>
> .if \store_to_vcpu == 0
> - push {r2}
> + push {r2-r4}
> .else
> str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> + add r12, vcpu, #CP15_OFFSET(c7_PAR)
> + strd r3, r4, [r12]
my compiler complains that the load should start with an even register,
so I've changed this with the patch below.
> .endif
> .endm
>
> @@ -319,12 +322,15 @@ vcpu .req r0 @ vcpu pointer always in r0
> */
> .macro write_cp15_state read_from_vcpu
> .if \read_from_vcpu == 0
> - pop {r2}
> + pop {r2-r4}
> .else
> ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> + add r12, vcpu, #CP15_OFFSET(c7_PAR)
> + ldrd r3, r4, [r12]
ditto
> .endif
>
> mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL
> + mcrr p15, 0, r3, r4, c7 @ PAR
>
> .if \read_from_vcpu == 0
> pop {r2-r12}
> --
> 1.8.2.3
>
Here's the fixup patch:
diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
index 2478af1..2b44b95 100644
--- a/arch/arm/kvm/interrupts_head.S
+++ b/arch/arm/kvm/interrupts_head.S
@@ -302,14 +302,14 @@ vcpu .req r0 @ vcpu pointer always in r0
.endif
mrc p15, 0, r2, c14, c1, 0 @ CNTKCTL
- mrrc p15, 0, r3, r4, c7 @ PAR
+ mrrc p15, 0, r4, r5, c7 @ PAR
.if \store_to_vcpu == 0
- push {r2-r4}
+ push {r2,r4-r5}
.else
str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
add r12, vcpu, #CP15_OFFSET(c7_PAR)
- strd r3, r4, [r12]
+ strd r4, r5, [r12]
.endif
.endm
@@ -322,15 +322,15 @@ vcpu .req r0 @ vcpu pointer always in r0
*/
.macro write_cp15_state read_from_vcpu
.if \read_from_vcpu == 0
- pop {r2-r4}
+ pop {r2,r4-r5}
.else
ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
add r12, vcpu, #CP15_OFFSET(c7_PAR)
- ldrd r3, r4, [r12]
+ ldrd r4, r5, [r12]
.endif
mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL
- mcrr p15, 0, r3, r4, c7 @ PAR
+ mcrr p15, 0, r4, r5, c7 @ PAR
.if \read_from_vcpu == 0
pop {r2-r12}
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
