[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/6] add support for booting from grub with x86_64

From: Luca
Subject: Re: [PATCH 1/6] add support for booting from grub with x86_64
Date: Wed, 2 Feb 2022 08:01:57 +0100

Il 28/01/22 20:56, Samuel Thibault ha scritto:

Luca Dariz, le ven. 28 janv. 2022 19:24:04 +0100, a ecrit:
* link kernel at 0x4000000 as the xen version, higher values causes
   linker errors.
* we can't use full segmentation in long mode, so we need to create a
   temporary mapping during early boot to be able to jump to high
* build direct map for first 4G in boothdr (seems required by Linux
* enable also write page access check in kernel mode

Please document all changes.

Will do

This is also changing ./configure behavior, also please document:

@@ -73,7 +73,7 @@
  /* This is the kernel address range in linear addresses.  */
  #ifdef __x86_64__
-#define LINEAR_MAX_KERNEL_ADDRESS      (0x00007fffffffffffUL)
+#define LINEAR_MAX_KERNEL_ADDRESS      (0xffffffffffffffffUL)

why this is needed (is it perhaps even a bugfix?), why the impact it has
is not a problem. (same for INTEL_PTE_PFN)

Strictly speaking, this should not be needed now, as we don't use high addresses yet; however, this seems to impact only VM_MAX_KERNEL_ADDRESS for 64-bit targets, which seems to be used only in pmap.c as a safety limit.

diff --git a/i386/intel/pmap.c b/i386/intel/pmap.c
index 3bf00659..91835b30 100644
--- a/i386/intel/pmap.c
+++ b/i386/intel/pmap.c
@@ -655,7 +655,7 @@ void pmap_bootstrap(void)
                                  pa_to_pte(_kvtophys((void *) kernel_page_dir
                                                      + i * INTEL_PGBYTES))
                                  | INTEL_PTE_VALID
+#if defined(MACH_PV_PAGETABLES) || defined(__x86_64__)

AIUI it'd rather be a

+#if !defined(MACH_HYP) || defined(MACH_PV_PAGETABLES)


In model_dep.c the CR0_WP bit is set only if MACH_HYP is not defined, I don't know if MACH_PV_PAGETABLES enables write protection in some other way. Additionally, with this definition it would be enabled also for 32-bit non-xen, changing the current behavior. I'll test it and if it works I'll change the condition.

+       movl    $p3table,%eax
+       or      $0b11,%eax

Please avoid magic numbers, use macros from headers so we know what 0b11

will do

+        mov     $0x200000,%eax   // 2MiB page, should be always available

(this one is fine however for instance: the comment says it all)

diff --git a/x86_64/interrupt.S b/x86_64/interrupt.S
index fccf6e28..eab643a5 100644
--- a/x86_64/interrupt.S
+++ b/x86_64/interrupt.S
@@ -41,12 +41,12 @@ ENTRY(interrupt)
        movl    8(%esp),%edx            /* set irq number as 3rd arg */
        movl    %edx,%ebx               /* copy irq number */
        shll    $2,%ebx                 /* irq * 4 */
-       movl    EXT(iunit)(%ebx),%edi   /* get device unit number as 1st arg */
+       movl    EXT(iunit)(%rbx),%edi   /* get device unit number as 1st arg */

Why do we need to switch to rbx?

This is needed when using high addresses for KERNEL_MAP_BASE because it fails to compile (I think because iunit's address was not fitting 32-bit anymore)

diff --git a/x86_64/locore.S b/x86_64/locore.S
index 612fc493..a7266dab 100644
--- a/x86_64/locore.S
+++ b/x86_64/locore.S
@@ -1136,7 +1136,7 @@ syscall_native:
        shll    $5,%eax                 /* manual indexing of mach_trap_t */
        xorq    %r10,%r10
-       movl    EXT(mach_trap_table)(%eax),%r10d
+       mov     EXT(mach_trap_table)(%rax),%r10

? mach_trap_arg_count is an int, not a long. We wouldn't need a long
anyway. Note that writing into the r10d register zero-clears the upper
part of r10.

This also failed to compile when using high addresses

                                        /* get number of arguments */
        andq    %r10,%r10
        jz      mach_call_call          /* skip argument copy if none */
@@ -1184,7 +1184,7 @@ mach_call_call:
  #endif /* DEBUG */
- call *EXT(mach_trap_table)+8(%eax)
+       call    *EXT(mach_trap_table)+8(%rax)

Similarly, why do we need to switch to rax?

Also failed to compile on high addresses. Maybe I should factor out these changes in another patch.

Thanks for the comments!


reply via email to

[Prev in Thread] Current Thread [Next in Thread]