Re: powerpc: Add an inline function to update HID0

Hi Michael,

On Tue, Aug 04, 2015 at 08:08:58PM +1000, Michael Ellerman wrote:

On Tue, 2015-04-08 at 08:30:58 UTC, "Gautham R. Shenoy" wrote:

quoted

Section 3.7 of Version 1.2 of the Power8 Processor User's Manual
prescribes that updates to HID0 be preceded by a SYNC instruction and
followed by an ISYNC instruction (Page 91).

Create a function name update_hid0() which follows this recipe and
invoke it from the static split core path.

Signed-off-by: Gautham R. Shenoy <redacted>
---
 arch/powerpc/include/asm/kvm_ppc.h       | 11 +++++++++++

Why is it in there? It's not KVM related per se.

Ok. Will fix this.

Where should it go? I think reg.h would be best, ideally near the definition
for HID0, though that's probably not possible because of ASSEMBLY requirements.
So at the bottom of reg.h ?

quoted

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index c6ef05b..325f1d6 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h

@@ -685,4 +685,15 @@ static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int ra, int rb)
 
 extern void xics_wake_cpu(int cpu);
 
+static inline void update_hid0(unsigned long hid0)
+{
+	/*
+	 *  The HID0 update should at the very least be preceded by a
+	 *  a SYNC instruction followed by an ISYNC instruction
+	 */
+	mb();
+	mtspr(SPRN_HID0, hid0);
+	isync();

That's going to turn into three separate inline asm blocks, which is maybe a
bit unfortunate. Have you checked the generated code is what we want, ie. just
sync, mtspr, isync ?

Yes, the objdump of subcore.o shows exactly these three instructions:
 7c 00 04 ac     sync    
 7c 70 fb a6     mtspr   1008,r3
 4c 00 01 2c     isync

cheers

--
Thanks and Regards
gautham.