Excerpts from Fabiano Rosas's message of March 3, 2021 1:04 am:
> Nicholas Piggin <npiggin@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>
> <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
