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

[Fabiano Rosas <farosas@linux.ibm.com>]

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.

>  
>  	/*
>  	 * 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.

> +	/* 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);
>  
>  	/*