On Thu, Aug 15, 2013 at 01:26:06PM +0900, Jonghwan Choi wrote:
> From: Marc Zyngier <marc.zyngier@xxxxxxx>
>
> This patch looks like it should be in the 3.10-stable tree, should we apply
> it?
Yes, please. Thanks.
>
> ------------------
>
> From: "Marc Zyngier <marc.zyngier@xxxxxxx>"
>
> commit 6a077e4ab9cbfbf279fb955bae05b03781c97013 upstream
>
> 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.
>
> [ Slightly tweaked to use an even register as first operand to ldrd
> and strd operations in interrupts_head.S - Christoffer ]
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> Signed-off-by: Jonghwan Choi <jhbird.choi@xxxxxxxxxxx>
> ---
> 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 e4956f4..11a5399 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 7bed755..5c8c128 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 8ca87ab..bc7892ba1 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -411,6 +411,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
> @@ -421,13 +425,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..2b44b95 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, r4, r5, c7 @ PAR
>
> .if \store_to_vcpu == 0
> - push {r2}
> + push {r2,r4-r5}
> .else
> str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> + add r12, vcpu, #CP15_OFFSET(c7_PAR)
> + strd r4, r5, [r12]
> .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-r5}
> .else
> ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
> + add r12, vcpu, #CP15_OFFSET(c7_PAR)
> + ldrd r4, r5, [r12]
> .endif
>
> mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL
> + mcrr p15, 0, r4, r5, c7 @ PAR
>
> .if \read_from_vcpu == 0
> pop {r2-r12}
> --
> 1.8.1.2
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html