Qemu-virt64-riscv BSP support SMP architecture #10887
Conversation
…ng the SMP architecture. Currently, the bsp: qemu-virt64-riscv does not support the SMP architecture, and some necessary interfaces are not implemented. Solution: Add the interface declarations to make the compilation pass. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
…rchitecture. Tests conducted on bsp: qemu-virt64-riscv. Currently, the command line cannot start normally. This is because the SMP architecture requires scheduling information update operations; secondly, it does not yet support context switching operations within interrupts. Solution: In the two functions (rt_hw_context_switch_to and rt_hw_context_switch) in context_gcc.S, add a call to rt_cpus_lock_status_restore to update the scheduler information. For the second issue, if scheduling is triggered in an interrupt, pcpu->irq_switch_flag will be set to 1; thus, rt_scheduler_do_irq_switch is called in interrupt_gcc.S to determine whether to perform context switching. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
…the spinlock. The specific implementation of the spinlock is added in risc-v/virt64/interrupt.c. Due to the need for atomic operations, a new file atomic_riscv.c (copied from the common directory) is added under risc-v/common64. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
…e SMP architecture. Currently, it does not support operation with two or more cores. Solution: Since the system runs in S-mode and does not support access to the mhartid register, the hartid is currently stored in the satp register (this register is not used when the bsp qemu-virt64-riscv runs RT-Thread). Additionally, logic for storing boot_hartid and multi-core initialization logic for the sp pointer have been added in startup_gcc.S. Logic for secondary core wake-up and entry has been added in cpuport.c. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
Add IPI handling logic based on the RISC-V architecture.We handle IPI-related requests in software interrupts. Up to this point, the RISC-V 64 architecture can support the 2-core SMP mode and has passed the SMP Utest. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
…figuration. Add dynamic startup based on core configuration. It should be noted that to pass the SMP Utest, the maximum priority needs to be configured to 256. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: libcpu_riscvReviewers: Yaochenger Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-11-19 10:26 CST)
📝 Review Instructions
|
|
|
|
@Tm-C-mT you need sign CLA first. |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds SMP (Symmetric Multi-Processing) support for the RISC-V virt64 BSP. The implementation includes IPI (Inter-Processor Interrupt) handling, spinlock mechanisms, atomic operations, and multi-core initialization.
Key changes:
- Adds IPI support with dedicated descriptor array and handler functions
- Implements ticket-based spinlock for SMP synchronization
- Adds atomic operation primitives using RISC-V AMO instructions
- Implements secondary CPU initialization and boot-up logic
- Updates trap handling to support per-CPU nested trap detection
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| libcpu/risc-v/virt64/interrupt.c | Adds IPI infrastructure, spinlock implementation, and interrupt priority functions |
| libcpu/risc-v/common64/trap.c | Updates for per-CPU trap nesting detection and IPI handler integration |
| libcpu/risc-v/common64/startup_gcc.S | Adds multi-hart initialization and per-CPU stack setup |
| libcpu/risc-v/common64/interrupt_gcc.S | Adds SMP-specific interrupt handling and local IRQ control |
| libcpu/risc-v/common64/cpuport.c | Adds secondary CPU entry point and context switch for SMP |
| libcpu/risc-v/common64/context_gcc.S | Adds CPU lock status restoration in context switches |
| libcpu/risc-v/common64/atomic_riscv.c | Implements RISC-V atomic operations using AMO instructions |
| bsp/qemu-virt64-riscv/run.sh | Updates to support dynamic SMP core configuration |
| bsp/qemu-virt64-riscv/qemu-dbg.sh | Updates to support dynamic SMP core configuration |
| bsp/qemu-virt64-riscv/link_cpus.lds | Defines the number of CPUs for linker |
| bsp/qemu-virt64-riscv/link.lds | Updates stack allocation for multiple CPUs |
| bsp/qemu-virt64-riscv/driver/board.c | Adds IPI initialization during board setup |
| bsp/qemu-virt64-riscv/SConstruct | Extracts CPU count from rtconfig.h and writes to linker script |
libcpu/risc-v/virt64/interrupt.c
Outdated
|
|
||
| void rt_hw_spin_lock_init(rt_hw_spinlock_t *_lock) | ||
| { | ||
| union rt_hw_spinlock_t *lock = (void *)_lock; |
There was a problem hiding this comment.
Bug/错误: The variable lock is declared but never used in the function. The next line directly uses _lock->slock, making this cast pointless. Either use the lock variable consistently or remove it.
中文: 声明的变量 lock 从未被使用。下一行直接使用了 _lock->slock，使得这个类型转换毫无意义。要么一致地使用 lock 变量，要么删除它。
| union rt_hw_spinlock_t *lock = (void *)_lock; |
There was a problem hiding this comment.
It has been modified according to the content.
libcpu/risc-v/virt64/interrupt.c
Outdated
|
|
||
| void rt_hw_ipi_send(int ipi_vector, unsigned int cpu_mask) | ||
| { | ||
| int cpuid = cpu_mask & -cpu_mask; // get the lowest set bit |
There was a problem hiding this comment.
Bug/错误: The logic to extract the CPU ID from cpu_mask is incorrect. The expression cpu_mask & -cpu_mask isolates the lowest set bit (e.g., 0b0100 becomes 0b0100), but this is not the bit position. For example, if cpu_mask = 4 (bit 2), the result is 4, not 2. This will cause an out-of-bounds array access. Use __builtin_ctz(cpu_mask) to get the bit position.
中文: 从 cpu_mask 提取 CPU ID 的逻辑不正确。表达式 cpu_mask & -cpu_mask 会分离出最低位的 1（例如 0b0100 变成 0b0100），但这不是位位置。例如，如果 cpu_mask = 4（第 2 位），结果是 4 而不是 2。这将导致数组越界访问。应使用 __builtin_ctz(cpu_mask) 获取位位置。
| int cpuid = cpu_mask & -cpu_mask; // get the lowest set bit | |
| int cpuid = __builtin_ctz(cpu_mask); // get the bit position of the lowest set bit |
There was a problem hiding this comment.
It has been modified according to the content.
libcpu/risc-v/common64/cpuport.c
Outdated
| return 0; | ||
| #else | ||
| /* Currently, the hartid is stored in the satp register. */ | ||
| uint32_t hart_id; |
There was a problem hiding this comment.
Bug/错误: Using uint32_t for hart_id is problematic on 64-bit systems. The satp CSR is 64-bit on RV64 systems. Using a 32-bit variable will truncate the value, potentially losing important bits. Change to rt_ubase_t or uint64_t.
中文: 使用 uint32_t 作为 hart_id 在 64 位系统上存在问题。在 RV64 系统上，satp CSR 是 64 位的。使用 32 位变量会截断值，可能丢失重要位。应改为 rt_ubase_t 或 uint64_t。
| uint32_t hart_id; | |
| rt_ubase_t hart_id; |
There was a problem hiding this comment.
It has been modified according to the content.
libcpu/risc-v/common64/startup_gcc.S
Outdated
| li t0, __STACKSIZE__ | ||
| add sp, sp, t0 | ||
| #else | ||
| csrw satp, a0 /* Currently, the hartid is stored in the satp register. */ |
There was a problem hiding this comment.
Best Practices/最佳实践: Storing the hartid in the satp register is unconventional and potentially problematic. The satp CSR is designed for address translation control and may be modified by MMU initialization code. Consider using a dedicated memory location or a different CSR (like sscratch temporarily, or a memory-mapped per-CPU data structure).
中文: 将 hartid 存储在 satp 寄存器中是非常规的，且可能存在问题。satp CSR 是为地址转换控制设计的，可能会被 MMU 初始化代码修改。建议使用专用的内存位置或不同的 CSR（如临时使用 sscratch，或使用内存映射的每 CPU 数据结构）。
| csrw satp, a0 /* Currently, the hartid is stored in the satp register. */ | |
| /* Removed storing hartid in satp register; hartid is stored in boot_hartid memory location. */ |
libcpu/risc-v/common64/cpuport.c
Outdated
| #ifdef RT_USING_SMP | ||
| /* ipi init */ | ||
| rt_hw_ipi_init(); | ||
| #endif /* RT_USING_SMP */ |
There was a problem hiding this comment.
Maintainability/可维护性: Redundant nested #ifdef RT_USING_SMP check inside secondary_cpu_entry() which is already within an #ifdef RT_USING_SMP block (line 159). Remove the inner conditional.
中文: 在 secondary_cpu_entry() 中存在冗余的 #ifdef RT_USING_SMP 嵌套检查，而该函数已经在 #ifdef RT_USING_SMP 块内（第 159 行）。应删除内部的条件编译。
| #ifdef RT_USING_SMP | |
| /* ipi init */ | |
| rt_hw_ipi_init(); | |
| #endif /* RT_USING_SMP */ | |
| /* ipi init */ | |
| rt_hw_ipi_init(); |
There was a problem hiding this comment.
It has been modified according to the content.
libcpu/risc-v/common64/cpuport.c
Outdated
| int hart, ret; | ||
|
|
||
| /* translate kernel virtual _start to physical address */ | ||
| entry_pa = (rt_uint64_t)&_start;//(rt_uint64_t)rt_kmem_v2p((void *)&_start); |
There was a problem hiding this comment.
Maintainability/可维护性: Commented-out code should be removed. If virtual-to-physical translation is needed, either implement it or add a TODO comment explaining why it's not needed currently.
中文: 应删除被注释的代码。如果需要虚拟地址到物理地址的转换，要么实现它，要么添加 TODO 注释解释为何当前不需要。
| entry_pa = (rt_uint64_t)&_start;//(rt_uint64_t)rt_kmem_v2p((void *)&_start); | |
| entry_pa = (rt_uint64_t)&_start; /* TODO: Virtual-to-physical translation is not needed here because &_start is already a physical address on this platform. */ |
There was a problem hiding this comment.
It has been modified according to the content.
|
hartid目前保存在satp寄存器，是asid部分么，如果是这样，smart版本会受影响 |
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
8561395 to
d892bf8
Compare
The CLA has now been signed. |
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
d892bf8 to
1df567e
Compare
| #include "encoding.h" | ||
| #include "riscv.h" | ||
| #include "interrupt.h" | ||
|
|
There was a problem hiding this comment.
| #include "sbi.h" |
目前保存在了satp的低位，也就是物理页号（PPN）字段，这应当也会影响smart版本？ 那是否能进行宏隔离：#if defined(RT_USING_SMP) && !defined(RT_USING_SMART), 因为对于rtthread-smart版本还没有好的hartid存储方案。但是对于rtthread版本存储在satp里应当不会有影响？ |
|
道理说 RT-Thread 版本也可能会开启 MMU，所以这么做可能还是会有问题，不过办法有的是。最主要的是 SMP 相关压力测试 OK 吗？ |
Fix issues with non-standard formatting Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
1df567e to
9cdbc36
Compare
OK的，我应该怎么做以完成压力测试呢 |
|
应该是有 utest 的 SMP 测试，可以选上运行看看 SMP 是否稳定 |
我看了一下src/utest/smp下的所有单元测试逻辑，也运行了这些单元测试，目前能顺利通过这些单元测试且符合预期。 |
可以看看之前我这个 PR 的一种思路：#10320 |
…rt framework/environment By defining a custom .percpu section and controlling the MMU mapping page table entries, different cores can access different physical memory when accessing the same virtual address (percpu_hartid), thereby storing their respective hartids. If the satp register is not used, the hartid is stored in the satp register. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
|
测试通过就可以了 |
Fix issues with non-standard formatting Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
|
作者可以把smp的utest添加到ci看护中，这样出现问题通过CI也可以查看到：https://github.com/RT-Thread/rt-thread/blob/master/.github/workflows/utest_auto_run.yml |
|
|
||
| #ifndef RT_USING_SMP | ||
| //if enable RT_USING_SMP, it will finished by rt_cpus_lock_status_restore. | ||
| #ifdef RT_USING_SMART |
|
|
||
| percpu_hartid = percpu_ptr; | ||
|
|
||
| // from virtual address to physical address |
看起来Smart-SMP模式有问题： https://github.com/RT-Thread/rt-thread/actions/runs/19415696503/job/55543632031?pr=10887 |
| int cpuid = __builtin_ctz(cpu_mask); // get the bit position of the lowest set bit | ||
| ipi_vectors[cpuid] |= (uint8_t)ipi_vector; | ||
| sbi_send_ipi((const unsigned long *)&cpu_mask); |
There was a problem hiding this comment.
[Concurrency/并发问题]: Race condition in IPI vector handling. The ipi_vectors[cpuid] is modified without atomic operations or locking, which can cause lost IPI events in SMP environments.
English: The code reads cpu_mask, extracts cpuid, and then performs a non-atomic OR operation on ipi_vectors[cpuid]. If multiple CPUs send IPIs to the same target CPU simultaneously, updates can be lost due to the read-modify-write race.
中文：代码读取 cpu_mask，提取 cpuid，然后对 ipi_vectors[cpuid] 执行非原子的 OR 操作。如果多个 CPU 同时向同一目标 CPU 发送 IPI，由于读-改-写竞争，更新可能会丢失。
Suggestion/建议: Use atomic OR operation:
rt_hw_atomic_or((volatile rt_atomic_t *)&ipi_vectors[cpuid], (rt_atomic_t)ipi_vector);
libcpu/risc-v/virt64/interrupt.c
Outdated
|
|
||
| ipi_vector = ipi_vectors[rt_cpu_get_id()]; | ||
| while (ipi_vector) | ||
| { | ||
| int bitpos = __builtin_ctz(ipi_vector); | ||
| ipi_vector &= ~(1 << bitpos); | ||
| if (bitpos < RT_MAX_IPI && ipi_desc[bitpos].handler != RT_NULL) | ||
| { | ||
| /* call the irq service routine */ | ||
| ipi_desc[bitpos].handler(bitpos, ipi_desc[bitpos].param); | ||
| } | ||
| } | ||
| ipi_vectors[rt_cpu_get_id()] = 0; |
There was a problem hiding this comment.
[Concurrency/并发问题]: Race condition in IPI handler. The clearing of ipi_vectors[rt_cpu_get_id()] is not atomic and can race with rt_hw_ipi_send().
English: Line 233 clears the IPI vector without synchronization. If another CPU calls rt_hw_ipi_send() between lines 222-233, the new IPI could be lost when line 233 executes.
中文：第 233 行在没有同步的情况下清除 IPI 向量。如果另一个 CPU 在第 222-233 行之间调用 rt_hw_ipi_send()，当第 233 行执行时，新的 IPI 可能会丢失。
Suggestion/建议: Use atomic operations to clear only the handled bits:
rt_hw_atomic_and((volatile rt_atomic_t *)&ipi_vectors[rt_cpu_get_id()],
(rt_atomic_t)~handled_vectors);And track which vectors were handled in the loop.
| ipi_vector = ipi_vectors[rt_cpu_get_id()]; | |
| while (ipi_vector) | |
| { | |
| int bitpos = __builtin_ctz(ipi_vector); | |
| ipi_vector &= ~(1 << bitpos); | |
| if (bitpos < RT_MAX_IPI && ipi_desc[bitpos].handler != RT_NULL) | |
| { | |
| /* call the irq service routine */ | |
| ipi_desc[bitpos].handler(bitpos, ipi_desc[bitpos].param); | |
| } | |
| } | |
| ipi_vectors[rt_cpu_get_id()] = 0; | |
| rt_uint32_t handled_vectors = 0; | |
| ipi_vector = ipi_vectors[rt_cpu_get_id()]; | |
| while (ipi_vector) | |
| { | |
| int bitpos = __builtin_ctz(ipi_vector); | |
| ipi_vector &= ~(1 << bitpos); | |
| handled_vectors |= (1 << bitpos); | |
| if (bitpos < RT_MAX_IPI && ipi_desc[bitpos].handler != RT_NULL) | |
| { | |
| /* call the irq service routine */ | |
| ipi_desc[bitpos].handler(bitpos, ipi_desc[bitpos].param); | |
| } | |
| } | |
| rt_hw_atomic_and((volatile rt_atomic_t *)&ipi_vectors[rt_cpu_get_id()], (rt_atomic_t)~handled_vectors); |
| @@ -0,0 +1 @@ | |||
| RT_CPUS_NR = 8; | |||
There was a problem hiding this comment.
[nitpick] [Best Practices/最佳实践]: Hardcoded maximum CPUs value in linker script.
English: The value RT_CPUS_NR = 8 is hardcoded in the linker script but may not match the actual RT_CPUS_NR configured in rtconfig.h. While the SConstruct script generates this file, having a default of 8 could be misleading.
中文：值 RT_CPUS_NR = 8 在链接器脚本中硬编码，但可能与 rtconfig.h 中配置的实际 RT_CPUS_NR 不匹配。虽然 SConstruct 脚本生成此文件，但默认值为 8 可能会产生误导。
Suggestion/建议: Add a comment explaining that this file is auto-generated:
/* Auto-generated by SConstruct - do not edit manually */
RT_CPUS_NR = 8;Or add this to a .gitignore to avoid committing the generated file.
| if grep -q "#define RT_USING_SMP" ./rtconfig.h 2>/dev/null; then | ||
| hart_num=$(grep "RT_CPUS_NR = [0-9]*;" ./link_cpus.lds | awk -F'[=;]' '{gsub(/ /, "", $2); print $2}') | ||
| if [ -z "$hart_num" ]; then | ||
| hart_num=1 | ||
| fi | ||
| QEMU_CMD="$QEMU_CMD -smp $hart_num" | ||
| fi |
There was a problem hiding this comment.
[nitpick] [Best Practices/最佳实践]: Missing error handling in shell script.
English: The script uses grep to check for RT_USING_SMP but doesn't handle the case where rtconfig.h doesn't exist (though 2>/dev/null suppresses errors). The awk command could also fail silently if the pattern doesn't match.
中文：脚本使用 grep 检查 RT_USING_SMP，但不处理 rtconfig.h 不存在的情况（尽管 2>/dev/null 抑制了错误）。如果模式不匹配，awk 命令也可能静默失败。
Suggestion/建议: Add validation:
if grep -q "#define RT_USING_SMP" ./rtconfig.h 2>/dev/null; then
hart_num=$(grep "RT_CPUS_NR = [0-9]*;" ./link_cpus.lds 2>/dev/null | awk -F'[=;]' '{gsub(/ /, "", $2); print $2}')
if [ -z "$hart_num" ] || [ "$hart_num" -lt 1 ]; then
echo "Warning: Invalid or missing RT_CPUS_NR, defaulting to 1"
hart_num=1
fi
QEMU_CMD="$QEMU_CMD -smp $hart_num"
fi| void rt_hw_percpu_hartid_init(rt_ubase_t *percpu_ptr, rt_ubase_t hartid) | ||
| { | ||
| rt_ubase_t *percpu_hartid_paddr; | ||
| rt_size_t percpu_size = (rt_size_t)((rt_ubase_t)&__percpu_end - (rt_ubase_t)&__percpu_start); | ||
|
|
||
| percpu_hartid = percpu_ptr; | ||
|
|
||
| // from virtual address to physical address | ||
| percpu_ptr = (rt_ubase_t *)((rt_ubase_t)percpu_ptr + (rt_ubase_t)rt_kmem_pvoff()); | ||
| percpu_hartid_paddr = percpu_ptr; | ||
|
|
||
| /* Save to the real area */ | ||
| *(rt_ubase_t *)((void *)percpu_hartid_paddr + hartid * percpu_size) = hartid; | ||
| } |
There was a problem hiding this comment.
[Security/安全问题]: Missing bounds check on hartid parameter.
English: The rt_hw_percpu_hartid_init() function (line 218) uses hartid to calculate an offset without validating that hartid < RT_CPUS_NR. An out-of-bounds hartid could cause buffer overflow when writing to the percpu area.
中文：rt_hw_percpu_hartid_init() 函数（第 218 行）使用 hartid 计算偏移量，而不验证 hartid < RT_CPUS_NR。越界的 hartid 可能会在写入 percpu 区域时导致缓冲区溢出。
Suggestion/建议: Add bounds checking:
void rt_hw_percpu_hartid_init(rt_ubase_t *percpu_ptr, rt_ubase_t hartid)
{
RT_ASSERT(hartid < RT_CPUS_NR);
rt_ubase_t *percpu_hartid_paddr;
rt_size_t percpu_size = (rt_size_t)((rt_ubase_t)&__percpu_end - (rt_ubase_t)&__percpu_start);
// ... rest of the function
}| unsigned long get_free_page(void) | ||
| { | ||
| static unsigned long page_off = 0UL; | ||
|
|
||
| if (page_off < ARCH_PAGE_SIZE / sizeof(unsigned long)) | ||
| { | ||
| return (unsigned long)(__init_page_array[page_off++].page); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
[nitpick] [Performance/性能问题]: Inefficient page allocation in early boot.
English: The get_free_page() function uses a simple linear allocator that doesn't track which pages are freed. Once page_off reaches the limit, no more pages can be allocated even if some were freed.
中文：get_free_page() 函数使用简单的线性分配器，不跟踪哪些页面被释放。一旦 page_off 达到限制，即使某些页面被释放，也无法再分配页面。
Suggestion/建议: Document that this is an early-stage allocator that doesn't support freeing, or implement a proper free list if pages need to be recycled during early boot.
| unsigned long get_free_page(void) | ||
| { | ||
| static unsigned long page_off = 0UL; | ||
|
|
||
| if (page_off < ARCH_PAGE_SIZE / sizeof(unsigned long)) | ||
| { | ||
| return (unsigned long)(__init_page_array[page_off++].page); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
[Concurrency/并发问题]: Non-thread-safe static variable in early page allocator.
English: The static variable page_off in get_free_page() (line 348) is modified without synchronization. If multiple CPUs call this function during early boot, they could receive the same page, causing corruption.
中文：get_free_page() 中的静态变量 page_off（第 348 行）在没有同步的情况下被修改。如果多个 CPU 在早期启动期间调用此函数，它们可能会收到相同的页面，导致损坏。
Suggestion/建议: Use atomic increment or ensure this function is only called from the boot CPU during early initialization:
unsigned long get_free_page(void)
{
static rt_atomic_t page_off = 0;
rt_atomic_t old_off = rt_hw_atomic_add(&page_off, 1);
if (old_off < ARCH_PAGE_SIZE / sizeof(unsigned long))
{
return (unsigned long)(__init_page_array[old_off].page);
}
return 0;
}
libcpu/risc-v/common64/cpuport.c
Outdated
| // if not enable MMU or pvoff==0, read hartid from satp register | ||
| rt_ubase_t hartid; | ||
| asm volatile("csrr %0, satp" : "=r"(hartid)); | ||
| return hartid; |
There was a problem hiding this comment.
[Bug/错误]: Incorrect hartid retrieval in rt_hw_cpu_id() when MMU is disabled.
English: When rt_kmem_pvoff() == 0, the code reads the entire satp register value as hartid (line 90). However, satp is a 64-bit register where only the lower bits should contain the hartid. If the MODE field or other bits are set, this will return an incorrect hartid.
中文：当 rt_kmem_pvoff() == 0 时，代码将整个 satp 寄存器值读取为 hartid（第 90 行）。然而，satp 是一个 64 位寄存器，其中只有低位应该包含 hartid。如果设置了 MODE 字段或其他位，这将返回不正确的 hartid。
Suggestion/建议: Mask the hartid value appropriately:
// if not enable MMU or pvoff==0, read hartid from satp register
rt_ubase_t hartid;
asm volatile("csrr %0, satp" : "=r"(hartid));
return hartid & 0xFFFF; // Assuming hartid fits in lower 16 bitsOr better yet, document the valid range and add an assertion.
| return hartid; | |
| /* Only lower 16 bits of satp are used for hartid; mask to avoid MODE/ASID/PPN bits */ | |
| return hartid & 0xFFFF; |
libcpu/risc-v/common64/context_gcc.S
Outdated
| /* | ||
| * Pass the previous CPU lock status to | ||
| * rt_cpus_lock_status_restore for restoration | ||
| */ |
There was a problem hiding this comment.
[nitpick] [Documentation/文档问题]: Comment has trailing whitespace and unclear formatting.
English: The comment spans lines 86-88 with inconsistent indentation and trailing spaces, making it harder to read.
中文：注释跨越第 86-88 行，缩进不一致且有尾随空格，使其难以阅读。
Suggestion/建议: Format the comment consistently:
/* Pass the previous CPU lock status to
* rt_cpus_lock_status_restore for restoration */ | for (hart = 0; hart < RT_CPUS_NR; hart++) | ||
| { | ||
| if (hart == boot_hartid) | ||
| continue; | ||
|
|
||
| ret = sbi_hsm_hart_start((unsigned long)hart, | ||
| (unsigned long)entry_pa, | ||
| 0UL); | ||
| if (ret) | ||
| { | ||
| rt_kprintf("sbi_hsm_hart_start failed for hart %d: %d\n", hart, ret); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[Bug/错误]: Missing validation in rt_hw_secondary_cpu_up().
English: The function iterates through all harts from 0 to RT_CPUS_NR but doesn't validate that the hart IDs are sequential or that boot_hartid < RT_CPUS_NR. If boot_hartid is a large value (e.g., on systems with non-contiguous hart IDs), the comparison on line 204 could be incorrect.
中文：该函数从 0 到 RT_CPUS_NR 迭代所有 hart，但不验证 hart ID 是否连续或 boot_hartid < RT_CPUS_NR。如果 boot_hartid 是一个大值（例如，在具有非连续 hart ID 的系统上），第 204 行的比较可能不正确。
Suggestion/建议: Add validation and document assumptions:
/* Assumes hart IDs are in range [0, RT_CPUS_NR) */
RT_ASSERT(boot_hartid < RT_CPUS_NR);
for (hart = 0; hart < RT_CPUS_NR; hart++)
{
if (hart == boot_hartid)
continue;
// ... rest of loop
}
好的，正在排查问题 |
|
我看CI通过了，请问是smart版本也OK了吗？@Tm-C-mT |
|
我在这个PR里面添加了Virt64的SMP-CI看护，等合并后可以看看效果 |
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
1487851 to
4985f64
Compare
目前来看是OK了 |
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
4985f64 to
8e6ded8
Compare
Fix some potential concurrency issues and code standardization problems. The .percpu section is only used when both ARCH_MM_MMU and RT_USING_SMP are enabled. However, there is a certain amount of space waste since no macro guard is added for it in the link script currently. The physical memory of QEMU started in CI is 128MB, so RT_HW_PAGE_END is modified from the original +256MB to +128MB. Signed-off-by: Mengchen Teng <teng_mengchen@163.com>
Tm-C-mT
force-pushed
the
qemu-virt64-riscv
branch
from
8e6ded8 to
c43d720
Compare
|
@Yaochenger 还请帮忙测试下 |
|
请移除工程中的重复文件 |
9 participants
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
目前qemu-virt64-riscv BSP不支持SMP模式下的运行，想让riscv64支持SMP架构。
这种修改方式是否合理？如不合理，修改后是否有可能被接受？
你的解决方案是什么 (what is your solution)
1、添加SMP架构下的必要接口，详见commit: 6c0a7d1
2、解决使能SMP后，hart为1时的启动挂死问题。添加rt_cpus_lock_status_restore与中断中的调度逻辑。详见commit: 7948f50
3、解决hart=2时的多核启动问题。hartid目前保存在了satp寄存器中，栈区地址根据hartid分配。并实现rt_hw_secondary_cpu_up与secondary_cpu_entry。详见commit: 402ec68
4、实现自旋锁及IPI处理逻辑。详见commit: c7cbe2e 及 8386d29。
5、qemu-virt64-riscv BSP根据config配置动态启动。详见commit: 7a770ce
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up