From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752552AbcELIRs (ORCPT ); Thu, 12 May 2016 04:17:48 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:32909 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750774AbcELIRo (ORCPT ); Thu, 12 May 2016 04:17:44 -0400 Date: Thu, 12 May 2016 10:17:39 +0200 From: Ingo Molnar To: Ard Biesheuvel Cc: Alex Thorlton , "linux-kernel@vger.kernel.org" , Dimitri Sivanich , Russ Anderson , Mike Travis , Matt Fleming , Borislav Petkov , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "x86@kernel.org" , "linux-efi@vger.kernel.org" Subject: Re: [PATCH 1/2] Create UV efi_call macros Message-ID: <20160512081739.GA25826@gmail.com> References: <1462996545-98387-1-git-send-email-athorlton@sgi.com> <1462996545-98387-2-git-send-email-athorlton@sgi.com> <20160512064606.GA30717@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Ard Biesheuvel wrote: > On 12 May 2016 at 08:46, Ingo Molnar wrote: > > > > * Alex Thorlton wrote: > > > >> +#define efi_call_virt(f, args...) \ > >> +({ \ > >> + efi_status_t __s; \ > >> + unsigned long flags; \ > >> + arch_efi_call_virt_setup(); \ > >> + local_save_flags(flags); \ > >> + __s = arch_efi_call_virt(f, args); \ > >> + efi_call_virt_check_flags(flags, __stringify(f)); \ > >> + arch_efi_call_virt_teardown(); \ > >> + __s; \ > >> +}) > >> + > >> +#define __efi_call_virt(f, args...) \ > >> +({ \ > >> + unsigned long flags; \ > >> + arch_efi_call_virt_setup(); \ > >> + local_save_flags(flags); \ > >> + arch_efi_call_virt(f, args); \ > >> + efi_call_virt_check_flags(flags, __stringify(f)); \ > >> + arch_efi_call_virt_teardown(); \ > >> +}) > >> + > >> +#define uv_call_virt(f, args...) \ > >> +({ \ > >> + efi_status_t __s; \ > >> + unsigned long flags; \ > >> + arch_efi_call_virt_setup(); \ > >> + local_save_flags(flags); \ > >> + __s = uv_efi_call_virt(f, args); \ > >> + efi_call_virt_check_flags(flags, __stringify(f)); \ > >> + arch_efi_call_virt_teardown(); \ > >> + __s; \ > >> +}) > > > > Btw., a very (very!) small stylistic nit that caught my eyes, and I realize that > > you just moved code, but could you please improve these macros a bit and make it > > look like regular kernel code? I.e. something like: > > > > #define efi_call_virt(f, args...) \ > > ({ \ > > efi_status_t __s; \ > > unsigned long flags; \ > > \ > > arch_efi_call_virt_setup(); \ > > \ > > local_save_flags(flags); \ > > __s = arch_efi_call_virt(f, args); \ > > efi_call_virt_check_flags(flags, __stringify(f)); \ > > arch_efi_call_virt_teardown(); \ > > \ > > __s; \ > > }) > > > > This delineates the various blocks of code: variables, setup, the saving/calling > > block plus the return code. > > > > (Assuming the EFI folks like the whole approach.) > > > > Fine by me, although having a newline after arch_efi_call_virt_setup() > but not before arch_efi_call_virt_teardown() seems rather arbitrary It's an oversight! :-) #define efi_call_virt(f, args...) \ ({ \ efi_status_t __s; \ unsigned long flags; \ \ arch_efi_call_virt_setup(); \ \ local_save_flags(flags); \ __s = arch_efi_call_virt(f, args); \ efi_call_virt_check_flags(flags, __stringify(f)); \ \ arch_efi_call_virt_teardown(); \ \ __s; \ }) But if it's too segmented this is fine too: #define efi_call_virt(f, args...) \ ({ \ efi_status_t __s; \ unsigned long flags; \ \ arch_efi_call_virt_setup(); \ local_save_flags(flags); \ __s = arch_efi_call_virt(f, args); \ efi_call_virt_check_flags(flags, __stringify(f)); \ arch_efi_call_virt_teardown(); \ \ __s; \ }) Thanks, Ingo