On 9/17/2026 2:58 AM, Chao Liu wrote:
On Wed, Sep 16, 2026 at 02:10:38PM +0800, Daniel Henrique Barboza wrote:
Hi Michael!
On 9/16/2026 3:35 AM, Michael Tokarev wrote:
On 8/28/26 23:39, Daniel Henrique Barboza wrote:
Running 'dtc' to generate a readable fdt for the virt machine produce
warnings:
$ dtc -I dtb -O dts virt.dtb > virt.dts
<stdout>: Warning (simple_bus_reg): /soc/mtimer@2000000: simple-bus unit address format
error, expected "2007ff8"
<stdout>: Warning (simple_bus_reg): /soc/mtimer@2008000: simple-bus unit address format
error, expected "200fff8"
This happens because the 'reg' field does not match the address in the
nodename. The right fix, pointed out by Chao in [1], is to make the
nodename match the reg value:
"In the current QEMU implement, `addr` is the MTIMECMP base address,
while the MTIME register is located at addr + RISCV_ACLINT_DEFAULT_MTIME."
In other words, the 'reg' value is correct but the nodename isn't.
We'll make the fix now, allowing the next patch to move the fixed version
of aclint FDT to fdt-common.
[1] https://lore.kernel.org/qemu-devel/[email protected]/
Cc: [email protected]
Suggested-by: Chao Liu <[email protected]>
Fixes: 954886ea6d ("hw/riscv: virt: Add optional ACLINT support to virt
machine")
Signed-off-by: Daniel Henrique Barboza <[email protected]>
---
hw/riscv/virt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
This patch is Cc'd to qemu-stable, but there's an interesting rebase twist here,
it looks like. This is commit v11.1.0-1035-g9d6ac47d318 in the master branch.
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a6ec9def16..83a4b7710d 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -234,7 +234,8 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
(s->memmap[VIRT_CLINT].size * socket);
size = s->memmap[VIRT_CLINT].size - RISCV_ACLINT_SWI_SIZE;
}
- name = g_strdup_printf("/soc/mtimer@%lx", addr);
+ name = g_strdup_printf("/soc/mtimer@%"HWADDR_PRIx,
+ addr + RISCV_ACLINT_DEFAULT_MTIME);
Before this commit, `add` variable is declared in this function as
unsigned long. So this particular change is wrong, because now the
format string does not match its argument:
../hw/riscv/virt.c: In function 'create_fdt_socket_aclint':
../hw/riscv/virt.c:408:28: error: format '%llx' expects argument of type 'long
long unsigned int', but argument 2 has type 'long unsigned int'
[-Werror=format=]
408 | name = g_strdup_printf("/soc/mtimer@%"HWADDR_PRIx,
| ^~~~~~~~~~~~~~~
409 | addr + RISCV_ACLINT_DEFAULT_MTIME);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
In file included from ../include/qemu/osdep.h:118,
from ../hw/riscv/virt.c:21:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/inttypes.h:38:19: note: format
string is defined here
38 | #define PRIx64 "llx"
However, the very next commit, v11.1.0-1036-ge6ea6bec5a3
"hw/riscv/fdt-common, virt.c: add riscv_create_fdt_socket_aclint()",
moves this whole function to another file, *and* changes the type of
`addr` variable to hwaddr. So now, everything fits.
But this single patch alone does not work.
Note also there's no mentions of this type change anywhere.
So, what was the actual issue ere?
I'm dropping this patch from the stable series for now, it is definitely
wrong.
It is ok to drop this. This is a FDT issue that has been around for several
years at this point and no one noticed.
In case you want to port it anyway the right fix would be this:
name = g_strdup_printf("/soc/mtimer@%lx", addr + RISCV_ACLINT_DEFAULT_MTIME);
This would fix the FDT nodename without messing with the format string.
It is also interesting that only win64 build of qemu detected this, all
other went just fine.
Yep, this went unnoticed on my end since it didn't break on Linux during my
testing ...
I also run into build failures somtimes. Just share my idea, QEMU's gitlab CI
is very useful! But be careful when choosing the base commit or branch.
I usually run Gitlab CI before submitting big series like this. The thing here
is
that we would need to do a full Gitlab CI run for every single patch to catch
something like this - it was a particular Windows runner that complained about
it,
and just with this patch standalone (applying patch 9 fixes the issue for it).
The silver lining here is that it's only a single Windows runner, so hopefully
the
overall impact when bisecting code is contained.
Thanks,
Daniel
If you just use the latest master branch, CI may fail..
Thanks,
Chao
Cheers,
Daniel
Thanks,
/mjt
