> Am 25.04.2016 um 10:08 schrieb Madhavan Srinivasan <maddy@xxxxxxxxxxxxxxxxxx>: > > > >> On Friday 08 April 2016 09:24 PM, Thomas Huth wrote: >> The SIAR register is available twice, one time as SPR 780 (unprivileged, >> but read-only), and one time as SPR 796 (privileged, but read and write). >> The Linux kernel code currently uses SPR 780 - and while this is OK for >> reading, writing to that register of course does not work. >> Since the KVM code tries to write to this register, too (see the mtspr >> in book3s_hv_rmhandlers.S), the contents of this register sometimes get >> lost for the guests, e.g. during migration of a VM. >> To fix this issue, simply switch to the other SPR numer 796 instead. > > IIUC, SIAR and SDAR are updated by hardware when we take > a pmu exception with sampling mode enabled (based on instr). > And these register contents are mainly for OS consumption. > So, we dont need to restore these register values at all, > kindly correct me if I missing something here. What if you migrate between a pmu event firing and the os reading siar? Or what if the host gets pmu events? Or we migrate the guest to a different pcpu? In all those cases we need to ensure the register contents are consistent. > > Maddy > >> >> Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx> >> --- >> Note: The perf code in core-book3s.c also seems to write to the SIAR >> SPR, so that might be affected by this issue, too - but I did >> not test the perf code, so I'm not sure about that part. Please write a small unit test that fires off pmu events constantly and checks whtether they arrive correctly. Run perf in parallel on the host to increase the chance for breakage. >> >> arch/powerpc/include/asm/reg.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h >> index f5f4c66..6630420 100644 >> --- a/arch/powerpc/include/asm/reg.h >> +++ b/arch/powerpc/include/asm/reg.h >> @@ -752,13 +752,13 @@ >> #define SPRN_PMC6 792 >> #define SPRN_PMC7 793 >> #define SPRN_PMC8 794 >> -#define SPRN_SIAR 780 >> #define SPRN_SDAR 781 >> #define SPRN_SIER 784 >> #define SIER_SIPR 0x2000000 /* Sampled MSR_PR */ >> #define SIER_SIHV 0x1000000 /* Sampled MSR_HV */ >> #define SIER_SIAR_VALID 0x0400000 /* SIAR contents valid */ >> #define SIER_SDAR_VALID 0x0200000 /* SDAR contents valid */ >> +#define SPRN_SIAR 796 I'm sure there's a reason (iSeries?) we used the r/o version before. Better introduce a new constant that gives us rw access and use that in the kvm entry/exit code. Alex >> #define SPRN_TACR 888 >> #define SPRN_TCSCR 889 >> #define SPRN_CSIGR 890 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
