On Wed, 16 May 2018 14:17:52 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> On 16/05/2018 13:14, Tony Krowiak wrote:
> > On 05/16/2018 04:51 AM, Pierre Morel wrote:
> >> On 07/05/2018 17:11, Tony Krowiak wrote:
> >>> @@ -1896,19 +1917,17 @@ static u64 kvm_s390_get_initial_cpuid(void)
> >>>
> >>> static void kvm_s390_crypto_init(struct kvm *kvm)
> >>> {
> >>> - if (!test_kvm_facility(kvm, 76))
> >>> + /*
> >>> + * If neither the AP instructions nor the message security assist
> >>> + * extension 3 (MSAX3) are installed, there is no need to
> >>> initialize a
> >>> + * crypto control block (CRYCB) for the guest.
> >>> + */
> >>> + if (!kvm_ap_instructions_available() && !test_kvm_facility(kvm,
> >>> 76))
> >>> return;
> >>>
> >>> kvm->arch.crypto.crycb = &kvm->arch.sie_page2->crycb;
> >>> - kvm_s390_set_crycb_format(kvm);
> >>
> >>
> >> For my point of view the all patch can be reduced to putting this
> >> call (kvm_s390_set_crycb_format(kvm);) before testing for facility 76.
> >>
> >> (and setting the format correctly in kvm_s390_set_crycb_format(kvm))
> >
> > I don't see what that buys us; it will just be reshuffling of the logic.
> > The idea here is that all of the code related to formatting the CRYCB for
> > use by the guest is contained in the kvm_s390_format_crycb(kvm) function.
> > We don't need a CRYCB, however, if the AP instructions are not installed
> > and the MSAX3 facility is not installed, so why even call
> > kvm_s390_format_crycb(kvm) in that case?
>
> It byes a lot of lines.
> I mean that you do exactly the same by only using 3 lines inserted
> instead of 65 changes.
> No logic change.
Sounds like a winner from my POV :)
