diff mbox

powerpc/kvm: Create proper names for the kvm_host_state PMU fields

Message ID 1404984871-15145-1-git-send-email-mpe@ellerman.id.au (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Ellerman July 10, 2014, 9:34 a.m. UTC
We have two arrays in kvm_host_state that contain register values for
the PMU. Currently we only create an asm-offsets symbol for the base of
the arrays, and do the array offset in the assembly code.

Creating an asm-offsets symbol for each field individually makes the
code much nicer to read, particularly for the MMCRx/SIxR/SDAR fields, and
might have helped us notice the recent double restore bug we had in this
code.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

This is on top of "powerpc/kvm: Remove redundant save of SIER AND MMCR2".

 arch/powerpc/kernel/asm-offsets.c       | 17 +++++++++++++++--
 arch/powerpc/kvm/book3s_hv_interrupts.S | 30 +++++++++++++++---------------
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 32 ++++++++++++++++----------------
 3 files changed, 46 insertions(+), 33 deletions(-)

Comments

Alexander Graf July 10, 2014, 10:16 a.m. UTC | #1
On 10.07.14 11:34, Michael Ellerman wrote:
> We have two arrays in kvm_host_state that contain register values for
> the PMU. Currently we only create an asm-offsets symbol for the base of
> the arrays, and do the array offset in the assembly code.
>
> Creating an asm-offsets symbol for each field individually makes the
> code much nicer to read, particularly for the MMCRx/SIxR/SDAR fields, and
> might have helped us notice the recent double restore bug we had in this
> code.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Acked-by: Alexander Graf <agraf@suse.de>

I still think this whole code path should just be C though.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman July 11, 2014, 4:46 a.m. UTC | #2
On Thu, 2014-07-10 at 12:16 +0200, Alexander Graf wrote:
> On 10.07.14 11:34, Michael Ellerman wrote:
> > We have two arrays in kvm_host_state that contain register values for
> > the PMU. Currently we only create an asm-offsets symbol for the base of
> > the arrays, and do the array offset in the assembly code.
> >
> > Creating an asm-offsets symbol for each field individually makes the
> > code much nicer to read, particularly for the MMCRx/SIxR/SDAR fields, and
> > might have helped us notice the recent double restore bug we had in this
> > code.
> >
> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> Acked-by: Alexander Graf <agraf@suse.de>

Thanks.

> I still think this whole code path should just be C though.

Yeah it probably should.

cheers


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

Patch

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index f5995a912213..9adadb1db9b8 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -645,8 +645,21 @@  int main(void)
 	HSTATE_FIELD(HSTATE_SAVED_XIRR, saved_xirr);
 	HSTATE_FIELD(HSTATE_HOST_IPI, host_ipi);
 	HSTATE_FIELD(HSTATE_PTID, ptid);
-	HSTATE_FIELD(HSTATE_MMCR, host_mmcr);
-	HSTATE_FIELD(HSTATE_PMC, host_pmc);
+	HSTATE_FIELD(HSTATE_MMCR0, host_mmcr[0]);
+	HSTATE_FIELD(HSTATE_MMCR1, host_mmcr[1]);
+	HSTATE_FIELD(HSTATE_MMCRA, host_mmcr[2]);
+	HSTATE_FIELD(HSTATE_SIAR, host_mmcr[3]);
+	HSTATE_FIELD(HSTATE_SDAR, host_mmcr[4]);
+	HSTATE_FIELD(HSTATE_MMCR2, host_mmcr[5]);
+	HSTATE_FIELD(HSTATE_SIER, host_mmcr[6]);
+	HSTATE_FIELD(HSTATE_PMC1, host_pmc[0]);
+	HSTATE_FIELD(HSTATE_PMC2, host_pmc[1]);
+	HSTATE_FIELD(HSTATE_PMC3, host_pmc[2]);
+	HSTATE_FIELD(HSTATE_PMC4, host_pmc[3]);
+	HSTATE_FIELD(HSTATE_PMC5, host_pmc[4]);
+	HSTATE_FIELD(HSTATE_PMC6, host_pmc[5]);
+	HSTATE_FIELD(HSTATE_PMC7, host_pmc[6]);
+	HSTATE_FIELD(HSTATE_PMC8, host_pmc[7]);
 	HSTATE_FIELD(HSTATE_PURR, host_purr);
 	HSTATE_FIELD(HSTATE_SPURR, host_spurr);
 	HSTATE_FIELD(HSTATE_DSCR, host_dscr);
diff --git a/arch/powerpc/kvm/book3s_hv_interrupts.S b/arch/powerpc/kvm/book3s_hv_interrupts.S
index 731be7478b27..16ef3163a8b6 100644
--- a/arch/powerpc/kvm/book3s_hv_interrupts.S
+++ b/arch/powerpc/kvm/book3s_hv_interrupts.S
@@ -97,15 +97,15 @@  END_FTR_SECTION_IFSET(CPU_FTR_ARCH_206)
 	mfspr	r5, SPRN_MMCR1
 	mfspr	r9, SPRN_SIAR
 	mfspr	r10, SPRN_SDAR
-	std	r7, HSTATE_MMCR(r13)
-	std	r5, HSTATE_MMCR + 8(r13)
-	std	r6, HSTATE_MMCR + 16(r13)
-	std	r9, HSTATE_MMCR + 24(r13)
-	std	r10, HSTATE_MMCR + 32(r13)
+	std	r7, HSTATE_MMCR0(r13)
+	std	r5, HSTATE_MMCR1(r13)
+	std	r6, HSTATE_MMCRA(r13)
+	std	r9, HSTATE_SIAR(r13)
+	std	r10, HSTATE_SDAR(r13)
 BEGIN_FTR_SECTION
 	mfspr	r9, SPRN_SIER
-	std	r8, HSTATE_MMCR + 40(r13)
-	std	r9, HSTATE_MMCR + 48(r13)
+	std	r8, HSTATE_MMCR2(r13)
+	std	r9, HSTATE_SIER(r13)
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
 	mfspr	r3, SPRN_PMC1
 	mfspr	r5, SPRN_PMC2
@@ -117,15 +117,15 @@  BEGIN_FTR_SECTION
 	mfspr	r10, SPRN_PMC7
 	mfspr	r11, SPRN_PMC8
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
-	stw	r3, HSTATE_PMC(r13)
-	stw	r5, HSTATE_PMC + 4(r13)
-	stw	r6, HSTATE_PMC + 8(r13)
-	stw	r7, HSTATE_PMC + 12(r13)
-	stw	r8, HSTATE_PMC + 16(r13)
-	stw	r9, HSTATE_PMC + 20(r13)
+	stw	r3, HSTATE_PMC1(r13)
+	stw	r5, HSTATE_PMC2(r13)
+	stw	r6, HSTATE_PMC3(r13)
+	stw	r7, HSTATE_PMC4(r13)
+	stw	r8, HSTATE_PMC5(r13)
+	stw	r9, HSTATE_PMC6(r13)
 BEGIN_FTR_SECTION
-	stw	r10, HSTATE_PMC + 24(r13)
-	stw	r11, HSTATE_PMC + 28(r13)
+	stw	r10, HSTATE_PMC7(r13)
+	stw	r11, HSTATE_PMC8(r13)
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
 31:
 
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 868347ef09fd..d68ecb33b52a 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -87,20 +87,20 @@  END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
 	cmpwi	r4, 0
 	beq	23f			/* skip if not */
 BEGIN_FTR_SECTION
-	ld	r3, HSTATE_MMCR(r13)
+	ld	r3, HSTATE_MMCR0(r13)
 	andi.	r4, r3, MMCR0_PMAO_SYNC | MMCR0_PMAO
 	cmpwi	r4, MMCR0_PMAO
 	beql	kvmppc_fix_pmao
 END_FTR_SECTION_IFSET(CPU_FTR_PMAO_BUG)
-	lwz	r3, HSTATE_PMC(r13)
-	lwz	r4, HSTATE_PMC + 4(r13)
-	lwz	r5, HSTATE_PMC + 8(r13)
-	lwz	r6, HSTATE_PMC + 12(r13)
-	lwz	r8, HSTATE_PMC + 16(r13)
-	lwz	r9, HSTATE_PMC + 20(r13)
+	lwz	r3, HSTATE_PMC1(r13)
+	lwz	r4, HSTATE_PMC2(r13)
+	lwz	r5, HSTATE_PMC3(r13)
+	lwz	r6, HSTATE_PMC4(r13)
+	lwz	r8, HSTATE_PMC5(r13)
+	lwz	r9, HSTATE_PMC6(r13)
 BEGIN_FTR_SECTION
-	lwz	r10, HSTATE_PMC + 24(r13)
-	lwz	r11, HSTATE_PMC + 28(r13)
+	lwz	r10, HSTATE_PMC7(r13)
+	lwz	r11, HSTATE_PMC8(r13)
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
 	mtspr	SPRN_PMC1, r3
 	mtspr	SPRN_PMC2, r4
@@ -112,18 +112,18 @@  BEGIN_FTR_SECTION
 	mtspr	SPRN_PMC7, r10
 	mtspr	SPRN_PMC8, r11
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
-	ld	r3, HSTATE_MMCR(r13)
-	ld	r4, HSTATE_MMCR + 8(r13)
-	ld	r5, HSTATE_MMCR + 16(r13)
-	ld	r6, HSTATE_MMCR + 24(r13)
-	ld	r7, HSTATE_MMCR + 32(r13)
+	ld	r3, HSTATE_MMCR0(r13)
+	ld	r4, HSTATE_MMCR1(r13)
+	ld	r5, HSTATE_MMCRA(r13)
+	ld	r6, HSTATE_SIAR(r13)
+	ld	r7, HSTATE_SDAR(r13)
 	mtspr	SPRN_MMCR1, r4
 	mtspr	SPRN_MMCRA, r5
 	mtspr	SPRN_SIAR, r6
 	mtspr	SPRN_SDAR, r7
 BEGIN_FTR_SECTION
-	ld	r8, HSTATE_MMCR + 40(r13)
-	ld	r9, HSTATE_MMCR + 48(r13)
+	ld	r8, HSTATE_MMCR2(r13)
+	ld	r9, HSTATE_SIER(r13)
 	mtspr	SPRN_MMCR2, r8
 	mtspr	SPRN_SIER, r9
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)