All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure
@ 2017-06-19 17:54 Qiuxu Zhuo
       [not found] ` <20170619175423.70656-1-qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Qiuxu Zhuo @ 2017-06-19 17:54 UTC (permalink / raw)
  To: matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
	ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A
  Cc: tony.luck-ral2JQCrhuEAvxtiuMwx3w,
	keescook-F7+t8E8rja9g9hUCZPvPmw,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, Qiuxu Zhuo

The 'nr_config_table' and 'config_table' (alreay in efi structure)
in efi structure provide a way for some driver(e.g. capsule-pstore
goes through the configuration table to extract crash capsules to
aid in debugging) iterates over the EFI configuration table array.

Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 arch/x86/platform/efi/efi.c | 1 +
 drivers/firmware/efi/efi.c  | 1 +
 include/linux/efi.h         | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 7e76a4d..bcda1b9 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -498,6 +498,7 @@ void __init efi_init(void)
 		return;
 
 	efi.config_table = (unsigned long)efi.systab->tables;
+	efi.nr_config_table = (unsigned long)efi.systab->nr_tables;
 	efi.fw_vendor	 = (unsigned long)efi.systab->fw_vendor;
 	efi.runtime	 = (unsigned long)efi.systab->runtime;
 
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..b511197 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -48,6 +48,7 @@ struct efi __read_mostly efi = {
 	.fw_vendor		= EFI_INVALID_TABLE_ADDR,
 	.runtime		= EFI_INVALID_TABLE_ADDR,
 	.config_table		= EFI_INVALID_TABLE_ADDR,
+	.nr_config_table	= EFI_INVALID_TABLE_ADDR,
 	.esrt			= EFI_INVALID_TABLE_ADDR,
 	.properties_table	= EFI_INVALID_TABLE_ADDR,
 	.mem_attr_table		= EFI_INVALID_TABLE_ADDR,
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ec36f42..bd5ff8f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -892,6 +892,7 @@ extern struct efi {
 	unsigned long fw_vendor;	/* fw_vendor */
 	unsigned long runtime;		/* runtime table */
 	unsigned long config_table;	/* config tables */
+	unsigned long nr_config_table;
 	unsigned long esrt;		/* ESRT table */
 	unsigned long properties_table;	/* properties table */
 	unsigned long mem_attr_table;	/* memory attributes table */
-- 
2.9.0.GIT

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure
       [not found] ` <20170619175423.70656-1-qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2017-06-20 23:28   ` Kees Cook
       [not found]     ` <CAGXu5jLuaU2H3y3VHYhR5qndcdD-pAgfC+_x+8PsACKjVjrmPw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2017-06-21 12:07   ` Ard Biesheuvel
  1 sibling, 1 reply; 5+ messages in thread
From: Kees Cook @ 2017-06-20 23:28 UTC (permalink / raw)
  To: Qiuxu Zhuo
  Cc: Matt Fleming, Ard Biesheuvel, Tony Luck,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

On Mon, Jun 19, 2017 at 10:54 AM, Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> The 'nr_config_table' and 'config_table' (alreay in efi structure)
> in efi structure provide a way for some driver(e.g. capsule-pstore
> goes through the configuration table to extract crash capsules to
> aid in debugging) iterates over the EFI configuration table array.
>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  arch/x86/platform/efi/efi.c | 1 +
>  drivers/firmware/efi/efi.c  | 1 +
>  include/linux/efi.h         | 1 +
>  3 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 7e76a4d..bcda1b9 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -498,6 +498,7 @@ void __init efi_init(void)
>                 return;
>
>         efi.config_table = (unsigned long)efi.systab->tables;
> +       efi.nr_config_table = (unsigned long)efi.systab->nr_tables;
>         efi.fw_vendor    = (unsigned long)efi.systab->fw_vendor;
>         efi.runtime      = (unsigned long)efi.systab->runtime;
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index b372aad..b511197 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -48,6 +48,7 @@ struct efi __read_mostly efi = {
>         .fw_vendor              = EFI_INVALID_TABLE_ADDR,
>         .runtime                = EFI_INVALID_TABLE_ADDR,
>         .config_table           = EFI_INVALID_TABLE_ADDR,
> +       .nr_config_table        = EFI_INVALID_TABLE_ADDR,
>         .esrt                   = EFI_INVALID_TABLE_ADDR,
>         .properties_table       = EFI_INVALID_TABLE_ADDR,
>         .mem_attr_table         = EFI_INVALID_TABLE_ADDR,
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index ec36f42..bd5ff8f 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -892,6 +892,7 @@ extern struct efi {
>         unsigned long fw_vendor;        /* fw_vendor */
>         unsigned long runtime;          /* runtime table */
>         unsigned long config_table;     /* config tables */
> +       unsigned long nr_config_table;

The other fields have (limited) comments. This should probably get one too.

-Kees

>         unsigned long esrt;             /* ESRT table */
>         unsigned long properties_table; /* properties table */
>         unsigned long mem_attr_table;   /* memory attributes table */
> --
> 2.9.0.GIT
>



-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure
       [not found]     ` <CAGXu5jLuaU2H3y3VHYhR5qndcdD-pAgfC+_x+8PsACKjVjrmPw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-06-21  8:36       ` Zhuo, Qiuxu
  0 siblings, 0 replies; 5+ messages in thread
From: Zhuo, Qiuxu @ 2017-06-21  8:36 UTC (permalink / raw)
  To: Kees Cook
  Cc: Matt Fleming, Ard Biesheuvel, Luck, Tony,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

> From: keescook@google.com [mailto:keescook@google.com] On Behalf Of Kees Cook
> Sent: Wednesday, June 21, 2017 7:28 AM

>>         unsigned long config_table;     /* config tables */
>> +       unsigned long nr_config_table;


> The other fields have (limited) comments. This should probably get one too.


OK. I'll add limited comment e.g. "the number of config tables" for it in next version.
Thanks!

BR
qiuxu


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure
       [not found] ` <20170619175423.70656-1-qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2017-06-20 23:28   ` Kees Cook
@ 2017-06-21 12:07   ` Ard Biesheuvel
       [not found]     ` <CAKv+Gu8fQmPRoxR67gm1yBvN0Z8ShY++TW07_RUYRsv-BAu_1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2017-06-21 12:07 UTC (permalink / raw)
  To: Qiuxu Zhuo
  Cc: Matt Fleming, Tony Luck, Kees Cook, linux-efi-u79uwXL29TY76Z2rM5mHXA

On 19 June 2017 at 19:54, Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> The 'nr_config_table' and 'config_table' (alreay in efi structure)
> in efi structure provide a way for some driver(e.g. capsule-pstore
> goes through the configuration table to extract crash capsules to
> aid in debugging) iterates over the EFI configuration table array.
>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  arch/x86/platform/efi/efi.c | 1 +
>  drivers/firmware/efi/efi.c  | 1 +
>  include/linux/efi.h         | 1 +
>  3 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 7e76a4d..bcda1b9 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -498,6 +498,7 @@ void __init efi_init(void)
>                 return;
>
>         efi.config_table = (unsigned long)efi.systab->tables;
> +       efi.nr_config_table = (unsigned long)efi.systab->nr_tables;

As it turns out, we never assign efi.config_table on ARM/arm64, and
this needs to be fixed, so I queued a patch in the EFI -next branch.
Could you rebase your series onto that, and make sure you add a
similar line as this one to uefi_init() in
drivers/firmware/efi/arm-init.c?

>         efi.fw_vendor    = (unsigned long)efi.systab->fw_vendor;
>         efi.runtime      = (unsigned long)efi.systab->runtime;
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index b372aad..b511197 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -48,6 +48,7 @@ struct efi __read_mostly efi = {
>         .fw_vendor              = EFI_INVALID_TABLE_ADDR,
>         .runtime                = EFI_INVALID_TABLE_ADDR,
>         .config_table           = EFI_INVALID_TABLE_ADDR,
> +       .nr_config_table        = EFI_INVALID_TABLE_ADDR,

nr_config_table is not a physical address, so please just initialize it to 0

>         .esrt                   = EFI_INVALID_TABLE_ADDR,
>         .properties_table       = EFI_INVALID_TABLE_ADDR,
>         .mem_attr_table         = EFI_INVALID_TABLE_ADDR,
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index ec36f42..bd5ff8f 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -892,6 +892,7 @@ extern struct efi {
>         unsigned long fw_vendor;        /* fw_vendor */
>         unsigned long runtime;          /* runtime table */
>         unsigned long config_table;     /* config tables */
> +       unsigned long nr_config_table;
>         unsigned long esrt;             /* ESRT table */
>         unsigned long properties_table; /* properties table */
>         unsigned long mem_attr_table;   /* memory attributes table */
> --
> 2.9.0.GIT
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure
       [not found]     ` <CAKv+Gu8fQmPRoxR67gm1yBvN0Z8ShY++TW07_RUYRsv-BAu_1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-06-22  8:42       ` Zhuo, Qiuxu
  0 siblings, 0 replies; 5+ messages in thread
From: Zhuo, Qiuxu @ 2017-06-22  8:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Matt Fleming, Luck, Tony, Kees Cook, linux-efi-u79uwXL29TY76Z2rM5mHXA

> From: linux-efi-owner@vger.kernel.org [mailto:linux-efi-owner@vger.kernel.org] On Behalf Of Ard Biesheuvel
>>         efi.config_table = (unsigned long)efi.systab->tables;
>> +       efi.nr_config_table = (unsigned long)efi.systab->nr_tables;
>
>As it turns out, we never assign efi.config_table on ARM/arm64, and this needs to be fixed, so I queued a patch in the EFI -next branch.
>Could you rebase your series onto that, and make sure you add a similar line as this one to uefi_init() in drivers/firmware/efi/arm-init.c?
   
OK,  will add it to  uefi_init() in drivers/firmware/efi/arm-init.c in v4.


>>         .config_table           = EFI_INVALID_TABLE_ADDR,
>> +       .nr_config_table        = EFI_INVALID_TABLE_ADDR,
>
>nr_config_table is not a physical address, so please just initialize it to 0

OK, will correct it in v4.

Thanks!

BR
qiuxu



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-06-22  8:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-19 17:54 [PATCH v3 1/2] efi: Add 'nr_config_table' variable in efi structure Qiuxu Zhuo
     [not found] ` <20170619175423.70656-1-qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-06-20 23:28   ` Kees Cook
     [not found]     ` <CAGXu5jLuaU2H3y3VHYhR5qndcdD-pAgfC+_x+8PsACKjVjrmPw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-21  8:36       ` Zhuo, Qiuxu
2017-06-21 12:07   ` Ard Biesheuvel
     [not found]     ` <CAKv+Gu8fQmPRoxR67gm1yBvN0Z8ShY++TW07_RUYRsv-BAu_1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-22  8:42       ` Zhuo, Qiuxu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.