Re: [PATCH v1] Export Runtime Configuration Interface table to sysfs

2019-08-08 Thread Narendra.K
On Thu, Aug 08, 2019 at 11:16:55AM +0300, Ard Biesheuvel wrote:
> 
> On Wed, 7 Aug 2019 at 16:09,  wrote:
> > On Thu, Jul 11, 2019 at 11:00:37PM +, Limonciello, Mario wrote:
[...]
> > Hi Ard,
> >
> > Does the version 1 of the patch look good ? Please share your thoughts.
> >
> 
> Thanks Narendra,
> 
> The patch looks mostly fine. I have pushed it to my efi/next branch,

Ard, thank you for the review comments and applying the patch.

> and I will let you know if the autobuilders find any problems.

Ok.

> 
> One possible enhancement would be to defer the second memremap() call
> until the first call to raw_table_read(), so the mapping only exists
> if you are actually interested in the contents of the table. If you do
> decide to make any followup changes, please send them as delta patches
> against 
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next

Ok. I will work on the enhancement suggestion. Thank you for the
suggestion. 

-- 
With regards,
Narendra K

Re: [PATCH 5.3 regression fix] efi-stub: Fix get_efi_config_table on mixed-mode setups

2019-08-08 Thread Jarkko Sakkinen
On Wed, Aug 07, 2019 at 11:59:03PM +0200, Hans de Goede wrote:
> Fix get_efi_config_table using the wrong structs when booting a
> 64 bit kernel on 32 bit firmware.
> 
> Cc: Matthew Garrett 
> Cc: Ard Biesheuvel 
> Cc: Jarkko Sakkinen 
> Fixes: 82d736ac56d7 ("Abstract out support for locating an EFI config table")
> Signed-off-by: Hans de Goede 

Acked-by: Jarkko Sakkinen 

/Jarkko


[PATCH v8 20/28] x86/asm: make some functions local

2019-08-08 Thread Jiri Slaby
There is a couple of assembly functions, which are invoked only locally
in the file they are defined. In C, we mark them "static". In assembly,
annotate them using SYM_{FUNC,CODE}_START_LOCAL (and switch their
ENDPROC to SYM_{FUNC,CODE}_END too). Whether we use FUNC or CODE,
depends on whether ENDPROC or END was used for a particular function
before.

Signed-off-by: Jiri Slaby 
Cc: "H. Peter Anvin" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: x...@kernel.org
Cc: Matt Fleming 
Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
---
 arch/x86/boot/compressed/efi_thunk_64.S |  8 
 arch/x86/entry/entry_64.S   | 21 +++--
 arch/x86/lib/copy_page_64.S |  4 ++--
 arch/x86/lib/memcpy_64.S| 12 ++--
 arch/x86/lib/memset_64.S|  8 
 arch/x86/platform/efi/efi_thunk_64.S| 12 ++--
 arch/x86/platform/pvh/head.S|  4 ++--
 7 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/arch/x86/boot/compressed/efi_thunk_64.S 
b/arch/x86/boot/compressed/efi_thunk_64.S
index d66000d23921..31312070db22 100644
--- a/arch/x86/boot/compressed/efi_thunk_64.S
+++ b/arch/x86/boot/compressed/efi_thunk_64.S
@@ -99,12 +99,12 @@ ENTRY(efi64_thunk)
ret
 ENDPROC(efi64_thunk)
 
-ENTRY(efi_exit32)
+SYM_FUNC_START_LOCAL(efi_exit32)
movqfunc_rt_ptr(%rip), %rax
push%rax
mov %rdi, %rax
ret
-ENDPROC(efi_exit32)
+SYM_FUNC_END(efi_exit32)
 
.code32
 /*
@@ -112,7 +112,7 @@ ENDPROC(efi_exit32)
  *
  * The stack should represent the 32-bit calling convention.
  */
-ENTRY(efi_enter32)
+SYM_FUNC_START_LOCAL(efi_enter32)
movl$__KERNEL_DS, %eax
movl%eax, %ds
movl%eax, %es
@@ -172,7 +172,7 @@ ENTRY(efi_enter32)
btsl$X86_CR0_PG_BIT, %eax
movl%eax, %cr0
lret
-ENDPROC(efi_enter32)
+SYM_FUNC_END(efi_enter32)
 
