Patchwork [1/8] KVM: PPC: Book3S PR: Load up SPRG3 register with guest value on guest entry

login
register
mail settings
Submitter Paul Mackerras
Date July 11, 2013, 11:49 a.m.
Message ID <20130711114943.GC21353@iris.ozlabs.ibm.com>
Download mbox | patch
Permalink /patch/258416/
State New
Headers show

Comments

Paul Mackerras - July 11, 2013, 11:49 a.m.
Unlike the other general-purpose SPRs, SPRG3 can be read by usermode
code, and is used in recent kernels to store the CPU and NUMA node
numbers so that they can be read by VDSO functions.  Thus we need to
load the guest's SPRG3 value into the real SPRG3 register when entering
the guest, and restore the host's value when exiting the guest.  We don't
need to save the guest SPRG3 value when exiting the guest as usermode
code can't modify SPRG3.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 arch/powerpc/kernel/asm-offsets.c    |  1 +
 arch/powerpc/kvm/book3s_interrupts.S | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
Alexander Graf - July 25, 2013, 1:38 p.m.
On 11.07.2013, at 13:49, Paul Mackerras wrote:

> Unlike the other general-purpose SPRs, SPRG3 can be read by usermode
> code, and is used in recent kernels to store the CPU and NUMA node
> numbers so that they can be read by VDSO functions.  Thus we need to
> load the guest's SPRG3 value into the real SPRG3 register when entering
> the guest, and restore the host's value when exiting the guest.  We don't
> need to save the guest SPRG3 value when exiting the guest as usermode
> code can't modify SPRG3.

This loads SPRG3 on every guest exit, which can happen a lot with instruction emulation. Since the kernel doesn't rely on the contents of SPRG3 we only have to care about it when not in KVM code, right?

So could we move this to kvmppc_core_vcpu_load/put instead?


Alex

> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> arch/powerpc/kernel/asm-offsets.c    |  1 +
> arch/powerpc/kvm/book3s_interrupts.S | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 6f16ffa..a67c76e 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -452,6 +452,7 @@ int main(void)
> 	DEFINE(VCPU_SPRG2, offsetof(struct kvm_vcpu, arch.shregs.sprg2));
> 	DEFINE(VCPU_SPRG3, offsetof(struct kvm_vcpu, arch.shregs.sprg3));
> #endif
> +	DEFINE(VCPU_SHARED_SPRG3, offsetof(struct kvm_vcpu_arch_shared, sprg3));
> 	DEFINE(VCPU_SHARED_SPRG4, offsetof(struct kvm_vcpu_arch_shared, sprg4));
> 	DEFINE(VCPU_SHARED_SPRG5, offsetof(struct kvm_vcpu_arch_shared, sprg5));
> 	DEFINE(VCPU_SHARED_SPRG6, offsetof(struct kvm_vcpu_arch_shared, sprg6));
> diff --git a/arch/powerpc/kvm/book3s_interrupts.S b/arch/powerpc/kvm/book3s_interrupts.S
> index 48cbbf8..17cfae5 100644
> --- a/arch/powerpc/kvm/book3s_interrupts.S
> +++ b/arch/powerpc/kvm/book3s_interrupts.S
> @@ -92,6 +92,11 @@ kvm_start_lightweight:
> 	PPC_LL	r3, VCPU_HFLAGS(r4)
> 	rldicl	r3, r3, 0, 63		/* r3 &= 1 */
> 	stb	r3, HSTATE_RESTORE_HID5(r13)
> +
> +	/* Load up guest SPRG3 value, since it's user readable */
> +	ld	r3, VCPU_SHARED(r4)
> +	ld	r3, VCPU_SHARED_SPRG3(r3)
> +	mtspr	SPRN_SPRG3, r3
> #endif /* CONFIG_PPC_BOOK3S_64 */
> 
> 	PPC_LL	r4, VCPU_SHADOW_MSR(r4)	/* get shadow_msr */
> @@ -123,6 +128,15 @@ kvmppc_handler_highmem:
> 	/* R7 = vcpu */
> 	PPC_LL	r7, GPR4(r1)
> 
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	/*
> +	 * Reload kernel SPRG3 value.
> +	 * No need to save guest value as usermode can't modify SPRG3.
> +	 */
> +	ld	r3, PACA_SPRG3(r13)
> +	mtspr	SPRN_SPRG3, r3
> +#endif /* CONFIG_PPC_BOOK3S_64 */
> +
> 	PPC_STL	r14, VCPU_GPR(R14)(r7)
> 	PPC_STL	r15, VCPU_GPR(R15)(r7)
> 	PPC_STL	r16, VCPU_GPR(R16)(r7)
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - July 25, 2013, 1:40 p.m.
On 25.07.2013, at 15:38, Alexander Graf wrote:

