From 7a8d64aa3aecdae5ed1231ebf46adcceede62713 Mon Sep 17 00:00:00 2001 From: Klim Tsoutsman Date: Wed, 20 Sep 2023 03:47:21 +1000 Subject: [PATCH 1/4] Add automated QEMU-based testing to CI (#1041) * Added `qemu_test` application that automatically runs all tests and returns a specific exit code from QEMU based on test output. * Added a `test` Make target that enables and runs `qemu_test`. * Added a CI workflow that runs the `test` target * Changed `test_mlx5`, `test_ixgbe`, and `test_block_io` to return an exit code of 0 if the required devices aren't connected. * Changed `test_identity_mapping`, `test_aligned_page_allocation`, and `test_filerw` to fail rather than print if they encounter an error. * Renamed `tls_test` to `test_tls` so it is detected by `qemu_test`. * Changed `test_channel` and `test_tls` to run all tests rather than specifying specific tests in the arguments. * Renamed `test_serial_echo` to `serial_echo` because it isn't really a test with defined success and failure conditions. * Changed `test_task_cancel` to always return 0, because task cancellation is not yet implemented in the mainline. * Skip `test_channel`, as it currently does not work. Signed-off-by: Klimenty Tsoutsman --- .github/workflows/check-clippy.yaml | 6 +- .github/workflows/docs.yaml | 4 +- .github/workflows/test.yaml | 20 +++ Cargo.lock | 70 ++++++---- Makefile | 11 ++ applications/qemu_test/Cargo.toml | 13 ++ applications/qemu_test/src/lib.rs | 120 ++++++++++++++++++ .../Cargo.toml | 2 +- .../src/lib.rs | 0 .../test_aligned_page_allocation/src/lib.rs | 18 ++- applications/test_block_io/Cargo.toml | 1 + applications/test_block_io/src/lib.rs | 19 ++- applications/test_channel/src/lib.rs | 51 ++------ applications/test_filerw/src/lib.rs | 10 +- applications/test_identity_mapping/src/lib.rs | 10 +- applications/test_ixgbe/src/lib.rs | 5 +- applications/test_mlx5/src/lib.rs | 9 +- applications/test_task_cancel/src/lib.rs | 6 + .../{tls_test => test_tls}/Cargo.toml | 2 +- .../{tls_test => test_tls}/src/lib.rs | 12 +- kernel/first_application/Cargo.toml | 1 + kernel/first_application/src/lib.rs | 3 +- theseus_features/Cargo.toml | 10 +- 23 files changed, 283 insertions(+), 120 deletions(-) create mode 100644 .github/workflows/test.yaml create mode 100644 applications/qemu_test/Cargo.toml create mode 100644 applications/qemu_test/src/lib.rs rename applications/{test_serial_echo => serial_echo}/Cargo.toml (95%) rename applications/{test_serial_echo => serial_echo}/src/lib.rs (100%) rename applications/{tls_test => test_tls}/Cargo.toml (94%) rename applications/{tls_test => test_tls}/src/lib.rs (86%) diff --git a/.github/workflows/check-clippy.yaml b/.github/workflows/check-clippy.yaml index a4eb6ef4ed..6456b4ca93 100644 --- a/.github/workflows/check-clippy.yaml +++ b/.github/workflows/check-clippy.yaml @@ -11,10 +11,10 @@ jobs: run: | git submodule update --init --recursive - name: "Install nasm" - run: sudo apt install nasm + run: | + sudo apt update + sudo apt install nasm - name: "Run Clippy" - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} working-directory: . run: | make clippy ARCH=x86_64 diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index d8f4764c90..edf5b21a5a 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -13,7 +13,9 @@ jobs: submodules: recursive - name: "Install nasm" - run: sudo apt install nasm + run: | + sudo apt update + sudo apt install nasm - name: Cache build artifacts uses: actions/cache@v3 diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 0000000000..b2e2f638f1 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,20 @@ +name: QEMU Test +on: + pull_request: + types: [synchronize, opened, reopened] +jobs: + run-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: "Initialize git submodules" + run: | + git submodule update --init --recursive + - name: "Install dependencies" + run: | + sudo apt update + sudo apt install make gcc nasm pkg-config grub-pc-bin mtools xorriso qemu qemu-kvm wget + - name: "Run tests" + working-directory: . + run: make test + timeout-minutes: 10 diff --git a/Cargo.lock b/Cargo.lock index 182e3e0153..65cc0e49c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1181,6 +1181,7 @@ dependencies = [ "log", "mod_mgmt", "path", + "qemu_test", "shell", "spawn", ] @@ -2837,6 +2838,23 @@ dependencies = [ "task", ] +[[package]] +name = "qemu-exit" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bb0fd6580eeed0103c054e3fba2c2618ff476943762f28a645b63b8692b21c9" + +[[package]] +name = "qemu_test" +version = "0.1.0" +dependencies = [ + "app_io", + "path", + "qemu-exit", + "spawn", + "task", +] + [[package]] name = "qp-trie" version = "0.8.1" @@ -3297,6 +3315,19 @@ dependencies = [ "syn 1.0.98", ] +[[package]] +name = "serial_echo" +version = "0.1.0" +dependencies = [ + "app_io", + "core2", + "io", + "log", + "serial_port", + "sync_irq", + "task", +] + [[package]] name = "serial_port" version = "0.1.0" @@ -3894,19 +3925,6 @@ dependencies = [ "task", ] -[[package]] -name = "test_serial_echo" -version = "0.1.0" -dependencies = [ - "app_io", - "core2", - "io", - "log", - "serial_port", - "sync_irq", - "task", -] - [[package]] name = "test_std_fs" version = "0.1.0" @@ -3948,6 +3966,16 @@ dependencies = [ "task", ] +[[package]] +name = "test_tls" +version = "0.1.0" +dependencies = [ + "app_io", + "log", + "test_thread_local", + "thread_local_macro", +] + [[package]] name = "test_wait_queue" version = "0.1.0" @@ -4012,6 +4040,7 @@ dependencies = [ "date", "deps", "example", + "first_application", "heap_eval", "hello", "hull", @@ -4027,6 +4056,7 @@ dependencies = [ "print_fault_log", "ps", "pwd", + "qemu_test", "raw_mode", "rm", "rq", @@ -4034,6 +4064,7 @@ dependencies = [ "rq_eval", "scheduler_eval", "seconds_counter", + "serial_echo", "shell", "swap", "test_aligned_page_allocation", @@ -4050,15 +4081,14 @@ dependencies = [ "test_preemption_counter", "test_restartable", "test_scheduler", - "test_serial_echo", "test_std_fs", "test_sync_block", "test_task_cancel", "test_thread_local", + "test_tls", "test_wait_queue", "test_wasmtime", "theseus_std", - "tls_test", "unified_channel", "unwind_test", "upd", @@ -4127,16 +4157,6 @@ dependencies = [ "sync_irq", ] -[[package]] -name = "tls_test" -version = "0.1.0" -dependencies = [ - "app_io", - "log", - "test_thread_local", - "thread_local_macro", -] - [[package]] name = "tock-registers" version = "0.7.0" diff --git a/Makefile b/Makefile index 3dc34a2ff5..79ba925199 100644 --- a/Makefile +++ b/Makefile @@ -1079,3 +1079,14 @@ endif @sudo cp -vf $(iso) /var/lib/tftpboot/theseus/ @sudo systemctl restart isc-dhcp-server @sudo systemctl restart tftpd-hpa + +test: export override QEMU_FLAGS += -device isa-debug-exit,iobase=0xf4,iosize=0x04 +test: export override QEMU_FLAGS += -nographic +test: export override FEATURES =--features theseus_tests --features first_application/qemu_test +test: $(iso) + # We exit with an exit code of 0 if QEMU's exit code is 17, and 2 otherwise. + # This is because `qemu_test` uses a value of 0x11 to indicate success. + $(QEMU_BIN) $(QEMU_FLAGS); \ + EXIT_CODE=$$?; \ + test $$EXIT_CODE -eq 17 && exit 0; \ + exit 2 diff --git a/applications/qemu_test/Cargo.toml b/applications/qemu_test/Cargo.toml new file mode 100644 index 0000000000..a279a2eab5 --- /dev/null +++ b/applications/qemu_test/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "qemu_test" +version = "0.1.0" +authors = ["Klim Tsoutsman "] +description = "Automated test runner" +edition = "2021" + +[dependencies] +app_io = { path = "../../kernel/app_io" } +path = { path = "../../kernel/path" } +qemu-exit = "3.0.2" +spawn = { path = "../../kernel/spawn" } +task = { path = "../../kernel/task" } diff --git a/applications/qemu_test/src/lib.rs b/applications/qemu_test/src/lib.rs new file mode 100644 index 0000000000..e6a46f726f --- /dev/null +++ b/applications/qemu_test/src/lib.rs @@ -0,0 +1,120 @@ +//! An automated test runner. +//! +//! The application assumes it is running in a QEMU virtual machine and exits +//! from QEMU with different exit codes depending on whether the tests passed or +//! failed. + +#![no_std] + +use alloc::{boxed::Box, string::String, vec::Vec}; + +use app_io::{print, println}; +use path::Path; +use qemu_exit::{QEMUExit, X86}; +use task::{ExitValue, KillReason}; + +extern crate alloc; + +static QEMU_EXIT_HANDLE: X86 = X86::new(0xf4, 0x11); + +pub fn main(_: Vec) -> isize { + task::set_kill_handler(Box::new(|_| { + QEMU_EXIT_HANDLE.exit_failure(); + })) + .unwrap(); + + let dir = task::get_my_current_task() + .map(|t| t.get_namespace().dir().clone()) + .expect("couldn't get namespace dir"); + + let object_files = dir.lock().list(); + + let test_paths = object_files + .into_iter() + .filter_map(|file_name| { + if file_name.starts_with("test_") { + // We must release the lock prior to calling `get_absolute_path` to avoid + // deadlock. + let file = dir.lock().get_file(file_name.as_ref()).unwrap(); + let path = file.lock().get_absolute_path(); + Some((file_name, Path::new(path))) + } else { + None + } + }) + .collect::>(); + + let total = test_paths.len(); + println!("running {} tests", total); + + let mut num_ignored = 0; + let mut num_failed = 0; + + for (file_name, path) in test_paths.into_iter() { + print!("test {} ... ", path); + if ignore(&file_name) { + num_ignored += 1; + println!("ignored"); + } else { + match run_test(path) { + Ok(_) => println!("ok"), + Err(_) => { + num_failed += 1; + println!("failed"); + } + } + } + } + + let result_str = if num_failed > 0 { "failed" } else { "ok" }; + let num_passed = total - num_failed; + println!( + "test result: {result_str}. {num_passed} passed; {num_failed} failed; {num_ignored} \ + ignored", + ); + + if num_failed == 0 { + QEMU_EXIT_HANDLE.exit_success(); + } else { + QEMU_EXIT_HANDLE.exit_failure(); + } +} + +#[allow(clippy::result_unit_err)] +pub fn run_test(path: Path) -> Result<(), ()> { + match spawn::new_application_task_builder(path, None) + .unwrap() + .argument(Vec::new()) + .spawn() + .unwrap() + .join() + .unwrap() + { + ExitValue::Completed(status) => match status.downcast_ref::() { + Some(0) => Ok(()), + _ => Err(()), + }, + ExitValue::Killed(KillReason::Requested) => unreachable!(), + ExitValue::Killed(KillReason::Panic(_)) => Err(()), + ExitValue::Killed(KillReason::Exception(_)) => Err(()), + } +} + +fn ignore(name: &str) -> bool { + const IGNORED_TESTS: [&str; 3] = [ + // `test_libc` requires extra Make commands to run. + "test_libc", + // `test_panic` panics on success, which isn't easily translatable to + // `ExitValue::Completed(0)`. + "test_panic", + // TODO: Remove + // `test_channel` has a bug that causes deadlock. + "test_channel", + ]; + for test in IGNORED_TESTS { + if name.starts_with(test) { + return true; + } + } + false +} diff --git a/applications/test_serial_echo/Cargo.toml b/applications/serial_echo/Cargo.toml similarity index 95% rename from applications/test_serial_echo/Cargo.toml rename to applications/serial_echo/Cargo.toml index 0cd47b8944..90a169434e 100644 --- a/applications/test_serial_echo/Cargo.toml +++ b/applications/serial_echo/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "test_serial_echo" +name = "serial_echo" version = "0.1.0" authors = ["Kevin Boos "] description = "a simple app for testing serial port I/O using higher-level I/O traits" diff --git a/applications/test_serial_echo/src/lib.rs b/applications/serial_echo/src/lib.rs similarity index 100% rename from applications/test_serial_echo/src/lib.rs rename to applications/serial_echo/src/lib.rs diff --git a/applications/test_aligned_page_allocation/src/lib.rs b/applications/test_aligned_page_allocation/src/lib.rs index ebe99e7c5f..7bf4e09f8d 100644 --- a/applications/test_aligned_page_allocation/src/lib.rs +++ b/applications/test_aligned_page_allocation/src/lib.rs @@ -27,17 +27,15 @@ fn rmain() -> Result<(), &'static str> { for num_pages in TEST_SET.into_iter() { for alignment in TEST_SET.into_iter() { println!("Attempting to allocate {num_pages} pages with alignment of {alignment} 4K pages..."); - match memory::allocate_pages_deferred( - AllocationRequest::AlignedTo { alignment_4k_pages: alignment }, + let (ap, _action) = memory::allocate_pages_deferred( + AllocationRequest::AlignedTo { + alignment_4k_pages: alignment, + }, num_pages, - ) { - Ok((ap, _action)) => { - assert_eq!(ap.start().number() % alignment, 0); - assert_eq!(ap.size_in_pages(), num_pages); - println!(" Success: {ap:?}"); - } - Err(e) => println!(" !! FAILURE: {e:?}"), - } + )?; + assert_eq!(ap.start().number() % alignment, 0); + assert_eq!(ap.size_in_pages(), num_pages); + println!(" Success: {ap:?}"); } } diff --git a/applications/test_block_io/Cargo.toml b/applications/test_block_io/Cargo.toml index 16332bdd81..ec361c3528 100644 --- a/applications/test_block_io/Cargo.toml +++ b/applications/test_block_io/Cargo.toml @@ -3,6 +3,7 @@ name = "test_block_io" version = "0.1.0" authors = ["Kevin Boos "] description = "a simple app for testing IO transfers for block devices" +edition = "2021" [dependencies] core2 = { version = "0.4.0", default-features = false, features = ["alloc", "nightly"] } diff --git a/applications/test_block_io/src/lib.rs b/applications/test_block_io/src/lib.rs index 5c7891dacd..0d5a977b3d 100644 --- a/applications/test_block_io/src/lib.rs +++ b/applications/test_block_io/src/lib.rs @@ -5,28 +5,25 @@ #![no_std] extern crate alloc; -#[macro_use] extern crate log; -// #[macro_use] extern crate app_io; -extern crate task; -extern crate io; -extern crate core2; -extern crate storage_manager; -extern crate ata; - use core::ops::{DerefMut}; use alloc::boxed::Box; use alloc::vec::Vec; use alloc::string::String; +use app_io::println; use ata::AtaDrive; use io::{ByteReader, ByteReaderWrapper, ByteReaderWriterWrapper, ByteWriter, ByteWriterWrapper, Reader, ReaderWriter}; +use log::{debug, error, info, trace}; pub fn main(_args: Vec) -> isize { - - let dev = storage_manager::storage_devices().next() - .expect("no storage devices exist"); + let dev = if let Some(dev) = storage_manager::storage_devices().next() { + dev + } else { + println!("no storage devices connected"); + return 0; + }; { // Call `StorageDevice` trait methods directly diff --git a/applications/test_channel/src/lib.rs b/applications/test_channel/src/lib.rs index de3b61f92a..b93cd9a261 100644 --- a/applications/test_channel/src/lib.rs +++ b/applications/test_channel/src/lib.rs @@ -65,11 +65,6 @@ pub fn main(args: Vec) -> isize { opts.optopt("x", "panic_in_send", "Injects a panic at specified message in sender in multiple tests (default no panic)", "SEND_PANIC"); opts.optopt("y", "panic_in_receive", "Injects a panic at specified message in receiver in multiple tests (default no panic)", "RECEIVE_PANIC"); - opts.optflag("r", "rendezvous", "run the test on the rendezvous-based synchronous channel"); - opts.optflag("a", "asynchronous", "run the test on the asynchronous buffered channel"); - opts.optflag("o", "oneshot", "run the 'oneshot' test variant, in which {ITER} tasks are spawned to send/receive one message each."); - opts.optflag("m", "multiple", "run the 'multiple' test, in which one sender and one receiver task are spawned to send/receive {ITER} messages."); - let matches = match opts.parse(args) { Ok(m) => m, Err(_f) => { @@ -110,49 +105,27 @@ pub fn main(args: Vec) -> isize { } fn rmain(matches: Matches) -> Result<(), &'static str> { - let mut did_something = false; - // If the user has specified panic instances as 'val', 'send_panic_pont' will be 'Some(val)'. // Similarly for 'receive_panic_point' as well. let send_panic_point = matches.opt_str("x").and_then(|i| i.parse::().ok()); let receive_panic_point = matches.opt_str("y").and_then(|i| i.parse::().ok()); - if matches.opt_present("r") { - if matches.opt_present("o") { - did_something = true; - println!("Running rendezvous channel test in oneshot mode."); - for _i in 0 .. iterations!() { - rendezvous_test_oneshot()?; - } - } - if matches.opt_present("m") { - did_something = true; - println!("Running rendezvous channel test in multiple mode."); - rendezvous_test_multiple(send_count!(), receive_count!(), send_panic_point, receive_panic_point)?; - } + println!("Running rendezvous channel test in oneshot mode."); + for _i in 0 .. iterations!() { + rendezvous_test_oneshot()?; } + println!("Running rendezvous channel test in multiple mode."); + rendezvous_test_multiple(send_count!(), receive_count!(), send_panic_point, receive_panic_point)?; - if matches.opt_present("a") { - if matches.opt_present("o") { - did_something = true; - println!("Running asynchronous channel test in oneshot mode."); - for _i in 0 .. iterations!() { - asynchronous_test_oneshot()?; - } - } - if matches.opt_present("m") { - did_something = true; - println!("Running asynchronous channel test in multiple mode."); - asynchronous_test_multiple(send_count!(), receive_count!(), send_panic_point, receive_panic_point)?; - } + println!("Running asynchronous channel test in oneshot mode."); + for _i in 0 .. iterations!() { + asynchronous_test_oneshot()?; } - if did_something { - println!("Test complete."); - Ok(()) - } else { - Err("no action performed, please select a test") - } + println!("Running asynchronous channel test in multiple mode."); + asynchronous_test_multiple(send_count!(), receive_count!(), send_panic_point, receive_panic_point)?; + + Ok(()) } diff --git a/applications/test_filerw/src/lib.rs b/applications/test_filerw/src/lib.rs index 3321c87839..8e144ade4a 100644 --- a/applications/test_filerw/src/lib.rs +++ b/applications/test_filerw/src/lib.rs @@ -97,11 +97,11 @@ fn test_filerw() -> Result<(), &'static str> { } pub fn main(_args: Vec) -> isize { - match test_filerw() { - Ok(()) => { }, - Err(err) => println!("{}", err) + Ok(()) => 0, + Err(err) => { + println!("error {}", err); + -1 + } } - - 0 } diff --git a/applications/test_identity_mapping/src/lib.rs b/applications/test_identity_mapping/src/lib.rs index abb927b278..3e66243459 100644 --- a/applications/test_identity_mapping/src/lib.rs +++ b/applications/test_identity_mapping/src/lib.rs @@ -26,13 +26,9 @@ fn rmain() -> Result<(), &'static str> { let flags = memory::PteFlags::new().valid(true); for num_pages in TEST_SET.into_iter() { println!("Attempting to create identity mapping of {num_pages} pages..."); - match memory::create_identity_mapping(num_pages, flags) { - Ok(mp) => { - assert_eq!(mp.size_in_pages(), num_pages); - println!(" Success: {mp:?}"); - } - Err(e) => println!(" !! FAILURE: {e:?}"), - } + let mp = memory::create_identity_mapping(num_pages, flags)?; + assert_eq!(mp.size_in_pages(), num_pages); + println!(" Success: {mp:?}"); } Ok(()) diff --git a/applications/test_ixgbe/src/lib.rs b/applications/test_ixgbe/src/lib.rs index f46105ad42..898f2c52ad 100644 --- a/applications/test_ixgbe/src/lib.rs +++ b/applications/test_ixgbe/src/lib.rs @@ -73,7 +73,10 @@ fn rmain(matches: &Matches, _opts: &Options) -> Result<(), &'static str> { let (dev_id, mac_address) = { let ixgbe_devs = get_ixgbe_nics_list().ok_or("Ixgbe NICs list not initialized")?; - if ixgbe_devs.is_empty() { return Err("No ixgbe device available"); } + if ixgbe_devs.is_empty() { + println!("no IXGBE devices available"); + return Ok(()); + } let nic = ixgbe_devs[0].lock(); (nic.device_id(), nic.mac_address()) }; diff --git a/applications/test_mlx5/src/lib.rs b/applications/test_mlx5/src/lib.rs index 7bdd84c492..fee04caa91 100644 --- a/applications/test_mlx5/src/lib.rs +++ b/applications/test_mlx5/src/lib.rs @@ -31,8 +31,13 @@ pub fn main(_args: Vec) -> isize { } fn rmain() -> Result<(), &'static str> { - - let mut nic = mlx5::get_mlx5_nic().ok_or("mlx5 nic isn't initialized")?.lock(); + let mut nic = match mlx5::get_mlx5_nic() { + Some(nic) => nic.lock(), + None => { + println!("MLX5 NIC isn't initialized"); + return Ok(()); + } + }; let mac_address = nic.mac_address(); let num_packets = 8192; diff --git a/applications/test_task_cancel/src/lib.rs b/applications/test_task_cancel/src/lib.rs index 64b6fce4ce..57ba5a993a 100644 --- a/applications/test_task_cancel/src/lib.rs +++ b/applications/test_task_cancel/src/lib.rs @@ -9,6 +9,9 @@ use core::sync::atomic::{AtomicBool, Ordering::Relaxed}; use spin::Mutex; pub fn main(_: Vec) -> isize { + 0 + // FIXME + /* let lock = Arc::new(Mutex::new(())); let task = spawn::new_task_builder(guard_hog, lock.clone()) .spawn() @@ -26,8 +29,10 @@ pub fn main(_: Vec) -> isize { let _ = lock.lock(); 0 + */ } +#[allow(dead_code)] #[inline(never)] fn guard_hog(lock: Arc>) { let _guard = lock.lock(); @@ -40,6 +45,7 @@ fn guard_hog(lock: Arc>) { } } +#[allow(dead_code)] #[inline(never)] fn lsda_generator() { static FALSE: AtomicBool = AtomicBool::new(false); diff --git a/applications/tls_test/Cargo.toml b/applications/test_tls/Cargo.toml similarity index 94% rename from applications/tls_test/Cargo.toml rename to applications/test_tls/Cargo.toml index 3cf7491fb9..fa65ed7a30 100644 --- a/applications/tls_test/Cargo.toml +++ b/applications/test_tls/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "tls_test" +name = "test_tls" version = "0.1.0" authors = ["Kevin Boos "] diff --git a/applications/tls_test/src/lib.rs b/applications/test_tls/src/lib.rs similarity index 86% rename from applications/tls_test/src/lib.rs rename to applications/test_tls/src/lib.rs index fa8157be54..584e311124 100644 --- a/applications/tls_test/src/lib.rs +++ b/applications/test_tls/src/lib.rs @@ -14,15 +14,9 @@ use alloc::vec::Vec; use alloc::string::String; -pub fn main(args: Vec) -> isize { - - match args.first() { - Some(first) if first == "macro" => { - test_macro(); - return 0; - } - _ => { } - } +pub fn main(_: Vec) -> isize { + println!("Testing TLS macro"); + test_macro(); println!("Invoking test_thread_local::test_tls()..."); test_thread_local::test_tls(10); diff --git a/kernel/first_application/Cargo.toml b/kernel/first_application/Cargo.toml index 5824ca42a7..a8a98b7e69 100644 --- a/kernel/first_application/Cargo.toml +++ b/kernel/first_application/Cargo.toml @@ -25,6 +25,7 @@ spawn = { path = "../spawn" } ## Note: if another application crate is used, make sure to change ## both this dependency and the invocation string in `lib.rs`. [target.'cfg(target_arch = "x86_64")'.dependencies] +qemu_test = { path = "../../applications/qemu_test", optional = true } shell = { path = "../../applications/shell" } ## Note: aarch64 doesn't yet support the full graphical `shell` application, diff --git a/kernel/first_application/src/lib.rs b/kernel/first_application/src/lib.rs index ddf75d59ed..817e7f0352 100644 --- a/kernel/first_application/src/lib.rs +++ b/kernel/first_application/src/lib.rs @@ -28,7 +28,8 @@ use path::Path; /// See the crate-level docs and this crate's `Cargo.toml` for more. const FIRST_APPLICATION_CRATE_NAME: &str = { - #[cfg(target_arch = "x86_64")] { "shell-" } + #[cfg(all(target_arch = "x86_64", feature = "qemu_test"))] { "qemu_test-" } + #[cfg(all(target_arch = "x86_64", not(feature = "qemu_test")))] { "shell-" } #[cfg(target_arch = "aarch64")] { "hello-" } }; diff --git a/theseus_features/Cargo.toml b/theseus_features/Cargo.toml index b223c9fea8..5eb9e46b0f 100644 --- a/theseus_features/Cargo.toml +++ b/theseus_features/Cargo.toml @@ -10,6 +10,7 @@ edition = "2021" [dependencies] theseus_std = { path = "../ports/theseus_std", optional = true } +first_application = { path = "../kernel/first_application", optional = true } ## Regular applications. cat = { path = "../applications/cat", optional = true } @@ -29,6 +30,7 @@ ps = { path = "../applications/ps", optional = true } pwd = { path = "../applications/pwd", optional = true } rm = { path = "../applications/rm", optional = true } rq = { path = "../applications/rq", optional = true } +serial_echo = { path = "../applications/serial_echo", optional = true } shell = { path = "../applications/shell", optional = true } swap = { path = "../applications/swap", optional = true } upd = { path = "../applications/upd", optional = true } @@ -47,6 +49,7 @@ hello = { path = "../applications/hello", optional = true } raw_mode = { path = "../applications/raw_mode", optional = true } print_fault_log = { path = "../applications/print_fault_log", optional = true } seconds_counter = { path = "../applications/seconds_counter", optional = true } +qemu_test = { path = "../applications/qemu_test", optional = true } test_aligned_page_allocation = { path = "../applications/test_aligned_page_allocation", optional = true } test_async = { path = "../applications/test_async", optional = true } test_backtrace = { path = "../applications/test_backtrace", optional = true } @@ -61,13 +64,12 @@ test_panic = { path = "../applications/test_panic", optional = true } test_preemption_counter = { path = "../applications/test_preemption_counter", optional = true } test_restartable = { path = "../applications/test_restartable", optional = true } test_scheduler = { path = "../applications/test_scheduler", optional = true } -test_serial_echo = { path = "../applications/test_serial_echo", optional = true } test_std_fs = { path = "../applications/test_std_fs", optional = true } test_sync_block = { path = "../applications/test_sync_block", optional = true } test_task_cancel = { path = "../applications/test_task_cancel", optional = true } +test_tls = { path = "../applications/test_tls", optional = true } test_wait_queue = { path = "../applications/test_wait_queue", optional = true } test_wasmtime = { path = "../applications/test_wasmtime", optional = true } -tls_test = { path = "../applications/tls_test", optional = true } ## Benchmark crates. @@ -124,6 +126,7 @@ theseus_apps = [ "pwd", "rm", "rq", + "serial_echo", "shell", "swap", "upd", @@ -160,12 +163,11 @@ theseus_tests = [ "test_preemption_counter", "test_restartable", "test_scheduler", - "test_serial_echo", "test_std_fs", "test_sync_block", "test_task_cancel", + "test_tls", "test_wait_queue", "test_wasmtime", - "tls_test", "unwind_test", ] From cae8ca8f5cdd380973824ecfbd0b5ceb4d62b588 Mon Sep 17 00:00:00 2001 From: Klim Tsoutsman Date: Wed, 20 Sep 2023 05:17:57 +1000 Subject: [PATCH 2/4] Set `TaskInner.pinned_cpu` when spawning pinned tasks (#1044) * When spawning a pinned task, `spawn` didn't previously set `inner.pinned_cpu` for the newly-created `Task`. * This is not currently a problem because the scheduler doesn't perform task migration across CPUs, but when that gets enabled (in #1042), it would cause the pinning choice to be ignore by the scheduler. Signed-off-by: Klimenty Tsoutsman --- Cargo.lock | 1 + kernel/spawn/Cargo.toml | 1 + kernel/spawn/src/lib.rs | 7 ++++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 65cc0e49c7..3260daba2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3518,6 +3518,7 @@ dependencies = [ "spin 0.9.4", "stack", "task", + "task_struct", "thread_local_macro", ] diff --git a/kernel/spawn/Cargo.toml b/kernel/spawn/Cargo.toml index ff1cb93dd4..c3ce259cfa 100644 --- a/kernel/spawn/Cargo.toml +++ b/kernel/spawn/Cargo.toml @@ -18,6 +18,7 @@ stack = { path = "../stack" } cpu = { path = "../cpu" } preemption = { path = "../preemption" } task = { path = "../task" } +task_struct = { path = "../task_struct" } runqueue = { path = "../runqueue" } scheduler = { path = "../scheduler" } mod_mgmt = { path = "../mod_mgmt" } diff --git a/kernel/spawn/src/lib.rs b/kernel/spawn/src/lib.rs index dbfc138bc1..043e6855e9 100755 --- a/kernel/spawn/src/lib.rs +++ b/kernel/spawn/src/lib.rs @@ -31,6 +31,7 @@ use spin::Mutex; use memory::{get_kernel_mmi_ref, MmiRef}; use stack::Stack; use task::{Task, TaskRef, RestartInfo, RunState, JoinableTaskRef, ExitableTaskRef, FailureCleanupFunction}; +use task_struct::ExposedTask; use mod_mgmt::{CrateNamespace, SectionType, SECTION_HASH_DELIMITER}; use path::Path; use fs_node::FileOrDir; @@ -381,7 +382,11 @@ impl TaskBuilder )?; // If a Task name wasn't provided, then just use the function's name. new_task.name = self.name.unwrap_or_else(|| String::from(core::any::type_name::())); - + + let exposed = ExposedTask { task: new_task }; + exposed.inner().lock().pinned_cpu = self.pin_on_cpu; + let ExposedTask { task: mut new_task } = exposed; + #[cfg(simd_personality)] { new_task.simd = self.simd; } From 1d9120178250e0744500bdffca8102e037299271 Mon Sep 17 00:00:00 2001 From: Kevin Boos <1139460+kevinaboos@users.noreply.github.com> Date: Wed, 20 Sep 2023 22:48:20 -0700 Subject: [PATCH 3/4] Add Discord invite badge to README (#1046) Meant to do this a while ago... --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index c1a364ddea..d0cd763e73 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@ [![Documentation](https://img.shields.io/badge/view-docs-blue)](https://theseus-os.github.io/Theseus/doc/___Theseus_Crates___/index.html) [![Book](https://img.shields.io/badge/view-book-blueviolet)](https://theseus-os.github.io/Theseus/book/index.html) [![Blog](https://img.shields.io/badge/view-blog-orange)](https://theseus-os.com) +[![Discord](https://img.shields.io/badge/Discord-%235865F2.svg?style=flat&logo=discord&logoColor=white)](https://discord.gg/NuUnqeYT8R) Theseus is a new OS written from scratch in [Rust](https://www.rust-lang.org/) to experiment with novel OS structure, better state management, and how to leverage **intralingual design** principles to shift OS responsibilities like resource management into the compiler. From ac51fc175126cab7d4fd2723a8fc3bb222f546b0 Mon Sep 17 00:00:00 2001 From: Nathan Royer <61582713+NathanRoyer@users.noreply.github.com> Date: Sun, 24 Sep 2023 09:39:54 +0200 Subject: [PATCH 4/4] aarch64: support FIQs and use them for TLB shootdown IPIs (#1039) * Add support for fast interrupts on aarch64, aka FIQs. FIQs are designed to be fast and can thus interrupt regular interrupts (IRQs) that are in the process of being handled. They are similar to NMIs on x86_64 in this regard, but can also be explicitly enabled/disabled. * Updated the GIC driver to support both Group 0 (FIQs) and Group 1 (IRQs). * `nano_core`, `captain`, and `ap_start` now enable/disable FIQs. * Broadcasting TLB shootdown IPIs now uses FIQs to ensure that TLB shootdowns occur instantly even if regular interrupts are disabled on one or more other CPUs. * Add a separate trait `Aarch64LocalInterruptController` for arch-specific features, which keeps the `LocalInterruptController` arch-agnostic. * This trait is primarily for configuring/handling fast interrupts (FIQs), but also for acknowledging interrupts, which x86_64 does not require. * The interrupt controller now allows enabling/disabling SPIs too. --------- Co-authored-by: Kevin Boos --- Cargo.lock | 2 +- kernel/ap_start/src/lib.rs | 3 + kernel/captain/src/lib.rs | 1 + kernel/context_switch_regular/src/aarch64.rs | 32 ++--- kernel/gic/src/gic/cpu_interface_gicv2.rs | 13 +- kernel/gic/src/gic/cpu_interface_gicv3.rs | 34 +++-- kernel/gic/src/gic/dist_interface.rs | 43 +++--- kernel/gic/src/gic/mod.rs | 56 +++++--- kernel/gic/src/gic/redist_interface.rs | 36 +++-- kernel/interrupt_controller/src/aarch64.rs | 143 ++++++++++++++----- kernel/interrupt_controller/src/lib.rs | 49 ++++--- kernel/interrupt_controller/src/x86_64.rs | 26 +--- kernel/interrupts/src/aarch64/mod.rs | 135 ++++++++++++----- kernel/interrupts/src/aarch64/table.s | 4 +- kernel/memory_aarch64/src/lib.rs | 45 ++++-- kernel/nano_core/src/lib.rs | 5 +- kernel/tlb_shootdown/src/lib.rs | 10 +- 17 files changed, 435 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3260daba2a..37c05c2764 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1661,7 +1661,7 @@ dependencies = [ [[package]] name = "irq_safety" version = "0.1.1" -source = "git+https://github.com/theseus-os/irq_safety#0e32fe775fdb93be1fc6f85b306d109aea5d920e" +source = "git+https://github.com/theseus-os/irq_safety#11bfab9f410a898df1e42ad6213488612e20c926" dependencies = [ "spin 0.9.4", ] diff --git a/kernel/ap_start/src/lib.rs b/kernel/ap_start/src/lib.rs index 6dadd5e0bd..08cb049fa4 100644 --- a/kernel/ap_start/src/lib.rs +++ b/kernel/ap_start/src/lib.rs @@ -52,6 +52,8 @@ pub fn kstart_ap( nmi_flags: u16, ) -> ! { irq_safety::disable_interrupts(); + #[cfg(target_arch = "aarch64")] + irq_safety::disable_fast_interrupts(); info!("Booted CPU {}, proc: {}, stack: {:#X} to {:#X}, nmi_lint: {}, nmi_flags: {:#X}", cpu_id, processor_id, _stack_start, _stack_end, nmi_lint, nmi_flags @@ -100,6 +102,7 @@ pub fn kstart_ap( #[cfg(target_arch = "aarch64")] { interrupts::init_ap(); + irq_safety::enable_fast_interrupts(); // Register this CPU as online in the system // This is the equivalent of `LocalApic::init` on aarch64 diff --git a/kernel/captain/src/lib.rs b/kernel/captain/src/lib.rs index 05a73df2e3..ced3b62b5b 100644 --- a/kernel/captain/src/lib.rs +++ b/kernel/captain/src/lib.rs @@ -123,6 +123,7 @@ pub fn init( interrupt_controller::init()?; interrupts::init()?; + irq_safety::enable_fast_interrupts(); // register BSP CpuId cpu::register_cpu(true)?; diff --git a/kernel/context_switch_regular/src/aarch64.rs b/kernel/context_switch_regular/src/aarch64.rs index 8e8bb1b6a3..71653e80dc 100644 --- a/kernel/context_switch_regular/src/aarch64.rs +++ b/kernel/context_switch_regular/src/aarch64.rs @@ -85,21 +85,21 @@ macro_rules! save_registers_regular { // Save all general purpose registers into the previous task. r#" // Make room on the stack for the registers. - sub sp, sp, #8 * 2 * 6 + sub sp, sp, #8 * 13 // Push registers on the stack, two at a time. - stp x19, x20, [sp, #8 * 2 * 0] - stp x21, x22, [sp, #8 * 2 * 1] - stp x23, x24, [sp, #8 * 2 * 2] - stp x25, x26, [sp, #8 * 2 * 3] - stp x27, x28, [sp, #8 * 2 * 4] - stp x29, x30, [sp, #8 * 2 * 5] + stp x19, x20, [sp, #8 * 0] + stp x21, x22, [sp, #8 * 2] + stp x23, x24, [sp, #8 * 4] + stp x25, x26, [sp, #8 * 6] + stp x27, x28, [sp, #8 * 8] + stp x29, x30, [sp, #8 * 10] // Push an OR of DAIF and NZCV flags of PSTATE mrs x29, DAIF mrs x30, NZCV orr x29, x29, x30 - str x29, [sp, #8 * 2 * 6] + str x29, [sp, #8 * 12] "# ); } @@ -133,20 +133,20 @@ macro_rules! restore_registers_regular { r#" // Pop DAIF and NZCV flags of PSTATE // These MSRs discard irrelevant bits; no AND is required. - ldr x29, [sp, #8 * 2 * 6] + ldr x29, [sp, #8 * 12] msr DAIF, x29 msr NZCV, x29 // Pop registers from the stack, two at a time. - ldp x29, x30, [sp, #8 * 2 * 5] - ldp x27, x28, [sp, #8 * 2 * 4] - ldp x25, x26, [sp, #8 * 2 * 3] - ldp x23, x24, [sp, #8 * 2 * 2] - ldp x21, x22, [sp, #8 * 2 * 1] - ldp x19, x20, [sp, #8 * 2 * 0] + ldp x29, x30, [sp, #8 * 10] + ldp x27, x28, [sp, #8 * 8] + ldp x25, x26, [sp, #8 * 6] + ldp x23, x24, [sp, #8 * 4] + ldp x21, x22, [sp, #8 * 2] + ldp x19, x20, [sp, #8 * 0] // Move the stack pointer back up. - add sp, sp, #8 * 2 * 6 + add sp, sp, #8 * 13 "# ); } diff --git a/kernel/gic/src/gic/cpu_interface_gicv2.rs b/kernel/gic/src/gic/cpu_interface_gicv2.rs index 58d8c81bca..698cbbc0fa 100644 --- a/kernel/gic/src/gic/cpu_interface_gicv2.rs +++ b/kernel/gic/src/gic/cpu_interface_gicv2.rs @@ -8,6 +8,7 @@ use super::Priority; use super::InterruptNumber; +use super::SPURIOUS_INTERRUPT_NUM; use volatile::{Volatile, ReadOnly, WriteOnly}; use zerocopy::FromBytes; @@ -42,7 +43,7 @@ pub struct CpuRegsP1 { // base offset } // enable group 0 -// const CTLR_ENGRP0: u32 = 0b01; +const CTLR_ENGRP0: u32 = 0b01; // enable group 1 const CTLR_ENGRP1: u32 = 0b10; @@ -51,6 +52,7 @@ impl CpuRegsP1 { /// Enables routing of group 1 interrupts for the current CPU. pub fn init(&mut self) { let mut reg = self.ctlr.read(); + reg |= CTLR_ENGRP0; reg |= CTLR_ENGRP1; self.ctlr.write(reg); } @@ -87,12 +89,17 @@ impl CpuRegsP1 { /// /// This tells the GIC that the requested interrupt is being /// handled by this CPU. - pub fn acknowledge_interrupt(&mut self) -> (InterruptNumber, Priority) { + /// + /// Returns None if a spurious interrupt is detected. + pub fn acknowledge_interrupt(&mut self) -> Option<(InterruptNumber, Priority)> { // Reading the interrupt number has the side effect // of acknowledging the interrupt. let int_num = self.acknowledge.read() as InterruptNumber; let priority = self.running_prio.read() as u8; - (int_num, priority) + match int_num { + SPURIOUS_INTERRUPT_NUM => None, + _ => Some((int_num, priority)) + } } } diff --git a/kernel/gic/src/gic/cpu_interface_gicv3.rs b/kernel/gic/src/gic/cpu_interface_gicv3.rs index 63072864b2..4a68d3db77 100644 --- a/kernel/gic/src/gic/cpu_interface_gicv3.rs +++ b/kernel/gic/src/gic/cpu_interface_gicv3.rs @@ -11,6 +11,8 @@ use core::arch::asm; use super::IpiTargetCpu; use super::Priority; use super::InterruptNumber; +use super::InterruptGroup; +use super::SPURIOUS_INTERRUPT_NUM; const SGIR_TARGET_ALL_OTHER_PE: u64 = 1 << 40; const IGRPEN_ENABLED: u64 = 1; @@ -28,7 +30,7 @@ pub fn init() { // Enable Group 0 // bit 0 = group 0 enable - // unsafe { asm!("msr ICC_IGRPEN0_EL1, {}", in(reg) IGRPEN_ENABLED) }; + unsafe { asm!("msr ICC_IGRPEN0_EL1, {}", in(reg) IGRPEN_ENABLED) }; // Enable Groupe 1 (non-secure) // bit 0 = group 1 (non-secure) enable @@ -61,9 +63,13 @@ pub fn set_minimum_priority(priority: Priority) { /// the current CPU. /// /// This implies that the CPU is ready to process interrupts again. -pub fn end_of_interrupt(int: InterruptNumber) { +pub fn end_of_interrupt(int: InterruptNumber, group: InterruptGroup) { let reg_value = int as u64; - unsafe { asm!("msr ICC_EOIR1_EL1, {}", in(reg) reg_value) }; + + match group { + InterruptGroup::Group0 => unsafe { asm!("msr ICC_EOIR0_EL1, {}", in(reg) reg_value) }, + InterruptGroup::Group1 => unsafe { asm!("msr ICC_EOIR1_EL1, {}", in(reg) reg_value) }, + } } /// Acknowledge the currently serviced interrupt and fetches its @@ -71,24 +77,33 @@ pub fn end_of_interrupt(int: InterruptNumber) { /// /// This tells the GIC that the requested interrupt is being /// handled by this CPU. -pub fn acknowledge_interrupt() -> (InterruptNumber, Priority) { +/// +/// Returns None if a spurious interrupt is detected. +pub fn acknowledge_interrupt(group: InterruptGroup) -> Option<(InterruptNumber, Priority)> { let int_num: u64; let priority: u64; // Reading the interrupt number has the side effect // of acknowledging the interrupt. + match group { + InterruptGroup::Group0 => unsafe { asm!("mrs {}, ICC_IAR0_EL1", out(reg) int_num) }, + InterruptGroup::Group1 => unsafe { asm!("mrs {}, ICC_IAR1_EL1", out(reg) int_num) }, + } + unsafe { - asm!("mrs {}, ICC_IAR1_EL1", out(reg) int_num); asm!("mrs {}, ICC_RPR_EL1", out(reg) priority); } let int_num = int_num & 0xffffff; let priority = priority & 0xff; - (int_num as InterruptNumber, priority as u8) + match int_num as InterruptNumber { + SPURIOUS_INTERRUPT_NUM => None, + n => Some((n, priority as u8)), + } } /// Generates an interrupt in CPU interfaces of the system -pub fn send_ipi(int_num: InterruptNumber, target: IpiTargetCpu) { +pub fn send_ipi(int_num: InterruptNumber, target: IpiTargetCpu, group: InterruptGroup) { let mut value = match target { IpiTargetCpu::Specific(cpu) => { let mpidr: cpu::MpidrValue = cpu.into(); @@ -118,5 +133,8 @@ pub fn send_ipi(int_num: InterruptNumber, target: IpiTargetCpu) { }; value |= (int_num as u64) << 24; - unsafe { asm!("msr ICC_SGI1R_EL1, {}", in(reg) value) }; + match group { + InterruptGroup::Group0 => unsafe { asm!("msr ICC_SGI0R_EL1, {}", in(reg) value) }, + InterruptGroup::Group1 => unsafe { asm!("msr ICC_SGI1R_EL1, {}", in(reg) value) }, + } } \ No newline at end of file diff --git a/kernel/gic/src/gic/dist_interface.rs b/kernel/gic/src/gic/dist_interface.rs index baa7fb7488..d271edd8fd 100644 --- a/kernel/gic/src/gic/dist_interface.rs +++ b/kernel/gic/src/gic/dist_interface.rs @@ -16,6 +16,7 @@ use super::IpiTargetCpu; use super::SpiDestination; use super::InterruptNumber; +use super::InterruptGroup; use super::Enabled; use super::Priority; use super::TargetList; @@ -75,7 +76,7 @@ pub struct DistRegsP6 { // base offset } // enable group 0 -// const CTLR_ENGRP0: u32 = 0b01; +const CTLR_ENGRP0: u32 = 0b01; // enable group 1 const CTLR_ENGRP1: u32 = 0b10; @@ -96,9 +97,6 @@ const SGIR_TARGET_ALL_OTHER_PE: u32 = 1 << 24; // 0 = route to specific PE const P6IROUTER_ANY_AVAILABLE_PE: u64 = 1 << 31; -// const GROUP_0: u32 = 0; -const GROUP_1: u32 = 1; - // bit 15: which interrupt group to target const SGIR_NSATT_GRP1: u32 = 1 << 15; @@ -112,6 +110,7 @@ impl DistRegsP1 { /// states. pub fn init(&mut self) -> Enabled { let mut reg = self.ctlr.read(); + reg |= CTLR_ENGRP0; reg |= CTLR_ENGRP1; reg |= CTLR_E1NWF; self.ctlr.write(reg); @@ -123,26 +122,26 @@ impl DistRegsP1 { } /// Returns whether the given SPI (shared peripheral interrupt) will be - /// forwarded by the distributor - pub fn is_spi_enabled(&self, int: InterruptNumber) -> Enabled { - // enabled? - read_array_volatile::<32>(&self.set_enable, int) > 0 - && - // part of group 1? - read_array_volatile::<32>(&self.group, int) == GROUP_1 + /// forwarded by the distributor. + pub fn get_spi_state(&self, int: InterruptNumber) -> Option { + if read_array_volatile::<32>(&self.set_enable, int) == 1 { + match read_array_volatile::<32>(&self.group, int) { + 0 => return Some(InterruptGroup::Group0), + 1 => return Some(InterruptGroup::Group1), + _ => { } + } + } + None } - /// Enables or disables the forwarding of a particular SPI (shared peripheral interrupt) - pub fn enable_spi(&mut self, int: InterruptNumber, enabled: Enabled) { - let reg_base = match enabled { - true => &mut self.set_enable, - false => &mut self.clear_enable, - }; - write_array_volatile::<32>(reg_base, int, 1); - - // whether we're enabling or disabling, - // set as part of group 1 - write_array_volatile::<32>(&mut self.group, int, GROUP_1); + /// Enables or disables the forwarding of a particular SPI (shared peripheral interrupt). + pub fn set_spi_state(&mut self, int: InterruptNumber, state: Option) { + if let Some(group) = state { + write_array_volatile::<32>(&mut self.group, int, group as u32); + write_array_volatile::<32>(&mut self.set_enable, int, 1); + } else { + write_array_volatile::<32>(&mut self.clear_enable, int, 1); + } } /// Returns the priority of an SPI. diff --git a/kernel/gic/src/gic/mod.rs b/kernel/gic/src/gic/mod.rs index 66f8247ac9..be09740ba2 100644 --- a/kernel/gic/src/gic/mod.rs +++ b/kernel/gic/src/gic/mod.rs @@ -19,6 +19,15 @@ use dist_interface::{DistRegsP1, DistRegsP6}; use cpu_interface_gicv2::CpuRegsP1; use redist_interface::{RedistRegsP1, RedistRegsSgiPpi}; +/// Whether an interrupt is put in Group 0 (FIQs) or Group 1 (IRQs) +#[repr(u32)] +pub enum InterruptGroup { + /// Group 0 is used for FIQs (fast interrupts) + Group0 = 0, + /// Group 1 is used for IRQs (regular interrupts) + Group1 = 1, +} + /// Boolean pub type Enabled = bool; @@ -122,6 +131,7 @@ impl SpiDestination { } const U32BITS: usize = u32::BITS as usize; +const SPURIOUS_INTERRUPT_NUM: InterruptNumber = 1023; // Reads one item of an array spanning across // multiple u32s. @@ -240,19 +250,29 @@ impl ArmGicDistributor { /// Returns whether the given interrupt is forwarded by the distributor. /// + /// # Return + /// * `None` if the interrupt is disabled. + /// * `Some(`[`InterruptGroup::Group0`]`)` if the interrupt is enabled and configured as a Group 0 interrupt. + /// * `Some(`[`InterruptGroup::Group1`]`)` if the interrupt is enabled and configured as a Group 1 interrupt. + /// /// Panics if `int` is not in the SPI range (>= 32). - pub fn get_spi_state(&self, int: InterruptNumber) -> Enabled { + pub fn get_spi_state(&self, int: InterruptNumber) -> Option { assert!(int >= 32, "get_spi_state: `int` must be >= 32"); - self.distributor().is_spi_enabled(int) + self.distributor().get_spi_state(int) } /// Enables or disables the forwarding of the given interrupt /// by the distributor. /// + /// # Arguments + /// * `int`: the interrupt number whose state we are modifying. + /// * `state`: whether the interrupt will be disabled (`None`), + /// enabled as `Group0` (regular IRQs), or enabled as `Group1` (fast interrupts, FIQs). + /// /// Panics if `int` is not in the SPI range (>= 32). - pub fn set_spi_state(&mut self, int: InterruptNumber, enabled: Enabled) { + pub fn set_spi_state(&mut self, int: InterruptNumber, state: Option) { assert!(int >= 32, "set_spi_state: `int` must be >= 32"); - self.distributor_mut().enable_spi(int, enabled) + self.distributor_mut().set_spi_state(int, state) } /// Returns the priority of the given interrupt. @@ -372,11 +392,11 @@ impl ArmGicCpuComponents { /// /// Panics if `int` is greater than or equal to 16; /// on aarch64, IPIs much be sent to an interrupt number less than 16. - pub fn send_ipi(&mut self, int: InterruptNumber, target: IpiTargetCpu) { + pub fn send_ipi(&mut self, int: InterruptNumber, target: IpiTargetCpu, group: InterruptGroup) { assert!(int < 16, "IPIs must have a number below 16 on ARMv8"); if let Self::V3 { .. } = self { - cpu_interface_gicv3::send_ipi(int, target) + cpu_interface_gicv3::send_ipi(int, target, group) } else { // we don't have access to the distributor... code would be: // dist_interface::send_ipi_gicv2(&mut dist_regs, int, target) @@ -394,10 +414,12 @@ impl ArmGicCpuComponents { /// being handled by this CPU. /// /// Returns a tuple of the interrupt's number and priority. - pub fn acknowledge_interrupt(&mut self) -> (InterruptNumber, Priority) { + /// + /// Returns None if a spurious interrupt is detected. + pub fn acknowledge_interrupt(&mut self, group: InterruptGroup) -> Option<(InterruptNumber, Priority)> { match self { - Self::V2 { registers, .. } => registers.acknowledge_interrupt(), - Self::V3 { .. } => cpu_interface_gicv3::acknowledge_interrupt(), + Self::V2 { registers, .. } => registers.acknowledge_interrupt(/* no way to specify a group in GICv2 */), + Self::V3 { .. } => cpu_interface_gicv3::acknowledge_interrupt(group), } } @@ -406,10 +428,10 @@ impl ArmGicCpuComponents { /// the current CPU. /// /// This implies that the CPU is ready to process interrupts again. - pub fn end_of_interrupt(&mut self, int: InterruptNumber) { + pub fn end_of_interrupt(&mut self, int: InterruptNumber, group: InterruptGroup) { match self { - Self::V2 { registers, .. } => registers.end_of_interrupt(int), - Self::V3 { .. } => cpu_interface_gicv3::end_of_interrupt(int), + Self::V2 { registers, .. } => registers.end_of_interrupt(int, /* no way to specify a group in GICv2 */), + Self::V3 { .. } => cpu_interface_gicv3::end_of_interrupt(int, group), } } @@ -417,17 +439,17 @@ impl ArmGicCpuComponents { /// /// Panics if `int` is greater than or equal to 32, which is beyond the range /// of local interrupt numbers. - pub fn get_interrupt_state(&self, int: InterruptNumber) -> Enabled { + pub fn get_interrupt_state(&self, int: InterruptNumber) -> Option { assert!(int < 32, "get_interrupt_state: `int` doesn't lie in the SGI/PPI (local interrupt) range"); if let Self::V3 { redist_regs } = self { - redist_regs.redist_sgippi.is_sgippi_enabled(int) + redist_regs.redist_sgippi.get_sgippi_state(int) } else { // there is no redistributor and we don't have access to the distributor log::error!("GICv2 doesn't support enabling/disabling local interrupt"); // should we panic? - true + Some(InterruptGroup::Group1) } } @@ -435,11 +457,11 @@ impl ArmGicCpuComponents { /// /// Panics if `int` is greater than or equal to 32, which is beyond the range /// of local interrupt numbers. - pub fn set_interrupt_state(&mut self, int: InterruptNumber, enabled: Enabled) { + pub fn set_interrupt_state(&mut self, int: InterruptNumber, state: Option) { assert!(int < 32, "set_interrupt_state: `int` doesn't lie in the SGI/PPI (local interrupt) range"); if let Self::V3 { redist_regs } = self { - redist_regs.redist_sgippi.enable_sgippi(int, enabled); + redist_regs.redist_sgippi.set_sgippi_state(int, state); } else { // there is no redistributor and we don't have access to the distributor log::error!("GICv2 doesn't support enabling/disabling local interrupt"); diff --git a/kernel/gic/src/gic/redist_interface.rs b/kernel/gic/src/gic/redist_interface.rs index 02461b343a..fb30b3f9f2 100644 --- a/kernel/gic/src/gic/redist_interface.rs +++ b/kernel/gic/src/gic/redist_interface.rs @@ -10,7 +10,7 @@ //! - Getting or setting the priority of PPIs & SGIs based on their numbers use super::InterruptNumber; -use super::Enabled; +use super::InterruptGroup; use super::Priority; use super::read_array_volatile; use super::write_array_volatile; @@ -78,9 +78,6 @@ const CTLR_DPG1NS: u32 = 1 << 25; /// If bit is set, the PE cannot be selected for group 0 "1 of N" interrupts. const CTLR_DPG0: u32 = 1 << 24; -/// const GROUP_0: u32 = 0; -const GROUP_1: u32 = 1; - /// This timeout value works on some ARM SoCs: /// - qemu's virt virtual machine /// @@ -150,26 +147,27 @@ impl RedistRegsP1 { impl RedistRegsSgiPpi { /// Returns whether the given SGI (software generated interrupts) or /// PPI (private peripheral interrupts) will be forwarded by the redistributor - pub fn is_sgippi_enabled(&self, int: InterruptNumber) -> Enabled { - read_array_volatile::<32>(&self.set_enable, int) > 0 - && - // part of group 1? - read_array_volatile::<32>(&self.group, int) == GROUP_1 + pub fn get_sgippi_state(&self, int: InterruptNumber) -> Option { + if read_array_volatile::<32>(&self.set_enable, int) == 1 { + match read_array_volatile::<32>(&self.group, int) { + 0 => return Some(InterruptGroup::Group0), + 1 => return Some(InterruptGroup::Group1), + _ => { } + } + } + None } /// Enables or disables the forwarding of a particular /// SGI (software generated interrupts) or PPI (private /// peripheral interrupts) - pub fn enable_sgippi(&mut self, int: InterruptNumber, enabled: Enabled) { - let reg = match enabled { - true => &mut self.set_enable, - false => &mut self.clear_enable, - }; - write_array_volatile::<32>(reg, int, 1); - - // whether we're enabling or disabling, - // set as part of group 1 - write_array_volatile::<32>(&mut self.group, int, GROUP_1); + pub fn set_sgippi_state(&mut self, int: InterruptNumber, state: Option) { + if let Some(group) = state { + write_array_volatile::<32>(&mut self.group, int, group as u32); + write_array_volatile::<32>(&mut self.set_enable, int, 1); + } else { + write_array_volatile::<32>(&mut self.clear_enable, int, 1); + } } /// Returns the priority of an SGI/PPI. diff --git a/kernel/interrupt_controller/src/aarch64.rs b/kernel/interrupt_controller/src/aarch64.rs index 6da4f16714..f2b3d7f63a 100644 --- a/kernel/interrupt_controller/src/aarch64.rs +++ b/kernel/interrupt_controller/src/aarch64.rs @@ -1,7 +1,10 @@ use { - gic::{ArmGicDistributor, ArmGicCpuComponents, SpiDestination, IpiTargetCpu, Version as GicVersion}, + gic::{ + ArmGicDistributor, ArmGicCpuComponents, SpiDestination, IpiTargetCpu, + Version as GicVersion, InterruptGroup, + }, arm_boards::{NUM_CPUS, BOARD_CONFIG, InterruptControllerConfig}, - core::array::try_from_fn, + core::{array::try_from_fn, cell::UnsafeCell}, sync_irq::IrqSafeMutex, cpu::current_cpu, spin::Once, @@ -49,9 +52,9 @@ pub fn init() -> Result<(), &'static str> { ArmGicCpuComponents::init(cpu_id, &version) })?; - Ok(cpu_ctlrs.map(|ctlr| { + Ok(cpu_ctlrs.map(|mut ctlr| { let mutex = IrqSafeMutex::new(ctlr); - LocalInterruptController(mutex) + LocalInterruptController(UnsafeCell::new(mutex)) })) })?; }, @@ -69,7 +72,10 @@ pub struct SystemInterruptController(IrqSafeMutex); /// Struct representing per-cpu-core interrupt controller chips. /// /// On aarch64 w/ GIC, this corresponds to a Redistributor & CPU interface. -pub struct LocalInterruptController(IrqSafeMutex); +pub struct LocalInterruptController(UnsafeCell>); + +unsafe impl Send for LocalInterruptController {} +unsafe impl Sync for LocalInterruptController {} impl SystemInterruptControllerApi for SystemInterruptController { fn get() -> &'static Self { @@ -112,19 +118,33 @@ impl SystemInterruptControllerApi for SystemInterruptController { fn set_destination( &self, sys_int_num: InterruptNumber, - destination: CpuId, + destination: Option, priority: Priority, ) -> Result<(), &'static str> { assert!(sys_int_num >= 32, "shared peripheral interrupts have a number >= 32"); + + let state = match destination.is_some() { + true => Some(InterruptGroup::Group1), + false => None, + }; + let mut dist = self.0.lock(); - dist.set_spi_target(sys_int_num as _, SpiDestination::Specific(destination)); - dist.set_spi_priority(sys_int_num as _, priority); + if let Some(destination) = destination { + dist.set_spi_target(sys_int_num as _, SpiDestination::Specific(destination)); + dist.set_spi_priority(sys_int_num as _, priority); + } + + dist.set_spi_state(sys_int_num as _, state); Ok(()) } } +macro_rules! lock { + ($this:ident) => (unsafe { $this.0.get().as_ref().unwrap().lock() }) +} + impl LocalInterruptControllerApi for LocalInterruptController { fn get() -> &'static Self { // how this function works: @@ -145,69 +165,128 @@ impl LocalInterruptControllerApi for LocalInterruptController { &ctrls[index] } - fn init_secondary_cpu_interface(&self) { - let mut cpu_ctrl = self.0.lock(); - cpu_ctrl.init_secondary_cpu_interface(); - } - fn id(&self) -> LocalInterruptControllerId { - let cpu_ctrl = self.0.lock(); + let cpu_ctrl = lock!(self); LocalInterruptControllerId(cpu_ctrl.get_cpu_interface_id()) } fn get_local_interrupt_priority(&self, num: InterruptNumber) -> Priority { assert!(num < 32, "local interrupts have a number < 32"); - let cpu_ctrl = self.0.lock(); + let cpu_ctrl = lock!(self); cpu_ctrl.get_interrupt_priority(num as _) } fn set_local_interrupt_priority(&self, num: InterruptNumber, priority: Priority) { assert!(num < 32, "local interrupts have a number < 32"); - let mut cpu_ctrl = self.0.lock(); + let mut cpu_ctrl = lock!(self); cpu_ctrl.set_interrupt_priority(num as _, priority); } fn is_local_interrupt_enabled(&self, num: InterruptNumber) -> bool { assert!(num < 32, "local interrupts have a number < 32"); - let cpu_ctrl = self.0.lock(); - cpu_ctrl.get_interrupt_state(num as _) + let cpu_ctrl = lock!(self); + match cpu_ctrl.get_interrupt_state(num as _) { + None => false, + Some(InterruptGroup::Group1) => true, + Some(InterruptGroup::Group0) => { + log::error!("Warning: found misconfigured local interrupt ({})", num); + true + }, + } } fn enable_local_interrupt(&self, num: InterruptNumber, enabled: bool) { assert!(num < 32, "local interrupts have a number < 32"); - let mut cpu_ctrl = self.0.lock(); - cpu_ctrl.set_interrupt_state(num as _, enabled); + let state = match enabled { + true => Some(InterruptGroup::Group1), + false => None, + }; + let mut cpu_ctrl = lock!(self); + cpu_ctrl.set_interrupt_state(num as _, state); } fn send_ipi(&self, num: InterruptNumber, dest: InterruptDestination) { use InterruptDestination::*; assert!(num < 16, "IPIs have a number < 16"); - let mut cpu_ctrl = self.0.lock(); - cpu_ctrl.send_ipi(num as _, match dest { + let dest = match dest { + SpecificCpu(cpu) => IpiTargetCpu::Specific(cpu), + AllOtherCpus => IpiTargetCpu::AllOtherCpus, + }; + + let mut cpu_ctrl = lock!(self); + + cpu_ctrl.send_ipi(num as _, dest, InterruptGroup::Group1); + } + + fn end_of_interrupt(&self, number: InterruptNumber) { + let mut cpu_ctrl = lock!(self); + cpu_ctrl.end_of_interrupt(number as _, InterruptGroup::Group1) + } +} + +impl AArch64LocalInterruptControllerApi for LocalInterruptController { + fn enable_fast_local_interrupt(&self, num: InterruptNumber, enabled: bool) { + assert!(num < 32, "local interrupts have a number < 32"); + let state = match enabled { + true => Some(InterruptGroup::Group0), + false => None, + }; + let mut cpu_ctrl = lock!(self); + cpu_ctrl.set_interrupt_state(num as _, state); + } + + fn send_fast_ipi(&self, num: InterruptNumber, dest: InterruptDestination) { + use InterruptDestination::*; + assert!(num < 16, "IPIs have a number < 16"); + + let dest = match dest { SpecificCpu(cpu) => IpiTargetCpu::Specific(cpu), AllOtherCpus => IpiTargetCpu::AllOtherCpus, - }); + }; + + let mut cpu_ctrl = lock!(self); + + cpu_ctrl.send_ipi(num as _, dest, InterruptGroup::Group0); } fn get_minimum_priority(&self) -> Priority { - let cpu_ctrl = self.0.lock(); + let cpu_ctrl = lock!(self); cpu_ctrl.get_minimum_priority() } fn set_minimum_priority(&self, priority: Priority) { - let mut cpu_ctrl = self.0.lock(); + let mut cpu_ctrl = lock!(self); cpu_ctrl.set_minimum_priority(priority) } - fn acknowledge_interrupt(&self) -> (InterruptNumber, Priority) { - let mut cpu_ctrl = self.0.lock(); - let (num, prio) = cpu_ctrl.acknowledge_interrupt(); - (num as _, prio) + fn acknowledge_interrupt(&self) -> Option<(InterruptNumber, Priority)> { + let mut cpu_ctrl = lock!(self); + let opt = cpu_ctrl.acknowledge_interrupt(InterruptGroup::Group1); + opt.map(|(num, prio)| (num as _, prio)) } - fn end_of_interrupt(&self, number: InterruptNumber) { - let mut cpu_ctrl = self.0.lock(); - cpu_ctrl.end_of_interrupt(number as _) + fn init_secondary_cpu_interface(&self) { + let mut cpu_ctrl = lock!(self); + cpu_ctrl.init_secondary_cpu_interface(); + } + + unsafe fn acknowledge_fast_interrupt(&self) -> Option<(InterruptNumber, Priority)> { + // we cannot lock here + // this has to be unsafe + let mut_mutex = self.0.get().as_mut().unwrap(); + let mut cpu_ctrl = mut_mutex.get_mut(); + + let opt = cpu_ctrl.acknowledge_interrupt(InterruptGroup::Group0); + opt.map(|(num, prio)| (num as _, prio)) + } + + unsafe fn end_of_fast_interrupt(&self, number: InterruptNumber) { + // we cannot lock here + // this has to be unsafe + let mut_mutex = self.0.get().as_mut().unwrap(); + let mut cpu_ctrl = mut_mutex.get_mut(); + + cpu_ctrl.end_of_interrupt(number as _, InterruptGroup::Group0) } } diff --git a/kernel/interrupt_controller/src/lib.rs b/kernel/interrupt_controller/src/lib.rs index 55c08fe697..92d7f10e24 100644 --- a/kernel/interrupt_controller/src/lib.rs +++ b/kernel/interrupt_controller/src/lib.rs @@ -52,7 +52,7 @@ pub trait SystemInterruptControllerApi { fn set_destination( &self, sys_int_num: InterruptNumber, - destination: CpuId, + destination: Option, priority: Priority, ) -> Result<(), &'static str>; } @@ -60,13 +60,6 @@ pub trait SystemInterruptControllerApi { pub trait LocalInterruptControllerApi { fn get() -> &'static Self; - /// Aarch64-specific way to initialize the secondary CPU interfaces. - /// - /// Must be called once from every secondary CPU. - /// - /// Always panics on x86_64. - fn init_secondary_cpu_interface(&self); - fn id(&self) -> LocalInterruptControllerId; fn get_local_interrupt_priority(&self, num: InterruptNumber) -> Priority; fn set_local_interrupt_priority(&self, num: InterruptNumber, priority: Priority); @@ -79,21 +72,45 @@ pub trait LocalInterruptControllerApi { /// If it's None, all CPUs except the sender receive the interrupt. fn send_ipi(&self, num: InterruptNumber, dest: InterruptDestination); + /// Tell the interrupt controller that the current interrupt has been handled. + fn end_of_interrupt(&self, number: InterruptNumber); +} + +/// AArch64-specific methods of a local interrupt controller +pub trait AArch64LocalInterruptControllerApi { + /// Same as [`LocalInterruptControllerApi::enable_local_interrupt`] but for fast interrupts (FIQs). + fn enable_fast_local_interrupt(&self, num: InterruptNumber, enabled: bool); + + /// Same as [`LocalInterruptControllerApi::send_ipi`] but for fast interrupts (FIQs). + fn send_fast_ipi(&self, num: InterruptNumber, dest: InterruptDestination); + /// Reads the minimum priority for an interrupt to reach this CPU. - /// - /// Note: aarch64-only, at the moment. fn get_minimum_priority(&self) -> Priority; /// Changes the minimum priority for an interrupt to reach this CPU. - /// - /// Note: aarch64-only, at the moment. fn set_minimum_priority(&self, priority: Priority); /// Aarch64-specific way to read the current pending interrupt number & priority. + fn acknowledge_interrupt(&self) -> Option<(InterruptNumber, Priority)>; + + /// Aarch64-specific way to initialize the secondary CPU interfaces. /// - /// Always panics on x86_64. - fn acknowledge_interrupt(&self) -> (InterruptNumber, Priority); + /// Must be called once from every secondary CPU. + fn init_secondary_cpu_interface(&self); - /// Tell the interrupt controller that the current interrupt has been handled. - fn end_of_interrupt(&self, number: InterruptNumber); + /// Same as [`Self::acknowledge_interrupt`] but for fast interrupts (FIQs) + /// + /// # Safety + /// + /// This is unsafe because it circumvents the internal Mutex. + /// It must only be used by the `interrupts` crate when handling an FIQ. + unsafe fn acknowledge_fast_interrupt(&self) -> Option<(InterruptNumber, Priority)>; + + /// Same as [`LocalInterruptControllerApi::end_of_interrupt`] but for fast interrupts (FIQs) + /// + /// # Safety + /// + /// This is unsafe because it circumvents the internal Mutex. + /// It must only be used by the `interrupts` crate when handling an FIQ. + unsafe fn end_of_fast_interrupt(&self, number: InterruptNumber); } diff --git a/kernel/interrupt_controller/src/x86_64.rs b/kernel/interrupt_controller/src/x86_64.rs index 917062e3f4..adbee306eb 100644 --- a/kernel/interrupt_controller/src/x86_64.rs +++ b/kernel/interrupt_controller/src/x86_64.rs @@ -55,7 +55,7 @@ impl SystemInterruptControllerApi for SystemInterruptController { fn set_destination( &self, sys_int_num: InterruptNumber, - destination: CpuId, + destination: Option, priority: Priority, ) -> Result<(), &'static str> { let mut int_ctlr = get_ioapic(self.id).expect("BUG: set_destination(): get_ioapic() returned None"); @@ -63,7 +63,11 @@ impl SystemInterruptControllerApi for SystemInterruptController { // no support for priority on x86_64 let _ = priority; - int_ctlr.set_irq(sys_int_num, destination.into(), sys_int_num /* <- is this correct? */) + if let Some(destination) = destination { + int_ctlr.set_irq(sys_int_num, destination.into(), sys_int_num) + } else { + Err("SystemInterruptController::set_destination: todo on x86: set the IOREDTBL MASK bit") + } } } @@ -73,10 +77,6 @@ impl LocalInterruptControllerApi for LocalInterruptController { unimplemented!() } - fn init_secondary_cpu_interface(&self) { - panic!("This must not be used on x86_64") - } - fn id(&self) -> LocalInterruptControllerId { let int_ctlr = get_my_apic().expect("BUG: id(): get_my_apic() returned None"); let int_ctlr = int_ctlr.read(); @@ -112,20 +112,6 @@ impl LocalInterruptControllerApi for LocalInterruptController { }); } - fn get_minimum_priority(&self) -> Priority { - // No priority support on x86_64 - Priority - } - - fn set_minimum_priority(&self, priority: Priority) { - // No priority support on x86_64 - let _ = priority; - } - - fn acknowledge_interrupt(&self) -> (InterruptNumber, Priority) { - panic!("This must not be used on x86_64") - } - fn end_of_interrupt(&self, _number: InterruptNumber) { let mut int_ctlr = get_my_apic().expect("BUG: end_of_interrupt(): get_my_apic() returned None"); let mut int_ctlr = int_ctlr.write(); diff --git a/kernel/interrupts/src/aarch64/mod.rs b/kernel/interrupts/src/aarch64/mod.rs index 716dd42506..3cae5f9fc4 100644 --- a/kernel/interrupts/src/aarch64/mod.rs +++ b/kernel/interrupts/src/aarch64/mod.rs @@ -11,7 +11,7 @@ use tock_registers::registers::InMemoryRegister; use interrupt_controller::{ LocalInterruptController, SystemInterruptController, InterruptDestination, - LocalInterruptControllerApi, SystemInterruptControllerApi, + LocalInterruptControllerApi, AArch64LocalInterruptControllerApi, SystemInterruptControllerApi, }; use kernel_config::time::CONFIG_TIMESLICE_PERIOD_MICROSECONDS; use arm_boards::BOARD_CONFIG; @@ -48,7 +48,7 @@ const MAX_IRQ_NUM: usize = 256; // it's an array of function pointers which are meant to handle IRQs. // Synchronous Exceptions (including syscalls) are not IRQs on aarch64; // this crate doesn't expose any way to handle them at the moment. -static IRQ_HANDLERS: IrqSafeRwLock<[InterruptHandler; MAX_IRQ_NUM]> = IrqSafeRwLock::new([default_irq_handler; MAX_IRQ_NUM]); +static IRQ_HANDLERS: IrqSafeRwLock<[Option; MAX_IRQ_NUM]> = IrqSafeRwLock::new([None; MAX_IRQ_NUM]); /// The Saved Program Status Register at the time of the exception. #[repr(transparent)] @@ -98,12 +98,6 @@ fn default_exception_handler(exc: &ExceptionContext, origin: &'static str) { loop { core::hint::spin_loop() } } -// called for all unhandled interrupt requests -extern "C" fn default_irq_handler(exc: &ExceptionContext) -> EoiBehaviour { - log::error!("Unhandled IRQ:\r\n{:?}\r\n[looping forever now]", exc); - loop { core::hint::spin_loop() } -} - fn read_timer_period_femtoseconds() -> u64 { let counter_freq_hz = CNTFRQ_EL0.get(); let fs_in_one_sec = 1_000_000_000_000_000; @@ -144,7 +138,10 @@ pub fn init_ap() { int_ctrl.init_secondary_cpu_interface(); int_ctrl.set_minimum_priority(0); - int_ctrl.enable_local_interrupt(TLB_SHOOTDOWN_IPI, true); + // on the bootstrap CPU, this is done in setup_tlb_shootdown_handler + int_ctrl.enable_fast_local_interrupt(TLB_SHOOTDOWN_IPI, true); + + // on the bootstrap CPU, this is done in init_timer int_ctrl.enable_local_interrupt(CPU_LOCAL_TIMER_IRQ, true); enable_timer(true); @@ -189,6 +186,8 @@ pub fn init_timer(timer_tick_handler: InterruptHandler) -> Result<(), &'static s /// This function registers an interrupt handler for an inter-processor interrupt /// and handles interrupt controller configuration for that interrupt. +/// +/// Returns an error if the specified interrupt number already has a registered handler. pub fn setup_ipi_handler(handler: InterruptHandler, local_num: InterruptNumber) -> Result<(), &'static str> { // register the handler if let Err(existing_handler) = register_interrupt(local_num, handler) { @@ -207,10 +206,30 @@ pub fn setup_ipi_handler(handler: InterruptHandler, local_num: InterruptNumber) Ok(()) } +/// This function registers an interrupt handler for the TLB Shootdown IPI +/// and handles interrupt controller configuration for that interrupt. +/// +/// Returns an error if the TLB Shootdown interrupt number already has a registered handler. +pub fn setup_tlb_shootdown_handler(handler: InterruptHandler) -> Result<(), &'static str> { + if let Err(existing_handler) = register_interrupt(TLB_SHOOTDOWN_IPI, handler) { + if handler as *const InterruptHandler != existing_handler { + return Err("A different interrupt handler has already been setup for that IPI"); + } + } + + { + // enable this interrupt as a Fast interrupt (FIQ / Group 0 interrupt) + let int_ctrl = LocalInterruptController::get(); + int_ctrl.enable_fast_local_interrupt(TLB_SHOOTDOWN_IPI, true); + } + + Ok(()) +} + /// Enables the PL011 "RX" SPI and routes it to the current CPU. pub fn init_pl011_rx_interrupt() -> Result<(), &'static str> { let int_ctrl = SystemInterruptController::get(); - int_ctrl.set_destination(PL011_RX_SPI, current_cpu(), u8::MAX) + int_ctrl.set_destination(PL011_RX_SPI, Some(current_cpu()), u8::MAX) } /// Disables the timer, schedules its next tick, and re-enables it @@ -255,15 +274,12 @@ pub fn register_interrupt(int_num: InterruptNumber, func: InterruptHandler) -> R let mut handlers = IRQ_HANDLERS.write(); let index = int_num as usize; - let value = handlers[index] as *const InterruptHandler; - let default = default_irq_handler as *const InterruptHandler; - - if value == default { - handlers[index] = func; - Ok(()) - } else { + if let Some(handler) = handlers[index] { error!("register_interrupt: the requested interrupt IRQ {} was already in use", index); - Err(value) + Err(handler as *const _) + } else { + handlers[index] = Some(func); + Ok(()) } } @@ -276,27 +292,35 @@ pub fn register_interrupt(int_num: InterruptNumber, func: InterruptHandler) -> R /// # Arguments /// * `int_num`: the interrupt number that needs to be deregistered /// * `func`: the handler that should currently be stored for 'interrupt_num' -pub fn deregister_interrupt(int_num: InterruptNumber, func: InterruptHandler) -> Result<(), *const InterruptHandler> { +pub fn deregister_interrupt(int_num: InterruptNumber, func: InterruptHandler) -> Result<(), Option<*const InterruptHandler>> { let mut handlers = IRQ_HANDLERS.write(); let index = int_num as usize; - let value = handlers[index] as *const InterruptHandler; let func = func as *const InterruptHandler; + let handler = handlers[index].map(|h| h as *const InterruptHandler); - if value == func { - handlers[index] = default_irq_handler; - Ok(()) - } else { + if handler != Some(func) { error!("deregister_interrupt: Cannot free interrupt due to incorrect handler function"); - Err(value) + Err(handler) + } else { + handlers[index] = None; + Ok(()) } } -/// Broadcast an Inter-Processor Interrupt to all other -/// cores in the system -pub fn send_ipi_to_all_other_cpus(irq_num: InterruptNumber) { +/// Broadcast an Inter-Processor Interrupt to all other CPU cores in the system +pub fn broadcast_ipi(ipi_num: InterruptNumber) { + let int_ctrl = LocalInterruptController::get(); + int_ctrl.send_ipi(ipi_num, InterruptDestination::AllOtherCpus); +} + +/// Broadcast the TLB Shootdown Inter-Processor Interrupt to all other +/// CPU cores in the system +/// +/// This IPI uses fast interrupts (FIQs) as an NMI alternative. +pub fn broadcast_tlb_shootdown_ipi() { let int_ctrl = LocalInterruptController::get(); - int_ctrl.send_ipi(irq_num, InterruptDestination::AllOtherCpus); + int_ctrl.send_fast_ipi(TLB_SHOOTDOWN_IPI, InterruptDestination::AllOtherCpus); } /// Send an "end of interrupt" signal, notifying the interrupt chip that @@ -414,21 +438,62 @@ extern "C" fn current_elx_synchronous(e: &mut ExceptionContext) { default_exception_handler(e, "current_elx_synchronous"); } +// When this is entered, FIQs are enabled / unmasked, because we use +// them as an NMI alternative, so they must be allowed at all times. +// +// Spurious interrupts are often the result of an FIQ being handled +// after we started handling an IRQ but before we acknowledged it. #[no_mangle] extern "C" fn current_elx_irq(exc: &mut ExceptionContext) { let (irq_num, _priority) = { let int_ctrl = LocalInterruptController::get(); - int_ctrl.acknowledge_interrupt() + match int_ctrl.acknowledge_interrupt() { + Some(irq_prio_tuple) => irq_prio_tuple, + None /* spurious interrupt */ => return, + } }; let index = irq_num as usize; - let handler = match index < MAX_IRQ_NUM { - true => IRQ_HANDLERS.read()[index], - false => default_irq_handler, + let handler = IRQ_HANDLERS.read().get(index).copied().flatten(); + let result = handler.map(|handler| handler(exc)); + + if let Some(result) = result { + if result == EoiBehaviour::HandlerDidNotSendEoi { + // will use LocalInterruptController + eoi(irq_num); + } + } else { + log::error!("Unhandled IRQ: {}\r\n{:?}\r\n[looping forever now]", irq_num, exc); + loop { core::hint::spin_loop() } + } +} + +// When this is entered, FIQs are disabled / masked: there must be +// only one FIQ (that we use as an NMI alternative) at a time. +// +// Currently, FIQs are only used for TLB shootdown. +#[no_mangle] +extern "C" fn current_elx_fiq(exc: &mut ExceptionContext) { + let (irq_num, _priority) = { + let int_ctrl = LocalInterruptController::get(); + let ack = unsafe { int_ctrl.acknowledge_fast_interrupt() }; + match ack { + Some(irq_prio_tuple) => irq_prio_tuple, + None /* spurious interrupt */ => return, + } }; - if handler(exc) == EoiBehaviour::HandlerDidNotSendEoi { - eoi(irq_num); + let handler = IRQ_HANDLERS.read().get(irq_num as usize).copied().flatten(); + let result = handler.map(|handler| handler(exc)); + + if let Some(result) = result { + if result == EoiBehaviour::HandlerDidNotSendEoi { + let int_ctrl = LocalInterruptController::get(); + unsafe { int_ctrl.end_of_fast_interrupt(irq_num) }; + } + } else { + log::error!("Unhandled FIQ: {}\r\n{:?}\r\n[looping forever now]", irq_num, exc); + loop { core::hint::spin_loop() } } } diff --git a/kernel/interrupts/src/aarch64/table.s b/kernel/interrupts/src/aarch64/table.s index c66a2f4045..c480eb7143 100644 --- a/kernel/interrupts/src/aarch64/table.s +++ b/kernel/interrupts/src/aarch64/table.s @@ -77,9 +77,11 @@ __exception_vector_start: .org 0x200 CALL_WITH_CONTEXT current_elx_synchronous .org 0x280 + // this first instruction re-enables (unmasks) fast interrupts so that IRQs can be interrupted by FIQs. + msr daifclr, #1 CALL_WITH_CONTEXT current_elx_irq .org 0x300 - FIQ_SUSPEND + CALL_WITH_CONTEXT current_elx_fiq .org 0x380 CALL_WITH_CONTEXT current_elx_serror diff --git a/kernel/memory_aarch64/src/lib.rs b/kernel/memory_aarch64/src/lib.rs index e7cd6a560f..04cd8c451f 100644 --- a/kernel/memory_aarch64/src/lib.rs +++ b/kernel/memory_aarch64/src/lib.rs @@ -16,35 +16,62 @@ use pte_flags::PteFlags; use boot_info::{BootInformation, ElfSection}; use kernel_config::memory::KERNEL_OFFSET; -#[cfg(any(target_arch = "aarch64", doc))] +#[cfg(any(doc, target_arch = "aarch64"))] use core::arch::asm; const THESEUS_ASID: u16 = 0; -#[cfg(any(target_arch = "aarch64", doc))] /// Flushes the specific virtual address in TLB. /// /// TLBI => tlb invalidate instruction -/// "va" => all translations at execution level -/// using the supplied address +/// "va" => all translations at execution level using the supplied address /// "e1" => execution level +#[cfg(any(doc, target_arch = "aarch64"))] pub fn tlb_flush_virt_addr(vaddr: VirtualAddress) { - #[cfg(target_arch = "aarch64")] - unsafe { asm!("tlbi vae1, {}", in(reg) vaddr.value()) }; + // unsure here: where should the original address ASID go? + // it's zero in theseus so it's not important for us + + // about the 48 bit shift: + // If the implementation supports 16 bits of ASID, then the + // upper 8 bits of the ASID must be written to 0 by software + // when the context being invalidated only uses 8 bits + let value = ((THESEUS_ASID as usize) << 48) | (vaddr.value() >> 12); + + unsafe { + asm!("tlbi vae1, {}", in(reg) value) + }; } -#[cfg(any(target_arch = "aarch64", doc))] +/* NO SUPPORT IN QEMU + +/// Flushes the specific virtual address in the TLB of any CPU +/// in the outer shareable domain. +/// +/// TLBI => tlb invalidate instruction +/// "va" => all translations at execution level using the supplied address +/// "e1" => execution level +/// "os" => outer shareable domain +#[cfg(any(doc, target_arch = "aarch64"))] +pub fn tlb_flush_virt_addr_all_cpus(vaddr: VirtualAddress) { + unsafe { + let value = ((THESEUS_ASID as usize) << 48) | (vaddr.value() >> 12); + asm!(".arch armv8.4-a\ntlbi vae1os, {}", in(reg) value) + }; +} + +*/ + /// Flushes all TLB entries with Theseus' ASID (=0). /// /// TLBI => tlb invalidate instruction /// "asid" => all entries with specific ASID /// "e1" => execution level +#[cfg(any(doc, target_arch = "aarch64"))] pub fn tlb_flush_by_theseus_asid() { - #[cfg(target_arch = "aarch64")] unsafe { asm!("tlbi aside1, {:x}", in(reg) THESEUS_ASID) }; } -#[cfg(any(target_arch = "aarch64", doc))] +#[cfg(any(doc, target_arch = "aarch64"))] pub use tlb_flush_by_theseus_asid as tlb_flush_all; /// Returns the current top-level page table address. diff --git a/kernel/nano_core/src/lib.rs b/kernel/nano_core/src/lib.rs index 87b6c2409c..7329a71ae4 100644 --- a/kernel/nano_core/src/lib.rs +++ b/kernel/nano_core/src/lib.rs @@ -88,6 +88,9 @@ where B: boot_info::BootInformation { irq_safety::disable_interrupts(); + #[cfg(target_arch = "aarch64")] + irq_safety::disable_fast_interrupts(); + println!("nano_core(): Entered early setup. Interrupts disabled."); #[cfg(target_arch = "x86_64")] @@ -131,7 +134,7 @@ where let logger_ports = [take_serial_port(SerialPortAddress::COM1)]; logger::early_init(None, IntoIterator::into_iter(logger_ports).flatten()); log::info!("initialized early logger with aarch64 serial ports."); - println!("nano_core(): initialized early logger with aarch64 serial ports."); + println!("nano_core(): initialized early logger with aarch64 serial ports."); } println!("nano_core(): initialized memory subsystem."); diff --git a/kernel/tlb_shootdown/src/lib.rs b/kernel/tlb_shootdown/src/lib.rs index 98f333b5dc..a9ad79e31e 100644 --- a/kernel/tlb_shootdown/src/lib.rs +++ b/kernel/tlb_shootdown/src/lib.rs @@ -28,7 +28,7 @@ pub fn init() { memory::set_broadcast_tlb_shootdown_cb(broadcast_tlb_shootdown); #[cfg(target_arch = "aarch64")] - interrupts::setup_ipi_handler(tlb_shootdown_ipi_handler, interrupts::TLB_SHOOTDOWN_IPI).unwrap(); + interrupts::setup_tlb_shootdown_handler(tlb_shootdown_ipi_handler).unwrap(); } /// Handles a TLB shootdown IPI requested by another CPU. @@ -40,6 +40,8 @@ pub fn init() { pub fn handle_tlb_shootdown_ipi() -> bool { let pages_to_invalidate = TLB_SHOOTDOWN_IPI_PAGES.read().clone(); if let Some(pages) = pages_to_invalidate { + // Note: logging in a NMI (x86_64) or FIQ (aarch64) context can cause deadlock, + // so this should only be used sparingly to help debug problems with TLB shootdowns. // log::trace!("handle_tlb_shootdown_ipi(): CPU {}, pages: {:?}", apic::current_cpu(), pages); for page in pages { tlb_flush_virt_addr(page.start_address()); @@ -97,7 +99,7 @@ fn broadcast_tlb_shootdown(pages_to_invalidate: PageRange) { } #[cfg(target_arch = "aarch64")] - interrupts::send_ipi_to_all_other_cpus(interrupts::TLB_SHOOTDOWN_IPI); + interrupts::broadcast_tlb_shootdown_ipi(); // wait for all other cores to handle this IPI // it must be a blocking, synchronous operation to ensure stale TLB entries don't cause problems @@ -111,6 +113,10 @@ fn broadcast_tlb_shootdown(pages_to_invalidate: PageRange) { // release lock TLB_SHOOTDOWN_IPI_LOCK.store(false, Ordering::Release); + + if false { + log::warn!("send_tlb_shootdown_ipi(): from CPU {:?}, complete", cpu::current_cpu()); + } } /// Interrupt Handler for TLB Shootdowns on aarch64