Re: [RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-31 Thread joeyli 
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), _keys);
> > +   size, _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, _SWSUSP_GUID,
> > , _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,
> > _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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-31 Thread joeyli 
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 j...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-30 Thread Matt Fleming 
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'.

>   /* 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), _keys);
> + size, _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, _SWSUSP_GUID,
>   , _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,
>   _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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-30 Thread Matt Fleming 
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 j...@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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-16 Thread Lee, Chun-Yi 
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;
 
/* 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), _keys);
+   size, _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, _SWSUSP_GUID,
, _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,
_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
-- 
1.8.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH 08/16] x86/efi: Carrying swsusp key by setup data

2015-07-16 Thread Lee, Chun-Yi 
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 j...@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;
 
/* 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
-- 
1.8.4.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/