From b5a3e456e8fe6203e720743be7309cf69809a45b Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Wed, 26 Aug 2026 16:23:31 +0200 Subject: [PATCH 01/12] boards/qemu-intel64: add citest configuration CI/NTFC testing config for intel64 Signed-off-by: raiden00pl --- Documentation/testing/ntfc.rst | 1 + .../configs/citest_smp/config.yaml | 14 ++ .../qemu-intel64/configs/citest_smp/defconfig | 161 ++++++++++++++++++ .../qemu-intel64/configs/citest_smp/run.sh | 54 ++++++ .../configs/citest_smp/session.json | 13 ++ 5 files changed, 243 insertions(+) create mode 100644 boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml create mode 100644 boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig create mode 100755 boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh create mode 100644 boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json diff --git a/Documentation/testing/ntfc.rst b/Documentation/testing/ntfc.rst index 76b780cbb21bb..47cb6e169bfc0 100644 --- a/Documentation/testing/ntfc.rst +++ b/Documentation/testing/ntfc.rst @@ -22,6 +22,7 @@ CI runtime test cases use NTFC. Configurations with an NTFC runner include: - :doc:`arm/imx6 `: ``sabre-6quad/citest`` - :doc:`arm64/qemu `: ``qemu-armv8a/citest`` - :doc:`arm64/qemu `: ``qemu-armv8a/citest_smp`` +- :doc:`x86_64/qemu `: ``qemu-intel64/citest_smp`` Running a CI Test Target Locally ================================ diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml new file mode 100644 index 0000000000000..cfce28d938844 --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/config.yaml @@ -0,0 +1,14 @@ +config: + cwd: './' + +product: + name: "qemu-intel64" + cores: + core0: + name: 'main' + device: 'qemu' + exec_path: 'qemu-system-x86_64' + exec_args: '-m 2G -smp 4 -cpu max -nographic -serial mon:stdio' + uptime: 60 + conf_path: './.config' + elf_path: './nuttx' diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig new file mode 100644 index 0000000000000..32b437b429766 --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/defconfig @@ -0,0 +1,161 @@ +# +# This file is autogenerated: PLEASE DO NOT EDIT IT. +# +# You can use "make menuconfig" to make any modifications to the installed .config file. +# You can then do "make savedefconfig" to generate a new defconfig file that includes your +# modifications. +# +# CONFIG_ARCH_INTEL64_HAVE_PCID is not set +# CONFIG_ARCH_X86_64_X2APIC is not set +CONFIG_16550_ADDRWIDTH=16 +CONFIG_16550_PCI_UART1=y +CONFIG_16550_PCI_UART1_CLOCK=1843200 +CONFIG_16550_PCI_UART1_DEVICE=0x0002 +CONFIG_16550_PCI_UART1_VENDOR=0x1b36 +CONFIG_16550_PCI_UART=y +CONFIG_16550_UART0=y +CONFIG_16550_UART0_BASE=0x3f8 +CONFIG_16550_UART0_CLOCK=1843200 +CONFIG_16550_UART0_IRQ=36 +CONFIG_16550_UART0_RXBUFSIZE=16 +CONFIG_16550_UART0_SERIAL_CONSOLE=y +CONFIG_16550_UART0_TXBUFSIZE=16 +CONFIG_ALLOW_MIT_COMPONENTS=y +CONFIG_ARCH="x86_64" +CONFIG_ARCH_BOARD="qemu-intel64" +CONFIG_ARCH_BOARD_INTEL64_QEMU=y +CONFIG_ARCH_CHIP="qemu" +CONFIG_ARCH_CHIP_INTEL64_QEMU=y +CONFIG_ARCH_INTEL64_HPET_ALARM=y +CONFIG_ARCH_INTERRUPTSTACK=8192 +CONFIG_ARCH_SETJMP_H=y +CONFIG_ARCH_SIZET_LONG=y +CONFIG_ARCH_X86_64=y +CONFIG_ARCH_X86_64_SSE3=y +CONFIG_ARCH_X86_64_SSE41=y +CONFIG_ARCH_X86_64_SSE42=y +CONFIG_ARCH_X86_64_SSE4A=y +CONFIG_ARCH_X86_64_SSSE3=y +CONFIG_BOARD_LOOPSPERMSEC=999 +CONFIG_BOOT_RUNFROMEXTSRAM=y +CONFIG_BUILTIN=y +CONFIG_CM_MM_TEST=y +CONFIG_CM_MUTEX_TEST=y +CONFIG_CM_PTHREAD_TEST=y +CONFIG_CM_SCHED_TEST=y +CONFIG_CM_SOCKET_TEST=y +CONFIG_CM_TIME_TEST=y +CONFIG_CONSOLE_SYSLOG=y +CONFIG_DEBUG_FEATURES=y +CONFIG_DEBUG_PCI=y +CONFIG_DEBUG_PCI_ERROR=y +CONFIG_DEBUG_PCI_INFO=y +CONFIG_DEBUG_PCI_WARN=y +CONFIG_DEBUG_SYMBOLS=y +CONFIG_DEFAULT_TASK_STACKSIZE=4194304 +CONFIG_DEV_SIMPLE_ADDRENV=y +CONFIG_EVENT_FD=y +CONFIG_EXAMPLES_HELLO=y +CONFIG_EXAMPLES_HIDKBD=y +CONFIG_EXAMPLES_PIPE=y +CONFIG_EXAMPLES_POPEN=y +CONFIG_FS_AIO=y +CONFIG_FS_FAT=y +CONFIG_FS_NAMED_SEMAPHORES=y +CONFIG_FS_NOTIFY=y +CONFIG_FS_PROCFS=y +CONFIG_FS_SHMFS=y +CONFIG_FS_TMPFS=y +CONFIG_HIDKBD_STACKSIZE=10240 +CONFIG_HIDMOUSE_STACKSIZE=10240 +CONFIG_IDLETHREAD_STACKSIZE=4194304 +CONFIG_INIT_ENTRYPOINT="nsh_main" +CONFIG_IOB_ALIGNMENT=64 +CONFIG_IOB_BUFSIZE=2048 +CONFIG_IOB_NBUFFERS=1024 +CONFIG_IOB_NCHAINS=255 +CONFIG_LIBC_LOCALE=y +CONFIG_LIBC_MEMFD_ERROR=y +CONFIG_LIBM=y +CONFIG_NET=y +CONFIG_NETDB_DNSCLIENT=y +CONFIG_NETDEV_LATEINIT=y +CONFIG_NETINIT_DRIPADDR=0xc0a80801 +CONFIG_NETINIT_IPADDR=0x0a000102 +CONFIG_NETUTILS_CODECS=y +CONFIG_NETUTILS_IPERF=y +CONFIG_NETUTILS_TELNETD=y +CONFIG_NETUTILS_TFTPC=y +CONFIG_NETUTILS_WEBCLIENT=y +CONFIG_NET_BROADCAST=y +CONFIG_NET_E1000=y +CONFIG_NET_ETH_PKTSIZE=1514 +CONFIG_NET_ICMP_SOCKET=y +CONFIG_NET_IGC=y +CONFIG_NET_LL_GUARDSIZE=32 +CONFIG_NET_MAX_LISTENPORTS=8 +CONFIG_NET_PKT=y +CONFIG_NET_STATISTICS=y +CONFIG_NET_TCP=y +CONFIG_NET_UDP=y +CONFIG_NSH_BUILTIN_APPS=y +CONFIG_NSH_READLINE=y +CONFIG_PCI=y +CONFIG_PCI_MSIX=y +CONFIG_PCI_QEMU_EDU=y +CONFIG_PCI_QEMU_TEST=y +CONFIG_PIPES=y +CONFIG_PREALLOC_CHILDSTATUS=16 +CONFIG_PRIORITY_INHERITANCE=y +CONFIG_PSEUDOFS_ATTRIBUTES=y +CONFIG_PSEUDOFS_FILE=y +CONFIG_PTHREAD_MUTEX_TYPES=y +CONFIG_PTHREAD_STACK_MIN=4194304 +CONFIG_RAM_SIZE=268435456 +CONFIG_SCHED_CHILD_STATUS=y +CONFIG_SCHED_HAVE_PARENT=y +CONFIG_SCHED_HPWORK=y +CONFIG_SCHED_IRQMONITOR=y +CONFIG_SCHED_LPNTHREADS=1 +CONFIG_SCHED_LPWORK=y +CONFIG_SCHED_THREAD_LOCAL=y +CONFIG_SCHED_TICKLESS=y +CONFIG_SIGNAL_FD=y +CONFIG_SIG_DEFAULT=y +CONFIG_SIG_EVTHREAD=y +CONFIG_SMP=y +CONFIG_START_DAY=3 +CONFIG_START_MONTH=3 +CONFIG_START_YEAR=2011 +CONFIG_SYSTEM_CLE=y +CONFIG_SYSTEM_NSH=y +CONFIG_SYSTEM_POPEN=y +CONFIG_SYSTEM_SETLOGMASK=y +CONFIG_TESTING_ATOMIC=y +CONFIG_TESTING_CMOCKA=y +CONFIG_TESTING_CRYPTO=y +CONFIG_TESTING_DRIVER_TEST=y +CONFIG_TESTING_FFF=y +CONFIG_TESTING_FSTEST=y +CONFIG_TESTING_GETPRIME=y +CONFIG_TESTING_HEAP=y +CONFIG_TESTING_LTP=y +CONFIG_TESTING_LTP_STACKSIZE=65536 +CONFIG_TESTING_MEMORY_STRESS=y +CONFIG_TESTING_OSTEST=y +CONFIG_TESTING_OSTEST_STACKSIZE=4194304 +CONFIG_TESTING_RAMTEST=y +CONFIG_TESTING_SMP=y +CONFIG_TESTING_TIMERJITTER=y +CONFIG_TESTS_TESTSUITES=y +CONFIG_TESTS_TESTSUITES_MOUNT_DIR="/tmp" +CONFIG_TIMER_FD=y +CONFIG_USBHOST=y +CONFIG_USBHOST_COMPOSITE=y +CONFIG_USBHOST_FT232R=y +CONFIG_USBHOST_HIDKBD=y +CONFIG_USBHOST_HIDMOUSE=y +CONFIG_USBHOST_MSC=y +CONFIG_USBHOST_WAITER=y +CONFIG_USBHOST_XHCI_PCI=y +CONFIG_USEC_PER_TICK=1 diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh new file mode 100755 index 0000000000000..b337e5891df3a --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +############################################################################ +# boards/x86_64/qemu/qemu-intel64/configs/citest_smp/run.sh +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. The +# ASF licenses this file to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance with the +# License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +############################################################################ + +set -o xtrace + +# start from nuttx dir +olddir=$(pwd) +nuttdir=${CURRENTCONFDIR}/../../../../../../ +cd ${nuttdir} + +# run NTFC +confpath=${CURRENTCONFDIR}/config.yaml +jsonconf=${CURRENTCONFDIR}/session.json +testpath=${NTFCDIR}/external/nuttx-testing +ntfc test --testpath=${testpath} --confpath=${confpath} --jsonconf=${jsonconf} + +ret="$?" +echo $ret + +# export test results +artifacts=${ARTIFACTCONFDIR}/ntfc +mkdir -p ${artifacts} + +# A retried build already exported the previous attempt; replace it, or +# "mv result" fails and the leftover directory makes the tree unclean. + +rm -rf ${artifacts}/result ${artifacts}/pytest.debug.log +mv pytest.debug.log ${artifacts} +mv result ${artifacts} + +# restore old dir +cd ${olddir} + +exit $ret diff --git a/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json new file mode 100644 index 0000000000000..2a8c05d05daae --- /dev/null +++ b/boards/x86_64/qemu/qemu-intel64/configs/citest_smp/session.json @@ -0,0 +1,13 @@ +{ + "module": { + "include_module": [], + "exclude_module": + [ + "Nuttx_System_Fs_Fs" + ], + "order": [] + }, + "args": { + "kv": [] + } +} From 5708874d1368949193656f715df71ff6b27f70ee Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Wed, 2 Sep 2026 15:24:45 +0200 Subject: [PATCH 02/12] arch/intel64: fix self-deadlock in intel64_oneshot_start() intel64_oneshot_start() takes g_oneshot_spin and then, if the timer is already running, calls intel64_oneshot_cancel(), which takes the same spinlock again. Spinlocks are not recursive, so the CPU spins forever on its own lock while holding the critical section; the HPET timer ISR on another CPU then blocks on g_cpu_irqlock and the system hangs. This is hit as soon as the tickless scheduler re-arms a running HPET oneshot timer under SMP (ostest task_restart, LTP aio tests). Stop the running timer inline instead of calling cancel: disable the interrupt, detach the ISR so up_enable_irq() does not assert on a busy IRQ, and clear the running flag. The ISR, comparator and interrupt enable are reprogrammed by the rest of the function anyway. Assisted-by: Claude Code Signed-off-by: raiden00pl --- arch/x86_64/src/intel64/intel64_oneshot.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/arch/x86_64/src/intel64/intel64_oneshot.c b/arch/x86_64/src/intel64/intel64_oneshot.c index a98b8cea55502..eb907eaed17ee 100644 --- a/arch/x86_64/src/intel64/intel64_oneshot.c +++ b/arch/x86_64/src/intel64/intel64_oneshot.c @@ -301,10 +301,21 @@ int intel64_oneshot_start(struct intel64_oneshot_s *oneshot, flags = spin_lock_irqsave(&g_oneshot_spin); if (oneshot->running) { - /* Yes.. then cancel it */ + /* Yes.. then stop it. Do NOT call intel64_oneshot_cancel() here: + * it takes g_oneshot_spin, which we already hold, and spinlocks are + * not recursive, so that deadlocks the CPU. Everything else that + * cancel would do (ISR, comparator, interrupt enable) is + * reprogrammed below anyway. + */ tmrinfo("Already running... cancelling\n"); - intel64_oneshot_cancel(oneshot, NULL); + +#ifndef CONFIG_INTEL64_HPET_FSB + INTEL64_TIM_DISABLEINT(oneshot->tch, oneshot->chan); + INTEL64_TIM_SETISR(oneshot->tch, oneshot->chan, NULL, NULL, false); +#endif + + oneshot->running = false; } /* Save the new handler and its argument */ From 36ac0f257fb092dada4a07ede914a37a08dd08ad Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 09:05:14 +0200 Subject: [PATCH 03/12] sched/wqueue: restore -ENOENT from work_cancel() for unqueued work work_cancel() used to return -ENOENT when the work structure was not in the queue, and callers depend on that: aio_cancel() tears down the AIO container (file_put() + aioc_free()) only when work_cancel() reports success, because a work item that is not queued may already be executing on a worker thread (see the comment in fs/aio/aio_cancel.c). Since commit 6f72f5481d ("sched/wqueue: Refactor delayed and periodical workqueue") work_cancel() returns OK unconditionally, and commit d2e01b90553 ("sched/wqueue: harden custom queue lifecycle") kept that behaviour and dropped -ENOENT from the function documentation. Under SMP the LTP aio_cancel tests then free the aio container and its file while the lpwork thread is still executing aio_write_worker() on it, which ends in a page fault in file_write() (f_inode == NULL) and a panic. Return -ENOENT again when the work is not queued, and document it. For the synchronous variant "not queued" alone does not tell whether the callback is running: the worker scan does, so report OK when a running callback was found and waited for, and -ENOENT only when the work was neither queued nor running. Assisted-by: Claude Code Signed-off-by: raiden00pl --- sched/wqueue/kwork_cancel.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/sched/wqueue/kwork_cancel.c b/sched/wqueue/kwork_cancel.c index 2329abaf70824..99c7774f48549 100644 --- a/sched/wqueue/kwork_cancel.c +++ b/sched/wqueue/kwork_cancel.c @@ -48,6 +48,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { irqstate_t flags; pid_t self = sync ? nxsched_gettid() : INVALID_PROCESS_ID; + int ret = -ENOENT; if (wqueue == NULL || work == NULL) { @@ -83,8 +84,18 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { work_timer_reset(wqueue); } + + ret = OK; } + /* Otherwise the work is not queued: either it was never queued or a + * worker has already dequeued it and may be executing its callback + * right now. Only the scan below can tell. Report -ENOENT if the + * work is neither queued nor running, so that callers (e.g. + * aio_cancel()) do not free resources that the callback is still + * using. + */ + if (sync) { for (wndx = 0; wndx < wqueue->nthreads; wndx++) @@ -93,6 +104,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { worker[wndx].wait_count++; sync_wait = &worker[wndx].wait; + ret = OK; break; } } @@ -102,7 +114,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, if (sync_wait == NULL) { - return OK; + return ret; } nxsem_wait_uninterruptible(sync_wait); @@ -130,6 +142,8 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, * Zero on success, a negated errno on failure * * -EINVAL - An invalid work queue was specified + * -ENOENT - The work is not queued (and, for the sync variant, not + * running either) * ****************************************************************************/ From fabd1f79922ca40aff306d70b8ef3e8dccf7f7a1 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 09:28:38 +0200 Subject: [PATCH 04/12] arch/intel64: fix nxstyle issues in intel64_hpet.c fix nxstyle issues in intel64_hpet.c Assisted-by: Claude Code Signed-off-by: raiden00pl --- arch/x86_64/src/intel64/intel64_hpet.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/x86_64/src/intel64/intel64_hpet.c b/arch/x86_64/src/intel64/intel64_hpet.c index 83ff189b1eb80..4e993c278308a 100644 --- a/arch/x86_64/src/intel64/intel64_hpet.c +++ b/arch/x86_64/src/intel64/intel64_hpet.c @@ -223,6 +223,7 @@ static void intel64_hpet_cmpset(struct intel64_tim_dev_s *dev, uint8_t timer, uint64_t cmp) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); intel64_hpet_putreg(hpet, HPET_TCOMP_OFFSET(timer), cmp); } @@ -239,6 +240,7 @@ static uint64_t intel64_hpet_cmpget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + DEBUGASSERT(timer < hpet->timers); return intel64_hpet_getreg(hpet, HPET_TCOMP_OFFSET(timer)); } @@ -255,6 +257,7 @@ static uint64_t intel64_hpet_intget(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return (intel64_hpet_getreg(hpet, HPET_GISR_OFFSET) & HPET_GISR_TINT(timer)); } @@ -271,6 +274,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, uint8_t timer) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + intel64_hpet_putreg(hpet, HPET_GISR_OFFSET, HPET_GISR_TINT(timer)); } @@ -285,6 +289,7 @@ static void intel64_hpet_intack(struct intel64_tim_dev_s *dev, static uint64_t intel64_hpet_cntget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_getreg(hpet, HPET_MCNTR_OFFSET); } @@ -300,6 +305,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, uint64_t cntr) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return intel64_hpet_putreg(hpet, HPET_MCNTR_OFFSET, cntr); } @@ -314,6 +320,7 @@ static void intel64_hpet_cntset(struct intel64_tim_dev_s *dev, static uint32_t intel64_hpet_perget(struct intel64_tim_dev_s *dev) { struct intel64_hpet_s *hpet = (struct intel64_hpet_s *)dev; + return hpet->clk_per_fs; } From 4aa86191ddd2a82173a7919f3e12781853ed7acd Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 13:25:42 +0200 Subject: [PATCH 05/12] arch/intel64: keep the HPET ISR attached when the timer is stopped intel64_hpet_setisr() with a NULL handler detached the ISR with irq_attach(irq, NULL), which installs irq_unexpected_isr(). The oneshot driver does this every time the timer expires or is re-armed, so an HPET interrupt already in flight to another CPU lands on the unexpected ISR and panics the system: irq_unexpected_isr: ERROR irq: 34 seen under SMP with the LTP test suite. Just mask the interrupt and keep the ISR attached; intel64_oneshot_handler() already treats an interrupt that arrives while the timer is not running as spurious. Assisted-by: Claude Code Signed-off-by: raiden00pl --- arch/x86_64/src/intel64/intel64_hpet.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/x86_64/src/intel64/intel64_hpet.c b/arch/x86_64/src/intel64/intel64_hpet.c index 4e993c278308a..36512ac6c7175 100644 --- a/arch/x86_64/src/intel64/intel64_hpet.c +++ b/arch/x86_64/src/intel64/intel64_hpet.c @@ -390,9 +390,12 @@ static int intel64_hpet_setisr(struct intel64_tim_dev_s *dev, uint8_t timer, if (handler == NULL) { - /* Disable interrupt */ + /* Disable the interrupt but keep the ISR attached. Detaching it here + * installs irq_unexpected_isr(), and an HPET interrupt that is already + * in flight to another CPU then panics the system. A stray interrupt + * is handled as spurious by the oneshot ISR instead. + */ - irq_attach(irq, handler, arg); up_disable_irq(irq); } else From c606e36b236bcb57fc098f6bd2540045075f0c1b Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 09:28:52 +0200 Subject: [PATCH 06/12] libs/libc/aio: fix nxstyle issues in lio_listio.c fix nxstyle issues in lio_listio.c Assisted-by: Claude Code Signed-off-by: raiden00pl --- libs/libc/aio/lio_listio.c | 113 +++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 55 deletions(-) diff --git a/libs/libc/aio/lio_listio.c b/libs/libc/aio/lio_listio.c index 42b6f3cfa9c03..2bbc37f8c3d2a 100644 --- a/libs/libc/aio/lio_listio.c +++ b/libs/libc/aio/lio_listio.c @@ -233,6 +233,7 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, if (status != OK) { int errcode = get_errno(); + ferr("ERROR sigprocmask failed: %d\n", errcode); DEBUGASSERT(errcode > 0); return -errcode; @@ -547,59 +548,61 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, status = OK; switch (aiocbp->aio_lio_opcode) { - case LIO_NOP: - { - /* Mark the do-nothing operation complete */ - - aiocbp->aio_result = OK; - } - break; - - case LIO_READ: - case LIO_WRITE: - { - if (aiocbp->aio_lio_opcode == LIO_READ) - { - /* Submit the asynchronous read operation */ - - status = aio_read(aiocbp); - } - else - { - /* Submit the asynchronous write operation */ - - status = aio_write(aiocbp); - } - - if (status < 0) - { - /* Failed to queue the I/O. Set up the error return. */ - - errcode = get_errno(); - ferr("ERROR: aio_read/write failed: %d\n", errcode); - DEBUGASSERT(errcode > 0); - aiocbp->aio_result = -errcode; - ret = ERROR; - } - else - { - /* Increment the count of successfully queue operations */ - - nqueued++; - } - } - break; - - default: - { - /* Make the invalid operation complete with an error */ - - ferr("ERROR: Unrecognized opcode: %d\n", - aiocbp->aio_lio_opcode); - aiocbp->aio_result = -EINVAL; - ret = ERROR; - } - break; + case LIO_NOP: + { + /* Mark the do-nothing operation complete */ + + aiocbp->aio_result = OK; + } + break; + + case LIO_READ: + case LIO_WRITE: + { + if (aiocbp->aio_lio_opcode == LIO_READ) + { + /* Submit the asynchronous read operation */ + + status = aio_read(aiocbp); + } + else + { + /* Submit the asynchronous write operation */ + + status = aio_write(aiocbp); + } + + if (status < 0) + { + /* Failed to queue the I/O. Set up the error return. */ + + errcode = get_errno(); + ferr("ERROR: aio_read/write failed: %d\n", errcode); + DEBUGASSERT(errcode > 0); + aiocbp->aio_result = -errcode; + ret = ERROR; + } + else + { + /* Increment the count of successfully queued + * operations + */ + + nqueued++; + } + } + break; + + default: + { + /* Make the invalid operation complete with an error */ + + ferr("ERROR: Unrecognized opcode: %d\n", + aiocbp->aio_lio_opcode); + aiocbp->aio_result = -EINVAL; + ret = ERROR; + } + break; } } } @@ -674,8 +677,8 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, * and this is the first error to be reported. */ - retcode = -status; - ret = ERROR; + retcode = -status; + ret = ERROR; } } } From 587176c1eca7d6d48b5222ed316d4da28dbc6a60 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 14:31:16 +0200 Subject: [PATCH 07/12] libs/libc/aio: fix lio_listio() racing with early I/O completion lio_listio(LIO_NOWAIT) submits the requests first and only afterwards installs the SIGPOLL handler and attaches the per-request private data in lio_sigsetup(). Under SMP the requests complete on the low priority work queue while this is going on, and aio_signal() queues SIGPOLL for each of them as it finishes: - A signal delivered for a request that lio_sigsetup() found already finished (aio_priv left NULL) made lio_sighandler() dereference a NULL private pointer (LTP lio_listio 10-1, 15-1, 2-1 crash with a page fault in lio_sighandler on qemu-intel64 SMP). - A signal delivered between the aio_result check and the aio_priv store of the last outstanding request was consumed without private data, no later signal arrived, and the caller was never notified. - If every request had completed by the time the loop ran, no handler invocation could ever see the private data and the caller hung. - After the completing handler had notified the caller, signals still queued for other requests ran the handler again with their own copy of the private data. Block SIGPOLL while the handler is installed and the private data is attached, so that the completion signals are delivered only once every entry is set up. Ignore a signal for a request without private data, notify the caller directly from lio_sigsetup() when everything had already completed, and detach the private data of every entry once the list is done so that late signals find nothing to do. Assisted-by: Claude Code Signed-off-by: raiden00pl --- libs/libc/aio/lio_listio.c | 76 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/libs/libc/aio/lio_listio.c b/libs/libc/aio/lio_listio.c index 2bbc37f8c3d2a..f4612aec819be 100644 --- a/libs/libc/aio/lio_listio.c +++ b/libs/libc/aio/lio_listio.c @@ -157,7 +157,18 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) /* Recover our private data from the AIO control block */ sighand = (FAR struct lio_sighand_s *)aiocbp->aio_priv; - DEBUGASSERT(sighand && sighand->list); + if (sighand == NULL) + { + /* This I/O completed before lio_sigsetup() attached our private data + * to it (the completion raced with lio_listio() on another CPU). + * Nothing to do: an entry that still carries the private data will + * finish the job, or lio_sigsetup() already notified the caller. + */ + + return; + } + + DEBUGASSERT(sighand->list); aiocbp->aio_priv = NULL; /* Check if all of the pending I/O has completed */ @@ -165,6 +176,26 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) ret = lio_checkio(sighand->list, sighand->nent); if (ret != -EINPROGRESS) { + FAR struct aiocb *other; + int i; + + /* Detach the private data of every other entry. A SIGPOLL that was + * dispatched for one of them before we got here still carries this + * handler and will be delivered later, possibly after the caller has + * been notified and its list is gone; with aio_priv cleared such a + * late invocation finds nothing to do. + */ + + for (i = 0; i < sighand->nent; i++) + { + other = sighand->list[i]; + if (other != NULL && other->aio_priv != NULL) + { + lib_free(other->aio_priv); + other->aio_priv = NULL; + } + } + /* All pending I/O has completed */ /* Restore the signal handler */ @@ -211,10 +242,12 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, FAR struct sigevent *sig) { FAR struct aiocb *aiocbp; + FAR struct aiocb *first = NULL; struct lio_sighand_s sighand; sigset_t set; struct sigaction act; int status; + int nprivs = 0; int i; /* Initialize the allocated structure */ @@ -225,11 +258,15 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, sighand.nent = nent; sighand.pid = _SCHED_GETPID(); - /* Make sure that SIGPOLL is not blocked */ + /* Block SIGPOLL until every entry carries its private data. The I/O is + * already in flight and may complete on another CPU at any time; a + * completion signal delivered before the loop below has attached the + * private data would be lost to lio_sighandler(). + */ sigemptyset(&set); sigaddset(&set, SIGPOLL); - status = sigprocmask(SIG_UNBLOCK, &set, &sighand.oprocmask); + status = sigprocmask(SIG_BLOCK, &set, &sighand.oprocmask); if (status != OK) { int errcode = get_errno(); @@ -271,6 +308,11 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, { FAR void *priv = NULL; + if (first == NULL) + { + first = aiocbp; + } + /* Check if I/O is pending for this entry */ if (aiocbp->aio_result == -EINPROGRESS) @@ -284,12 +326,40 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, memcpy(priv, (FAR void *)&sighand, sizeof(struct lio_sighand_s)); + nprivs++; } aiocbp->aio_priv = priv; } } + if (nprivs == 0 && first != NULL) + { + /* Every I/O completed while the signal handler was being set up, so + * no lio_sighandler() invocation will see our private data. Finish + * the job here: restore the signal state and notify the caller. + */ + + sigaction(SIGPOLL, &sighand.oact, NULL); + sigprocmask(SIG_SETMASK, &sighand.oprocmask, NULL); + return nxsig_notification(sighand.pid, &sighand.sig, SI_ASYNCIO, + &first->aio_sigwork); + } + + /* Let the completion signals through. Any that arrived meanwhile are + * delivered right here. + */ + + status = sigprocmask(SIG_UNBLOCK, &set, NULL); + if (status != OK) + { + int errcode = get_errno(); + + ferr("ERROR sigprocmask failed: %d\n", errcode); + DEBUGASSERT(errcode > 0); + return -errcode; + } + return OK; } From b8b439542227e78bdfe6e5d48840ffa9e2eb4553 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 14:31:16 +0200 Subject: [PATCH 08/12] fs/aio: advance the pending list in aio_cancel() before work_cancel() The "cancel everything on this descriptor" loop in aio_cancel() only moved to the next container inside the branch taken when work_cancel() succeeded. When work_cancel() reports -ENOENT because a worker thread is already executing the request, the loop restarted its search from the same container and spun forever, hanging the task (and, since the scan runs with interrupts enabled but never yields, that CPU) without any output. This was hit by the LTP aio_cancel tests on SMP. Capture the next container before calling work_cancel() so the loop always makes progress. Assisted-by: Claude Code Signed-off-by: raiden00pl --- fs/aio/aio_cancel.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/aio/aio_cancel.c b/fs/aio/aio_cancel.c index ceb31dd4a4106..d715033afae48 100644 --- a/fs/aio/aio_cancel.c +++ b/fs/aio/aio_cancel.c @@ -180,6 +180,13 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp) if (aioc) { + /* Advance to the next container now: work_cancel() may report + * that this one is already executing (-ENOENT), and we must + * not examine the same container again in that case. + */ + + next = (FAR struct aio_container_s *)aioc->aioc_link.flink; + /* Yes... attempt to cancel the I/O. There are two * possibilities:* (1) the work has already been started and * is no longer queued, or (2) the work has not been started @@ -195,8 +202,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp) * transfers */ - next = - (FAR struct aio_container_s *)aioc->aioc_link.flink; pid = aioc->aioc_pid; aiocbp = aioc_decant(aioc); DEBUGASSERT(aiocbp); From 2a47cc2365463a7a10da568b6dbb0cf824a00b01 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 20:31:47 +0200 Subject: [PATCH 09/12] drivers/serial: sample xmit.head once per iteration in uart_xmitchars() uart_putxmitchar() advances xmit.head from thread context without holding the critical section, so on SMP the head index can move, and wrap around, while uart_xmitchars() runs in the TX interrupt on another CPU. Since commit b319c27f03e ("serial: Added APIs for receiving and sending multiple chars") the sendbuf path of uart_xmitchars() reads xmit.head twice: once to decide whether the pending data is contiguous and again to compute its length. If the producer wraps the index in between, the computed length goes negative, is passed to sendbuf() as a huge size_t and the driver transmits memory far beyond the ring buffer. The per-byte path reads the index only once and is not affected, which is why this went unnoticed: the batch path is only used by drivers that implement sendbuf, and the 16550 driver gained it in commit 45c38d8592b ("drivers/serial/16550: add polling mode support for serial drivers"). qemu-intel64 with SMP is the first configuration combining a sendbuf driver with a producer running on another CPU. On qemu-intel64 SMP this shows up as an endless stream of NUL bytes on the console (captured with gdb: head = 1, tail = 8, size = 16, and u16550_sendbuf() called with size = (size_t)-7), which makes the ntfc test harness fail every test that runs while the flood lasts. Read the head index once per loop iteration and use that snapshot for both the contiguity test and the length. The producer only ever moves the index forward, so a stale snapshot merely sends less now. Assisted-by: Claude Code Signed-off-by: raiden00pl --- drivers/serial/serial_io.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/serial/serial_io.c b/drivers/serial/serial_io.c index 0f8bc6b270869..22ee415901709 100644 --- a/drivers/serial/serial_io.c +++ b/drivers/serial/serial_io.c @@ -57,14 +57,22 @@ void uart_xmitchars(FAR uart_dev_t *dev) { uint16_t nbytes = 0; + sbuf_size_t head; #ifdef CONFIG_SMP irqstate_t flags = enter_critical_section(); #endif - /* Send while we still have data in the TX buffer & room in the fifo */ + /* Send while we still have data in the TX buffer & room in the fifo. + * + * uart_putxmitchar() advances xmit.head from thread context without + * holding the critical section, so on SMP it can move (and wrap) while + * we are in here. Sample it once per iteration: a stale value only + * makes us send less now, whereas reading it twice can turn the batch + * length negative and send from far beyond the buffer. + */ - while (dev->xmit.head != dev->xmit.tail && uart_txready(dev)) + while ((head = dev->xmit.head) != dev->xmit.tail && uart_txready(dev)) { /* Send the next byte */ @@ -72,9 +80,9 @@ void uart_xmitchars(FAR uart_dev_t *dev) { ssize_t sent; - if (dev->xmit.tail < dev->xmit.head) + if (dev->xmit.tail < head) { - sent = dev->xmit.head - dev->xmit.tail; + sent = head - dev->xmit.tail; } else { From 1f9860d533970cfb45261f46562baf603713a95a Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 09:40:31 +0200 Subject: [PATCH 10/12] drivers/serial: read the consumer index once in uart_recvchars() Same issue as the previous commit, on the receive side: uart_read() advances recv.tail from thread context without holding the critical section, but the recvbuf batch path of uart_recvchars() reads recv.tail several times (the full check, the watermark count and the free-space computation). If uart_read() moves and wraps the index in between, the computed free space goes negative and is passed to recvbuf() as a huge size_t, which lets the driver store past the end of the ring buffer. Read recv.tail once per loop iteration and derive everything from that snapshot. The consumer only ever moves the index forward, so a stale snapshot merely stores less now. Assisted-by: Claude Code Signed-off-by: raiden00pl --- drivers/serial/serial_io.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/serial/serial_io.c b/drivers/serial/serial_io.c index 22ee415901709..9eabbab24cb80 100644 --- a/drivers/serial/serial_io.c +++ b/drivers/serial/serial_io.c @@ -165,6 +165,7 @@ void uart_recvchars(FAR uart_dev_t *dev) int signo = 0; #endif uint16_t nbytes = 0; + sbuf_size_t tail; /* Loop putting characters into the receive buffer until there are no * further characters to available. @@ -173,22 +174,32 @@ void uart_recvchars(FAR uart_dev_t *dev) while (uart_rxavailable(dev)) { int nexthead = rxbuf->head + 1 < rxbuf->size ? rxbuf->head + 1 : 0; - bool is_full = (nexthead == rxbuf->tail); + bool is_full; FAR char *pbuf = NULL; char ch; + /* uart_read() advances recv.tail from thread context without holding + * the critical section, so on SMP it can move (and wrap) while we are + * in here. Sample it once per iteration and derive the free space + * from that snapshot: a stale value only makes us store less now, + * whereas reading it twice can turn the batch length negative. + */ + + tail = rxbuf->tail; + is_full = (nexthead == tail); + #ifdef CONFIG_SERIAL_IFLOWCONTROL_WATERMARKS unsigned int nbuffered; /* How many bytes are buffered */ - if (rxbuf->head >= rxbuf->tail) + if (rxbuf->head >= tail) { - nbuffered = rxbuf->head - rxbuf->tail; + nbuffered = rxbuf->head - tail; } else { - nbuffered = rxbuf->size - rxbuf->tail + rxbuf->head; + nbuffered = rxbuf->size - tail + rxbuf->head; } /* Is the level now above the watermark level that we need to report? */ @@ -231,11 +242,11 @@ void uart_recvchars(FAR uart_dev_t *dev) if (!is_full) { - if (rxbuf->tail > rxbuf->head) + if (tail > rxbuf->head) { - nbytes = rxbuf->tail - rxbuf->head - 1; + nbytes = tail - rxbuf->head - 1; } - else if (rxbuf->tail) + else if (tail) { nbytes = rxbuf->size - rxbuf->head; } From 04992c64022bd7ce9050a81ee2dfe4a797bcec5c Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 10:23:27 +0200 Subject: [PATCH 11/12] arch/intel64: don't clear the oneshot handler from the HPET ISR intel64_oneshot_handler() cleared oneshot->handler and oneshot->arg after picking them up, without holding g_oneshot_spin, while intel64_oneshot_start() re-arms the timer under that lock from another CPU. Now that the HPET ISR stays attached across a re-arm, a stale interrupt can interleave with start(): it reads the freshly installed handler, clears it, and start() then sets running = true again, so the genuine expiry that follows finds running == true with a NULL handler and jumps to address zero from interrupt context (page fault at RIP 0 in the CPU0 IDLE task while the LTP lio_listio tests were running), or the alarm is simply lost and the tickless system stops. The handler and its argument are owned by start() and cancel(); the ISR only needs to read them. Leave them alone in the ISR and skip the call if none is installed. The remaining effect of a stale interrupt is an early invocation of the alarm callback, which is harmless: the tickless scheduler re-evaluates its expirations and re-arms the timer. Assisted-by: Claude Code Signed-off-by: raiden00pl --- arch/x86_64/src/intel64/intel64_oneshot.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/arch/x86_64/src/intel64/intel64_oneshot.c b/arch/x86_64/src/intel64/intel64_oneshot.c index eb907eaed17ee..27168a329f88b 100644 --- a/arch/x86_64/src/intel64/intel64_oneshot.c +++ b/arch/x86_64/src/intel64/intel64_oneshot.c @@ -100,18 +100,21 @@ static int intel64_oneshot_handler(int irg_num, void * context, void *arg) INTEL64_TIM_ACKINT(oneshot->tch, oneshot->chan); #endif - /* The timer is no longer running */ + /* The timer is no longer running. Only pick up the handler here; + * it is owned by intel64_oneshot_start()/cancel(), which may be + * re-arming the timer on another CPU right now. Clearing it from + * the ISR could leave a re-armed timer without a handler, and the + * next expiry would then jump through a NULL pointer. + */ oneshot->running = false; - - /* Forward the event, clearing out any vestiges */ - oneshot_handler = (oneshot_handler_t)oneshot->handler; - oneshot->handler = NULL; oneshot_arg = (void *)oneshot->arg; - oneshot->arg = NULL; - oneshot_handler(oneshot_arg); + if (oneshot_handler != NULL) + { + oneshot_handler(oneshot_arg); + } } else { From c6fe98dd2f8939cd5fc0c166d2aacb4ccb0fc5bf Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 14:56:32 +0200 Subject: [PATCH 12/12] fs/aio: decant the container only after the I/O has been performed The aio worker functions started by decanting the container, which frees the container back to the pool and drops the file reference, and only then performed the I/O through the aiocb. Nothing was holding the file open while the read/write/fsync was in progress, and the request was no longer on the pending list, so a concurrent close() or aio_cancel() could pull the file from under the running operation. Keep the container until the operation has completed and decant it just before signalling completion. Assisted-by: Claude Code Signed-off-by: raiden00pl --- fs/aio/aio_fsync.c | 8 +++++++- fs/aio/aio_read.c | 8 +++++++- fs/aio/aio_write.c | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c index cc4ad40dbbbe7..5484dfa9ed5cf 100644 --- a/fs/aio/aio_fsync.c +++ b/fs/aio/aio_fsync.c @@ -79,7 +79,12 @@ static void aio_fsync_worker(FAR void *arg) #ifdef CONFIG_PRIORITY_INHERITANCE prio = aioc->aioc_prio; #endif - aiocbp = aioc_decant(aioc); + aiocbp = aioc->aioc_aiocbp; + + /* Perform the I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the fsync using aioc_filep */ @@ -96,6 +101,7 @@ static void aio_fsync_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c index b3e366215e24b..8c2dca953af14 100644 --- a/fs/aio/aio_read.c +++ b/fs/aio/aio_read.c @@ -79,7 +79,12 @@ static void aio_read_worker(FAR void *arg) #ifdef CONFIG_PRIORITY_INHERITANCE prio = aioc->aioc_prio; #endif - aiocbp = aioc_decant(aioc); + aiocbp = aioc->aioc_aiocbp; + + /* Perform the I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the file read using: * @@ -105,6 +110,7 @@ static void aio_read_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c index 0ba11dc357ea9..cefd3bc9abe2d 100644 --- a/fs/aio/aio_write.c +++ b/fs/aio/aio_write.c @@ -82,7 +82,12 @@ static void aio_write_worker(FAR void *arg) #ifdef CONFIG_PRIORITY_INHERITANCE prio = aioc->aioc_prio; #endif - aiocbp = aioc_decant(aioc); + aiocbp = aioc->aioc_aiocbp; + + /* Perform the I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Call fcntl(F_GETFL) to get the file open mode. */ @@ -133,6 +138,7 @@ static void aio_write_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE