| Message ID | 1549014725-28216-2-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] KVM: s390: vsie: fix Do the CRYCB validation first | expand |
On Fri, 1 Feb 2019 10:52:05 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > The case when the SIE for guest3 is not setup for using > encryption keys nor Adjunct processor but the guest2 > does use these features was not properly handled. > > This leads SIE entry for guest3 to crash with validity intercept > because the guest2, not having the use of encryption keys nor > Adjunct Processor did not initialize the CRYCB designation. > > In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA > are set in guest3 a format 0 CRYCB is allowed for guest3 > and the CRYCB designation in the SIE for guest3 is not checked > on SIE entry. > > Let's allow the CRYCD designation to be ignored when the s/CRYCD/CRYCB/ > SIE for guest3 is not initialized for encryption key usage > nor AP. > > Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) I think the canonical format is Fixes: d6f6959ac587 ("KVM: s390: vsie: Do the CRYCB validation first") > CC: stable@vger.kernel.org # 4.20+ > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/kvm/vsie.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index a153257..a748f76 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > if (!apie_h && !key_msk) > return 0; > > + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) > + return 0; > + > if (!crycb_addr) > return set_validity_icpt(scb_s, 0x0039U); > Looks sane to me, but I'll let vsie experts comment.
On 01.02.19 10:52, Pierre Morel wrote: > The case when the SIE for guest3 is not setup for using > encryption keys nor Adjunct processor but the guest2 > does use these features was not properly handled. > > This leads SIE entry for guest3 to crash with validity intercept > because the guest2, not having the use of encryption keys nor > Adjunct Processor did not initialize the CRYCB designation. > > In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA > are set in guest3 a format 0 CRYCB is allowed for guest3 > and the CRYCB designation in the SIE for guest3 is not checked > on SIE entry. > > Let's allow the CRYCD designation to be ignored when the > SIE for guest3 is not initialized for encryption key usage > nor AP. > > Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/kvm/vsie.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index a153257..a748f76 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > if (!apie_h && !key_msk) > return 0; > > + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) > + return 0; > + > if (!crycb_addr) > return set_validity_icpt(scb_s, 0x0039U); > > The original patch said "We need to handle the validity checks for the crycb, no matter what the settings for the keywrappings are. So lets move the keywrapping checks after we have done the validy checks." Can you explain why keywrapping now is important? These patches seem to contradict.
On 01/02/2019 11:50, Cornelia Huck wrote: > On Fri, 1 Feb 2019 10:52:05 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> The case when the SIE for guest3 is not setup for using >> encryption keys nor Adjunct processor but the guest2 >> does use these features was not properly handled. >> >> This leads SIE entry for guest3 to crash with validity intercept >> because the guest2, not having the use of encryption keys nor >> Adjunct Processor did not initialize the CRYCB designation. >> >> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >> are set in guest3 a format 0 CRYCB is allowed for guest3 >> and the CRYCB designation in the SIE for guest3 is not checked >> on SIE entry. >> >> Let's allow the CRYCD designation to be ignored when the > > s/CRYCD/CRYCB/ > >> SIE for guest3 is not initialized for encryption key usage >> nor AP. >> >> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) > > I think the canonical format is > > Fixes: d6f6959ac587 ("KVM: s390: vsie: Do the CRYCB validation first") > >> > > CC: stable@vger.kernel.org # 4.20+ > >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> --- >> arch/s390/kvm/vsie.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index a153257..a748f76 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> if (!apie_h && !key_msk) >> return 0; >> >> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >> + return 0; >> + >> if (!crycb_addr) >> return set_validity_icpt(scb_s, 0x0039U); >> > > Looks sane to me, but I'll let vsie experts comment. > Thanks Pierre
On 01/02/2019 11:56, David Hildenbrand wrote: > On 01.02.19 10:52, Pierre Morel wrote: >> The case when the SIE for guest3 is not setup for using >> encryption keys nor Adjunct processor but the guest2 >> does use these features was not properly handled. >> >> This leads SIE entry for guest3 to crash with validity intercept >> because the guest2, not having the use of encryption keys nor >> Adjunct Processor did not initialize the CRYCB designation. >> >> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >> are set in guest3 a format 0 CRYCB is allowed for guest3 >> and the CRYCB designation in the SIE for guest3 is not checked >> on SIE entry. >> >> Let's allow the CRYCD designation to be ignored when the >> SIE for guest3 is not initialized for encryption key usage >> nor AP. >> >> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> --- >> arch/s390/kvm/vsie.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index a153257..a748f76 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> if (!apie_h && !key_msk) >> return 0; >> >> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >> + return 0; >> + >> if (!crycb_addr) >> return set_validity_icpt(scb_s, 0x0039U); >> >> > > The original patch said > > "We need to handle the validity checks for the crycb, no matter what the > settings for the keywrappings are. So lets move the keywrapping checks > after we have done the validy checks." > > Can you explain why keywrapping now is important? These patches seem to > contradict. > No it does not, having the flags set or not is part of the validity check. but, I acted too fast. The problem seems to be even clearer: key_msk is defined as int key_msk = test_kvm_facility(vcpu->kvm, 76); If it is defined, as it should for a mask, as (scb_o->ecb3 & (ECB3_AES | ECB3_DEA)) all is clear..., key_msk is not use but for this test, so I do not understand why it is set as facility 76. so I think I better do: diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index a153257..30843a8 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -289,7 +289,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) unsigned long *b1, *b2; u8 ecb3_flags; int apie_h; - int key_msk = test_kvm_facility(vcpu->kvm, 76); + int key_msk = scb_o->ecb3 & (ECB3_AES | ECB3_DEA); int fmt_o = crycbd_o & CRYCB_FORMAT_MASK; int fmt_h = vcpu->arch.sie_block->crycbd & CRYCB_FORMAT_MASK; int ret = 0; So just define a mask a mask. I verify the functionality and test on Monday and if in between it seems better to you so too I post the patch. Thanks, Pierre
On 01.02.2019 14:37, Pierre Morel wrote: > On 01/02/2019 11:56, David Hildenbrand wrote: >> On 01.02.19 10:52, Pierre Morel wrote: >>> The case when the SIE for guest3 is not setup for using >>> encryption keys nor Adjunct processor but the guest2 >>> does use these features was not properly handled. >>> >>> This leads SIE entry for guest3 to crash with validity intercept >>> because the guest2, not having the use of encryption keys nor >>> Adjunct Processor did not initialize the CRYCB designation. >>> >>> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >>> are set in guest3 a format 0 CRYCB is allowed for guest3 >>> and the CRYCB designation in the SIE for guest3 is not checked >>> on SIE entry. >>> >>> Let's allow the CRYCD designation to be ignored when the >>> SIE for guest3 is not initialized for encryption key usage >>> nor AP. >>> >>> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>> --- >>> arch/s390/kvm/vsie.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>> index a153257..a748f76 100644 >>> --- a/arch/s390/kvm/vsie.c >>> +++ b/arch/s390/kvm/vsie.c >>> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>> if (!apie_h && !key_msk) >>> return 0; >>> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >>> + return 0; >>> + >>> if (!crycb_addr) >>> return set_validity_icpt(scb_s, 0x0039U); >>> >> >> The original patch said >> >> "We need to handle the validity checks for the crycb, no matter what the >> settings for the keywrappings are. So lets move the keywrapping checks >> after we have done the validy checks." >> >> Can you explain why keywrapping now is important? These patches seem to >> contradict. >> > > No it does not, having the flags set or not is part of the validity check. > but, I acted too fast. > > The problem seems to be even clearer: > key_msk is defined as > int key_msk = test_kvm_facility(vcpu->kvm, 76); > > If it is defined, as it should for a mask, as > (scb_o->ecb3 & (ECB3_AES | ECB3_DEA)) > > all is clear..., key_msk is not use but for this test, so I do not understand why it is set as facility 76. > > so I think I better do: > > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index a153257..30843a8 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -289,7 +289,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > unsigned long *b1, *b2; > u8 ecb3_flags; > int apie_h; > - int key_msk = test_kvm_facility(vcpu->kvm, 76); > + int key_msk = scb_o->ecb3 & (ECB3_AES | ECB3_DEA); > int fmt_o = crycbd_o & CRYCB_FORMAT_MASK; > int fmt_h = vcpu->arch.sie_block->crycbd & CRYCB_FORMAT_MASK; > int ret = 0; > > > So just define a mask a mask. > I verify the functionality and test on Monday and if in between it seems better to you so too I post the patch. Can you spin a v2 with an improved patch description outlining the problem according to this discussion?
On 01.02.19 14:37, Pierre Morel wrote: > On 01/02/2019 11:56, David Hildenbrand wrote: >> On 01.02.19 10:52, Pierre Morel wrote: >>> The case when the SIE for guest3 is not setup for using >>> encryption keys nor Adjunct processor but the guest2 >>> does use these features was not properly handled. >>> >>> This leads SIE entry for guest3 to crash with validity intercept >>> because the guest2, not having the use of encryption keys nor >>> Adjunct Processor did not initialize the CRYCB designation. >>> >>> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >>> are set in guest3 a format 0 CRYCB is allowed for guest3 >>> and the CRYCB designation in the SIE for guest3 is not checked >>> on SIE entry. >>> >>> Let's allow the CRYCD designation to be ignored when the >>> SIE for guest3 is not initialized for encryption key usage >>> nor AP. >>> >>> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>> --- >>> arch/s390/kvm/vsie.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>> index a153257..a748f76 100644 >>> --- a/arch/s390/kvm/vsie.c >>> +++ b/arch/s390/kvm/vsie.c >>> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>> if (!apie_h && !key_msk) >>> return 0; >>> >>> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >>> + return 0; >>> + >>> if (!crycb_addr) >>> return set_validity_icpt(scb_s, 0x0039U); >>> >>> >> >> The original patch said >> >> "We need to handle the validity checks for the crycb, no matter what the >> settings for the keywrappings are. So lets move the keywrapping checks >> after we have done the validy checks." >> >> Can you explain why keywrapping now is important? These patches seem to >> contradict. >> > > No it does not, having the flags set or not is part of the validity check. > but, I acted too fast. > > The problem seems to be even clearer: > key_msk is defined as > int key_msk = test_kvm_facility(vcpu->kvm, 76); > > If it is defined, as it should for a mask, as > (scb_o->ecb3 & (ECB3_AES | ECB3_DEA)) > > all is clear..., key_msk is not use but for this test, so I do not > understand why it is set as facility 76. > > so I think I better do: > > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index a153257..30843a8 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -289,7 +289,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, > struct vsie_page *vsie_page) > unsigned long *b1, *b2; > u8 ecb3_flags; > int apie_h; > - int key_msk = test_kvm_facility(vcpu->kvm, 76); > + int key_msk = scb_o->ecb3 & (ECB3_AES | ECB3_DEA); > int fmt_o = crycbd_o & CRYCB_FORMAT_MASK; > int fmt_h = vcpu->arch.sie_block->crycbd & CRYCB_FORMAT_MASK; > int ret = 0; > > > So just define a mask a mask. > I verify the functionality and test on Monday and if in between it > seems better to you so too I post the patch. Without documentation at hand I cannot really judge what is the right approach. You have to read the architecture description very carefully and thimk about the different scenarios. I cannot help here. test_kvm_facility(vcpu->kvm, 76) means "hardware knows key wrapping exists" scb_o->ecb3 & (ECB3_AES | ECB3_DEA) means "key wrapping was actually enabled for AES or DEA" > > Thanks, > Pierre >
On 04/02/2019 16:15, David Hildenbrand wrote: > On 01.02.19 14:37, Pierre Morel wrote: >> On 01/02/2019 11:56, David Hildenbrand wrote: >>> On 01.02.19 10:52, Pierre Morel wrote: >>>> The case when the SIE for guest3 is not setup for using >>>> encryption keys nor Adjunct processor but the guest2 >>>> does use these features was not properly handled. >>>> >>>> This leads SIE entry for guest3 to crash with validity intercept >>>> because the guest2, not having the use of encryption keys nor >>>> Adjunct Processor did not initialize the CRYCB designation. >>>> >>>> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >>>> are set in guest3 a format 0 CRYCB is allowed for guest3 >>>> and the CRYCB designation in the SIE for guest3 is not checked >>>> on SIE entry. >>>> >>>> Let's allow the CRYCD designation to be ignored when the >>>> SIE for guest3 is not initialized for encryption key usage >>>> nor AP. >>>> >>>> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>>> --- >>>> arch/s390/kvm/vsie.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>> index a153257..a748f76 100644 >>>> --- a/arch/s390/kvm/vsie.c >>>> +++ b/arch/s390/kvm/vsie.c >>>> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>> if (!apie_h && !key_msk) >>>> return 0; >>>> >>>> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >>>> + return 0; >>>> + >>>> if (!crycb_addr) >>>> return set_validity_icpt(scb_s, 0x0039U); >>>> >>>> >>> >>> The original patch said >>> >>> "We need to handle the validity checks for the crycb, no matter what the >>> settings for the keywrappings are. So lets move the keywrapping checks >>> after we have done the validy checks." >>> >>> Can you explain why keywrapping now is important? These patches seem to >>> contradict. >>> >> >> No it does not, having the flags set or not is part of the validity check. >> but, I acted too fast. >> >> The problem seems to be even clearer: >> key_msk is defined as >> int key_msk = test_kvm_facility(vcpu->kvm, 76); >> >> If it is defined, as it should for a mask, as >> (scb_o->ecb3 & (ECB3_AES | ECB3_DEA)) >> >> all is clear..., key_msk is not use but for this test, so I do not >> understand why it is set as facility 76. >> >> so I think I better do: >> >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index a153257..30843a8 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -289,7 +289,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, >> struct vsie_page *vsie_page) >> unsigned long *b1, *b2; >> u8 ecb3_flags; >> int apie_h; >> - int key_msk = test_kvm_facility(vcpu->kvm, 76); >> + int key_msk = scb_o->ecb3 & (ECB3_AES | ECB3_DEA); >> int fmt_o = crycbd_o & CRYCB_FORMAT_MASK; >> int fmt_h = vcpu->arch.sie_block->crycbd & CRYCB_FORMAT_MASK; >> int ret = 0; >> >> >> So just define a mask a mask. >> I verify the functionality and test on Monday and if in between it >> seems better to you so too I post the patch. > > Without documentation at hand I cannot really judge what is the right > approach. You have to read the architecture description very carefully > and thimk about the different scenarios. I cannot help here. > > test_kvm_facility(vcpu->kvm, 76) means "hardware knows key wrapping exists" > > scb_o->ecb3 & (ECB3_AES | ECB3_DEA) means "key wrapping was actually > enabled for AES or DEA" Right. We also must take care of the format. So not sure my propositions are right. And helas I have the documentation :) So ... I have to read it again again. Regards, Pierre > >> >> Thanks, >> Pierre >> > >
On 04/02/2019 15:48, Christian Borntraeger wrote: > > > On 01.02.2019 14:37, Pierre Morel wrote: >> On 01/02/2019 11:56, David Hildenbrand wrote: >>> On 01.02.19 10:52, Pierre Morel wrote: >>>> The case when the SIE for guest3 is not setup for using >>>> encryption keys nor Adjunct processor but the guest2 >>>> does use these features was not properly handled. >>>> >>>> This leads SIE entry for guest3 to crash with validity intercept >>>> because the guest2, not having the use of encryption keys nor >>>> Adjunct Processor did not initialize the CRYCB designation. >>>> >>>> In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA >>>> are set in guest3 a format 0 CRYCB is allowed for guest3 >>>> and the CRYCB designation in the SIE for guest3 is not checked >>>> on SIE entry. >>>> >>>> Let's allow the CRYCD designation to be ignored when the >>>> SIE for guest3 is not initialized for encryption key usage >>>> nor AP. >>>> >>>> Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>>> --- >>>> arch/s390/kvm/vsie.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>> index a153257..a748f76 100644 >>>> --- a/arch/s390/kvm/vsie.c >>>> +++ b/arch/s390/kvm/vsie.c >>>> @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>> if (!apie_h && !key_msk) >>>> return 0; >>>> + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) >>>> + return 0; >>>> + >>>> if (!crycb_addr) >>>> return set_validity_icpt(scb_s, 0x0039U); >>>> >>> >>> The original patch said >>> >>> "We need to handle the validity checks for the crycb, no matter what the >>> settings for the keywrappings are. So lets move the keywrapping checks >>> after we have done the validy checks." >>> >>> Can you explain why keywrapping now is important? These patches seem to >>> contradict. >>> >> >> No it does not, having the flags set or not is part of the validity check. >> but, I acted too fast. >> >> The problem seems to be even clearer: >> key_msk is defined as >> int key_msk = test_kvm_facility(vcpu->kvm, 76); >> >> If it is defined, as it should for a mask, as >> (scb_o->ecb3 & (ECB3_AES | ECB3_DEA)) >> >> all is clear..., key_msk is not use but for this test, so I do not understand why it is set as facility 76. >> >> so I think I better do: >> >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index a153257..30843a8 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -289,7 +289,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> unsigned long *b1, *b2; >> u8 ecb3_flags; >> int apie_h; >> - int key_msk = test_kvm_facility(vcpu->kvm, 76); >> + int key_msk = scb_o->ecb3 & (ECB3_AES | ECB3_DEA); >> int fmt_o = crycbd_o & CRYCB_FORMAT_MASK; >> int fmt_h = vcpu->arch.sie_block->crycbd & CRYCB_FORMAT_MASK; >> int ret = 0; >> >> >> So just define a mask a mask. >> I verify the functionality and test on Monday and if in between it seems better to you so too I post the patch. > > Can you spin a v2 with an improved patch description outlining the problem according > to this discussion? > Yes I will do. Seems I need more analyzing of the problem Regards, Pierre
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index a153257..a748f76 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -300,6 +300,9 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) if (!apie_h && !key_msk) return 0; + if (!(scb_o->eca & ECA_APIE) && !(scb_o->ecb3 & (ECB3_AES | ECB3_DEA))) + return 0; + if (!crycb_addr) return set_validity_icpt(scb_s, 0x0039U);
The case when the SIE for guest3 is not setup for using encryption keys nor Adjunct processor but the guest2 does use these features was not properly handled. This leads SIE entry for guest3 to crash with validity intercept because the guest2, not having the use of encryption keys nor Adjunct Processor did not initialize the CRYCB designation. In the case where none of ECA_APIE, ECB3_AES or ECB3_DEA are set in guest3 a format 0 CRYCB is allowed for guest3 and the CRYCB designation in the SIE for guest3 is not checked on SIE entry. Let's allow the CRYCD designation to be ignored when the SIE for guest3 is not initialized for encryption key usage nor AP. Fixup: d6f6959 (KVM: s390: vsie: Do the CRYCB validation first) Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- arch/s390/kvm/vsie.c | 3 +++ 1 file changed, 3 insertions(+)