From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC107C46464 for ; Fri, 10 Aug 2018 08:49:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AF7C9217C6 for ; Fri, 10 Aug 2018 08:49:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AF7C9217C6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728036AbeHJLSb (ORCPT ); Fri, 10 Aug 2018 07:18:31 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47528 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726997AbeHJLSb (ORCPT ); Fri, 10 Aug 2018 07:18:31 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD48480D2F6B; Fri, 10 Aug 2018 08:49:34 +0000 (UTC) Received: from gondolin (dhcp-192-222.str.redhat.com [10.33.192.222]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6B4CD2026D65; Fri, 10 Aug 2018 08:49:31 +0000 (UTC) Date: Fri, 10 Aug 2018 10:49:29 +0200 From: Cornelia Huck To: Tony Krowiak Cc: Harald Freudenberger , Tony Krowiak , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, freude@de.ibm.com, schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com, borntraeger@de.ibm.com, kwankhede@nvidia.com, bjsdjshi@linux.vnet.ibm.com, pbonzini@redhat.com, alex.williamson@redhat.com, pmorel@linux.vnet.ibm.com, alifm@linux.vnet.ibm.com, mjrosato@linux.vnet.ibm.com, jjherne@linux.vnet.ibm.com, thuth@redhat.com, pasic@linux.vnet.ibm.com, berrange@redhat.com, fiuczy@linux.vnet.ibm.com, buendgen@de.ibm.com, frankja@linux.ibm.com Subject: Re: [PATCH v8 04/22] s390/zcrypt: Integrate ap_asm.h into include/asm/ap.h. Message-ID: <20180810104929.6d40edef.cohuck@redhat.com> In-Reply-To: <323af125-f078-919c-3117-f9022f5529ae@linux.ibm.com> References: <1533739472-7172-1-git-send-email-akrowiak@linux.vnet.ibm.com> <1533739472-7172-5-git-send-email-akrowiak@linux.vnet.ibm.com> <20180809110645.33b20c1f.cohuck@redhat.com> <323af125-f078-919c-3117-f9022f5529ae@linux.ibm.com> Organization: Red Hat GmbH MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 10 Aug 2018 08:49:35 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 10 Aug 2018 08:49:35 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'cohuck@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 9 Aug 2018 12:06:56 -0400 Tony Krowiak wrote: > On 08/09/2018 05:17 AM, Harald Freudenberger wrote: > > On 09.08.2018 11:06, Cornelia Huck wrote: > >> On Wed, 8 Aug 2018 10:44:14 -0400 > >> Tony Krowiak wrote: > >> > >>> From: Harald Freudenberger > >>> > >>> Move all the inline functions from the ap bus header > >>> file ap_asm.h into the in-kernel api header file > >>> arch/s390/include/asm/ap.h so that KVM can make use > >>> of all the low level AP functions. > >>> > >>> Signed-off-by: Harald Freudenberger > >>> Signed-off-by: Christian Borntraeger > >> You should add your own s-o-b if you are sending on patches written by > >> others (even if it does not matter in the end, when they are merged > >> through a different path anyway.) > >> > >>> --- > >>> arch/s390/include/asm/ap.h | 284 ++++++++++++++++++++++++++++++++++++---- > >>> drivers/s390/crypto/ap_asm.h | 261 ------------------------------------ > >>> drivers/s390/crypto/ap_bus.c | 21 +--- > >>> drivers/s390/crypto/ap_bus.h | 1 + > >>> drivers/s390/crypto/ap_card.c | 1 - > >>> drivers/s390/crypto/ap_queue.c | 1 - > >>> 6 files changed, 259 insertions(+), 310 deletions(-) > >>> delete mode 100644 drivers/s390/crypto/ap_asm.h > >>> > >>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h > >>> index c1bedb4..046e044 100644 > >>> --- a/arch/s390/include/asm/ap.h > >>> +++ b/arch/s390/include/asm/ap.h > >>> @@ -47,6 +47,50 @@ struct ap_queue_status { > >>> }; > >>> > >>> /** > >>> + * ap_intructions_available() - Test if AP instructions are available. > >>> + * > >>> + * Returns 0 if the AP instructions are installed. > >> Stumbled over this when I was looking at the usage in patch 7: if I see > >> a function called '_available' return 0, I'd assume that whatever the > >> function tests for is *not* available. > >> > >> Rather call this function ap_instructions_check_availability() (and > >> keep the return code convention), or switch this to return 0 if not > >> available and !0 if available? > > Good catch, Cony you are right. I'll fix this to return 1 if AP instructions > > are available and 0 if not. However, this patch will come via Martin's pipe > > to the Linus Torwald kernel sources. > > Is your intent to simply indicate whether the AP instructions are > available or > not; or is the intention to indicate whether the AP instructions are > available > and if not, they why? In the former, then I agree that a boolean should be > returned; however, if the case is the latter, then what you have is fine but > maybe the function name should be changed as Connie suggests. So, can this actually fail for any reason other than "instructions not installed"? Even if it did, the end result is that the instructions are not usable -- I don't think the "why" would be interesting at that point. > > >>> + */ > >>> +static inline int ap_instructions_available(void) > >>> +{ > >>> + register unsigned long reg0 asm ("0") = AP_MKQID(0, 0); > >>> + register unsigned long reg1 asm ("1") = -ENODEV; > >>> + register unsigned long reg2 asm ("2"); > >>> + > >>> + asm volatile( > >>> + " .long 0xb2af0000\n" /* PQAP(TAPQ) */ > >>> + "0: la %0,0\n" > >>> + "1:\n" > >>> + EX_TABLE(0b, 1b) > >>> + : "+d" (reg1), "=d" (reg2) > >>> + : "d" (reg0) > >>> + : "cc"); > >>> + return reg1; > >>> +}