.data
.balign 8
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 50c19edbf639..6d6cb84952ff 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1099,7 +1099,8 @@ idtentry hypervisor_callback xen_do_hypervisor_callback 
has_error_code=0
  * existing activation in its critical region -- if so, we pop the current
  * activation and restart the handler using the previous one.
  */
-ENTRY(xen_do_hypervisor_callback)  /* 
do_hypervisor_callback(struct *pt_regs) */
+/* do_hypervisor_callback(struct *pt_regs) */
+SYM_CODE_START_LOCAL(xen_do_hypervisor_callback)
 
 /*
  * Since we don't modify %rdi, evtchn_do_upall(struct *pt_regs) will
@@ -1117,7 +1118,7 @@ ENTRY(xen_do_hypervisor_callback) /* 
do_hypervisor_callback(struct *pt_regs) */
callxen_maybe_preempt_hcall
 #endif
jmp error_exit
-END(xen_do_hypervisor_callback)
+SYM_CODE_END(xen_do_hypervisor_callback)
 
 /*
  * Hypervisor uses this for application faults while it executes.
@@ -1212,7 +1213,7 @@ idtentry machine_checkdo_mce  
has_error_code=0paranoid=1
  * Use slow, but surefire "are we in kernel?" check.
  * Return: ebx=0: need swapgs on exit, ebx=1: otherwise
  */
-ENTRY(paranoid_entry)
+SYM_CODE_START_LOCAL(paranoid_entry)
UNWIND_HINT_FUNC
cld
PUSH_AND_CLEAR_REGS save_ret=1
@@ -1239,7 +1240,7 @@ ENTRY(paranoid_entry)
SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
 
ret
-END(paranoid_entry)
+SYM_CODE_END(paranoid_entry)
 
 /*
  * "Paranoid" exit path from exception stack.  This is invoked
@@ -1253,7 +1254,7 @@ END(paranoid_entry)
  *
  * On entry, ebx is "no swapgs" flag (1: don't need swapgs, 0: need it)
  */
-ENTRY(paranoid_exit)
+SYM_CODE_START_LOCAL(paranoid_exit)
UNWIND_HINT_REGS
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF_DEBUG
@@ -1270,12 +1271,12 @@ ENTRY(paranoid_exit)
RESTORE_CR3 scratch_reg=%rbx save_reg=%r14
 .Lparanoid_exit_restore:
jmp restore_regs_and_return_to_kernel
-END(paranoid_exit)
+SYM_CODE_END(paranoid_exit)
 
 /*
  * Save all registers in pt_regs, and switch GS if needed.
  */
-ENTRY(error_entry)
+SYM_CODE_START_LOCAL(error_entry)
UNWIND_HINT_FUNC
cld
PUSH_AND_CLEAR_REGS save_ret=1
@@ -1350,16 +1351,16 @@ ENTRY(error_entry)
callfixup_bad_iret
mov %rax, %rsp
jmp .Lerror_entry_from_usermode_after_swapgs
-END(error_entry)
+SYM_CODE_END(error_entry)
 
