Skip to content

Conversation

buhenxihuan
Copy link
Contributor

Since the feature of dtb parsing has not yet been merged into the mainline, the address in the toml configuration is still used to refresh the dcache

@buhenxihuan buhenxihuan requested review from luodeb and aarkegz April 17, 2025 08:57
@buhenxihuan buhenxihuan force-pushed the master branch 2 times, most recently from f62070c to c7724fa Compare April 18, 2025 01:54
Copy link
Contributor

@aarkegz aarkegz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should create a arch-independent invalidate_dcache functions, and provide stub implementations for x86_64 and riscv?

@buhenxihuan
Copy link
Contributor Author

buhenxihuan commented Apr 18, 2025

Maybe we should create a arch-independent invalidate_dcache functions, and provide stub implementations for x86_64 and riscv?

If x86 and riscv also need to be refreshed, then such an interface is indeed needed.

@ZR233
Copy link
Contributor

ZR233 commented Apr 28, 2025

Maybe we should create a arch-independent invalidate_dcache functions, and provide stub implementations for x86_64 and riscv?

If x86 and riscv also need to be refreshed, then such an interface is indeed needed.

arceos主线曾提起过cache相关功能,但未合并,且没给出未合并原因,与这个PR功能重复 arceos-org/arceos#210 @aarkegz

@buhenxihuan buhenxihuan changed the title feat: add dcache invalidate support rk3588 smp May 12, 2025
Copy link
Contributor

@aarkegz aarkegz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small fixes and changes are necessary.

@aarkegz aarkegz requested a review from Copilot May 15, 2025 07:31
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables support for RK3588 SMP on aarch64 by revising how virtual CPUs are activated and integrating new cache management functions along with updated configuration and build scripts.

  • Update vCPU activation logic to map physical CPU IDs to vCPU IDs from the configuration
  • Introduce a new cache module (with assembly routines) and integrate cache cleaning in image loading
  • Update Makefile and platform configuration files to support GICv3 and RK3588-specific naming

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/vmm/vcpus.rs Adjusts vCPU activation to use configuration mapping instead of direct physical IDs
src/vmm/mod.rs Conditionally includes the new cache module for aarch64
src/vmm/images.rs Invokes cache cleaning for memory regions on aarch64
src/vmm/cache.rs & src/vmm/cache.S Introduces new cache management routines via assembly
scripts/make/qemu.mk, features.mk, Makefile Updates build scripts for GICv3 support and RK3588 platform identification
configs/*.toml Updates configuration files for the RK3588 SMP and related platforms
Comments suppressed due to low confidence (2)

src/vmm/vcpus.rs:298

  • Consider enhancing the error message with additional context or instructions to help diagnose misconfiguration issues if the physical CPU ID is missing from the mapping.
.expect(&format!("Physical CPU ID {} not found in VM configuration", target_cpu));

Makefile:158

  • [nitpick] Ensure that the platform name 'aarch64-rk3588j-hv' is consistently used across all related configuration and documentation files to avoid confusion.
else ifeq ($(PLAT_NAME), aarch64-rk3588j-hv)

buhenxihuan and others added 5 commits May 15, 2025 15:51
…e vCPU startup logic

- Add phys_cpu_ids configuration item in linux-rk3588-aarch64-smp.toml
- Modify vcpu_run function to start corresponding vCPU according to configured physical CPU ID mapping
- Optimize vCPU startup logic to improve system startup efficiency and stability
* move cache invalidators into `utils::arch` mod

* formatted

* fixed missing `pub` vis

* fix two missing commas in `qemu.mk`, which caused `make ARCH=aarch64 run` failed to start
Copy link
Contributor

@aarkegz aarkegz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Remote CI cannot run with PR from another repo. I've tested this PR on another local branch, so it's safe to do a manual config override.

@aarkegz aarkegz merged commit 32f9ac6 into arceos-hypervisor:master May 15, 2025
40 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants