From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pierre Morel Subject: Re: [PATCH v1] KVM: s390: vsie: fix Do the CRYCB validation first Date: Fri, 1 Feb 2019 14:37:30 +0100 Message-ID: <3bb397e9-d679-1152-1b42-d633682b3272@linux.ibm.com> References: <1549014725-28216-1-git-send-email-pmorel@linux.ibm.com> <1549014725-28216-2-git-send-email-pmorel@linux.ibm.com> Reply-To: pmorel@linux.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Archive: List-Post: To: David Hildenbrand , borntraeger@de.ibm.com Cc: linux-kernel@vger.kernel.org, cohuck@redhat.com, linux-s390@vger.kernel.org, kvm@vger.kernel.org, frankja@linux.ibm.com, akrowiak@linux.ibm.com, pasic@linux.ibm.com, Claudio Imbrenda List-ID: 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 >> Reported-by: Claudio Imbrenda >> --- >> 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 -- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany