linux-kernel - Re: [PATCH v5 02/13] KVM: s390: refactor crypto initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180516142113.3d23d356.cohuck@redhat.com>
Date:   Wed, 16 May 2018 14:21:13 +0200
From:   Cornelia Huck <cohuck@...hat.com>
To:     Pierre Morel <pmorel@...ux.ibm.com>
Cc:     Tony Krowiak <akrowiak@...ux.vnet.ibm.com>,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, freude@...ibm.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, borntraeger@...ibm.com,
        kwankhede@...dia.com, bjsdjshi@...ux.vnet.ibm.com,
        pbonzini@...hat.com, alex.williamson@...hat.com,
        pmorel@...ux.vnet.ibm.com, alifm@...ux.vnet.ibm.com,
        mjrosato@...ux.vnet.ibm.com, jjherne@...ux.vnet.ibm.com,
        thuth@...hat.com, pasic@...ux.vnet.ibm.com, berrange@...hat.com,
        fiuczy@...ux.vnet.ibm.com, buendgen@...ibm.com
Subject: Re: [PATCH v5 02/13] KVM: s390: refactor crypto initialization

On Wed, 16 May 2018 14:17:52 +0200
Pierre Morel <pmorel@...ux.ibm.com> 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 :)