[PATCH 5/5] KVM: PPC: Book3S HV: Reject mflags=2 (LPCR[AIL]=2) ADDR_TRANS_MODE mode

[PATCH 5/5] KVM: PPC: Book3S HV: Reject mflags=2 (LPCR[AIL]=2) ADDR_TRANS_MODE mode

[Nicholas Piggin]

AIL=2 mode has no known users, so is not well tested or supported.
Disallow guests from selecting this mode because it may become
deprecated in future versions of the architecture.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 36d16740748a..4295ccdbee26 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -784,6 +784,11 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
 		vcpu->arch.dawr  = value1;
 		vcpu->arch.dawrx = value2;
 		return H_SUCCESS;
+	case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
+		/* KVM does not support mflags=2 (AIL=2) */
+		if (mflags != 0 && mflags != 3)
+			return H_UNSUPPORTED_FLAG_START;
+		return H_TOO_HARD;
 	default:
 		return H_TOO_HARD;
 	}
-- 
2.20.1

Re: [PATCH 5/5] KVM: PPC: Book3S HV: Reject mflags=2 (LPCR[AIL]=2) ADDR_TRANS_MODE mode

[Paul Mackerras]

On Mon, May 20, 2019 at 10:56:59AM +1000, Nicholas Piggin wrote:
> AIL=2 mode has no known users, so is not well tested or supported.
> Disallow guests from selecting this mode because it may become
> deprecated in future versions of the architecture.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Given that H_SET_MODE_RESOURCE_ADDR_TRANS_MODE gets punted to
userspace (QEMU), why are we enforcing this here rather than in QEMU?

If there is a reason to do this here rather than in QEMU, then the
patch description should really comment on why we're rejecting AIL=1
as well as AIL=2.

> ---
>  arch/powerpc/kvm/book3s_hv.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 36d16740748a..4295ccdbee26 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -784,6 +784,11 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>  		vcpu->arch.dawr  = value1;
>  		vcpu->arch.dawrx = value2;
>  		return H_SUCCESS;
> +	case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
> +		/* KVM does not support mflags=2 (AIL=2) */
> +		if (mflags != 0 && mflags != 3)
> +			return H_UNSUPPORTED_FLAG_START;
> +		return H_TOO_HARD;
>  	default:
>  		return H_TOO_HARD;
>  	}
> -- 
> 2.20.1

Paul.

Re: [PATCH 5/5] KVM: PPC: Book3S HV: Reject mflags=2 (LPCR[AIL]=2) ADDR_TRANS_MODE mode

[Nicholas Piggin]

Paul Mackerras's on June 17, 2019 3:58 pm:
> On Mon, May 20, 2019 at 10:56:59AM +1000, Nicholas Piggin wrote:
>> AIL=2 mode has no known users, so is not well tested or supported.
>> Disallow guests from selecting this mode because it may become
>> deprecated in future versions of the architecture.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> Given that H_SET_MODE_RESOURCE_ADDR_TRANS_MODE gets punted to
> userspace (QEMU), why are we enforcing this here rather than in QEMU?

QEMU (appears to) support AIL=2. I don't propose to remove that
support (which could in theory be a regression for users). It
can't have ever worked properly for KVM guests though AFAIKS. Is
that reasonable?

> If there is a reason to do this here rather than in QEMU, then the
> patch description should really comment on why we're rejecting AIL=1
> as well as AIL=2.

Sure.

Thanks for review of the other patches as well, I think all your
points were valid so I'll fix them up before reposting.

Thanks,
Nick

> 
>> ---
>>  arch/powerpc/kvm/book3s_hv.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 36d16740748a..4295ccdbee26 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -784,6 +784,11 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>>  		vcpu->arch.dawr  = value1;
>>  		vcpu->arch.dawrx = value2;
>>  		return H_SUCCESS;
>> +	case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
>> +		/* KVM does not support mflags=2 (AIL=2) */
>> +		if (mflags != 0 && mflags != 3)
>> +			return H_UNSUPPORTED_FLAG_START;
>> +		return H_TOO_HARD;
>>  	default:
>>  		return H_TOO_HARD;
>>  	}
>> -- 
>> 2.20.1
> 
> Paul.
>