Message ID | 20191024114059.102802-28-frankja@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: s390: Add support for protected VMs | expand |
On 24.10.19 13:40, Janosch Frank wrote: > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Can you add why this is necessary and how handle_stop() is intended to work in prot mode? How is SIGP handled in general in prot mode? (which intercepts are handled by QEMU) Would it be valid for user space to inject a STOP interrupt with "flags & KVM_S390_STOP_FLAG_STORE_STATUS" - I think not (legacy QEMU only) I think we should rather disallow injecting such stop interrupts (KVM_S390_STOP_FLAG_STORE_STATUS) in prot mode in the first place. Also, we should disallow prot virt without user_sigp. > --- > arch/s390/kvm/intercept.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c > index 37cb62bc261b..a89738e4f761 100644 > --- a/arch/s390/kvm/intercept.c > +++ b/arch/s390/kvm/intercept.c > @@ -72,7 +72,8 @@ static int handle_stop(struct kvm_vcpu *vcpu) > if (!stop_pending) > return 0; > > - if (flags & KVM_S390_STOP_FLAG_STORE_STATUS) { > + if (flags & KVM_S390_STOP_FLAG_STORE_STATUS && > + !kvm_s390_pv_is_protected(vcpu->kvm)) { > rc = kvm_s390_vcpu_store_status(vcpu, > KVM_S390_STORE_STATUS_NOADDR); > if (rc) >
On 24/10/2019 13.40, Janosch Frank wrote: > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/intercept.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c > index 37cb62bc261b..a89738e4f761 100644 > --- a/arch/s390/kvm/intercept.c > +++ b/arch/s390/kvm/intercept.c > @@ -72,7 +72,8 @@ static int handle_stop(struct kvm_vcpu *vcpu) > if (!stop_pending) > return 0; > > - if (flags & KVM_S390_STOP_FLAG_STORE_STATUS) { > + if (flags & KVM_S390_STOP_FLAG_STORE_STATUS && > + !kvm_s390_pv_is_protected(vcpu->kvm)) { > rc = kvm_s390_vcpu_store_status(vcpu, > KVM_S390_STORE_STATUS_NOADDR); > if (rc) Can this still happen at all that we get here with KVM_S390_STOP_FLAG_STORE_STATUS in the protected case? I'd rather expect that SIGP is completely handled by the UV already, so userspace should have no need to inject a SIGP_STOP anymore? Or did I get that wrong? Anyway, I guess it can not hurt to add this check anyway, so: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index 37cb62bc261b..a89738e4f761 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -72,7 +72,8 @@ static int handle_stop(struct kvm_vcpu *vcpu) if (!stop_pending) return 0; - if (flags & KVM_S390_STOP_FLAG_STORE_STATUS) { + if (flags & KVM_S390_STOP_FLAG_STORE_STATUS && + !kvm_s390_pv_is_protected(vcpu->kvm)) { rc = kvm_s390_vcpu_store_status(vcpu, KVM_S390_STORE_STATUS_NOADDR); if (rc)
Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- arch/s390/kvm/intercept.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)