From 14aee3d5cc765ec81b1d36d64ef449ce42524fd1 Mon Sep 17 00:00:00 2001 From: Zhang Junyang Date: Tue, 11 Nov 2025 17:31:46 +0800 Subject: [PATCH] Unify and fix RISC-V symbol names --- .../style-guidelines/asm-guidelines.md | 7 +++++++ .../loongarch/boot/{boot.S => bsp_boot.S} | 1 + ostd/src/arch/loongarch/boot/mod.rs | 2 +- ostd/src/arch/riscv/boot/ap_boot.S | 18 +++++++++--------- .../arch/riscv/boot/{boot.S => bsp_boot.S} | 19 ++++++++++--------- ostd/src/arch/riscv/boot/mod.rs | 2 +- 6 files changed, 29 insertions(+), 20 deletions(-) rename ostd/src/arch/loongarch/boot/{boot.S => bsp_boot.S} (98%) rename ostd/src/arch/riscv/boot/{boot.S => bsp_boot.S} (91%) diff --git a/book/src/to-contribute/style-guidelines/asm-guidelines.md b/book/src/to-contribute/style-guidelines/asm-guidelines.md index ef36b9b21..be315d3d3 100644 --- a/book/src/to-contribute/style-guidelines/asm-guidelines.md +++ b/book/src/to-contribute/style-guidelines/asm-guidelines.md @@ -49,6 +49,13 @@ foo: The assembler treats the directives `.global` and `.globl` as the same. For clarity, however, `.global` is preferred. +Note that a Rust crate is a single translation unit, so `global_asm!` labels in +different modules within the same crate share the same global namespace. In RISC-V, +we have experienced that defining two labels with the same name in different modules +can nondeterministically cause duplicate symbol error. The `.L` prefix does not help. +Therefore, add custom prefixes to your labels to avoid name clashes. Nevertheless, +use `.global` to export symbols that need to be visible within the same crate. + In x86-64, if an executable section contains a mix of 32-bit and 64-bit code, the `.code64` and `.code32` directives are treated as function attributes. ```asm diff --git a/ostd/src/arch/loongarch/boot/boot.S b/ostd/src/arch/loongarch/boot/bsp_boot.S similarity index 98% rename from ostd/src/arch/loongarch/boot/boot.S rename to ostd/src/arch/loongarch/boot/bsp_boot.S index b1c9442ea..751a036e3 100644 --- a/ostd/src/arch/loongarch/boot/boot.S +++ b/ostd/src/arch/loongarch/boot/bsp_boot.S @@ -34,6 +34,7 @@ .section ".boot", "awx", @progbits +# `_start` is also the entrypoint specified in the linker script. .global _start _start: # Set DMW0 (kernel) diff --git a/ostd/src/arch/loongarch/boot/mod.rs b/ostd/src/arch/loongarch/boot/mod.rs index b38c44ec8..3505af261 100644 --- a/ostd/src/arch/loongarch/boot/mod.rs +++ b/ostd/src/arch/loongarch/boot/mod.rs @@ -19,7 +19,7 @@ use crate::{ mm::paddr_to_vaddr, }; -global_asm!(include_str!("boot.S")); +global_asm!(include_str!("bsp_boot.S")); static EFI_SYSTEM_TABLE: Once<&'static EfiSystemTable> = Once::new(); diff --git a/ostd/src/arch/riscv/boot/ap_boot.S b/ostd/src/arch/riscv/boot/ap_boot.S index e0ada269b..1c0ebc698 100644 --- a/ostd/src/arch/riscv/boot/ap_boot.S +++ b/ostd/src/arch/riscv/boot/ap_boot.S @@ -35,7 +35,7 @@ ap_boot_start: # Check if the write to satp succeeds. # Reference: . csrr t3, satp - beq t3, t1, flush_tlb_ap + beq t3, t1, ap_flush_tlb # This AP doesn't support Sv48. So the `__ap_boot_page_table_pointer` must # point to a Sv39 page table since we assume that all harts support a same @@ -47,13 +47,13 @@ ap_boot_start: # Check again if the write to satp succeeds. csrr t0, satp - beq t0, t1, flush_tlb_ap + beq t0, t1, ap_flush_tlb # If the CPU doesn't support either Sv48 or Sv39 this is actually reachable. -unreachable_pa_ap: - j unreachable_pa_ap +ap_unreachable_pa: + j ap_unreachable_pa -flush_tlb_ap: +ap_flush_tlb: sfence.vma # Flush TLB. # Now we need to switch to virtual addressing. @@ -87,11 +87,11 @@ __ap_boot_cpu_id_tail: ap_boot_virtual: # Atomically update the CPU ID tail and load the previous value to t1. lla t0, __ap_boot_cpu_id_tail -cmpxchg_load: +ap_cmpxchg_load: lr.d t1, (t0) addi t2, t1, 1 sc.d t2, t2, (t0) - bnez t2, cmpxchg_load + bnez t2, ap_cmpxchg_load # Get the AP boot info array pointer using absolute addressing. lla t2, __ap_boot_info_array_pointer @@ -118,5 +118,5 @@ cmpxchg_load: lla t1, riscv_ap_early_entry jr t1 -unreachable_va_ap: - j unreachable_va_ap +ap_unreachable_va: + j ap_unreachable_va diff --git a/ostd/src/arch/riscv/boot/boot.S b/ostd/src/arch/riscv/boot/bsp_boot.S similarity index 91% rename from ostd/src/arch/riscv/boot/boot.S rename to ostd/src/arch/riscv/boot/bsp_boot.S index de9c461e6..af8f1bad6 100644 --- a/ostd/src/arch/riscv/boot/boot.S +++ b/ostd/src/arch/riscv/boot/bsp_boot.S @@ -19,6 +19,7 @@ KERNEL_VMA_OFFSET = 0xffffffff00000000 .section ".boot", "awx", @progbits +# `_start` is also the entrypoint specified in the linker script. .global _start _start: # Arguments passed from SBI: @@ -46,7 +47,7 @@ _start: # Check if the write to satp succeeds. If not, try Sv39. # Reference: . csrr t1, satp - beq t0, t1, flush_tlb_bsp + beq t0, t1, bsp_flush_tlb # Try loading the Sv39 page table. lla t0, sv39_boot_l3pt @@ -57,20 +58,20 @@ _start: # Check again if the write to satp succeeds. csrr t1, satp - beq t0, t1, flush_tlb_bsp + beq t0, t1, bsp_flush_tlb # If the CPU doesn't support either Sv48 or Sv39 this is actually reachable. -unreachable_pa_bsp: - j unreachable_pa_bsp +bsp_unreachable_pa: + j bsp_unreachable_pa -flush_tlb_bsp: +bsp_flush_tlb: sfence.vma # Update SP/PC to use the virtual address. li t1, KERNEL_VMA_OFFSET lla sp, boot_stack_top or sp, sp, t1 - lla t0, _start_virt - KERNEL_VMA_OFFSET + lla t0, bsp_boot_virt - KERNEL_VMA_OFFSET or t0, t0, t1 jr t0 @@ -120,7 +121,7 @@ boot_stack_top: .text -_start_virt: +bsp_boot_virt: # Initialize GP to the CPU-local storage's base address. .extern __cpu_local_start lla gp, __cpu_local_start @@ -129,5 +130,5 @@ _start_virt: lla t0, riscv_boot jr t0 -unreachable_va_bsp: - j unreachable_va_bsp +bsp_unreachable_va: + j bsp_unreachable_va diff --git a/ostd/src/arch/riscv/boot/mod.rs b/ostd/src/arch/riscv/boot/mod.rs index 572b84103..198493c2e 100644 --- a/ostd/src/arch/riscv/boot/mod.rs +++ b/ostd/src/arch/riscv/boot/mod.rs @@ -18,7 +18,7 @@ use crate::{ mm::paddr_to_vaddr, }; -global_asm!(include_str!("boot.S")); +global_asm!(include_str!("bsp_boot.S")); /// The Flattened Device Tree of the platform. pub static DEVICE_TREE: Once = Once::new();