diff mbox series

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

Message ID 20210305150638.2675513-2-npiggin@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series KVM: PPC: Book3S: C-ify the P9 entry/exit code | expand
Related show

Commit Message

Nicholas Piggin March 5, 2021, 3:05 p.m. UTC
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(-)

Comments

Fabiano Rosas March 8, 2021, 3:26 p.m. UTC | #1
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);
>  
>  	/*
Nicholas Piggin March 9, 2021, 1:11 a.m. UTC | #2
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);
>>  
>>  	/*
>
diff mbox series

Patch

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