-ENTRY(error_exit)
+SYM_CODE_START_LOCAL(error_exit)
UNWIND_HINT_REGS
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
testb   $3, CS(%rsp)
jz  retint_kernel
jmp retint_user
-END(error_exit)
+SYM_CODE_END(error_exit)
 
 /*
  * Runs on exception stack.  Xen PV does not go through this path at all,
diff --git a/arch/x86/lib/copy_page_64.S b/arch/x86/lib/copy_page_64.S
index fd2d09afa097..f505870bd93b 100644
--- 

Re: [PATCH v1] Export Runtime Configuration Interface table to sysfs

2019-08-08 Thread Ard Biesheuvel
On Wed, 7 Aug 2019 at 16:09,  wrote:
>
> On Thu, Jul 11, 2019 at 11:00:37PM +, Limonciello, Mario wrote:
> > > -Original Message-
> > > From: K, Narendra
> > > Sent: Wednesday, July 10, 2019 11:59 AM
> > > To: linux-efi@vger.kernel.org; ard.biesheu...@linaro.org; 
> > > pjo...@redhat.com
> > > Cc: K, Narendra; Hayes, Stuart; Limonciello, Mario
> > > Subject: [PATCH v1] Export Runtime Configuration Interface table to sysfs
> > >
> > > From: Narendra K 
> > >
> > > System firmware advertises the address of the 'Runtime Configuration 
> > > Interface
> > > table version 2 (RCI2)' via an EFI Configuration Table entry. This code 
> > > retrieves
> > > the RCI2 table from the address and exports it to sysfs as a binary 
> > > attribute 'rci2'
> > > under /sys/firmware/efi/tables directory.
> > > The approach adopted is similar to the attribute 'DMI' under
> > > /sys/firmware/dmi/tables.
> > >
> > > RCI2 table contains BIOS HII in XML format and is used to populate BIOS 
> > > setup
> > > page in Dell EMC OpenManage Server Administrator tool.
> > > The BIOS setup page contains BIOS tokens which can be configured.
> > >
> > > Signed-off-by: Narendra K 
> >
> > Reviewed-by: Mario Limonciello 
>
> Hi Ard,
>
> Does the version 1 of the patch look good ? Please share your thoughts.
>

Thanks Narendra,

The patch looks mostly fine. I have pushed it to my efi/next branch,
and I will let you know if the autobuilders find any problems.

One possible enhancement would be to defer the second memremap() call
until the first call to raw_table_read(), so the mapping only exists
if you are actually interested in the contents of the table. If you do
decide to make any followup changes, please send them as delta patches
against https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next

Thanks,
Ard.




> >
> > > ---
> > > Hi Ard, the review comment in the v0 version of the patch suggested that 
> > > the
> > > kconfig symbol be set to Y for X86. I made a change to the suggestion.
> > > In the v1 version, I have set the symbol to N by default and added a note 
> > > to the
> > > help section to make it Y for Dell EMC PowerEdge systems. If it needs to 
> > > be
> > > changed, I will resubmit the patch after changing it to implement the 
> > > suggestion.
> > >
> > > The patch is created on 'next' branch of efi tree.
> > >
> > > v0 -> v1:
> > > - Introduced a new Kconfig symbol CONFIG_EFI_RCI2_TABLE and compile
> > > RCI2 table support if it is set. Set the symbol to N by default.
> > > - Removed calling 'efi_rci2_sysfs_init' from drivers/firmware/efi/efi.c 
> > > and made
> > > it a 'late_initcall' in drivers/firmware/efi/rci2_table.c.
> > > Removed the function declaration from include/linux/efi.h.
> > >
> > > RFC -> v0:
> > > - Removed rci2 table from struct efi and defined it in rci2_table.c 
> > > similar to the
> > > way uv_systab_phys is defined in arch/x86/platform/uv/bios_uv.c
> > > - Removed the oem_tables array and added rci2 to common_tables array
> > > - Removed the string 'rci2' from the common_tables array so that it is not
> > > printed in dmesg.
> > > - Merged function 'efi_rci2_table_init' into 'efi_rci2_sysfs_init' 
> > > function to avoid
> > > calling early_memremap/unmap functions.
>
> --
> With regards,
> Narendra K


Re: [PATCH 5.3 regression fix] efi-stub: Fix get_efi_config_table on mixed-mode setups

2019-08-08 Thread Ard Biesheuvel
On Thu, 8 Aug 2019 at 02:03, Matthew Garrett  wrote:
>
> On Wed, Aug 7, 2019 at 2:59 PM Hans de Goede  wrote:
> >
> > Fix get_efi_config_table using the wrong structs when booting a
> > 64 bit kernel on 32 bit firmware.
> >
> > Cc: Matthew Garrett 
> > Cc: Ard Biesheuvel 
> > Cc: Jarkko Sakkinen 
> > Fixes: 82d736ac56d7 ("Abstract out support for locating an EFI config 
> > table")
> > Signed-off-by: Hans de Goede 
>
> Acked-By: Matthew Garrett 
>

Thanks for fixing this, Hans.

Reviewed-by: Ard Biesheuvel 

> Good catch. I think fixing this is preferable to reverting - the
> duplicate events are visible to userland, so there's a risk that apps
> will end up depending on them if there's a release that behaves that
> way.

Agreed, I will send this out as a fix.

> Presumably mixed mode isn't a thing on ARM?

Nope. I should have realised this when we made this routine generic,
but I failed to spot it. ARM is either strictly 32-bit or strictly
64-bit.