[PATCH v2 28/37] KVM: PPC: Book3S HV P9: Add helpers for OS SPR handling

[Nicholas Piggin]

Excerpts from Fabiano Rosas's message of March 3, 2021 1:04 am:
> Nicholas Piggin <npiggin at gmail.com> writes:
> 
>> This is a first step to wrapping supervisor and user SPR saving and
>> loading up into helpers, which will then be called independently in
>> bare metal and nested HV cases in order to optimise SPR access.
>>
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
> 
> <snip>
> 
>> +/* vcpu guest regs must already be saved */
>> +static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
>> +				    struct p9_host_os_sprs *host_os_sprs)
>> +{
>> +	mtspr(SPRN_PSPB, 0);
>> +	mtspr(SPRN_WORT, 0);
>> +	mtspr(SPRN_UAMOR, 0);
>> +	mtspr(SPRN_PSPB, 0);
> 
> Not your fault, but PSPB is set twice here.

Yeah you're right.

>> +
>> +	mtspr(SPRN_DSCR, host_os_sprs->dscr);
>> +	mtspr(SPRN_TIDR, host_os_sprs->tidr);
>> +	mtspr(SPRN_IAMR, host_os_sprs->iamr);
>> +
>> +	if (host_os_sprs->amr != vcpu->arch.amr)
>> +		mtspr(SPRN_AMR, host_os_sprs->amr);
>> +
>> +	if (host_os_sprs->fscr != vcpu->arch.fscr)
>> +		mtspr(SPRN_FSCR, host_os_sprs->fscr);
>> +}
>> +
> 
> <snip>
> 
>> @@ -3605,34 +3666,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>>  	vcpu->arch.dec_expires = dec + tb;
>>  	vcpu->cpu = -1;
>>  	vcpu->arch.thread_cpu = -1;
>> -	vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
>> -
>> -	vcpu->arch.iamr = mfspr(SPRN_IAMR);
>> -	vcpu->arch.pspb = mfspr(SPRN_PSPB);
>> -	vcpu->arch.fscr = mfspr(SPRN_FSCR);
>> -	vcpu->arch.tar = mfspr(SPRN_TAR);
>> -	vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
>> -	vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
>> -	vcpu->arch.bescr = mfspr(SPRN_BESCR);
>> -	vcpu->arch.wort = mfspr(SPRN_WORT);
>> -	vcpu->arch.tid = mfspr(SPRN_TIDR);
>> -	vcpu->arch.amr = mfspr(SPRN_AMR);
>> -	vcpu->arch.uamor = mfspr(SPRN_UAMOR);
>> -	vcpu->arch.dscr = mfspr(SPRN_DSCR);
>> -
>> -	mtspr(SPRN_PSPB, 0);
>> -	mtspr(SPRN_WORT, 0);
>> -	mtspr(SPRN_UAMOR, 0);
>> -	mtspr(SPRN_DSCR, host_dscr);
>> -	mtspr(SPRN_TIDR, host_tidr);
>> -	mtspr(SPRN_IAMR, host_iamr);
>> -	mtspr(SPRN_PSPB, 0);
>>
>> -	if (host_amr != vcpu->arch.amr)
>> -		mtspr(SPRN_AMR, host_amr);
>> +	restore_p9_host_os_sprs(vcpu, &host_os_sprs);
>>
>> -	if (host_fscr != vcpu->arch.fscr)
>> -		mtspr(SPRN_FSCR, host_fscr);
>> +	store_spr_state(vcpu);
> 
> store_spr_state should come first, right? We want to save the guest
> state before restoring the host state.

Yes good catch. I switched that back around later but looks like I
never brought the fix back to the right patch. Interestingly, things 
pretty much work like this if the guest or host doesn't do anything
much with the SPRs!

Thanks,
Nick