Re: [PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
> On 13-Aug-2021, at 9:54 AM, Nicholas Piggin wrote: > > Excerpts from Athira Rajeev's message of August 9, 2021 1:03 pm: >> >> >>> On 26-Jul-2021, at 9:19 AM, Nicholas Piggin wrote: > > >>> +static void freeze_pmu(unsigned long mmcr0, unsigned long mmcra) >>> +{ >>> + if (!(mmcr0 & MMCR0_FC)) >>> + goto do_freeze; >>> + if (mmcra & MMCRA_SAMPLE_ENABLE) >>> + goto do_freeze; >>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) { >>> + if (!(mmcr0 & MMCR0_PMCCEXT)) >>> + goto do_freeze; >>> + if (!(mmcra & MMCRA_BHRB_DISABLE)) >>> + goto do_freeze; >>> + } >>> + return; >>> + >>> +do_freeze: >>> + mmcr0 = MMCR0_FC; >>> + mmcra = 0; >>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) { >>> + mmcr0 |= MMCR0_PMCCEXT; >>> + mmcra = MMCRA_BHRB_DISABLE; >>> + } >>> + >>> + mtspr(SPRN_MMCR0, mmcr0); >>> + mtspr(SPRN_MMCRA, mmcra); >>> + isync(); >>> +} >>> + >> Hi Nick, >> >> After feezing pmu, do we need to clear “pmcregs_in_use” as well? > > Not until we save the values out of the registers. pmcregs_in_use = 0 > means our hypervisor is free to clear our PMU register contents. > >> Also can’t we unconditionally do the MMCR0/MMCRA/ freeze settings in here ? >> do we need the if conditions for FC/PMCCEXT/BHRB ? > > I think it's possible, but pretty minimal advantage. I would prefer to > set them the way perf does for now. Sure Nick, Other changes looks good to me. Reviewed-by: Athira Rajeev Thanks Athira > If we can move this code into perf/ > it should become easier for you to tweak things. > > Thanks, > Nick
Re: [PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
Excerpts from Athira Rajeev's message of August 9, 2021 1:03 pm: > > >> On 26-Jul-2021, at 9:19 AM, Nicholas Piggin wrote: >> +static void freeze_pmu(unsigned long mmcr0, unsigned long mmcra) >> +{ >> +if (!(mmcr0 & MMCR0_FC)) >> +goto do_freeze; >> +if (mmcra & MMCRA_SAMPLE_ENABLE) >> +goto do_freeze; >> +if (cpu_has_feature(CPU_FTR_ARCH_31)) { >> +if (!(mmcr0 & MMCR0_PMCCEXT)) >> +goto do_freeze; >> +if (!(mmcra & MMCRA_BHRB_DISABLE)) >> +goto do_freeze; >> +} >> +return; >> + >> +do_freeze: >> +mmcr0 = MMCR0_FC; >> +mmcra = 0; >> +if (cpu_has_feature(CPU_FTR_ARCH_31)) { >> +mmcr0 |= MMCR0_PMCCEXT; >> +mmcra = MMCRA_BHRB_DISABLE; >> +} >> + >> +mtspr(SPRN_MMCR0, mmcr0); >> +mtspr(SPRN_MMCRA, mmcra); >> +isync(); >> +} >> + > Hi Nick, > > After feezing pmu, do we need to clear “pmcregs_in_use” as well? Not until we save the values out of the registers. pmcregs_in_use = 0 means our hypervisor is free to clear our PMU register contents. > Also can’t we unconditionally do the MMCR0/MMCRA/ freeze settings in here ? > do we need the if conditions for FC/PMCCEXT/BHRB ? I think it's possible, but pretty minimal advantage. I would prefer to set them the way perf does for now. If we can move this code into perf/ it should become easier for you to tweak things. Thanks, Nick
Re: [PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
> On 26-Jul-2021, at 9:19 AM, Nicholas Piggin wrote: > > Implement the P9 path PMU save/restore code in C, and remove the > POWER9/10 code from the P7/8 path assembly. > > -449 cycles (8533) POWER9 virt-mode NULL hcall > > Signed-off-by: Nicholas Piggin > --- > arch/powerpc/include/asm/asm-prototypes.h | 5 - > arch/powerpc/kvm/book3s_hv.c | 205 -- > arch/powerpc/kvm/book3s_hv_interrupts.S | 13 +- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 43 + > 4 files changed, 200 insertions(+), 66 deletions(-) > > diff --git a/arch/powerpc/include/asm/asm-prototypes.h > b/arch/powerpc/include/asm/asm-prototypes.h > index 222823861a67..41b8a1e1144a 100644 > --- a/arch/powerpc/include/asm/asm-prototypes.h > +++ b/arch/powerpc/include/asm/asm-prototypes.h > @@ -141,11 +141,6 @@ static inline void kvmppc_restore_tm_hv(struct kvm_vcpu > *vcpu, u64 msr, > bool preserve_nv) { } > #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ > > -void kvmhv_save_host_pmu(void); > -void kvmhv_load_host_pmu(void); > -void kvmhv_save_guest_pmu(struct kvm_vcpu *vcpu, bool pmu_in_use); > -void kvmhv_load_guest_pmu(struct kvm_vcpu *vcpu); > - > void kvmppc_p9_enter_guest(struct kvm_vcpu *vcpu); > > long kvmppc_h_set_dabr(struct kvm_vcpu *vcpu, unsigned long dabr); > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 2eef708c4354..d20b579ddcdf 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -3735,6 +3735,188 @@ static noinline void kvmppc_run_core(struct > kvmppc_vcore *vc) > trace_kvmppc_run_core(vc, 1); > } > > +/* > + * Privileged (non-hypervisor) host registers to save. > + */ > +struct p9_host_os_sprs { > + unsigned long dscr; > + unsigned long tidr; > + unsigned long iamr; > + unsigned long amr; > + unsigned long fscr; > + > + unsigned int pmc1; > + unsigned int pmc2; > + unsigned int pmc3; > + unsigned int pmc4; > + unsigned int pmc5; > + unsigned int pmc6; > + unsigned long mmcr0; > + unsigned long mmcr1; > + unsigned long mmcr2; > + unsigned long mmcr3; > + unsigned long mmcra; > + unsigned long siar; > + unsigned long sier1; > + unsigned long sier2; > + unsigned long sier3; > + unsigned long sdar; > +}; > + > +static void freeze_pmu(unsigned long mmcr0, unsigned long mmcra) > +{ > + if (!(mmcr0 & MMCR0_FC)) > + goto do_freeze; > + if (mmcra & MMCRA_SAMPLE_ENABLE) > + goto do_freeze; > + if (cpu_has_feature(CPU_FTR_ARCH_31)) { > + if (!(mmcr0 & MMCR0_PMCCEXT)) > + goto do_freeze; > + if (!(mmcra & MMCRA_BHRB_DISABLE)) > + goto do_freeze; > + } > + return; > + > +do_freeze: > + mmcr0 = MMCR0_FC; > + mmcra = 0; > + if (cpu_has_feature(CPU_FTR_ARCH_31)) { > + mmcr0 |= MMCR0_PMCCEXT; > + mmcra = MMCRA_BHRB_DISABLE; > + } > + > + mtspr(SPRN_MMCR0, mmcr0); > + mtspr(SPRN_MMCRA, mmcra); > + isync(); > +} > + Hi Nick, After feezing pmu, do we need to clear “pmcregs_in_use” as well? Also can’t we unconditionally do the MMCR0/MMCRA/ freeze settings in here ? do we need the if conditions for FC/PMCCEXT/BHRB ? Thanks Athira > +static void save_p9_host_pmu(struct p9_host_os_sprs *host_os_sprs) > +{ > + if (ppc_get_pmu_inuse()) { > + /* > + * It might be better to put PMU handling (at least for the > + * host) in the perf subsystem because it knows more about what > + * is being used. > + */ > + > + /* POWER9, POWER10 do not implement HPMC or SPMC */ > + > + host_os_sprs->mmcr0 = mfspr(SPRN_MMCR0); > + host_os_sprs->mmcra = mfspr(SPRN_MMCRA); > + > + freeze_pmu(host_os_sprs->mmcr0, host_os_sprs->mmcra); > + > + host_os_sprs->pmc1 = mfspr(SPRN_PMC1); > + host_os_sprs->pmc2 = mfspr(SPRN_PMC2); > + host_os_sprs->pmc3 = mfspr(SPRN_PMC3); > + host_os_sprs->pmc4 = mfspr(SPRN_PMC4); > + host_os_sprs->pmc5 = mfspr(SPRN_PMC5); > + host_os_sprs->pmc6 = mfspr(SPRN_PMC6); > + host_os_sprs->mmcr1 = mfspr(SPRN_MMCR1); > + host_os_sprs->mmcr2 = mfspr(SPRN_MMCR2); > + host_os_sprs->sdar = mfspr(SPRN_SDAR); > + host_os_sprs->siar = mfspr(SPRN_SIAR); > + host_os_sprs->sier1 = mfspr(SPRN_SIER); > + > + if (cpu_has_feature(CPU_FTR_ARCH_31)) { > + host_os_sprs->mmcr3 = mfspr(SPRN_MMCR3); > + host_os_sprs->sier2 = mfspr(SPRN_SIER2); > + host_os_sprs->sier3 = mfspr(SPRN_SIER3); > + } > + } > +} > + > +static void load_p9_guest_pmu(struct kvm_vcpu *vcpu) > +{ > + mtspr(SPRN_PMC1, vcpu->arch.pmc[0]); > + mtspr(SPRN_PMC2,
[PATCH v1 17/55] KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
Implement the P9 path PMU save/restore code in C, and remove the POWER9/10 code from the P7/8 path assembly. -449 cycles (8533) POWER9 virt-mode NULL hcall Signed-off-by: Nicholas Piggin --- arch/powerpc/include/asm/asm-prototypes.h | 5 - arch/powerpc/kvm/book3s_hv.c | 205 -- arch/powerpc/kvm/book3s_hv_interrupts.S | 13 +- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 43 + 4 files changed, 200 insertions(+), 66 deletions(-) diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h index 222823861a67..41b8a1e1144a 100644 --- a/arch/powerpc/include/asm/asm-prototypes.h +++ b/arch/powerpc/include/asm/asm-prototypes.h @@ -141,11 +141,6 @@ static inline void kvmppc_restore_tm_hv(struct kvm_vcpu *vcpu, u64 msr, bool preserve_nv) { } #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */ -void kvmhv_save_host_pmu(void); -void kvmhv_load_host_pmu(void); -void kvmhv_save_guest_pmu(struct kvm_vcpu *vcpu, bool pmu_in_use); -void kvmhv_load_guest_pmu(struct kvm_vcpu *vcpu); - void kvmppc_p9_enter_guest(struct kvm_vcpu *vcpu); long kvmppc_h_set_dabr(struct kvm_vcpu *vcpu, unsigned long dabr); diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 2eef708c4354..d20b579ddcdf 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -3735,6 +3735,188 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc) trace_kvmppc_run_core(vc, 1); } +/* + * Privileged (non-hypervisor) host registers to save. + */ +struct p9_host_os_sprs { + unsigned long dscr; + unsigned long tidr; + unsigned long iamr; + unsigned long amr; + unsigned long fscr; + + unsigned int pmc1; + unsigned int pmc2; + unsigned int pmc3; + unsigned int pmc4; + unsigned int pmc5; + unsigned int pmc6; + unsigned long mmcr0; + unsigned long mmcr1; + unsigned long mmcr2; + unsigned long mmcr3; + unsigned long mmcra; + unsigned long siar; + unsigned long sier1; + unsigned long sier2; + unsigned long sier3; + unsigned long sdar; +}; + +static void freeze_pmu(unsigned long mmcr0, unsigned long mmcra) +{ + if (!(mmcr0 & MMCR0_FC)) + goto do_freeze; + if (mmcra & MMCRA_SAMPLE_ENABLE) + goto do_freeze; + if (cpu_has_feature(CPU_FTR_ARCH_31)) { + if (!(mmcr0 & MMCR0_PMCCEXT)) + goto do_freeze; + if (!(mmcra & MMCRA_BHRB_DISABLE)) + goto do_freeze; + } + return; + +do_freeze: + mmcr0 = MMCR0_FC; + mmcra = 0; + if (cpu_has_feature(CPU_FTR_ARCH_31)) { + mmcr0 |= MMCR0_PMCCEXT; + mmcra = MMCRA_BHRB_DISABLE; + } + + mtspr(SPRN_MMCR0, mmcr0); + mtspr(SPRN_MMCRA, mmcra); + isync(); +} + +static void save_p9_host_pmu(struct p9_host_os_sprs *host_os_sprs) +{ + if (ppc_get_pmu_inuse()) { + /* +* It might be better to put PMU handling (at least for the +* host) in the perf subsystem because it knows more about what +* is being used. +*/ + + /* POWER9, POWER10 do not implement HPMC or SPMC */ + + host_os_sprs->mmcr0 = mfspr(SPRN_MMCR0); + host_os_sprs->mmcra = mfspr(SPRN_MMCRA); + + freeze_pmu(host_os_sprs->mmcr0, host_os_sprs->mmcra); + + host_os_sprs->pmc1 = mfspr(SPRN_PMC1); + host_os_sprs->pmc2 = mfspr(SPRN_PMC2); + host_os_sprs->pmc3 = mfspr(SPRN_PMC3); + host_os_sprs->pmc4 = mfspr(SPRN_PMC4); + host_os_sprs->pmc5 = mfspr(SPRN_PMC5); + host_os_sprs->pmc6 = mfspr(SPRN_PMC6); + host_os_sprs->mmcr1 = mfspr(SPRN_MMCR1); + host_os_sprs->mmcr2 = mfspr(SPRN_MMCR2); + host_os_sprs->sdar = mfspr(SPRN_SDAR); + host_os_sprs->siar = mfspr(SPRN_SIAR); + host_os_sprs->sier1 = mfspr(SPRN_SIER); + + if (cpu_has_feature(CPU_FTR_ARCH_31)) { + host_os_sprs->mmcr3 = mfspr(SPRN_MMCR3); + host_os_sprs->sier2 = mfspr(SPRN_SIER2); + host_os_sprs->sier3 = mfspr(SPRN_SIER3); + } + } +} + +static void load_p9_guest_pmu(struct kvm_vcpu *vcpu) +{ + mtspr(SPRN_PMC1, vcpu->arch.pmc[0]); + mtspr(SPRN_PMC2, vcpu->arch.pmc[1]); + mtspr(SPRN_PMC3, vcpu->arch.pmc[2]); + mtspr(SPRN_PMC4, vcpu->arch.pmc[3]); + mtspr(SPRN_PMC5, vcpu->arch.pmc[4]); + mtspr(SPRN_PMC6, vcpu->arch.pmc[5]); + mtspr(SPRN_MMCR1, vcpu->arch.mmcr[1]); + mtspr(SPRN_MMCR2, vcpu->arch.mmcr[2]); + mtspr(SPRN_SDAR, vcpu->arch.sdar); + mtspr(SPRN_SIAR, vcpu->arch.siar); + mtspr(SPRN_SIER,