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@xxxxxxxxxxxxx>
Reported-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
---
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
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
