| Message ID | 20200203131957.383915-36-borntraeger@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add support for protected VMs | expand |
On Mon, 3 Feb 2020 08:19:55 -0500 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > From: Janosch Frank <frankja@linux.ibm.com> > > We're not allowed to inject interrupts on those intercept codes. As our Spell out what these codes actually are? > 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> > --- > arch/s390/kvm/kvm-s390.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index d4dc156e2c3e..137ae5dc9101 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4050,6 +4050,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; > @@ -4082,10 +4083,15 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > } > exit_reason = sie64a(vcpu->arch.sie_block, > vcpu->run->s.regs.gprs); > + /* This will likely be moved into a new function. */ Hm? > if (kvm_s390_pv_is_protected(vcpu->kvm)) { > memcpy(vcpu->run->s.regs.gprs, > sie_page->pv_grregs, > sizeof(sie_page->pv_grregs)); > + 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);
On 06.02.20 11:10, Cornelia Huck wrote: > On Mon, 3 Feb 2020 08:19:55 -0500 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> From: Janosch Frank <frankja@linux.ibm.com> >> >> We're not allowed to inject interrupts on those intercept codes. As our > > Spell out what these codes actually are? We're not allowed to inject interrupts on intercepts that leave the guest state in an "in-beetween" state where the next SIE entry will do a continuation. Namely secure instruction interception and secure prefix interception... >> @@ -4082,10 +4083,15 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) >> } >> exit_reason = sie64a(vcpu->arch.sie_block, >> vcpu->run->s.regs.gprs); >> + /* This will likely be moved into a new function. */ > > Hm? Unless you have a good naming for such function, will remove this comment. I tried that but I the end result was not better or worse. > >> if (kvm_s390_pv_is_protected(vcpu->kvm)) { >> memcpy(vcpu->run->s.regs.gprs, >> sie_page->pv_grregs, >> sizeof(sie_page->pv_grregs)); >> + 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); >
On 03/02/2020 14.19, Christian Borntraeger wrote: > From: Janosch Frank <frankja@linux.ibm.com> > > We're not allowed to inject interrupts on those intercept codes. 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> > --- > arch/s390/kvm/kvm-s390.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index d4dc156e2c3e..137ae5dc9101 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4050,6 +4050,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; > @@ -4082,10 +4083,15 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > } > exit_reason = sie64a(vcpu->arch.sie_block, > vcpu->run->s.regs.gprs); > + /* This will likely be moved into a new function. */ As already mentioned by Cornelia: Please remove the above comment. > if (kvm_s390_pv_is_protected(vcpu->kvm)) { > memcpy(vcpu->run->s.regs.gprs, > sie_page->pv_grregs, > sizeof(sie_page->pv_grregs)); ... but I'd like to suggest to add a comment before the following if-statement with some information from the patch description. Otherwise this will later be one of the code spots where you scratch your head while looking at it and wonder why this masking of bits is done here. > + 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); > Thomas
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index d4dc156e2c3e..137ae5dc9101 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4050,6 +4050,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; @@ -4082,10 +4083,15 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) } exit_reason = sie64a(vcpu->arch.sie_block, vcpu->run->s.regs.gprs); + /* This will likely be moved into a new function. */ if (kvm_s390_pv_is_protected(vcpu->kvm)) { memcpy(vcpu->run->s.regs.gprs, sie_page->pv_grregs, sizeof(sie_page->pv_grregs)); + 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);