| Message ID | 20200214222658.12946-34-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add support for protected VMs | expand |
On 14.02.20 23:26, Christian Borntraeger wrote: > From: Janosch Frank <frankja@linux.ibm.com> > > We're not allowed to inject interrupts on intercepts that leave the > guest state in an "in-between" state where the next SIE entry will do a > continuation, namely secure instruction interception (104) and secure > prefix interception (112). > As our PSW is just a copy of the real one that will be replaced on the > next exit, we can mask out the interrupt bits in the PSW to make sure > that we do not inject anything. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: Thomas Huth <thuth@redhat.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > [borntraeger@de.ibm.com: patch merging, splitting, fixing] > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index b6113285f47f..1b6963bbc96f 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4025,6 +4025,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason) > return vcpu_post_run_fault_in_sie(vcpu); > } > > +#define PSW_INT_MASK (PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_MCHECK) > static int __vcpu_run(struct kvm_vcpu *vcpu) > { > int rc, exit_reason; > @@ -4061,6 +4062,16 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > memcpy(vcpu->run->s.regs.gprs, > sie_page->pv_grregs, > sizeof(sie_page->pv_grregs)); > + /* > + * We're not allowed to inject interrupts on intercepts > + * that leave the guest state in an "in-between" state > + * where the next SIE entry will do a continuation. > + * Fence interrupts in our "internal" PSW. > + */ > + if (vcpu->arch.sie_block->icptcode == ICPT_PV_INSTR || > + vcpu->arch.sie_block->icptcode == ICPT_PV_PREF) { > + vcpu->arch.sie_block->gpsw.mask &= ~PSW_INT_MASK; > + } Is this actually the right approach? I mean, you lose PSW masks, but you only want to teach the interrupt delivery code to skip delivering interrupts until the intercept has been handled. Does not look clean to me TBH.
On 18.02.20 10:53, David Hildenbrand wrote: > On 14.02.20 23:26, Christian Borntraeger wrote: >> From: Janosch Frank <frankja@linux.ibm.com> >> >> We're not allowed to inject interrupts on intercepts that leave the >> guest state in an "in-between" state where the next SIE entry will do a >> continuation, namely secure instruction interception (104) and secure >> prefix interception (112). >> As our PSW is just a copy of the real one that will be replaced on the >> next exit, we can mask out the interrupt bits in the PSW to make sure >> that we do not inject anything. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> [borntraeger@de.ibm.com: patch merging, splitting, fixing] >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/kvm/kvm-s390.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index b6113285f47f..1b6963bbc96f 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -4025,6 +4025,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason) >> return vcpu_post_run_fault_in_sie(vcpu); >> } >> >> +#define PSW_INT_MASK (PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_MCHECK) >> static int __vcpu_run(struct kvm_vcpu *vcpu) >> { >> int rc, exit_reason; >> @@ -4061,6 +4062,16 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) >> memcpy(vcpu->run->s.regs.gprs, >> sie_page->pv_grregs, >> sizeof(sie_page->pv_grregs)); >> + /* >> + * We're not allowed to inject interrupts on intercepts >> + * that leave the guest state in an "in-between" state >> + * where the next SIE entry will do a continuation. >> + * Fence interrupts in our "internal" PSW. >> + */ >> + if (vcpu->arch.sie_block->icptcode == ICPT_PV_INSTR || >> + vcpu->arch.sie_block->icptcode == ICPT_PV_PREF) { >> + vcpu->arch.sie_block->gpsw.mask &= ~PSW_INT_MASK; >> + } > > Is this actually the right approach? I mean, you lose PSW masks, but you > only want to teach the interrupt delivery code to skip delivering > interrupts until the intercept has been handled. Does not look clean to > me TBH. After reading the patch description again :) Reviewed-by: David Hildenbrand <david@redhat.com>
On 18.02.20 10:53, David Hildenbrand wrote: > On 14.02.20 23:26, Christian Borntraeger wrote: >> From: Janosch Frank <frankja@linux.ibm.com> >> >> We're not allowed to inject interrupts on intercepts that leave the >> guest state in an "in-between" state where the next SIE entry will do a >> continuation, namely secure instruction interception (104) and secure >> prefix interception (112). >> As our PSW is just a copy of the real one that will be replaced on the >> next exit, we can mask out the interrupt bits in the PSW to make sure >> that we do not inject anything. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Thomas Huth <thuth@redhat.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> [borntraeger@de.ibm.com: patch merging, splitting, fixing] >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> --- >> arch/s390/kvm/kvm-s390.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index b6113285f47f..1b6963bbc96f 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -4025,6 +4025,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason) >> return vcpu_post_run_fault_in_sie(vcpu); >> } >> >> +#define PSW_INT_MASK (PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_MCHECK) >> static int __vcpu_run(struct kvm_vcpu *vcpu) >> { >> int rc, exit_reason; >> @@ -4061,6 +4062,16 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) >> memcpy(vcpu->run->s.regs.gprs, >> sie_page->pv_grregs, >> sizeof(sie_page->pv_grregs)); >> + /* >> + * We're not allowed to inject interrupts on intercepts >> + * that leave the guest state in an "in-between" state >> + * where the next SIE entry will do a continuation. >> + * Fence interrupts in our "internal" PSW. >> + */ >> + if (vcpu->arch.sie_block->icptcode == ICPT_PV_INSTR || >> + vcpu->arch.sie_block->icptcode == ICPT_PV_PREF) { >> + vcpu->arch.sie_block->gpsw.mask &= ~PSW_INT_MASK; >> + } > > Is this actually the right approach? I mean, you lose PSW masks, but you > only want to teach the interrupt delivery code to skip delivering > interrupts until the intercept has been handled. Does not look clean to > me TBH. The only purpose of the PSW mask in PV is to tell the hypervisor what interrupts are deliverable or if we are in wait state. In fact those bits are the only ones that we see. So I think its fair to use those.
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index b6113285f47f..1b6963bbc96f 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4025,6 +4025,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason) return vcpu_post_run_fault_in_sie(vcpu); } +#define PSW_INT_MASK (PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_MCHECK) static int __vcpu_run(struct kvm_vcpu *vcpu) { int rc, exit_reason; @@ -4061,6 +4062,16 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) memcpy(vcpu->run->s.regs.gprs, sie_page->pv_grregs, sizeof(sie_page->pv_grregs)); + /* + * We're not allowed to inject interrupts on intercepts + * that leave the guest state in an "in-between" state + * where the next SIE entry will do a continuation. + * Fence interrupts in our "internal" PSW. + */ + if (vcpu->arch.sie_block->icptcode == ICPT_PV_INSTR || + vcpu->arch.sie_block->icptcode == ICPT_PV_PREF) { + vcpu->arch.sie_block->gpsw.mask &= ~PSW_INT_MASK; + } } local_irq_disable(); __enable_cpu_timer_accounting(vcpu);