From ffa7cdc41255533ac475ce1d41d22f1ba76caf5e Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Wed, 15 Jul 2026 15:50:43 +0200 Subject: [PATCH 1/3] The switch from the physical timer (CNTP_) to the virtual timer (CNTV_) updated the FDT parsing in init(), which now selects the third interrupt triplet of the "arm,armv8-timer" node (Virtual, PPI11 / INTID 27). init_cpu() still selected the second triplet (Non-secure Phys, PPI 14 / INTID 30), so every application processor enabled the physical timer PPI in its redistributor while set_oneshot_timer programs the virtual timer. Timer PPIs are per-core: sleeping tasks on the secondary cores were never woken up again. This is why thread_test with --smp 4 timed out: of eight threads only the two scheduled on the boot core finished; the six on the secondary cores slept forever. Skip the same two triplets in init_cpu() so all cores enable the virtual timer interrupt. --- src/arch/aarch64/kernel/interrupts.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/arch/aarch64/kernel/interrupts.rs b/src/arch/aarch64/kernel/interrupts.rs index 0717b2222f..483385c92d 100644 --- a/src/arch/aarch64/kernel/interrupts.rs +++ b/src/arch/aarch64/kernel/interrupts.rs @@ -449,7 +449,15 @@ pub fn init_cpu() { if let Some(timer_node) = fdt.find_compatible(&["arm,armv8-timer", "arm,armv7-timer"]) { let irq_slice = timer_node.property("interrupts").unwrap().value; - /* Secure Phys IRQ */ + // Select the Virtual Timer triplet, matching `init()`: the kernel + // programs the virtual timer (CNTV_*), and timer PPIs are per-core, + // so every application processor has to enable the *same* interrupt + // in its redistributor that the boot core selected. + /* Secure Phys IRQ — skip */ + let (_irqtype, irq_slice) = irq_slice.split_at(size_of::()); + let (_irq, irq_slice) = irq_slice.split_at(size_of::()); + let (_irqflags, irq_slice) = irq_slice.split_at(size_of::()); + /* Non-secure Phys IRQ — skip */ let (_irqtype, irq_slice) = irq_slice.split_at(size_of::()); let (_irq, irq_slice) = irq_slice.split_at(size_of::()); let (_irqflags, irq_slice) = irq_slice.split_at(size_of::()); From 1c5f0294540b7e0bd21e149d5fd02b99e0555776 Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Fri, 17 Jul 2026 10:40:22 +0200 Subject: [PATCH 2/3] xtask: wait for the stdin echo instead of a fixed delay test_stdin fed each message to the guest with a one-second pause and then terminated QEMU immediately, reading the echoed output only afterwards. On a slow target the last echo had not been written yet when QEMU was killed, so the collected output ended mid-line and the assertion failed intermittently. This showed up on aarch64_be, which is emulated without hardware acceleration. Collect the guest's stdout on a background thread and wait until every message has been echoed (bounded by a 60 s timeout) before terminating QEMU. The check no longer depends on the guest keeping pace with a fixed schedule. --- xtask/src/ci/qemu.rs | 48 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/xtask/src/ci/qemu.rs b/xtask/src/ci/qemu.rs index 39803a0cd9..ff46d090d0 100644 --- a/xtask/src/ci/qemu.rs +++ b/xtask/src/ci/qemu.rs @@ -3,7 +3,8 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr, TcpStream, UdpSocket}; use std::path::Path; use std::process::{Child, Command, ExitStatus, Stdio}; use std::str::from_utf8; -use std::time::Duration; +use std::sync::mpsc; +use std::time::{Duration, Instant}; use std::{env, fs, io, thread}; use anyhow::{Context, Result, bail, ensure}; @@ -570,9 +571,27 @@ fn get_frequency() -> u64 { } fn test_stdin(child: &mut Child) -> Result<()> { + /// Upper bound for the guest to echo back every message. This only + /// caps the failure case — the test stops waiting as soon as the last + /// message shows up. + const ECHO_TIMEOUT: Duration = Duration::from_secs(60); + thread::sleep(Duration::from_secs(10)); let messages = ["Hello, there!", "Hello, again!", "Bye-bye!"]; + // Reading blocks, so the echo is collected on a separate thread while + // the main thread feeds stdin. + let stdout = child.stdout.take().unwrap(); + let (tx, rx) = mpsc::channel(); + thread::spawn(move || { + for line in BufReader::new(stdout).lines() { + let Ok(line) = line else { break }; + if tx.send(line).is_err() { + break; + } + } + }); + let mut stdin = child.stdin.take().unwrap(); for message in messages { writeln!(&mut stdin, "{message}")?; @@ -580,20 +599,31 @@ fn test_stdin(child: &mut Child) -> Result<()> { thread::sleep(Duration::from_secs(1)); } - child.terminate()?; + // Wait for the guest to echo everything back before terminating QEMU. + // Terminating after a fixed delay cut the output off mid-line on slow + // targets, such as aarch64_be, which is emulated without hardware + // acceleration. + let mut stdout_lines = Vec::new(); + let mut missing = messages.to_vec(); + let deadline = Instant::now() + ECHO_TIMEOUT; + while !missing.is_empty() { + let Some(timeout) = deadline.checked_duration_since(Instant::now()) else { + break; + }; + let Ok(line) = rx.recv_timeout(timeout) else { + break; + }; + missing.retain(|message| !line.contains(message)); + stdout_lines.push(line); + } - let stdout = child.stdout.take().unwrap(); - let stdout_lines = BufReader::new(stdout) - .lines() - .collect::, _>>()?; + child.terminate()?; for line in &stdout_lines { println!("{line}"); } - for message in messages { - assert!(stdout_lines.iter().any(|line| line.contains(message))); - } + assert!(missing.is_empty(), "missing from output: {missing:?}"); Ok(()) } From 2f10654572d860865cb001696671e9d5f8c74733 Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Sun, 19 Jul 2026 11:33:48 +0200 Subject: [PATCH 3/3] fix(aarch64/interrupts): enable the same timer PPI on every core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit init() and init_cpu() each parsed the "arm,armv8-timer" device-tree node on their own and picked a triplet by hand, so the two could disagree about which interrupt the kernel uses. The kernel arms the physical timer (CNTP_CVAL_EL0/CNTP_CTL_EL0), whose PPI is the Non-secure Phys triplet (INTID 30), but init_cpu() selected the Virtual triplet (INTID 27). Timer PPIs are per-core, so every application processor enabled an interrupt that never fires and tasks sleeping on the secondary cores were never woken up again — thread_test hung until the QEMU timeout on aarch64_be with SMP 4. Parse the timer interrupt once in init(), cache the resolved IntId and trigger mode in TIMER_IRQ, and let init_cpu() enable that exact interrupt in its redistributor. This removes the duplicated FDT parsing that let the two sites drift apart. --- src/arch/aarch64/kernel/interrupts.rs | 94 +++++++++++---------------- 1 file changed, 37 insertions(+), 57 deletions(-) diff --git a/src/arch/aarch64/kernel/interrupts.rs b/src/arch/aarch64/kernel/interrupts.rs index 483385c92d..0a5deafb6b 100644 --- a/src/arch/aarch64/kernel/interrupts.rs +++ b/src/arch/aarch64/kernel/interrupts.rs @@ -33,6 +33,10 @@ pub(crate) const SGI_RESCHED: u8 = 1; /// Number of the timer interrupt static mut TIMER_INTERRUPT: u32 = 0; +/// Timer interrupt and its trigger mode, parsed once from the device tree by +/// [`init`]. Timer PPIs are per-core, so [`init_cpu`] reuses this to enable the +/// exact same interrupt in every application processor's redistributor. +static TIMER_IRQ: OnceCell<(IntId, Trigger)> = OnceCell::new(); /// Number of the UART interrupt static mut UART_INTERRUPT: u32 = 0; /// Possible interrupt handlers @@ -263,6 +267,28 @@ pub fn wakeup_core(core_id: CoreId) { .unwrap(); } +/// Decodes an `(interrupt-type, interrupt-number, flags)` triplet from a device +/// tree `interrupts` property into its [`IntId`] and [`Trigger`]. +fn decode_interrupt(irqtype: u32, irq: u32, irqflags: u32) -> (IntId, Trigger) { + let id = if irqtype == 1 { + IntId::ppi(irq) + } else if irqtype == 0 { + IntId::spi(irq) + } else { + panic!("Invalid interrupt type"); + }; + + let trigger = if (irqflags & 0xf) == 4 || (irqflags & 0xf) == 8 { + Trigger::Level + } else if (irqflags & 0xf) == 2 || (irqflags & 0xf) == 1 { + Trigger::Edge + } else { + panic!("Invalid interrupt level!"); + }; + + (id, trigger) +} + pub(crate) fn init() { info!("Initialize generic interrupt controller"); @@ -356,25 +382,15 @@ pub(crate) fn init() { .lock() .insert(u8::try_from(irq).unwrap() + PPI_START, "Timer"); + // Cache the parsed interrupt so that every application processor enables + // the exact same PPI in its redistributor (see `init_cpu`). + let (timer_irqid, trigger) = decode_interrupt(irqtype, irq, irqflags); + TIMER_IRQ.set((timer_irqid, trigger)).unwrap(); + // enable timer interrupt - let timer_irqid = if irqtype == 1 { - IntId::ppi(irq) - } else if irqtype == 0 { - IntId::spi(irq) - } else { - panic!("Invalid interrupt type"); - }; gic.set_interrupt_priority(timer_irqid, Some(cpu_id), 0x00) .unwrap(); - if (irqflags & 0xf) == 4 || (irqflags & 0xf) == 8 { - gic.set_trigger(timer_irqid, Some(cpu_id), Trigger::Level) - .unwrap(); - } else if (irqflags & 0xf) == 2 || (irqflags & 0xf) == 1 { - gic.set_trigger(timer_irqid, Some(cpu_id), Trigger::Edge) - .unwrap(); - } else { - panic!("Invalid interrupt level!"); - } + gic.set_trigger(timer_irqid, Some(cpu_id), trigger).unwrap(); gic.enable_interrupt(timer_irqid, Some(cpu_id), true) .unwrap(); } @@ -445,49 +461,13 @@ pub fn init_cpu() { GicCpuInterface::enable_group1(true); GicCpuInterface::set_priority_mask(0xff); - let fdt = env::fdt().unwrap(); - - if let Some(timer_node) = fdt.find_compatible(&["arm,armv8-timer", "arm,armv7-timer"]) { - let irq_slice = timer_node.property("interrupts").unwrap().value; - // Select the Virtual Timer triplet, matching `init()`: the kernel - // programs the virtual timer (CNTV_*), and timer PPIs are per-core, - // so every application processor has to enable the *same* interrupt - // in its redistributor that the boot core selected. - /* Secure Phys IRQ — skip */ - let (_irqtype, irq_slice) = irq_slice.split_at(size_of::()); - let (_irq, irq_slice) = irq_slice.split_at(size_of::()); - let (_irqflags, irq_slice) = irq_slice.split_at(size_of::()); - /* Non-secure Phys IRQ — skip */ - let (_irqtype, irq_slice) = irq_slice.split_at(size_of::()); - let (_irq, irq_slice) = irq_slice.split_at(size_of::()); - let (_irqflags, irq_slice) = irq_slice.split_at(size_of::()); - /* Non-secure Phys IRQ */ - let (irqtype, irq_slice) = irq_slice.split_at(size_of::()); - let (irq, irq_slice) = irq_slice.split_at(size_of::()); - let (irqflags, _irq_slice) = irq_slice.split_at(size_of::()); - let irqtype = u32::from_be_bytes(irqtype.try_into().unwrap()); - let irq = u32::from_be_bytes(irq.try_into().unwrap()); - let irqflags = u32::from_be_bytes(irqflags.try_into().unwrap()); - - // enable timer interrupt - let timer_irqid = if irqtype == 1 { - IntId::ppi(irq) - } else if irqtype == 0 { - IntId::spi(irq) - } else { - panic!("Invalid interrupt type"); - }; + // Enable the same timer interrupt that `init()` parsed from the device + // tree. The timer is a per-core PPI, so it has to be enabled in every + // redistributor; reusing the cached value keeps all cores in sync. + if let Some(&(timer_irqid, trigger)) = TIMER_IRQ.get() { gic.set_interrupt_priority(timer_irqid, Some(cpu_id), 0x00) .unwrap(); - if (irqflags & 0xf) == 4 || (irqflags & 0xf) == 8 { - gic.set_trigger(timer_irqid, Some(cpu_id), Trigger::Level) - .unwrap(); - } else if (irqflags & 0xf) == 2 || (irqflags & 0xf) == 1 { - gic.set_trigger(timer_irqid, Some(cpu_id), Trigger::Edge) - .unwrap(); - } else { - panic!("Invalid interrupt level!"); - } + gic.set_trigger(timer_irqid, Some(cpu_id), trigger).unwrap(); gic.enable_interrupt(timer_irqid, Some(cpu_id), true) .unwrap(); }