Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754533AbbGaPcF (ORCPT ); Fri, 31 Jul 2015 11:32:05 -0400 Received: from smtp.nue.novell.com ([195.135.221.5]:60968 "EHLO smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753555AbbGaPcC (ORCPT ); Fri, 31 Jul 2015 11:32:02 -0400 Date: Fri, 31 Jul 2015 23:31:47 +0800 From: joeyli To: Matt Fleming Cc: "Lee, Chun-Yi" , linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, linux-pm@vger.kernel.org, "Wysocki, Rafael J" , Matthew Garrett , Len Brown , Pavel Machek , Josh Boyer , Vojtech Pavlik , Jiri Kosina , "H. Peter Anvin" Subject: Re: [RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data Message-ID: <20150731153147.GE2067@linux-rxt1.site> References: <1437056730-15247-1-git-send-email-jlee@suse.com> <1437056730-15247-9-git-send-email-jlee@suse.com> <1438273809.11322.21.camel@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1438273809.11322.21.camel@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2747 Lines: 73 On Thu, Jul 30, 2015 at 05:30:09PM +0100, Matt Fleming wrote: > On Thu, 2015-07-16 at 22:25 +0800, Lee, Chun-Yi wrote: > > For forwarding swsusp key from EFI stub to boot kernel, this patch > > allocates setup data for carrying swsusp key, size and the status > > of efi operating. > > > > Signed-off-by: Lee, Chun-Yi > > --- > > arch/x86/boot/compressed/eboot.c | 29 +++++++++++++++++++++++++---- > > arch/x86/include/uapi/asm/bootparam.h | 1 + > > 2 files changed, 26 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c > > index 97b356f..5e1476e 100644 > > --- a/arch/x86/boot/compressed/eboot.c > > +++ b/arch/x86/boot/compressed/eboot.c > > @@ -1392,19 +1392,23 @@ free_mem_map: > > > > static void setup_swsusp_keys(struct boot_params *params) > > { > > - unsigned long key_size, attributes; > > + struct setup_data *setup_data, *swsusp_setup_data; > > struct swsusp_keys *swsusp_keys; > > + int size = 0; > > + unsigned long key_size, attributes; > > efi_status_t status; > > Why the local variable shuffling? It'd be better to leave key_size and > attributes alone. > > Also, 'size' doesn't look like it should be int. If you're passing it to > allocate_pool it really wants to be 'unsigned long'. > I see, I will change that. > > /* Allocate setup_data to carry keys */ > > + size = sizeof(struct setup_data) + sizeof(struct swsusp_keys); > > status = efi_call_early(allocate_pool, EFI_LOADER_DATA, > > - sizeof(struct swsusp_keys), &swsusp_keys); > > + size, &swsusp_setup_data); > > if (status != EFI_SUCCESS) { > > efi_printk(sys_table, "Failed to alloc mem for swsusp_keys\n"); > > return; > > } > > > > - memset(swsusp_keys, 0, sizeof(struct swsusp_keys)); > > + memset(swsusp_setup_data, 0, size); > > + swsusp_keys = (struct swsusp_keys *)swsusp_setup_data->data; > > > > status = efi_call_early(get_variable, SWSUSP_KEY, &EFI_SWSUSP_GUID, > > &attributes, &key_size, swsusp_keys->swsusp_key); > > @@ -1413,7 +1417,9 @@ static void setup_swsusp_keys(struct boot_params *params) > > memset(swsusp_keys->swsusp_key, 0, SWSUSP_DIGEST_SIZE); > > status = efi_call_early(set_variable, SWSUSP_KEY, > > &EFI_SWSUSP_GUID, attributes, 0, NULL); > > - if (status == EFI_SUCCESS) { > > + if (status) > > + goto clean_fail; > > Please use the EFI status codes explicitly. > Here also, I will check status explicitly. Thanks a lot! Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/