Skip to content

Fix qos-test and updated CI pipeline #50

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
ho28 merged 3 commits into from
Jun 25, 2026
Merged

Fix qos-test and updated CI pipeline #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
430d8f9
hw/arm/boot: skip fdt_pack in qtest mode
hungmtruong Jun 24, 2026
3c5d009
tests/acpi: regenerate expected HEST binary
hungmtruong Jun 24, 2026
0575c7d
ci: disable vhost and simplify workflow
hungmtruong Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 48 additions & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Comment thread
hungmtruong marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# ci.yml, WR QEMU github CI configuration file.
#
# Copyright (c) 2024 Wind River Systems, Inc.
# Copyright (c) 2026 Wind River Systems, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 2 as
Expand All @@ -18,6 +18,12 @@
name: WR QEMU CI

on:
workflow_dispatch:
inputs:
clean_build:
description: 'Clear ccache for a fully clean build'
type: boolean
default: false
push:
branches:
- wr-integration
Expand All @@ -35,18 +41,28 @@ jobs:
repository: Wind-River/qemu
path: qemu
# Use a specific branch or tag if needed
ref: wr-integration
# ref: wr-integration

- name: Cache apt packages
uses: actions/cache@v4
with:
path: ~/apt-cache
key: apt-${{ runner.os }}-${{ hashFiles('qemu/.github/workflows/ci.yml') }}
restore-keys: |
apt-${{ runner.os }}-

- name: Set up dependencies
run: |
sudo mkdir -p ~/apt-cache
sudo apt-get update
sudo apt-get install -y \
sudo apt-get install -y -o Dir::Cache::Archives="$HOME/apt-cache" \
acpica-tools \
autoconf \
automake \
bison \
bzip2 \
build-essential \
ccache \
cmake \
curl \
flex \
Expand Down Expand Up @@ -74,17 +90,44 @@ jobs:
vsftpd \
wget \
zlib1g-dev

- name: Clear ccache (if requested)
if: ${{ github.event.inputs.clean_build == 'true' }}
run: ccache --clear

- name: Cache ccache
uses: actions/cache@v4
with:
path: ~/.cache/ccache
key: ccache-${{ runner.os }}-${{ github.sha }}
restore-keys: |
ccache-${{ runner.os }}-

- name: Configure ccache
run: |
ccache --set-config=max_size=1G
ccache --zero-stats

- name: Build QEMU
working-directory: qemu
run: |
./configure --target-list="aarch64-softmmu,x86_64-softmmu" --enable-fdt --enable-plugins --enable-slirp --enable-capstone
make -j $(cat /proc/cpuinfo | grep "^processor" | wc -l)
export PATH="/usr/lib/ccache:$PATH"
Comment thread
hungmtruong marked this conversation as resolved.
./configure --target-list="aarch64-softmmu,x86_64-softmmu" --enable-fdt --enable-plugins --enable-slirp --enable-capstone --disable-vhost-net --disable-vhost-user
make -j$(nproc)
make plugins
make check-build

- name: Print ccache stats
run: ccache --show-stats

- name: Run QEMU tests
working-directory: qemu
run: |
V=1 make check

- name: Upload test logs on failure
if: failure()
uses: actions/upload-artifact@v4
with:
name: meson-test-logs
path: qemu/build/meson-logs/testlog.txt
17 changes: 13 additions & 4 deletions hw/arm/boot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "qemu/units.h"
#include "system/qtest.h"
#include "qemu/bswap.h"

/* Kernel boot protocol is specified in the kernel docs
Expand Down Expand Up @@ -640,10 +641,18 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
* By default QEMU generates a 1 MiB FDT.
* Let's pack it to save some room.
*/
rc = fdt_pack(fdt);
/* Should only fail if we've built a corrupted tree */
g_assert(rc == 0);
size = fdt_totalsize(fdt);
/*
* Skip in qtest mode — fdt_pack causes the aarch64 qos-test
* to hang for reasons that are not fully understood. Since no
* kernel boots in qtest, fdt_pack is unnecessary and skipping
* it allows the tests to pass.
*/
if (!qtest_enabled()) {
rc = fdt_pack(fdt);
/* Should only fail if we've built a corrupted tree */

@ho28 ho28 Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the detailed commit message - but what exactly does "it triggers a BQL deadlock
in the virtio-scsi iothread dataplane stop path" mean, and how is it related to the size of the FDT?

I know you probably don't have the answer right now. If you want to look into and find out, then can you update the commit message and this comment with that info? Otherwise, just drop the specifics and mention that this causes qtest issues for reasons that are unclear to us.

@hungmtruong hungmtruong Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Used GDB to look into this, and this is what I have found:

The deadlock is between two threads during qos-test:

  • main thread: holds BQL, calls virtio_scsi_dataplane_stop(), waits for the iothread
  • iothread: completing a SCSI DMA unmap to address 0, needs BQL

Regarding the size of the FDT, I am not sure.

@ho28 ho28 Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ok - I ask because the fix appears to be to not pack the DTB for the qtest. I'm fine with that fix. But its not clear to me how the size of the DTB affects synchronization around the Big Qemu Lock (BQL). i'm pushing on it because conceptually the DTB and the virtio scsi data path have nothing to do with each other. if theres a race condition somewhere, it would be nice to know the exact steps that race and how we end up in the deadlock vs the normal functioning path. But like i said before, it's not necessary to find this answer for us to ship this fix.

@hungmtruong hungmtruong Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I have fixed the commit message.

g_assert(rc == 0);
size = fdt_totalsize(fdt);
}


/* Put the DTB into the memory map as a ROM image: this will ensure
Expand Down
Binary file modified tests/data/acpi/aarch64/virt/HEST
View file
Open in desktop
Comment thread
hungmtruong marked this conversation as resolved.
Binary file not shown.
Loading