Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes and improvements in GIC driver #910

Merged
merged 11 commits into from
Apr 6, 2023

Conversation

NathanRoyer
Copy link
Member

@NathanRoyer NathanRoyer commented Mar 28, 2023

Small changes to the gic crate:

  • Differentiate SpiDestination & IpiTargetCpu, which were previously TargetCpu
  • Use CpuId & MpidrValue when appropriate
  • Fixed TargetList so that it doesn't allow routing SPIs & IPIs to arbitrary CpuId values
  • Implemented TryFrom<u64> for MpidrValue to identify existing CPUs based on GIC register contents

Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Everything looks generally fine, I just made a few suggestions for minor improvements.

kernel/gic/src/gic/mod.rs Outdated Show resolved Hide resolved
kernel/gic/src/gic/mod.rs Outdated Show resolved Hide resolved
kernel/cpu/src/aarch64.rs Outdated Show resolved Hide resolved
kernel/gic/src/gic/mod.rs Show resolved Hide resolved
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Looks solid, just left one comment clarifying SpiDestination behavior

kernel/gic/src/gic/mod.rs Show resolved Hide resolved
kevinaboos added a commit that referenced this pull request Apr 4, 2023
* New `arm_boards` crate: per-board definitions for aarch64 builds
  * The default is for QEMU's basic `virt` machine spec.
  * Currently, this specifies the number of CPUs, their IDs, and
    the interrupt controller configuration including one GIC
    redistributor base address per core.
  * The `gic` crate now uses this instead of defining its own
    internal configuration values specific to QEMU.
  * `multicore_bringup` now uses the list of `CpuId`s from
    the selected ARM board configuration.

* The redistributor initialization routine code now enables
  the dispatch of "1 of N"-distributed SPIs.
  * All CPUs' redistributors are now initialized by the
    bootstrap processor as part of `interrupts::init()`.

* Incorporates two fixes from #910:
  * Fix a bug in `get_spi_target` & `set_spi_target` where
    an atomic u32 read/write was used to manipulate a
    u64 MMIO register.
  * Introduce `Offset32` and `Offset64` types to distinguish
    32-bit and 64-bit registers more clearly.

Co-authored-by: Kevin Boos <kevinaboos@gmail.com>
github-actions bot pushed a commit that referenced this pull request Apr 4, 2023
* New `arm_boards` crate: per-board definitions for aarch64 builds
  * The default is for QEMU's basic `virt` machine spec.
  * Currently, this specifies the number of CPUs, their IDs, and
    the interrupt controller configuration including one GIC
    redistributor base address per core.
  * The `gic` crate now uses this instead of defining its own
    internal configuration values specific to QEMU.
  * `multicore_bringup` now uses the list of `CpuId`s from
    the selected ARM board configuration.

* The redistributor initialization routine code now enables
  the dispatch of "1 of N"-distributed SPIs.
  * All CPUs' redistributors are now initialized by the
    bootstrap processor as part of `interrupts::init()`.

* Incorporates two fixes from #910:
  * Fix a bug in `get_spi_target` & `set_spi_target` where
    an atomic u32 read/write was used to manipulate a
    u64 MMIO register.
  * Introduce `Offset32` and `Offset64` types to distinguish
    32-bit and 64-bit registers more clearly.

Co-authored-by: Kevin Boos <kevinaboos@gmail.com> d2ed7ac
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

left a few comments about iterator usage and type conversion implementations

kernel/gic/src/gic/dist_interface.rs Outdated Show resolved Hide resolved
kernel/gic/src/gic/mod.rs Outdated Show resolved Hide resolved
kernel/gic/src/gic/mod.rs Outdated Show resolved Hide resolved
kernel/arm_boards/src/mpidr.rs Outdated Show resolved Hide resolved
@kevinaboos kevinaboos merged commit b408d0a into theseus-os:theseus_main Apr 6, 2023
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