Skip to content

Separate lock-free fields from Zone and make fields private#270

Merged
li041 merged 1 commit intosyswonder:devfrom
li041:zone-refactor
Mar 19, 2026
Merged

Separate lock-free fields from Zone and make fields private#270
li041 merged 1 commit intosyswonder:devfrom
li041:zone-refactor

Conversation

@li041
Copy link
Contributor

@li041 li041 commented Mar 16, 2026

Overview

This PR refactors the Zone structure to clearly separate fields that require synchronization from fields that do not, and improves encapsulation by removing direct public field access.

Changes

1. Separate lock-free fields

Previously, all fields were stored directly inside Zone, even though some fields do not require synchronization.

This PR introduces a new structure:

pub struct ZoneInner

which contains fields that require synchronization. ZoneInner is protected by an RwLock inside Zone.

Fields that do not require whole locking remain directly in Zone, such as:

  • name
  • id
  • is_err

This separation makes it explicit which fields require locking and avoids unnecessary locking for immutable or atomic data.

2. Make struct fields private

All fields in Zone and ZoneInner were previously declared as pub.
They are now made private and accessed through accessor methods.

This prevents external code from directly mutating internal state and ensures that access patterns follow the intended synchronization model.

Result

After this change:

  • Lock-free fields are clearly separated from synchronized state.
  • Access to internal fields is controlled through APIs.

@li041 li041 requested review from Copilot, dallasxy and liulog and removed request for Copilot March 16, 2026 09:18
@li041 li041 requested review from Copilot and liulog and removed request for liulog March 16, 2026 09:19
@li041 li041 marked this pull request as draft March 16, 2026 09:22
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 refactors zone state management by splitting Zone into lock-free fields and synchronized state (ZoneInner) behind an RwLock, and updates call sites to use accessor methods instead of direct public field access.

Changes:

  • Introduce ZoneInner (locked) and move mutable/shared state from Zone into it; keep id/name/is_err lock-free (atomic/immutable).
  • Replace Arc<RwLock<Zone>> with Arc<Zone> and update zone interactions across PCI, MMIO, hypercalls, and irqchip/device/arch code to use Zone::read() / Zone::write().
  • Adjust PCI APIs for more read-only access (e.g., VirtualRootComplex::get_device_by_base(&self, ...)).

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/zone.rs Introduces ZoneInner behind RwLock, makes fields private, updates zone list types and zone lifecycle helpers.
src/pci/pci_test.rs Updates PCI tests to access vpci_bus via ZoneInner guards + accessors.
src/pci/pci_struct.rs Makes get_device_by_base take &self (read-only lookup).
src/pci/pci_handler.rs Updates zone access patterns to use ZoneInner accessors; reduces write-lock usage where possible.
src/pci/pci_config.rs Uses a ZoneInner write guard (inner) to mutate PCI/MMIO-related zone state.
src/memory/mmio.rs Switches to zone.id() and uses ZoneInner::find_mmio_region via zone.read().
src/hypercall/mod.rs Updates cpu_set access via accessors; changes irqchip reset call site to avoid relying on ZoneInner guard.
src/device/sifive_ccache/mod.rs Registers MMIO regions via self.write().mmio_region_register(...).
src/device/irqchip/plic/mod.rs Removes outer zone lock usage and updates irq_bitmap/cpu_* access via ZoneInner methods.
src/device/irqchip/pic/mod.rs Updates to self.id() for DMA table clearing.
src/device/irqchip/pic/ioapic.rs Updates IOAPIC MMIO registration to go through ZoneInner write guard.
src/device/irqchip/gicv3/vgic.rs Updates MMIO registration and IRQ bitmap init to use ZoneInner guards/accessors; updates GPM queries.
src/device/irqchip/gicv3/mod.rs Updates irq bitmap iteration and uses self.id() for ITS reset.
src/device/irqchip/gicv3/gits.rs Updates GPM access and cpu_set accessors for command analysis and address translation.
src/device/irqchip/gicv2/vgic.rs Updates MMIO registration, GPM insertion, and irq bitmap init to use ZoneInner guards/accessors.
src/device/irqchip/aia/vimsic.rs Updates to accessor-based cpu_set and GPM mutation through ZoneInner guard.
src/device/irqchip/aia/vaplic.rs Updates cpu_set access to use accessor methods.
src/device/irqchip/aia/mod.rs Removes outer .read() usage (zone now Arc<Zone>), updates id usage and IRQ bitmap iteration via ZoneInner.
src/device/eic7700_syscrg.rs Registers syscon MMIO regions via a single ZoneInner write guard.
src/cpu_data.rs Changes per-cpu zone handle to Option<Arc<Zone>> and updates GPM activation access pattern.
src/arch/x86_64/zone.rs Updates PT init and arch configuration to mutate/query through ZoneInner guards/accessors.
src/arch/x86_64/mmio.rs Updates GPM queries to use gpm() accessor.
src/arch/x86_64/ipi.rs Uses Zone::cpu_set() accessor (no direct field access).
src/arch/x86_64/hypercall.rs Updates GPM queries to use gpm() accessor.
src/arch/riscv64/zone.rs Uses ZoneInner guard/accessors for GPM insertion and MMIO registration; updates logging.
src/arch/riscv64/cpu.rs Updates cpu_set access to use accessor method.
src/arch/loongarch64/zone.rs Uses ZoneInner guard/accessors for GPM insertion/deletion and MMIO registration; updates logging and queries.
src/arch/loongarch64/trap.rs Updates zone id access to id() (no outer lock).
src/arch/aarch64/zone.rs Uses ZoneInner guard/accessors for PT init and IOMMU PT init; improves non-root error handling for unsupported mem types.
src/arch/aarch64/trap.rs Updates zone id / cpu_set access to accessors.
src/arch/aarch64/ivc.rs Uses ZoneInner guard/accessors for GPM insertion and MMIO registration; updates zone id usage.
src/arch/aarch64/hypercall.rs Updates GPM queries to use gpm() accessor.
src/arch/aarch64/cpu.rs Updates cpu_set access to use accessor method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@li041 li041 force-pushed the zone-refactor branch 2 times, most recently from 5501906 to 9f9c2cc Compare March 16, 2026 09:48
@li041 li041 marked this pull request as ready for review March 16, 2026 11:20
@liulog liulog self-requested a review March 17, 2026 05:16
@ForeverYolo
Copy link
Contributor

Has this PR been tested on any development board so far?

@ForeverYolo
Copy link
Contributor

@li041

@li041
Copy link
Contributor Author

li041 commented Mar 19, 2026

Has this PR been tested on any development board so far?

Yeah, this PR has been tested on qemu platform and Rk3588.

@li041 li041 merged commit 1a9da8a into syswonder:dev Mar 19, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants