On Monday 25 April 2016 01:45 PM, Alexander Graf wrote: > >> 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. Ok got it. Let me try perf record with sample_addr type. Maddy > >> 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
