[U-Boot] [PATCH 1/3] x86: Change 4-level page table base address to low memory

At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/i386/cpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index 208ef08..af42431 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -462,6 +462,7 @@ int cpu_has_64bit(void) has_long_mode(); }
+#define PAGETABLE_BASE 0x80000 #define PAGETABLE_SIZE (6 * 4096)
/** @@ -523,10 +524,7 @@ int cpu_jump_to_64bit_uboot(ulong target) uint32_t *pgtable; func_t func;
- /* TODO(sjg@chromium.org): Find a better place for this */ - pgtable = (uint32_t *)0x1000000; - if (!pgtable) - return -ENOMEM; + pgtable = (uint32_t *)PAGETABLE_BASE;
build_pagetable(pgtable);

Before jumping to 64-bit U-Boot proper, SPL copies the cpu_call64() function to a hardcoded address 0x3000000. This can have potential conflicts with application usage. Switch the destination address to be allocated from the heap to avoid such risk.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/i386/call64.S | 4 ++++ arch/x86/cpu/i386/cpu.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/i386/call64.S b/arch/x86/cpu/i386/call64.S index 8f86728..275063c 100644 --- a/arch/x86/cpu/i386/call64.S +++ b/arch/x86/cpu/i386/call64.S @@ -79,6 +79,10 @@ lret_target: mov %eax, %eax /* Clear bits 63:32 */ jmp *%eax /* Jump to the 64-bit target */
+.globl call64_stub_size +call64_stub_size: + .long . - cpu_call64 + .data .align 16 .globl gdt64 diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index af42431..e4b5514 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -523,18 +523,23 @@ int cpu_jump_to_64bit_uboot(ulong target) typedef void (*func_t)(ulong pgtable, ulong setup_base, ulong target); uint32_t *pgtable; func_t func; + char *ptr;
pgtable = (uint32_t *)PAGETABLE_BASE;
build_pagetable(pgtable);
- /* TODO(sjg@chromium.org): Find a better place for this */ - char *ptr = (char *)0x3000000; + extern long call64_stub_size; + ptr = malloc(call64_stub_size); + if (!ptr) { + printf("Failed to allocate the cpu_call64 stub\n"); + return -ENOMEM; + } char *gdt = (char *)0x3100000;
extern char gdt64[];
- memcpy(ptr, cpu_call64, 0x1000); + memcpy(ptr, cpu_call64, call64_stub_size); memcpy(gdt, gdt64, 0x100);
/*

On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
Before jumping to 64-bit U-Boot proper, SPL copies the cpu_call64() function to a hardcoded address 0x3000000. This can have potential conflicts with application usage. Switch the destination address to be allocated from the heap to avoid such risk.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/call64.S | 4 ++++ arch/x86/cpu/i386/cpu.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Feb 2, 2019 at 2:06 PM Simon Glass sjg@chromium.org wrote:
On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
Before jumping to 64-bit U-Boot proper, SPL copies the cpu_call64() function to a hardcoded address 0x3000000. This can have potential conflicts with application usage. Switch the destination address to be allocated from the heap to avoid such risk.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/call64.S | 4 ++++ arch/x86/cpu/i386/cpu.c | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

It is unnecessary to use a RAM version GDT for 64-bit U-Boot proper. In fact we can just use the ROM version directly, which not only eliminates the risk of being overwritten by application, but also removes the complexity of patching the cpu_call64().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
arch/x86/cpu/i386/cpu.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index e4b5514..3bde44e 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -535,23 +535,9 @@ int cpu_jump_to_64bit_uboot(ulong target) printf("Failed to allocate the cpu_call64 stub\n"); return -ENOMEM; } - char *gdt = (char *)0x3100000; - - extern char gdt64[]; - memcpy(ptr, cpu_call64, call64_stub_size); - memcpy(gdt, gdt64, 0x100);
- /* - * TODO(sjg@chromium.org): This manually inserts the pointers into - * the code. Tidy this up to avoid this. - */ func = (func_t)ptr; - ulong ofs = (ulong)cpu_call64 - (ulong)ptr; - *(ulong *)(ptr + 7) = (ulong)gdt; - *(ulong *)(ptr + 0xc) = (ulong)gdt + 2; - *(ulong *)(ptr + 0x13) = (ulong)gdt; - *(ulong *)(ptr + 0x117 - 0xd4) -= ofs;
/* * Copy U-Boot from ROM

Hi Bin,
On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
It is unnecessary to use a RAM version GDT for 64-bit U-Boot proper. In fact we can just use the ROM version directly, which not only eliminates the risk of being overwritten by application, but also removes the complexity of patching the cpu_call64().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 14 -------------- 1 file changed, 14 deletions(-)
I am not sure what issue I had with this, but I'm pleased that this works.
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Feb 2, 2019 at 2:06 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
It is unnecessary to use a RAM version GDT for 64-bit U-Boot proper. In fact we can just use the ROM version directly, which not only eliminates the risk of being overwritten by application, but also removes the complexity of patching the cpu_call64().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 14 -------------- 1 file changed, 14 deletions(-)
I am not sure what issue I had with this, but I'm pleased that this works.
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

On 1/31/19 5:22 PM, Bin Meng wrote:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com
Thanks for addressing this. It fixes the problem reported in
x86: #define CONFIG_LOADADDR 0x1100000 https://lists.denx.de/pipermail/u-boot/2019-January/356108.html
I have set aforementioned patch to superseded.
Tested on qemu-x86_64.
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de

Am 31.01.2019 um 17:22 schrieb Bin Meng bmeng.cn@gmail.com:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Can't you dynamically allocate the PT too?
Alex
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index 208ef08..af42431 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -462,6 +462,7 @@ int cpu_has_64bit(void) has_long_mode(); }
+#define PAGETABLE_BASE 0x80000 #define PAGETABLE_SIZE (6 * 4096)
/** @@ -523,10 +524,7 @@ int cpu_jump_to_64bit_uboot(ulong target) uint32_t *pgtable; func_t func;
- /* TODO(sjg@chromium.org): Find a better place for this */
- pgtable = (uint32_t *)0x1000000;
- if (!pgtable)
return -ENOMEM;
pgtable = (uint32_t *)PAGETABLE_BASE;
build_pagetable(pgtable);
-- 2.7.4

