[v1,01/12] KVM: PPC: Book3S HV P9: Restore host CTRL SPR after guest exit

The host CTRL (runlatch) value is not restored after guest exit. The
host CTRL should always be 1 except in CPU idle code, so this can result
in the host running with runlatch clear, and potentially switching to
a different vCPU which then runs with runlatch clear as well.

This has little effect on P9 machines, CTRL is only responsible for some
PMU counter logic in the host and so other than corner cases of software
relying on that, or explicitly reading the runlatch value (Linux does
not appear to be affected but it's possible non-Linux guests could be),
there should be no execution correctness problem, though it could be
used as a covert channel between guests.

There may be microcontrollers, firmware or monitoring tools that sample
the runlatch value out-of-band, however since the register is writable
by guests, these values would (should) not be relied upon for correct
operation of the host, so suboptimal performance or incorrect reporting
should be the worst problem.

Fixes: 95a6432ce9038 ("KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 3 +++
 1 file changed, 3 insertions(+)

Nicholas Piggin <npiggin@gmail.com> writes:

> The host CTRL (runlatch) value is not restored after guest exit. The
> host CTRL should always be 1 except in CPU idle code, so this can result
> in the host running with runlatch clear, and potentially switching to
> a different vCPU which then runs with runlatch clear as well.
>
> This has little effect on P9 machines, CTRL is only responsible for some
> PMU counter logic in the host and so other than corner cases of software
> relying on that, or explicitly reading the runlatch value (Linux does
> not appear to be affected but it's possible non-Linux guests could be),
> there should be no execution correctness problem, though it could be
> used as a covert channel between guests.
>
> There may be microcontrollers, firmware or monitoring tools that sample
> the runlatch value out-of-band, however since the register is writable
> by guests, these values would (should) not be relied upon for correct
> operation of the host, so suboptimal performance or incorrect reporting
> should be the worst problem.
>
> Fixes: 95a6432ce9038 ("KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kvm/book3s_hv.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 13bad6bf4c95..208a053c9adf 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3728,7 +3728,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;
> +	/* Save guest CTRL register, set runlatch to 1 */
>  	vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
> +	if (!(vcpu->arch.ctrl & 1))
> +		mtspr(SPRN_CTRLT, vcpu->arch.ctrl | 1);

Maybe ditch the comment and use the already defined CTRL_RUNLATCH?

>
>  	vcpu->arch.iamr = mfspr(SPRN_IAMR);
>  	vcpu->arch.pspb = mfspr(SPRN_PSPB);

Excerpts from Fabiano Rosas's message of April 13, 2021 12:06 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> The host CTRL (runlatch) value is not restored after guest exit. The
>> host CTRL should always be 1 except in CPU idle code, so this can result
>> in the host running with runlatch clear, and potentially switching to
>> a different vCPU which then runs with runlatch clear as well.
>>
>> This has little effect on P9 machines, CTRL is only responsible for some
>> PMU counter logic in the host and so other than corner cases of software
>> relying on that, or explicitly reading the runlatch value (Linux does
>> not appear to be affected but it's possible non-Linux guests could be),
>> there should be no execution correctness problem, though it could be
>> used as a covert channel between guests.
>>
>> There may be microcontrollers, firmware or monitoring tools that sample
>> the runlatch value out-of-band, however since the register is writable
>> by guests, these values would (should) not be relied upon for correct
>> operation of the host, so suboptimal performance or incorrect reporting
>> should be the worst problem.
>>
>> Fixes: 95a6432ce9038 ("KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests")
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/kvm/book3s_hv.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 13bad6bf4c95..208a053c9adf 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -3728,7 +3728,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;
>> +	/* Save guest CTRL register, set runlatch to 1 */
>>  	vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
>> +	if (!(vcpu->arch.ctrl & 1))
>> +		mtspr(SPRN_CTRLT, vcpu->arch.ctrl | 1);
> 
> Maybe ditch the comment and use the already defined CTRL_RUNLATCH?

I did it this way so you can more easily match up the C with the 
existing asm version.

I have a later patch to clean up CTRL handling a bit (in both C and 
asm).

Thanks,
Nick