Re: [PATCH v3 01/41] KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from Fabiano Rosas's message of March 9, 2021 1:26 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> These are already disallowed by H_SET_MODE from the guest, also disallow
>> these by updating LPCR directly.
>>
>> AIL modes can affect the host interrupt behaviour while the guest LPCR
>> value is set, so filter it here too.
>>
>> Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/kvm/book3s_hv.c        | 11 +++++++++--
>>  arch/powerpc/kvm/book3s_hv_nested.c |  7 +++++--
>>  2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 13bad6bf4c95..c40eeb20be39 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -803,7 +803,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>>  		vcpu->arch.dawrx1 = value2;
>>  		return H_SUCCESS;
>>  	case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
>> -		/* KVM does not support mflags=2 (AIL=2) */
>> +		/*
>> +		 * KVM does not support mflags=2 (AIL=2) and AIL=1 is reserved.
>> +		 * Keep this in synch with kvmppc_set_lpcr.
>> +		 */
>>  		if (mflags != 0 && mflags != 3)
>>  			return H_UNSUPPORTED_FLAG_START;
>>  		return H_TOO_HARD;
>> @@ -1667,8 +1670,12 @@ static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu, u64 new_lpcr,
>>  	 * On POWER8 and POWER9 userspace can also modify AIL (alt. interrupt loc.).
>>  	 */
>>  	mask = LPCR_DPFD | LPCR_ILE | LPCR_TC;
>> -	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> +	if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>  		mask |= LPCR_AIL;
>> +		/* LPCR[AIL]=1/2 is disallowed */
>> +		if ((new_lpcr & LPCR_AIL) && (new_lpcr & LPCR_AIL) != LPCR_AIL_3)
>> +			new_lpcr &= ~LPCR_AIL;
>> +	}
>>  	/*
>>  	 * On POWER9, allow userspace to enable large decrementer for the
>>  	 * guest, whether or not the host has it enabled.
>> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
>> index 2fe1fea4c934..b496079e02f7 100644
>> --- a/arch/powerpc/kvm/book3s_hv_nested.c
>> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
>> @@ -139,9 +139,12 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> 
> We're missing the patch that moves the lpcr setting into
> sanitise_hv_regs.

Oh yes sorry, mistyped the format-patch command.

>>  
>>  	/*
>>  	 * Don't let L1 change LPCR bits for the L2 except these:
>> +	 * Keep this in sync with kvmppc_set_lpcr.
>>  	 */
>> -	mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
>> -		LPCR_LPES | LPCR_MER;
>> +	mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_LD | LPCR_LPES | LPCR_MER;
> 
> I think this line's change belongs in patch 33 doesn't it? Otherwise you
> are clearing a bit below that is not present in the mask so it would
> never be used anyway.

Ah yes, thank you. Will fix.

> 
>> +	/* LPCR[AIL]=1/2 is disallowed */
>> +	if ((hr->lpcr & LPCR_AIL) && (hr->lpcr & LPCR_AIL) != LPCR_AIL_3)
>> +		hr->lpcr &= ~LPCR_AIL;
>>  	hr->lpcr = (vc->lpcr & ~mask) | (hr->lpcr & mask);
>>  
>>  	/*
>