| Message ID | 1437056730-15247-9-git-send-email-jlee@suse.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Rafael Wysocki |
| Headers | show |
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 <jlee@suse.com> > --- > 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'. > /* 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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <jlee@suse.com> > > --- > > 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-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; /* 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; + else { efi_printk(sys_table, "Cleaned existing swsusp key\n"); status = EFI_NOT_FOUND; } @@ -1433,6 +1439,21 @@ static void setup_swsusp_keys(struct boot_params *params) if (status != EFI_SUCCESS) efi_printk(sys_table, "Failed to set swsusp key\n"); } + +clean_fail: + swsusp_setup_data->type = SETUP_SWSUSP_KEYS; + swsusp_setup_data->len = sizeof(struct swsusp_keys); + swsusp_setup_data->next = 0; + swsusp_keys->skey_status = efi_status_to_err(status); + + setup_data = (struct setup_data *)params->hdr.setup_data; + while (setup_data && setup_data->next) + setup_data = (struct setup_data *)setup_data->next; + + if (setup_data) + setup_data->next = (unsigned long)swsusp_setup_data; + else + params->hdr.setup_data = (unsigned long)swsusp_setup_data; } #else static void setup_swsusp_keys(struct boot_params *params) {} diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index ab456dc..12e0a203 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -7,6 +7,7 @@ #define SETUP_DTB 2 #define SETUP_PCI 3 #define SETUP_EFI 4 +#define SETUP_SWSUSP_KEYS 5 /* ram_size flags */ #define RAMDISK_IMAGE_START_MASK 0x07FF
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 <jlee@suse.com> --- arch/x86/boot/compressed/eboot.c | 29 +++++++++++++++++++++++++---- arch/x86/include/uapi/asm/bootparam.h | 1 + 2 files changed, 26 insertions(+), 4 deletions(-)