[PATCH v4 03/14] x86/boot: Move gdt_l1e caching out of traps_init()

Andrew Cooper posted 14 patches 3 days, 4 hours ago
[PATCH v4 03/14] x86/boot: Move gdt_l1e caching out of traps_init()
Posted by Andrew Cooper 3 days, 4 hours ago
Commit 564d261687c0 ("x86/ctxt-switch: Document and improve GDT handling") put
the initialisation of {,compat_}gdt_l1e into traps_init() but this wasn't a
great choice.  Instead, put it in smp_prepare_cpus() which performs the BSP
preparation of variables normally set up by cpu_smpboot_alloc() for APs.

This removes an implicit dependency that prevents traps_init() moving earlier
than move_xen() in the boot sequence.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

v4:
 * New

I'm on the fence about the ASSERT(), but I'm getting rather tired of unstated
dependencies.  For a PV64 guest using SYSEXIT to enter the guest, it's the
first interrupt/exception which references the GDT, which could be after the
guest is running.
---
 xen/arch/x86/domain.c      |  2 ++
 xen/arch/x86/smpboot.c     | 11 +++++++++++
 xen/arch/x86/traps-setup.c |  7 -------
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 8eb1509782ef..e658c2d647b7 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2029,6 +2029,8 @@ static always_inline bool need_full_gdt(const struct domain *d)
 
 static void update_xen_slot_in_full_gdt(const struct vcpu *v, unsigned int cpu)
 {
+    ASSERT(per_cpu(gdt_l1e, cpu).l1); /* Confirm these have been cached. */
+
     l1e_write(pv_gdt_ptes(v) + FIRST_RESERVED_GDT_PAGE,
               !is_pv_32bit_vcpu(v) ? per_cpu(gdt_l1e, cpu)
                                    : per_cpu(compat_gdt_l1e, cpu));
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 961bdf53331c..491cbbba33ae 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1167,6 +1167,17 @@ void __init smp_prepare_cpus(void)
     initialize_cpu_data(0); /* Final full version of the data */
     print_cpu_info(0);
 
+    /*
+     * Cache {,compat_}gdt_l1e for the BSP now that physically relocation is
+     * done.  It must be after physical relocation of Xen, and before the
+     * first context_switch().
+     */
+    this_cpu(gdt_l1e) =
+        l1e_from_pfn(virt_to_mfn(boot_gdt), __PAGE_HYPERVISOR_RW);
+    if ( IS_ENABLED(CONFIG_PV32) )
+        this_cpu(compat_gdt_l1e) =
+            l1e_from_pfn(virt_to_mfn(boot_compat_gdt), __PAGE_HYPERVISOR_RW);
+
     boot_cpu_physical_apicid = get_apic_id();
     x86_cpu_to_apicid[0] = boot_cpu_physical_apicid;
 
diff --git a/xen/arch/x86/traps-setup.c b/xen/arch/x86/traps-setup.c
index d77be8f83921..c5fc71c75bca 100644
--- a/xen/arch/x86/traps-setup.c
+++ b/xen/arch/x86/traps-setup.c
@@ -341,13 +341,6 @@ void __init traps_init(void)
 
     init_ler();
 
-    /* Cache {,compat_}gdt_l1e now that physically relocation is done. */
-    this_cpu(gdt_l1e) =
-        l1e_from_pfn(virt_to_mfn(boot_gdt), __PAGE_HYPERVISOR_RW);
-    if ( IS_ENABLED(CONFIG_PV32) )
-        this_cpu(compat_gdt_l1e) =
-            l1e_from_pfn(virt_to_mfn(boot_compat_gdt), __PAGE_HYPERVISOR_RW);
-
     percpu_traps_init();
 }
 
-- 
2.39.5


Re: [PATCH v4 03/14] x86/boot: Move gdt_l1e caching out of traps_init()
Posted by Jan Beulich 15 hours ago
On 28.02.2026 00:16, Andrew Cooper wrote:
> Commit 564d261687c0 ("x86/ctxt-switch: Document and improve GDT handling") put
> the initialisation of {,compat_}gdt_l1e into traps_init() but this wasn't a
> great choice.  Instead, put it in smp_prepare_cpus() which performs the BSP
> preparation of variables normally set up by cpu_smpboot_alloc() for APs.
> 
> This removes an implicit dependency that prevents traps_init() moving earlier
> than move_xen() in the boot sequence.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> I'm on the fence about the ASSERT(), but I'm getting rather tired of unstated
> dependencies.  For a PV64 guest using SYSEXIT to enter the guest, it's the
> first interrupt/exception which references the GDT, which could be after the
> guest is running.

I think that's okay to have there. "Unstated dependencies" is of course a wide
field, and going too far with assertions is also a risk.

Jan