On 23.08.2018 08:44, Pierre Morel wrote:
> On 22/08/2018 19:06, David Hildenbrand wrote:
>> On 22.08.2018 18:51, Pierre Morel wrote:
>>> Currently the CRYCB format used in the host for the
>>> shadowed CRYCB is FORMAT2 while no check is done if
>>> AP instructions are supported in the host.
>>>
>>> We better use the format the host calculated for the
>>> guest 1 as the host already tested it against its
>>> facility set.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
>>> ---
>>> arch/s390/kvm/vsie.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>> index 56a9d47..0b12916 100644
>>> --- a/arch/s390/kvm/vsie.c
>>> +++ b/arch/s390/kvm/vsie.c
>>> @@ -154,6 +154,7 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>> const u32 crycb_addr = crycbd_o & 0x7ffffff8U;
>>> unsigned long *b1, *b2;
>>> u8 ecb3_flags;
>>> + unsigned long g1_fmt;
>>>
>>> scb_s->crycbd = 0;
>>> if (!(crycbd_o == CRYCB_FORMAT1))
>>> @@ -180,8 +181,8 @@ static int shadow_crycb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>> return set_validity_icpt(scb_s, 0x0035U);
>>>
>>> scb_s->ecb3 |= ecb3_flags;
>>> - scb_s->crycbd = ((__u32)(__u64) &vsie_page->crycb) | CRYCB_FORMAT1 |
>>> - CRYCB_FORMAT2;
>>> + g1_fmt = vcpu->arch.sie_block->crycbd & 0x03;
>>> + scb_s->crycbd = ((__u32)(__u64) &vsie_page->crycb) | g1_fmt;
>>>
>>> /* xor both blocks in one run */
>>> b1 = (unsigned long *) vsie_page->crycb.dea_wrapping_key_mask;
>>>
>>
>> This is wrong. I remember that with APXA, if FORMAT2 is available, we
>> should always use FORMAT2. That's why we explicitly convert it here.
>>
>
> You are right if FORMAT2 is available we should use FORMAT2
> but the intention here is to use what KVM crypto init function did,
> assuming it did the right thing.
>
> Eventually we are running on a host without AP and we should use FORMAT1.
>
> Isn't it correct?
Yes and no :)
No APXA -> FORMAT2 bit is ignored (and that is one of the reasons why I
am being so strict about simulating HW behavior correctly in nested code
:) )
This only holds as long as we are not using AP. Because from a MSA3
perspective, FORMAT1==FORMAT2 (apart from the length/alignment, which is
fine for us).
Once we support AP (via ECA.28), we'll properly have to create either a
Format0/Format1/Format2. Then, there is actually a semantically
difference ("different fields used").
>
> Regards,
> Pierre
>
>
--
Thanks,
David / dhildenb
