Skip to content

Conversation

@FlowerBlackG
Copy link
Member

Wrap read and write operations on consts::NCPU so users won't see unsafe anymore.

@enkerewpo enkerewpo requested a review from Copilot April 20, 2025 16:55
@enkerewpo enkerewpo added the feature New feature or request label Apr 20, 2025
@enkerewpo enkerewpo added this to the hvisor-v0.1.2 milestone Apr 20, 2025
Copy link
Contributor

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 wraps all direct accesses to the unsafe global NCPU with safe accessor functions to hide unsafety from users.

  • Replace direct unsafe reads/writes on NCPU with consts::ncpu() and consts::set_ncpu().
  • Update related call sites across zone, main, hypercall, irqchip, and other modules.
  • Remove the now-unused MAX_CPUS constant from virtio_trampoline.

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/zone.rs Updated Zone initialization to use consts::ncpu() instead of unsafe consts::NCPU.
src/main.rs Replaced the unsafe block that assigned to consts::NCPU with a call to consts::set_ncpu(ncpu).
src/ivc.rs Renamed the unused tuple field from is_new to _is_new in the ivc record insertion.
src/hypercall/mod.rs Updated loops and conditionals to use consts::ncpu() instead of unsafe consts::NCPU.
src/device/virtio_trampoline.rs Removed the unused MAX_CPUS constant to enforce dynamic CPU count.
src/device/irqchip/gicv3/vgic.rs Replaced unsafe consts::NCPU calls with safe consts::ncpu() calls in loops/conditions.
src/device/irqchip/gicv3/mod.rs Updated PendingIrqs initialization to use consts::ncpu().
src/device/irqchip/gicv3/gicr.rs Replaced unsafe access to consts::NCPU with consts::ncpu() in iteration.
src/device/irqchip/gicv2/mod.rs Updated PendingIrqs initialization to use consts::ncpu().
src/consts.rs Added safe accessor functions (ncpu and set_ncpu) replacing direct unsafe access.
src/arch/aarch64/mm.rs Updated setup_parange and assert conditions to use consts::ncpu() instead of unsafe consts::NCPU.
Comments suppressed due to low confidence (2)

src/ivc.rs:160

  • [nitpick] The use of '_is_new' properly indicates that the variable is intentionally unused; consider adding a brief in-line comment if this placeholder might be revisited in future changes.
if let Ok((_is_new, start_paddr)) = insert_ivc_record(ivc_config, self.id as _)

src/device/virtio_trampoline.rs:43

  • Since MAX_CPUS is removed in favor of a dynamic CPU count via consts::ncpu(), please ensure no residual references to this constant remain elsewhere in the codebase.
pub const MAX_CPUS: usize = 4;

@enkerewpo enkerewpo requested a review from KouweiLee April 25, 2025 13:05
@KouweiLee KouweiLee closed this Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants