Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-10 Thread Chao Fan
On Mon, Sep 10, 2018 at 10:13:49PM +0200, Rafael J. Wysocki wrote:
>On Mon, Sep 10, 2018 at 2:41 PM Chao Fan  wrote:
>>
>> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
>> tables.
>
>Why?

Sorry for that, I have sent the cover letter to you.

>
>> Since some operations are not needed here,
>
>What operations?

Such as some operations related to the map between physical address
and virtual address. Here in compressed period, use physical address
directely.

>
>> functions are simplified. Functions will be used to dig SRAT tables to get
>> information of memory, so that KASLR can the memory in immovable node.
>
>So do you want to parse SRAT only or do something in addition to that?

Only read and get memory information, won't change SRAT.
So ACPI code will not be changed.

Thanks,
Chao Fan

>
>> And also, these functions won't influence the initialization of
>> ACPI after start_kernel().
>
>




Random crashes with i386 and efi boots

2018-09-10 Thread Guenter Roeck
Hi folks,

even after commit eeb89e2bb1ac ("x86/efi: Load fixmap GDT in
efi_call_phys_epilog()"), my i386/efi qemu boot tests still crash randomly
(roughly 5-10% of the time). As before, I don't see much useful output in
the qemu log (this time it doesn't even complain about a triple fault). 

Debugging shows that the crash happens in efi_call_phys_epilog().
A sample log from a crashed test run is attached below. It appears that
the crash happens if there is an interrupt at a critical section of the
code.

While playing with the code, I found a possible fix.

diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 05ca14222463..9959657127f4 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -85,10 +85,9 @@ pgd_t * __init efi_call_phys_prolog(void)
 
 void __init efi_call_phys_epilog(pgd_t *save_pgd)
 {
+   load_fixmap_gdt(0);
load_cr3(save_pgd);
__flush_tlb_all();
-
-   load_fixmap_gdt(0);
 }

This restores the execution order prior to commit eeb89e2bb1ac.
I have no real idea what I am doing, so this change is to be taken with
a grain of salt. All I can say is that 100 boots with the above change
were successful while the current upstream code (v4.19-rc3) crashes on
a regular basis (in a controlled test I observed 6 failures out of 100
boots).

It would be great if someone with a bit more experience can have another
look and figure out the underlying problem. Please let me know if I can
provide additional information.

Thanks,
Guenter


IN:
# efi_call_phys_epilog(save_pgd);
0xd8f9c12c:  8b 45 f0 movl -0x10(%ebp), %eax
0xd8f9c12f:  e8 49 01 00 00   calll0xd8f9c27d


IN:
# efi_call_phys_epilog():
# load_cr3();
0xd8f9c27d:  55   pushl%ebp
0xd8f9c27e:  05 00 00 00 40   addl $0x4000, %eax
0xd8f9c283:  89 e5movl %esp, %ebp
0xd8f9c285:  0f 22 d8 movl %eax, %cr3

CR3 update: CR3=1904e000

IN:
# __flush_tlb_all();
0xd8f9c288:  e8 c8 5e 2b ff   calll0xd8252155

EAX=1904e000 EBX= ECX=d8f9c126 EDX=d8eafd60
ESI=1f09a000 EDI=0030 EBP=d8e99f3c ESP=d8e99f3c
EIP=d8f9c288 EFL=00200207 [-PC] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =007b   00cff300 DPL=3 DS   [-WA]
CS =0060   00cf9a00 DPL=0 CS32 [-R-]
SS =0068   00cf9300 DPL=0 DS   [-WA]
DS =007b   00cff300 DPL=3 DS   [-WA]
FS =00d8 06c92000  008f9300 DPL=0 DS16 [-WA]
GS =00e0 dfcd09c0 0018 00409100 DPL=0 DS   [--A]
LDT=   8200 DPL=0 LDT
TR =0080 ff803000 206b 8900 DPL=0 TSS32-avl
GDT= 1fcc 00ff
IDT= ff80 07ff
CR0=80050033 CR2=ffd17000 CR3=1904e000 CR4=00040690
DR0= DR1= DR2= DR3=
DR6=0ff0 DR7=0400
CCS=4000 CCD=1904e000 CCO=ADDL
EFER=
Servicing hardware INT=0x30
  508: v=30 e= i=0 cpl=0 IP=0060:d8f9c288 pc=d8f9c288 SP=0068:d8e99f3c 
env->regs[R_EAX]=1904e000
X=1904e000 EBX= ECX=d8f9c126 EDX=d8eafd60
ESI=1f09a000 EDI=0030 EBP=d8e99f3c ESP=d8e99f3c
EIP=d8f9c288 EFL=00200207 [-PC] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =007b   00cff300 DPL=3 DS   [-WA]
CS =0060   00cf9a00 DPL=0 CS32 [-R-]
SS =0068   00cf9300 DPL=0 DS   [-WA]
DS =007b   00cff300 DPL=3 DS   [-WA]
FS =00d8 06c92000  008f9300 DPL=0 DS16 [-WA]
GS =00e0 dfcd09c0 0018 00409100 DPL=0 DS   [--A]
LDT=   8200 DPL=0 LDT
TR =0080 ff803000 206b 8900 DPL=0 TSS32-avl
GDT= 1fcc 00ff
IDT= ff80 07ff
CR0=80050033 CR2=ffd17000 CR3=1904e000 CR4=00040690
DR0= DR1= DR2= DR3=
DR6=0ff0 DR7=0400
CCS=0005 CCD=1904e000 CCO=EFLAGS
EFER=
check_exception old: 0x new 0xe
  509: v=0e e= i=0 cpl=0 IP=0060:d8f9c288 pc=d8f9c288 SP=0068:d8e99f3c 
CR2=1fcc0060
EAX=1904e000 EBX= ECX=d8f9c126 EDX=d8eafd60
ESI=1f09a000 EDI=0030 EBP=d8e99f3c ESP=d8e99f3c
EIP=d8f9c288 EFL=00200207 [-PC] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =007b   00cff300 DPL=3 DS   [-WA]
CS =0060   00cf9a00 DPL=0 CS32 [-R-]
SS =0068   00cf9300 DPL=0 DS   [-WA]
DS =007b   00cff300 DPL=3 DS   [-WA]
FS =00d8 06c92000  008f9300 DPL=0 DS16 [-WA]
GS =00e0 dfcd09c0 0018 00409100 DPL=0 DS   [--A]
LDT=   8200 DPL=0 LDT
TR =0080 ff803000 206b 8900 DPL=0 TSS32-avl
GDT= 1fcc 00ff
IDT= ff80 07ff
CR0=80050033 CR2=1fcc0060 CR3=1904e000 CR4=00040690
DR0= DR1= DR2= DR3=
DR6=0ff0 DR7=0400
CCS=0005 CCD=1904e000 CCO=EFLAGS
EFER=
check_exception old: 0xe new 0xe
  510: v=08 e= i=0 cpl=0 IP=0060:d8f9c288 pc=d8f9c288 SP=0068:d8e99f3c 

Re: [PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-10 Thread Rafael J. Wysocki
On Mon, Sep 10, 2018 at 2:41 PM Chao Fan  wrote:
>
> Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
> tables.

Why?

> Since some operations are not needed here,

What operations?

> functions are simplified. Functions will be used to dig SRAT tables to get
> information of memory, so that KASLR can the memory in immovable node.

So do you want to parse SRAT only or do something in addition to that?

> And also, these functions won't influence the initialization of
> ACPI after start_kernel().


RE: [PATCH V5 0/2] Add efi page fault handler to recover from page

2018-09-10 Thread Prakhya, Sai Praneeth
Hi All,

> Changes from V4 to V5:
> --
> 1. Drop config option that enables efi page fault handler, instead make
>it default.
> 2. Call schedule() in an infinite loop to account for spurious wake ups.
> 3. Introduce "NONE" as an efi runtime service function identifier so that
>it could be used in efi_recover_from_page_fault() to check if the page
>fault was indeed triggered by an efi runtime service.

Please note that apart from dropping the config knob and accounting for 
spurious wake ups, I have added a third change.

I realized that when kernel command line option efi=old_map is passed, the efi 
page fault handler in V4 wouldn't work because, it checks for efi_mm.

In the efi page fault handler, I have also added an explicit check for x86_64 
because 
AFAIK, x86_32 bit machines do not suffer from this problem (Boris, please 
correct me if I am wrong).
Also, I was unable to trigger page faults with buggy OVMF for x86_32.

Just wanted to state the third change explicitly here so that it doesn't escape 
reviewers eyes.

Regards,
Sai


[PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

2018-09-10 Thread Sai Praneeth Prakhya
From: Sai Praneeth 

As per the UEFI specification, after the call to ExitBootServices(),
accesses by the firmware to any memory regions except
EFI_RUNTIME_SERVICES_ regions is considered illegal. A buggy
firmware could trigger these illegal accesses when an efi runtime
service is invoked and if this happens when the kernel is up and
running, the kernel hangs.

Kernel hangs because the memory region requested by the firmware isn't
mapped in efi_pgd, which causes a page fault in ring 0 and the kernel
fails to handle it, leading to die(). To save kernel from hanging, add
an efi specific page fault handler which recovers from such faults by
1. If the efi runtime service is efi_reset_system(), reboot the machine
   through BIOS.
2. If the efi runtime service is _not_ efi_reset_system(), then, freeze
   efi_rts_wq and schedule a new process.

The efi page fault handler offers us two advantages:
1. Recovers from potential hangs that could be caused by buggy firmware.
2. Shout loud that the firmware is buggy and hence is not a kernel bug.

Tested-by: Bhupesh Sharma 
Suggested-by: Matt Fleming 
Based-on-code-from: Ricardo Neri 
Signed-off-by: Sai Praneeth Prakhya 
Cc: Al Stone 
Cc: Borislav Petkov 
Cc: Ingo Molnar 
Cc: Andy Lutomirski 
Cc: Bhupesh Sharma 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ard Biesheuvel 
---
 arch/x86/include/asm/efi.h  |  1 +
 arch/x86/mm/fault.c |  9 
 arch/x86/platform/efi/quirks.c  | 78 +
 drivers/firmware/efi/runtime-wrappers.c |  8 
 include/linux/efi.h |  8 +++-
 5 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..c1a655f099ef 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -140,6 +140,7 @@ extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
 extern void efi_delete_dummy_variable(void);
 extern void efi_switch_mm(struct mm_struct *mm);
+extern int efi_recover_from_page_fault(unsigned long phys_addr);
 
 struct efi_setup_data {
u64 fw_vendor;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 2aafa6ab6103..cc2a2e3a4095 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -16,6 +16,7 @@
 #include /* prefetchw*/
 #include /* exception_enter(), ...   */
 #include  /* faulthandler_disabled()  */
+#include  /* efi_recover_from_page_fault()*/
 
 #include /* boot_cpu_has, ...*/
 #include  /* dotraplinkage, ...   */
@@ -24,6 +25,7 @@
 #include   /* emulate_vsyscall */
 #include   /* struct vm86  */
 #include/* vma_pkey()   */
+#include/* efi_recover_from_page_fault()*/
 
 #define CREATE_TRACE_POINTS
 #include 
@@ -790,6 +792,13 @@ no_context(struct pt_regs *regs, unsigned long error_code,
return;
 
/*
+* Buggy firmware could access regions which might page fault, try to
+* recover from such faults.
+*/
+   if (efi_recover_from_page_fault(address))
+   return;
+
+   /*
 * Oops. The kernel tried to access some bad page. We'll have to
 * terminate things with extreme prejudice:
 */
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 844d31cb8a0c..3920ae8cab2a 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define EFI_MIN_RESERVE 5120
 
@@ -654,3 +655,80 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, 
void *kbuff,
 }
 
 #endif
+
+/*
+ * If any access by any efi runtime service causes a page fault, then,
+ * 1. If it's efi_reset_system(), reboot through BIOS.
+ * 2. If any other efi runtime service, then
+ *a. Return error status to the efi caller process.
+ *b. Disable EFI Runtime Services forever and
+ *c. Freeze efi_rts_wq and schedule new process.
+ *
+ * @return: Returns 0, if the page fault is not handled. This function
+ * will never return if the page fault is handled successfully.
+ */
+int efi_recover_from_page_fault(unsigned long phys_addr)
+{
+   if (!IS_ENABLED(CONFIG_X86_64))
+   return 0;
+
+   /*
+* Make sure that an efi runtime service caused the page fault.
+* "efi_mm" cannot be used to check if the page fault had occurred
+* in the firmware context because efi=old_map doesn't use efi_pgd.
+*/
+   if (efi_rts_work.efi_rts_id == NONE)
+   return 0;
+
+   /*
+* Address range 0x - 0x0fff is always mapped in the efi_pgd, so
+* page faulting on these addresses isn't expected.
+*/
+   if (phys_addr >= 0x && 

[PATCH V5 0/2] Add efi page fault handler to recover from page

2018-09-10 Thread Sai Praneeth Prakhya
From: Sai Praneeth 

There may exist some buggy UEFI firmware implementations that access efi
memory regions other than EFI_RUNTIME_SERVICES_ even after
the kernel has assumed control of the platform. This violates UEFI
specification. Hence, provide a efi specific page fault handler which
recovers from page faults caused by buggy firmware.

Page faults triggered by firmware happen at ring 0 and if unhandled,
hangs the kernel. So, provide an efi specific page fault handler to:
1. Avoid panics/hangs caused by buggy firmware.
2. Shout loud that the firmware is buggy and hence is not a kernel bug.

The efi page fault handler will check if the access is by
efi_reset_system().
1. If so, then the efi page fault handler will reboot the machine
   through BIOS and not through efi_reset_system().
2. If not, then the efi page fault handler will freeze efi_rts_wq and
   schedules a new process.

This issue was reported by Al Stone when he saw that reboot via EFI hangs
the machine. Upon debugging, I found that it's efi_reset_system() that's
touching memory regions which it shouldn't. To reproduce the same
behavior, I have hacked OVMF and made efi_reset_system() buggy. Along
with efi_reset_system(), I have also modified get_next_high_mono_count()
and set_virtual_address_map(). They illegally access both boot time and
other efi regions.

Testing the patch set:
--
1. Download buggy firmware from here [1].
2. Run a qemu instance with this buggy BIOS and boot mainline kernel.
Add reboot=efi to the kernel command line arguments and after the kernel
is up and running, type "reboot". The kernel should hang while rebooting.
3. With the same setup, boot kernel after applying patches and the
reboot should work fine. Also please notice warning/error messages
printed by kernel.

Changes from RFC to V1:
---
1. Drop "long jump" technique of dealing with illegal access and instead
   use scheduling away from efi_rts_wq.

Changes from V1 to V2:
--
1. Shortened config name to CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS from
   CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES.
2. Made the config option available only to expert users.
3. efi_free_boot_services() should be called only when
   CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS is not enabled. Previously, this
   was part of init/main.c file. As it is an architecture agnostic code,
   moved the change to arch/x86/platform/efi/quirks.c file.

Changes from V2 to V3:
--
1. Drop treating illegal access to EFI_BOOT_SERVICES_ regions
   separately from illegal accesses to other regions like
   EFI_CONVENTIONAL_MEMORY or EFI_LOADER_.
   In previous versions, illegal access to EFI_BOOT_SERVICES_
   regions were handled by mapping requested region to efi_pgd but from
   V3 they are handled similar to illegal access to other regions i.e by
   freezing efi_rts_wq and scheduling new process.
2. Change __efi_init_fixup attribute to __efi_init.

Changes from V3 to V4:
--
1. Drop saving original memory map passed by kernel. It also means less
   checks in efi page fault handler.
2. Change the config name to EFI_PAGE_FAULT_HANDLER to reflect it's
   functionality more appropriately.

Changes from V4 to V5:
--
1. Drop config option that enables efi page fault handler, instead make
   it default.
2. Call schedule() in an infinite loop to account for spurious wake ups.
3. Introduce "NONE" as an efi runtime service function identifier so that
   it could be used in efi_recover_from_page_fault() to check if the page
   fault was indeed triggered by an efi runtime service.

Note:
-
Patch set based on "next" branch in efi tree.

[1] https://drive.google.com/drive/folders/1VozKTms92ifyVHAT0ZDQe55ZYL1UE5wt

Sai Praneeth (2):
  efi: Make efi_rts_work accessible to efi page fault handler
  x86/efi: Add efi page fault handler to recover from page faults caused
by the firmware

 arch/x86/include/asm/efi.h  |  1 +
 arch/x86/mm/fault.c |  9 
 arch/x86/platform/efi/quirks.c  | 78 +
 drivers/firmware/efi/runtime-wrappers.c | 61 +++---
 include/linux/efi.h | 42 ++
 5 files changed, 147 insertions(+), 44 deletions(-)

Tested-by: Bhupesh Sharma 
Suggested-by: Matt Fleming 
Based-on-code-from: Ricardo Neri 
Signed-off-by: Sai Praneeth Prakhya 
Cc: Al Stone 
Cc: Borislav Petkov 
Cc: Ingo Molnar 
Cc: Andy Lutomirski 
Cc: Bhupesh Sharma 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ard Biesheuvel 

-- 
2.7.4



[PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler

2018-09-10 Thread Sai Praneeth Prakhya
From: Sai Praneeth 

After the kernel has booted, if any accesses by firmware causes a page
fault, the efi page fault handler would freeze efi_rts_wq and schedules
a new process. To do this, the efi page fault handler needs
efi_rts_work. Hence, make it accessible.

There will be no race conditions in accessing this structure, because,
all the calls to efi runtime services are already serialized.

Tested-by: Bhupesh Sharma 
Suggested-by: Matt Fleming 
Based-on-code-from: Ricardo Neri 
Signed-off-by: Sai Praneeth Prakhya 
Cc: Al Stone 
Cc: Borislav Petkov 
Cc: Ingo Molnar 
Cc: Andy Lutomirski 
Cc: Bhupesh Sharma 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ard Biesheuvel 
---
 drivers/firmware/efi/runtime-wrappers.c | 53 ++---
 include/linux/efi.h | 36 ++
 2 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c 
b/drivers/firmware/efi/runtime-wrappers.c
index aa66cbf23512..b18b2d864c2c 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -45,39 +45,7 @@
 #define __efi_call_virt(f, args...) \
__efi_call_virt_pointer(efi.systab->runtime, f, args)
 
-/* efi_runtime_service() function identifiers */
-enum efi_rts_ids {
-   GET_TIME,
-   SET_TIME,
-   GET_WAKEUP_TIME,
-   SET_WAKEUP_TIME,
-   GET_VARIABLE,
-   GET_NEXT_VARIABLE,
-   SET_VARIABLE,
-   QUERY_VARIABLE_INFO,
-   GET_NEXT_HIGH_MONO_COUNT,
-   UPDATE_CAPSULE,
-   QUERY_CAPSULE_CAPS,
-};
-
-/*
- * efi_runtime_work:   Details of EFI Runtime Service work
- * @arg<1-5>:  EFI Runtime Service function arguments
- * @status:Status of executing EFI Runtime Service
- * @efi_rts_id:EFI Runtime Service function identifier
- * @efi_rts_comp:  Struct used for handling completions
- */
-struct efi_runtime_work {
-   void *arg1;
-   void *arg2;
-   void *arg3;
-   void *arg4;
-   void *arg5;
-   efi_status_t status;
-   struct work_struct work;
-   enum efi_rts_ids efi_rts_id;
-   struct completion efi_rts_comp;
-};
+struct efi_runtime_work efi_rts_work;
 
 /*
  * efi_queue_work: Queue efi_runtime_service() and wait until it's done
@@ -91,7 +59,6 @@ struct efi_runtime_work {
  */
 #define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5)
\
 ({ \
-   struct efi_runtime_work efi_rts_work;   \
efi_rts_work.status = EFI_ABORTED;  \
\
init_completion(_rts_work.efi_rts_comp);\
@@ -184,18 +151,16 @@ static DEFINE_SEMAPHORE(efi_runtime_lock);
  */
 static void efi_call_rts(struct work_struct *work)
 {
-   struct efi_runtime_work *efi_rts_work;
void *arg1, *arg2, *arg3, *arg4, *arg5;
efi_status_t status = EFI_NOT_FOUND;
 
-   efi_rts_work = container_of(work, struct efi_runtime_work, work);
-   arg1 = efi_rts_work->arg1;
-   arg2 = efi_rts_work->arg2;
-   arg3 = efi_rts_work->arg3;
-   arg4 = efi_rts_work->arg4;
-   arg5 = efi_rts_work->arg5;
+   arg1 = efi_rts_work.arg1;
+   arg2 = efi_rts_work.arg2;
+   arg3 = efi_rts_work.arg3;
+   arg4 = efi_rts_work.arg4;
+   arg5 = efi_rts_work.arg5;
 
-   switch (efi_rts_work->efi_rts_id) {
+   switch (efi_rts_work.efi_rts_id) {
case GET_TIME:
status = efi_call_virt(get_time, (efi_time_t *)arg1,
   (efi_time_cap_t *)arg2);
@@ -253,8 +218,8 @@ static void efi_call_rts(struct work_struct *work)
 */
pr_err("Requested executing invalid EFI Runtime Service.\n");
}
-   efi_rts_work->status = status;
-   complete(_rts_work->efi_rts_comp);
+   efi_rts_work.status = status;
+   complete(_rts_work.efi_rts_comp);
 }
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 401e4b254e30..855992b15269 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1659,7 +1659,43 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
+/* efi_runtime_service() function identifiers */
+enum efi_rts_ids {
+   GET_TIME,
+   SET_TIME,
+   GET_WAKEUP_TIME,
+   SET_WAKEUP_TIME,
+   GET_VARIABLE,
+   GET_NEXT_VARIABLE,
+   SET_VARIABLE,
+   QUERY_VARIABLE_INFO,
+   GET_NEXT_HIGH_MONO_COUNT,
+   UPDATE_CAPSULE,
+   QUERY_CAPSULE_CAPS,
+};
+
+/*
+ * efi_runtime_work:   Details of EFI Runtime Service work
+ * @arg<1-5>:  EFI Runtime Service function arguments
+ * @status:Status of executing EFI Runtime Service
+ * @efi_rts_id:EFI Runtime Service 

Re: [PATCH] efi: take size of partition entry from GPT header

2018-09-10 Thread Ard Biesheuvel
On 10 September 2018 at 21:03, Eugene Korenevsky  wrote:
> Use gpt_header.sizeof_partition_entry instead of sizeof(gpt_entry)
> for GPT entry size.
> According to UEFI spec, GPT entry size is specified in GPT header.
>

I think this patch is probably correct, but could you please provide
more information:
- why do you need this patch
- which UEFI spec section/paragraph are you quoting


> Signed-off-by: Eugene Korenevsky 
> ---
>  block/partitions/efi.c | 26 --
>  1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index 39f70d968754..29f7ec585e1d 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -428,11 +428,6 @@ static int is_gpt_valid(struct parsed_partitions *state, 
> u64 lba,
>  (unsigned long 
> long)le64_to_cpu((*gpt)->first_usable_lba));
> goto fail;
> }
> -   /* Check that sizeof_partition_entry has the correct value */
> -   if (le32_to_cpu((*gpt)->sizeof_partition_entry) != sizeof(gpt_entry)) 
> {
> -   pr_debug("GUID Partition Entry Size check failed.\n");
> -   goto fail;
> -   }
>

Shouldn't we assert that sizeof_partition_entry >= sizeof(gpt_entry) ?

> /* Sanity check partition table size */
> pt_size = (u64)le32_to_cpu((*gpt)->num_partition_entries) *
> @@ -692,7 +687,7 @@ static int find_valid_gpt(struct parsed_partitions 
> *state, gpt_header **gpt,
>  int efi_partition(struct parsed_partitions *state)
>  {
> gpt_header *gpt = NULL;
> -   gpt_entry *ptes = NULL;
> +   gpt_entry *ptes = NULL, *pte;
> u32 i;
> unsigned ssz = bdev_logical_block_size(state->bdev) / 512;
>
> @@ -704,13 +699,16 @@ int efi_partition(struct parsed_partitions *state)
>
> pr_debug("GUID Partition Table is valid!  Yea!\n");
>
> -   for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < 
> state->limit-1; i++) {
> +   pte = ptes;
> +   for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < 
> state->limit-1;
> +   i++,
> +   pte = (gpt_entry *)((u8*)pte + 
> gpt->sizeof_partition_entry)) {
> struct partition_meta_info *info;
> unsigned label_count = 0;
> unsigned label_max;
> -   u64 start = le64_to_cpu(ptes[i].starting_lba);
> -   u64 size = le64_to_cpu(ptes[i].ending_lba) -
> -  le64_to_cpu(ptes[i].starting_lba) + 1ULL;
> +   u64 start = le64_to_cpu(pte->starting_lba);
> +   u64 size = le64_to_cpu(pte->ending_lba) -
> +  le64_to_cpu(pte->starting_lba) + 1ULL;
>
> if (!is_pte_valid([i], last_lba(state->bdev)))
> continue;
> @@ -718,18 +716,18 @@ int efi_partition(struct parsed_partitions *state)
> put_partition(state, i+1, start * ssz, size * ssz);
>
> /* If this is a RAID volume, tell md */
> -   if (!efi_guidcmp(ptes[i].partition_type_guid, 
> PARTITION_LINUX_RAID_GUID))
> +   if (!efi_guidcmp(pte->partition_type_guid, 
> PARTITION_LINUX_RAID_GUID))
> state->parts[i + 1].flags = ADDPART_FLAG_RAID;
>
> info = >parts[i + 1].info;
> -   efi_guid_to_str([i].unique_partition_guid, info->uuid);
> +   efi_guid_to_str(>unique_partition_guid, info->uuid);
>
> /* Naively convert UTF16-LE to 7 bits. */
> label_max = min(ARRAY_SIZE(info->volname) - 1,
> -   ARRAY_SIZE(ptes[i].partition_name));
> +   ARRAY_SIZE(pte->partition_name));
> info->volname[label_max] = 0;
> while (label_count < label_max) {
> -   u8 c = ptes[i].partition_name[label_count] & 0xff;
> +   u8 c = pte->partition_name[label_count] & 0xff;
> if (c && !isprint(c))
> c = '!';
> info->volname[label_count] = c;
> --
> 2.18.0


[PATCH] efi: take size of partition entry from GPT header

2018-09-10 Thread Eugene Korenevsky
Use gpt_header.sizeof_partition_entry instead of sizeof(gpt_entry)
for GPT entry size.
According to UEFI spec, GPT entry size is specified in GPT header.

Signed-off-by: Eugene Korenevsky 
---
 block/partitions/efi.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index 39f70d968754..29f7ec585e1d 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -428,11 +428,6 @@ static int is_gpt_valid(struct parsed_partitions *state, 
u64 lba,
 (unsigned long 
long)le64_to_cpu((*gpt)->first_usable_lba));
goto fail;
}
-   /* Check that sizeof_partition_entry has the correct value */
-   if (le32_to_cpu((*gpt)->sizeof_partition_entry) != sizeof(gpt_entry)) {
-   pr_debug("GUID Partition Entry Size check failed.\n");
-   goto fail;
-   }
 
/* Sanity check partition table size */
pt_size = (u64)le32_to_cpu((*gpt)->num_partition_entries) *
@@ -692,7 +687,7 @@ static int find_valid_gpt(struct parsed_partitions *state, 
gpt_header **gpt,
 int efi_partition(struct parsed_partitions *state)
 {
gpt_header *gpt = NULL;
-   gpt_entry *ptes = NULL;
+   gpt_entry *ptes = NULL, *pte;
u32 i;
unsigned ssz = bdev_logical_block_size(state->bdev) / 512;
 
@@ -704,13 +699,16 @@ int efi_partition(struct parsed_partitions *state)
 
pr_debug("GUID Partition Table is valid!  Yea!\n");
 
-   for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < 
state->limit-1; i++) {
+   pte = ptes;
+   for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < 
state->limit-1;
+   i++,
+   pte = (gpt_entry *)((u8*)pte + 
gpt->sizeof_partition_entry)) {
struct partition_meta_info *info;
unsigned label_count = 0;
unsigned label_max;
-   u64 start = le64_to_cpu(ptes[i].starting_lba);
-   u64 size = le64_to_cpu(ptes[i].ending_lba) -
-  le64_to_cpu(ptes[i].starting_lba) + 1ULL;
+   u64 start = le64_to_cpu(pte->starting_lba);
+   u64 size = le64_to_cpu(pte->ending_lba) -
+  le64_to_cpu(pte->starting_lba) + 1ULL;
 
if (!is_pte_valid([i], last_lba(state->bdev)))
continue;
@@ -718,18 +716,18 @@ int efi_partition(struct parsed_partitions *state)
put_partition(state, i+1, start * ssz, size * ssz);
 
/* If this is a RAID volume, tell md */
-   if (!efi_guidcmp(ptes[i].partition_type_guid, 
PARTITION_LINUX_RAID_GUID))
+   if (!efi_guidcmp(pte->partition_type_guid, 
PARTITION_LINUX_RAID_GUID))
state->parts[i + 1].flags = ADDPART_FLAG_RAID;
 
info = >parts[i + 1].info;
-   efi_guid_to_str([i].unique_partition_guid, info->uuid);
+   efi_guid_to_str(>unique_partition_guid, info->uuid);
 
/* Naively convert UTF16-LE to 7 bits. */
label_max = min(ARRAY_SIZE(info->volname) - 1,
-   ARRAY_SIZE(ptes[i].partition_name));
+   ARRAY_SIZE(pte->partition_name));
info->volname[label_max] = 0;
while (label_count < label_max) {
-   u8 c = ptes[i].partition_name[label_count] & 0xff;
+   u8 c = pte->partition_name[label_count] & 0xff;
if (c && !isprint(c))
c = '!';
info->volname[label_count] = c;
-- 
2.18.0


[PATCH v6 1/3] x86/boot: Add acpitb.c to parse acpi tables

2018-09-10 Thread Chao Fan
Imitate the ACPI code of parsing ACPI tables to dig and read ACPI
tables. Since some operations are not needed here, functions are
simplified. Functions will be used to dig SRAT tables to get
information of memory, so that KASLR can the memory in immovable node.

And also, these functions won't influence the initialization of
ACPI after start_kernel().

Signed-off-by: Chao Fan 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Ard Biesheuvel 
Cc: linux-efi@vger.kernel.org
Cc: linux-a...@vger.kernel.org
---
 arch/x86/boot/compressed/Makefile |   4 +
 arch/x86/boot/compressed/acpitb.c | 382 ++
 arch/x86/boot/compressed/misc.h   |   8 +
 3 files changed, 394 insertions(+)
 create mode 100644 arch/x86/boot/compressed/acpitb.c

diff --git a/arch/x86/boot/compressed/Makefile 
b/arch/x86/boot/compressed/Makefile
index 28764dacf018..394d7d93da7a 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -83,6 +83,10 @@ ifdef CONFIG_X86_64
vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
 
+ifdef CONFIG_MEMORY_HOTREMOVE
+vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpitb.o
+endif
+
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
diff --git a/arch/x86/boot/compressed/acpitb.c 
b/arch/x86/boot/compressed/acpitb.c
new file mode 100644
index ..66c515401076
--- /dev/null
+++ b/arch/x86/boot/compressed/acpitb.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+#define BOOT_CTYPE_H
+#include "misc.h"
+
+#include 
+#include 
+#include 
+#include 
+
+extern unsigned long get_cmd_line_ptr(void);
+
+#define STATIC
+#include 
+
+#ifdef CONFIG_MEMORY_HOTREMOVE
+struct mem_vector {
+   unsigned long long start;
+   unsigned long long size;
+};
+/* Store the immovable memory regions */
+struct mem_vector immovable_mem[MAX_NUMNODES*2];
+#endif
+
+#ifdef CONFIG_EFI
+/* Search EFI table for rsdp table. */
+static bool efi_get_rsdp_addr(acpi_physical_address *rsdp_addr)
+{
+   efi_system_table_t *systab;
+   bool find_rsdp = false;
+   bool efi_64 = false;
+   void *config_tables;
+   struct efi_info *e;
+   char *sig;
+   int size;
+   int i;
+
+   e = _params->efi_info;
+   sig = (char *)>efi_loader_signature;
+
+   if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4))
+   efi_64 = true;
+   else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4))
+   efi_64 = false;
+   else {
+   debug_putstr("Wrong EFI loader signature.\n");
+   return false;
+   }
+
+   /* Get systab from boot params. Based on efi_init(). */
+#ifdef CONFIG_X86_32
+   if (e->efi_systab_hi || e->efi_memmap_hi) {
+   debug_putstr("Table located above 4GB, disabling EFI.\n");
+   return false;
+   }
+   systab = (efi_system_table_t *)e->efi_systab;
+#else
+   systab = (efi_system_table_t *)(
+   e->efi_systab | ((__u64)e->efi_systab_hi<<32));
+#endif
+
+   if (systab == NULL)
+   return false;
+
+   /*
+* Get EFI tables from systab. Based on efi_config_init() and
+* efi_config_parse_tables(). Only dig the useful tables but not
+* do the initialization jobs.
+*/
+   size = efi_64 ? sizeof(efi_config_table_64_t) :
+   sizeof(efi_config_table_32_t);
+
+   for (i = 0; i < systab->nr_tables; i++) {
+   efi_guid_t guid;
+   unsigned long table;
+
+   config_tables = (void *)(systab->tables + size * i);
+   if (efi_64) {
+   efi_config_table_64_t *tmp_table;
+
+   tmp_table = (efi_config_table_64_t *)config_tables;
+   guid = tmp_table->guid;
+   table = tmp_table->table;
+#ifndef CONFIG_64BIT
+   if (table >> 32) {
+   debug_putstr("Table located above 4G, disabling 
EFI.\n");
+   return false;
+   }
+#endif
+   } else {
+   efi_config_table_32_t *tmp_table;
+
+   tmp_table = (efi_config_table_32_t *)config_tables;
+   guid = tmp_table->guid;
+   table = tmp_table->table;
+   }
+
+   /*
+* Get rsdp from EFI tables.
+* If ACPI20 table found, use it and return true.
+* If ACPI20 table not found, but ACPI table found,
+* use the ACPI table and return true.
+* If neither ACPI table nor ACPI20 table found,
+* return false.
+*/
+   if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) {
+   *rsdp_addr = (acpi_physical_address)table;
+   find_rsdp = true;
+   } else if 

Re: [PATCH] x86/efi: earlyprintk - Add 64bit efi fb address support

2018-09-10 Thread Ingo Molnar


* Aaron Ma  wrote:

>   if (efi_fb)
>   return (efi_fb + start);

Just noticed this detail unrelated to your patch: return is not a function.

Thanks,

Ingo