> 
> On 11.07.2013, at 13:49, Paul Mackerras wrote:
> 
>> Unlike the other general-purpose SPRs, SPRG3 can be read by usermode
>> code, and is used in recent kernels to store the CPU and NUMA node
>> numbers so that they can be read by VDSO functions.  Thus we need to
>> load the guest's SPRG3 value into the real SPRG3 register when entering
>> the guest, and restore the host's value when exiting the guest.  We don't
>> need to save the guest SPRG3 value when exiting the guest as usermode
>> code can't modify SPRG3.
> 
> This loads SPRG3 on every guest exit, which can happen a lot with instruction emulation. Since the kernel doesn't rely on the contents of SPRG3 we only have to care about it when not in KVM code, right?
> 
> So could we move this to kvmppc_core_vcpu_load/put instead?

but then again if all the shadow copy code is negligible performance wise, so is this probably. Applied to kvm-ppc-queue.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 6f16ffa..a67c76e 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -452,6 +452,7 @@  int main(void)
 	DEFINE(VCPU_SPRG2, offsetof(struct kvm_vcpu, arch.shregs.sprg2));
 	DEFINE(VCPU_SPRG3, offsetof(struct kvm_vcpu, arch.shregs.sprg3));
 #endif
+	DEFINE(VCPU_SHARED_SPRG3, offsetof(struct kvm_vcpu_arch_shared, sprg3));
 	DEFINE(VCPU_SHARED_SPRG4, offsetof(struct kvm_vcpu_arch_shared, sprg4));
 	DEFINE(VCPU_SHARED_SPRG5, offsetof(struct kvm_vcpu_arch_shared, sprg5));
 	DEFINE(VCPU_SHARED_SPRG6, offsetof(struct kvm_vcpu_arch_shared, sprg6));
diff --git a/arch/powerpc/kvm/book3s_interrupts.S b/arch/powerpc/kvm/book3s_interrupts.S
index 48cbbf8..17cfae5 100644
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -92,6 +92,11 @@  kvm_start_lightweight:
 	PPC_LL	r3, VCPU_HFLAGS(r4)
 	rldicl	r3, r3, 0, 63		/* r3 &= 1 */
 	stb	r3, HSTATE_RESTORE_HID5(r13)
+
+	/* Load up guest SPRG3 value, since it's user readable */
+	ld	r3, VCPU_SHARED(r4)
+	ld	r3, VCPU_SHARED_SPRG3(r3)
+	mtspr	SPRN_SPRG3, r3
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
 	PPC_LL	r4, VCPU_SHADOW_MSR(r4)	/* get shadow_msr */
@@ -123,6 +128,15 @@  kvmppc_handler_highmem:
 	/* R7 = vcpu */
 	PPC_LL	r7, GPR4(r1)
 
+#ifdef CONFIG_PPC_BOOK3S_64
+	/*
+	 * Reload kernel SPRG3 value.
+	 * No need to save guest value as usermode can't modify SPRG3.
+	 */
+	ld	r3, PACA_SPRG3(r13)
+	mtspr	SPRN_SPRG3, r3
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
 	PPC_STL	r14, VCPU_GPR(R14)(r7)
 	PPC_STL	r15, VCPU_GPR(R15)(r7)
 	PPC_STL	r16, VCPU_GPR(R16)(r7)