| Message ID | 20190520005659.18628-5-npiggin@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/5] KVM: PPC: Book3S: Define and use SRR1_MSR_BITS | expand |
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.
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. >
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; }
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(+)