Re: [RFC PATCH 27/43] KVM: PPC: Book3S HV P9: Move host OS save/restore functions to built-in

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Athira Rajeev's message of July 8, 2021 3:32 pm:
> 
> 
>> On 22-Jun-2021, at 4:27 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>> 
>> Move the P9 guest/host register switching functions to the built-in
>> P9 entry code, and export it for nested to use as well.
>> 
>> This allows more flexibility in scheduling these supervisor privileged
>> SPR accesses with the HV privileged and PR SPR accesses in the low level
>> entry code.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c          | 351 +-------------------------
>> arch/powerpc/kvm/book3s_hv.h          |  39 +++
>> arch/powerpc/kvm/book3s_hv_p9_entry.c | 332 ++++++++++++++++++++++++
>> 3 files changed, 372 insertions(+), 350 deletions(-)
>> create mode 100644 arch/powerpc/kvm/book3s_hv.h
>> 
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 35749b0b663f..a7660af22161 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -79,6 +79,7 @@
>> #include <asm/dtl.h>
>> 
>> #include "book3s.h"
>> +#include "book3s_hv.h"
>> 
>> #define CREATE_TRACE_POINTS
>> #include "trace_hv.h"
>> @@ -3675,356 +3676,6 @@ 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 switch_pmu_to_guest(struct kvm_vcpu *vcpu,
>> -				struct p9_host_os_sprs *host_os_sprs)
>> -{
>> -	struct lppaca *lp;
>> -	int load_pmu = 1;
>> -
>> -	lp = vcpu->arch.vpa.pinned_addr;
>> -	if (lp)
>> -		load_pmu = lp->pmcregs_in_use;
>> -
>> -	if (load_pmu)
>> -	      vcpu->arch.hfscr |= HFSCR_PM;
>> -
>> -	/* Save host */
>> -	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);
>> -		}
>> -	}
>> -
>> -#ifdef CONFIG_PPC_PSERIES
>> -	if (kvmhv_on_pseries()) {
>> -		if (vcpu->arch.vpa.pinned_addr) {
>> -			struct lppaca *lp = vcpu->arch.vpa.pinned_addr;
>> -			get_lppaca()->pmcregs_in_use = lp->pmcregs_in_use;
>> -		} else {
>> -			get_lppaca()->pmcregs_in_use = 1;
>> -		}
>> -	}
>> -#endif
>> -
>> -	/* Load guest */
>> -	if (vcpu->arch.hfscr & HFSCR_PM) {
>> -		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, vcpu->arch.sier[0]);
>> -
>> -		if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>> -			mtspr(SPRN_MMCR3, vcpu->arch.mmcr[4]);
> 
> 
> Hi Nick,
> 
> Have a doubt here..
> For MMCR3, it is  vcpu->arch.mmcr[3) ?

Hey, yea it is you're right. Good catch.

Thanks,
Nick