Hi Alex,
On Fri, Feb 1, 2019 at 2:30 AM Alexander Graf agraf@suse.de wrote:
Am 31.01.2019 um 17:22 schrieb Bin Meng bmeng.cn@gmail.com:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Can't you dynamically allocate the PT too?
The dynamically allocated PT only makes sense when in SPL. It then becomes an arbitrary address again when entering in the 64-bit proper.
Regards, Bin

Am 01.02.2019 um 00:40 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Alex,
On Fri, Feb 1, 2019 at 2:30 AM Alexander Graf agraf@suse.de wrote:
Am 31.01.2019 um 17:22 schrieb Bin Meng bmeng.cn@gmail.com:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Can't you dynamically allocate the PT too?
The dynamically allocated PT only makes sense when in SPL. It then becomes an arbitrary address again when entering in the 64-bit proper.
I'm not sure I follow? On aarch64, we allocate every level dynamically. I feel like I'm missing a piece of the puzzle here :)
Alex
Regards, Bin

Hi Alex,
On Fri, Feb 1, 2019 at 8:01 AM Alexander Graf agraf@suse.de wrote:
Am 01.02.2019 um 00:40 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Alex,
On Fri, Feb 1, 2019 at 2:30 AM Alexander Graf agraf@suse.de wrote:
Am 31.01.2019 um 17:22 schrieb Bin Meng bmeng.cn@gmail.com:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Can't you dynamically allocate the PT too?
The dynamically allocated PT only makes sense when in SPL. It then becomes an arbitrary address again when entering in the 64-bit proper.
I'm not sure I follow? On aarch64, we allocate every level dynamically. I feel like I'm missing a piece of the puzzle here :)
The current x86 implementation is the SPL allocates the page table for the 64-bit U-Boot. We can certainly change the implementation but I would leave that for future changes.
Regards, Bin

Am 01.02.2019 um 08:29 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Alex,
On Fri, Feb 1, 2019 at 8:01 AM Alexander Graf agraf@suse.de wrote:
Am 01.02.2019 um 00:40 schrieb Bin Meng bmeng.cn@gmail.com:
Hi Alex,
On Fri, Feb 1, 2019 at 2:30 AM Alexander Graf agraf@suse.de wrote:
Am 31.01.2019 um 17:22 schrieb Bin Meng bmeng.cn@gmail.com:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Can't you dynamically allocate the PT too?
The dynamically allocated PT only makes sense when in SPL. It then becomes an arbitrary address again when entering in the 64-bit proper.
I'm not sure I follow? On aarch64, we allocate every level dynamically. I feel like I'm missing a piece of the puzzle here :)
The current x86 implementation is the SPL allocates the page table for the 64-bit U-Boot. We can certainly change the implementation but I would leave that for future changes.
I see. Works for me :)
Alex
Regards, Bin

On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 2/2/19 7:06 AM, Simon Glass wrote:
On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hello Bin,
this patch is needed together with
test/py: use default load address for tftp https://lists.denx.de/pipermail/u-boot/2019-January/356265.html
to get the EFI patch queue merged.
Should Tom pick the patch series?
Best regards
Heinrich

Hi Heinrich,
On Tue, Feb 5, 2019 at 7:32 PM Heinrich Schuchardt xypron.debian@gmx.de wrote:
On 2/2/19 7:06 AM, Simon Glass wrote:
On Thu, 31 Jan 2019 at 09:17, Bin Meng bmeng.cn@gmail.com wrote:
At present the 4-level page table base address for 64-bit U-Boot proper is assigned an address that conflicts with CONFIG_LOADADDR. Change it to an address within the low memory range instead.
Fixes crashes seen when 'dhcp' on QEMU x86_64 with "-net nic -net user,tftp=.,bootfile=u-boot".
Reported-by: Alexander Graf agraf@suse.de Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/i386/cpu.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hello Bin,
this patch is needed together with
test/py: use default load address for tftp https://lists.denx.de/pipermail/u-boot/2019-January/356265.html
to get the EFI patch queue merged.
Should Tom pick the patch series?
Sorry I just came back from Chinese new year holiday vacation. I will send PR to Tom ASAP.
applied to u-boot-x86, thanks!
Regards, Bin
participants (5)
-
Alexander Graf
-
Bin Meng
-
Heinrich Schuchardt
-
Heinrich Schuchardt
-
Simon Glass