[Bug 1905297] Moved bug report
This is an automated cleanup. This bug report has been moved to QEMU's new bug tracker on gitlab.com and thus gets marked as 'expired' now. Please continue with the discussion here: https://gitlab.com/qemu-project/qemu/-/issues/468 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1905297 Title: Zynq7000 UART clock reset initialization Status in QEMU: Expired Bug description: Hello, we have come across a strange behavior in the Zynq7000 model of Qemu that seems to have been introduced between 5.0.0 and 5.1.0. The reset values of the SLCR register, in particular those for UART_CLK_CTRL, are such that the UARTs should find functional clocks. Up to 5.0.0 this was also the behavior that was implemented in QEMU. Starting in 5.1.0, we found that - despite correct reset values [1] - the UARTs are non-functional upon reset. Some investigation revealed that the cause for that is that the corresponding clocks are not properly initialized. Between 5.0.0 and 5.1.0, there are three commits that touch the Zynq UART clocks [2]. The last of them [3] triggers the faulty behavior. Attached is a patch that applies 5.2.0-rc2 and yields a functional UART. We surmise that the underlying device release issue runs much deeper, so it is only meant to identify the issue. [1] hw/misc/zynq_slcr.c static void zynq_slcr_reset_init(Object *obj, ResetType type) s->regs[R_UART_CLK_CTRL] = 0x3F03; [2] 38867cb7ec90..5b49a34c6800 [3] commit 5b49a34c6800d0cb917f959d8e75e5775f0fac3f (refs/bisect/bad) Author: Damien Hedde Date: Mon Apr 6 15:52:50 2020 +0200 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1905297/+subscriptions
[Bug 1905297] Re: Zynq7000 UART clock reset initialization
I guess the patch has never been sent to the qemu-devel mailing list and thus was never considered for inclusion. Anyway, let's move this ticket over to the new bug tracker at gitlab.com, maybe it gets more attention there... ** Changed in: qemu Status: Incomplete => Expired ** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #468 https://gitlab.com/qemu-project/qemu/-/issues/468 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1905297 Title: Zynq7000 UART clock reset initialization Status in QEMU: Expired Bug description: Hello, we have come across a strange behavior in the Zynq7000 model of Qemu that seems to have been introduced between 5.0.0 and 5.1.0. The reset values of the SLCR register, in particular those for UART_CLK_CTRL, are such that the UARTs should find functional clocks. Up to 5.0.0 this was also the behavior that was implemented in QEMU. Starting in 5.1.0, we found that - despite correct reset values [1] - the UARTs are non-functional upon reset. Some investigation revealed that the cause for that is that the corresponding clocks are not properly initialized. Between 5.0.0 and 5.1.0, there are three commits that touch the Zynq UART clocks [2]. The last of them [3] triggers the faulty behavior. Attached is a patch that applies 5.2.0-rc2 and yields a functional UART. We surmise that the underlying device release issue runs much deeper, so it is only meant to identify the issue. [1] hw/misc/zynq_slcr.c static void zynq_slcr_reset_init(Object *obj, ResetType type) s->regs[R_UART_CLK_CTRL] = 0x3F03; [2] 38867cb7ec90..5b49a34c6800 [3] commit 5b49a34c6800d0cb917f959d8e75e5775f0fac3f (refs/bisect/bad) Author: Damien Hedde Date: Mon Apr 6 15:52:50 2020 +0200 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1905297/+subscriptions
[Bug 1779955] Re: qemu linux-user requires read permissions on memory passed to syscalls that should only need write access
[Expired for QEMU because there has been no activity for 60 days.]
** Changed in: qemu
Status: Incomplete => Expired
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1779955
Title:
qemu linux-user requires read permissions on memory passed to syscalls
that should only need write access
Status in QEMU:
Expired
Bug description:
When read() function takes an mmap'ed address as output buffer, it
returns EFAULT. The expected behavior is it should just work.
The following code works for qemu-system-arm, but not for qemu-arm-
static.
QEMU version affected: latest release 2.12.0.
Steps to reproduce (please substitute /path/to/qemu-arm-static with
the path of the binary, and /tmp/a.cpp with the example source code
attached):
# First register binfmt_misc
[hidden]$ docker run --rm --privileged multiarch/qemu-user-static:register
--reset
# Compile the code and run
[hidden]$ docker run --rm -it -v /tmp/a.cpp:/tmp/a.cpp -v
/path/to/qemu-arm-static:/usr/bin/qemu-arm-static arm32v7/ubuntu:18.04 bash -c
'{ apt update -y && apt install -y g++; } >& /dev/null && g++ -std=c++14
/tmp/a.cpp -o /tmp/a.out && echo hehe > /tmp/haha.txt && /tmp/a.out'
ofd=3
ftruncate=0
mmap=0xff3f5000
fd=4
0xff3f5023 -1 14
The expected result in qemu-system-arm as well as natively on x86_64 host:
hidden$ ./a.out
ofd=3
ftruncate=0
mmap=0xb6fb7000
fd=4
0xb6fb7023 5 0
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1779955/+subscriptions
[Bug 1785734] Re: movdqu partial write at page boundary
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1785734 Title: movdqu partial write at page boundary Status in QEMU: Expired Bug description: In TCG mode, when a 16-byte write instruction (such as movdqu) is executed at a page boundary and causes a page fault, a partial write is executed in the first page. See the attached code for an example. Tested on the qemu-3.0.0-rc1 release. % gcc -m32 qemu-bug2.c && ./a.out && echo && qemu-i386 ./a.out [snip] page fault: addr=0x70001000 err=0x7 *(0x7ff8+ 0) = aa *(0x7ff8+ 1) = aa *(0x7ff8+ 2) = aa *(0x7ff8+ 3) = aa *(0x7ff8+ 4) = aa *(0x7ff8+ 5) = aa *(0x7ff8+ 6) = aa *(0x7ff8+ 7) = aa *(0x7ff8+ 8) = 55 *(0x7ff8+ 9) = 55 *(0x7ff8+10) = 55 *(0x7ff8+11) = 55 *(0x7ff8+12) = 55 *(0x7ff8+13) = 55 *(0x7ff8+14) = 55 *(0x7ff8+15) = 55 [snip] page fault: addr=0x70001000 err=0x6 *(0x7ff8+ 0) = 77 *(0x7ff8+ 1) = 66 *(0x7ff8+ 2) = 55 *(0x7ff8+ 3) = 44 *(0x7ff8+ 4) = 33 *(0x7ff8+ 5) = 22 *(0x7ff8+ 6) = 11 *(0x7ff8+ 7) = 0 *(0x7ff8+ 8) = 55 *(0x7ff8+ 9) = 55 *(0x7ff8+10) = 55 *(0x7ff8+11) = 55 *(0x7ff8+12) = 55 *(0x7ff8+13) = 55 *(0x7ff8+14) = 55 *(0x7ff8+15) = 55 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1785734/+subscriptions
[Bug 1862874] Re: java may stuck for a long time in system mode with "-cpu max"
[Expired for QEMU because there has been no activity for 60 days.]
** Changed in: qemu
Status: Incomplete => Expired
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1862874
Title:
java may stuck for a long time in system mode with "-cpu max"
Status in QEMU:
Expired
Bug description:
Bug Description:
Run "java -version" in guest VM, java may stuck for a long time (several
hours) and then recover.
Steps to reproduce:
1. Launch VM by attached simple script: launch.sh
2. Execute "java -version" and then print "date" in a loop
while :
do
/home/bot/jdk/bin/java -version
date
done
3. A long time gap will be observed: may > 24 hours.
Technical details:
* host: x86_64 Linux 4.15.0-70-generic
* qemu v4.2.0
* java: tried two versions: openjdk-11-jre-headless or compiled java-13
* command-line: (See details in launch.sh)
/home/bot/qemu/qemu-build/qemu-4.2.0/binaries/bin/qemu-system-x86_64 \
-drive "file=${img},format=qcow2" \
-drive "file=${user_data},format=raw" \
-cpu max \
-m 24G \
-serial mon:stdio \
-smp 8 \
-nographic \
;
* Observed by java core dump generated by "kill -SIGSEGV" when java stucked:
Different pthreads are blocked on their own condition variables:
Id Target Id Frame
1Thread 0x7f48a041a080 (LWP 22470) __GI_raise (sig=sig@entry=6)
at ../sysdeps/unix/sysv/linux/raise.c:51
2Thread 0x7f487197d700 (LWP 22473) 0x7f489f5c49f3 in
futex_wait_cancelable (private=, expected=0,
futex_word=0x7f48980197c0)
at ../sysdeps/unix/sysv/linux/futex-internal.h:88
3Thread 0x7f4861b89700 (LWP 22483) 0x7f489f5c4ed9 in
futex_reltimed_wait_cancelable (private=,
reltime=0x7f4861b88960, expected=0,
futex_word=0x7f489801b084)
at ../sysdeps/unix/sysv/linux/futex-internal.h:142
4Thread 0x7f4861e8c700 (LWP 22480) 0x7f489f5c76d6 in
futex_abstimed_wait_cancelable (private=0, abstime=0x0, expected=0,
futex_word=0x7f48980107c0)
at ../sysdeps/unix/sysv/linux/futex-internal.h:205
5Thread 0x7f4861c8a700 (LWP 22482) 0x7f489f5c4ed9 in
futex_reltimed_wait_cancelable (private=,
reltime=0x7f4861c89800, expected=0,
futex_word=0x7f489801ed44)
at ../sysdeps/unix/sysv/linux/futex-internal.h:142
6Thread 0x7f48a0418700 (LWP 22471) 0x7f4880b13200 in ?? ()
7Thread 0x7f48703ea700 (LWP 22478) 0x7f489f5c49f3 in
futex_wait_cancelable (private=, expected=0,
futex_word=0x7f489801dfc0)
at ../sysdeps/unix/sysv/linux/futex-internal.h:88
8Thread 0x7f48702e9700 (LWP 22479) 0x7f489f5c49f3 in
futex_wait_cancelable (private=, expected=0,
futex_word=0x7f489838cd84)
at ../sysdeps/unix/sysv/linux/futex-internal.h:88
9Thread 0x7f4870f71700 (LWP 22475) 0x7f489f5c49f3 in
futex_wait_cancelable (private=, expected=0,
futex_word=0x7f489801a300)
at ../sysdeps/unix/sysv/linux/futex-internal.h:88
10 Thread 0x7f487187b700 (LWP 22474) 0x7f489f5c76d6 in
futex_abstimed_wait_cancelable (private=0, abstime=0x0, expected=0,
futex_word=0x7f48980cf770)
at ../sysdeps/unix/sysv/linux/futex-internal.h:205
11 Thread 0x7f4871a7f700 (LWP 22472) 0x7f489f5c76d6 in
futex_abstimed_wait_cancelable (private=0, abstime=0x0, expected=0,
futex_word=0x7f489809ba30)
at ../sysdeps/unix/sysv/linux/futex-internal.h:205
12 Thread 0x7f4861d8b700 (LWP 22481) 0x7f489f5c4ed9 in
futex_reltimed_wait_cancelable (private=,
reltime=0x7f4861d8a680, expected=0,
futex_word=0x7f489801ed44)
at ../sysdeps/unix/sysv/linux/futex-internal.h:142
13 Thread 0x7f48704ec700 (LWP 22477) 0x7f489f5c4ed9 in
futex_reltimed_wait_cancelable (private=,
reltime=0x7f48704eb910, expected=0,
futex_word=0x7f489801d120)
at ../sysdeps/unix/sysv/linux/futex-internal.h:142
14 Thread 0x7f4870e6f700 (LWP 22476) 0x7f489f5c4ed9 in
futex_reltimed_wait_cancelable (private=,
reltime=0x7f4870e6eb20, expected=0,
futex_word=0x7f489828abd0)
at ../sysdeps/unix/sysv/linux/futex-internal.h:142
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1862874/+subscriptions
[Bug 1870331] Re: default nic device created even though supplied by configfile
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1870331 Title: default nic device created even though supplied by configfile Status in QEMU: Expired Bug description: QEMU emulator version 4.1.94 Documentation states that qemu will create a default nic as long as not explicitly forbidden (-nic none) or defined ( e.g. -nic ). Observation: Even though qemu-system-ppc is started with "-readconfig" (which defines a nic), another nic gets created. When additionally suppling "-nic none", only the nic of the config file is created. As matter of fact, if all items that are defined in config file are supplied as command line arguments, no further nic is created. Expectation: When a nic is defined in config file, the default guest-nic should not get created. That would match behavior when all items, defined in config file are supplied as command line arguments. Attached config file. (qemu) info pci Bus 0, device 0, function 0: Host bridge: PCI device 1057:0002 PCI subsystem 1af4:1100 id "" Bus 0, device 1, function 0: VGA controller: PCI device 1234: PCI subsystem 1af4:1100 BAR0: 32 bit prefetchable memory at 0x8000 [0x80ff]. BAR2: 32 bit memory at 0x8100 [0x81000fff]. BAR6: 32 bit memory at 0x [0xfffe]. id "" Bus 0, device 2, function 0: Ethernet controller: PCI device 10ec:8029 PCI subsystem 1af4:1100 IRQ 23. BAR0: I/O at 0x1000 [0x10ff]. BAR6: 32 bit memory at 0x [0x0007fffe]. id "" Bus 0, device 3, function 0: Ethernet controller: PCI device 10ec:8029 PCI subsystem 1af4:1100 IRQ 24. BAR0: I/O at 0x1100 [0x11ff]. BAR6: 32 bit memory at 0x [0x0007fffe]. id "" To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1870331/+subscriptions
[Bug 1897481] Re: qemu crashes with VGA pass-through, e-GPU, nvidia 1060
[Expired for QEMU because there has been no activity for 60 days.]
** Changed in: qemu
Status: Incomplete => Expired
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1897481
Title:
qemu crashes with VGA pass-through, e-GPU, nvidia 1060
Status in QEMU:
Expired
Bug description:
I try to pass-through nvidia 1060 6gb card, which is connected via
ExpressCard (EXP-GDC converter).
I can successfully run my virtual machine without pass-through, but
when I try to add the devices, qemu crashes.
The coredump contains:
Stack trace of thread 3289311:
#0 0x00614c49 memory_region_update_container_subregions
(qemu-system-x86_64 + 0x214c49)
#1 0x005c0e8c vfio_probe_nvidia_bar0_quirk (qemu-system-x86_64 +
0x1c0e8c)
#2 0x005bcec0 vfio_realize (qemu-system-x86_64 + 0x1bcec0)
#3 0x0079b423 pci_qdev_realize (qemu-system-x86_64 + 0x39b423)
#4 0x006facda device_set_realized (qemu-system-x86_64 + 0x2facda)
#5 0x00887e57 property_set_bool (qemu-system-x86_64 + 0x487e57)
#6 0x0088ac48 object_property_set (qemu-system-x86_64 + 0x48ac48)
#7 0x0088d1d2 object_property_set_qobject (qemu-system-x86_64 +
0x48d1d2)
#8 0x0088b1f7 object_property_set_bool (qemu-system-x86_64 +
0x48b1f7)
#9 0x00693785 qdev_device_add (qemu-system-x86_64 + 0x293785)
#10 0x0061aad0 device_init_func (qemu-system-x86_64 + 0x21aad0)
#11 0x0098c87b qemu_opts_foreach (qemu-system-x86_64 + 0x58c87b)
#12 0x006211cb qemu_init (qemu-system-x86_64 + 0x2211cb)
#13 0x005002aa main (qemu-system-x86_64 + 0x1002aa)
#14 0x7fce8af21152 __libc_start_main (libc.so.6 + 0x28152)
#15 0x0050087e _start (qemu-system-x86_64 + 0x10087e)
The whole running command is pretty long, since I use libvirt to
manage my machines:
LC_ALL=C \
PATH=/usr/local/sbin:/usr/local/bin:/usr/bin \
HOME=/var/lib/libvirt/qemu/domain-2-Win10 \
XDG_DATA_HOME=/var/lib/libvirt/qemu/domain-2-Win10/.local/share \
XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain-2-Win10/.cache \
XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain-2-Win10/.config \
QEMU_AUDIO_DRV=spice \
/usr/bin/qemu-system-x86_64 \
-name guest=Win10,debug-threads=on \
-S \
-blockdev
'{"driver":"file","filename":"/usr/share/edk2-ovmf/x64/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/Win10_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
\
-machine
pc-q35-5.1,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
\
-cpu host,migratable=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff \
-m 8192 \
-overcommit mem-lock=off \
-smp 2,sockets=2,cores=1,threads=1 \
-uuid 7043c77b-4903-4527-8089-9679d9a17fee \
-no-user-config \
-nodefaults \
-chardev stdio,mux=on,id=charmonitor \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=localtime,driftfix=slew \
-global kvm-pit.lost_tick_policy=delay \
-no-hpet \
-no-shutdown \
-global ICH9-LPC.disable_s3=1 \
-global ICH9-LPC.disable_s4=1 \
-boot strict=on \
-device
pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
\
-device pcie-root-port,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1 \
-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2 \
-device pcie-root-port,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3 \
-device pcie-root-port,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4 \
-device pcie-root-port,port=0x15,chassis=6,id=pci.6,bus=pcie.0,addr=0x2.0x5 \
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 \
-blockdev '{"driver":"file","filename":"/home/sergiy/VirtualBox
VMs/win4games.img","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}'
\
-device ide-hd,bus=ide.0,drive=libvirt-2-format,id=sata0-0-0,bootindex=1 \
-blockdev
'{"driver":"file","filename":"/home/sergiy/Downloads/Win10_2004_Ukrainian_x64.iso","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-1-format","read-only":true,"driver":"raw","file":"libvirt-1-storage"}'
\
-device ide-cd,bus=ide.1,drive=libvirt-1-format,id=sata0-0-1 \
-chardev pty,id=charserial0 \
-device
[Bug 1869006] Re: PCIe cards passthrough to TCG guest works on 2GB of guest memory but fails on 4GB (vfio_dma_map invalid arg)
[Expired for QEMU because there has been no activity for 60 days.]
** Changed in: qemu
Status: Incomplete => Expired
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1869006
Title:
PCIe cards passthrough to TCG guest works on 2GB of guest memory but
fails on 4GB (vfio_dma_map invalid arg)
Status in QEMU:
Expired
Bug description:
During one meeting coworker asked "did someone tried to passthrough
PCIe card to other arch guest?" and I decided to check it.
Plugged SATA and USB3 controllers into spare slots on mainboard and
started playing. On 1GB VM instance it worked (both cold- and hot-
plugged). On 4GB one it did not:
Błąd podczas uruchamiania domeny: internal error: process exited while
connecting to monitor: 2020-03-25T13:43:39.107524Z qemu-system-aarch64: -device
vfio-pci,host=:29:00.0,id=hostdev0,bus=pci.3,addr=0x0: VFIO_MAP_DMA: -22
2020-03-25T13:43:39.107560Z qemu-system-aarch64: -device
vfio-pci,host=:29:00.0,id=hostdev0,bus=pci.3,addr=0x0: vfio :29:00.0:
failed to setup container for group 28: memory listener initialization failed:
Region mach-virt.ram: vfio_dma_map(0x563169753c80, 0x4000, 0x1,
0x7fb2a3e0) = -22 (Invalid argument)
Traceback (most recent call last):
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in
cb_wrapper
callback(asyncjob, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
callback(*args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line
66, in newfn
ret = fn(self, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/domain.py", line 1279, in
startup
self._backend.create()
File "/usr/lib64/python3.8/site-packages/libvirt.py", line 1234, in create
if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirt.libvirtError: internal error: process exited while connecting to
monitor: 2020-03-25T13:43:39.107524Z qemu-system-aarch64: -device
vfio-pci,host=:29:00.0,id=hostdev0,bus=pci.3,addr=0x0: VFIO_MAP_DMA: -22
2020-03-25T13:43:39.107560Z qemu-system-aarch64: -device
vfio-pci,host=:29:00.0,id=hostdev0,bus=pci.3,addr=0x0: vfio :29:00.0:
failed to setup container for group 28: memory listener initialization failed:
Region mach-virt.ram: vfio_dma_map(0x563169753c80, 0x4000, 0x1,
0x7fb2a3e0) = -22 (Invalid argument)
I played with memory and 3054 MB is maximum value possible to boot VM with
coldplugged host PCIe cards.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1869006/+subscriptions
[Bug 1902306] Re: Allow setting usb storage device ID parameters
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1902306 Title: Allow setting usb storage device ID parameters Status in QEMU: Expired Bug description: Some stubborn software requires certain VID/PID/Serial to authenticate and refuses to start in emulation. This poses a problem with unsupported programs which often require keeping an ancient hardware praying that the USB stick will not die before the (often defunct) company making it. Virtualizing such environment is desired. However, QEMU doesn't allow setting VID/PID/Serial/Name of emulated USB devices, but instead uses hardcoded values: https://github.com/qemu/qemu/blob/c99fa56b95a72f6debd50a280561895d078ae020/hw/usb /dev-storage.c#L95 This request (including a patch) was already made in 2015 on the list but never got any response: https://lists.nongnu.org/archive/html /qemu-discuss/2015-07/msg00072.html WDYT of adding such functionality? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1902306/+subscriptions
[Bug 1873769] Re: SB16 audio playback freezes emulation in Windows 95 guest
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1873769 Title: SB16 audio playback freezes emulation in Windows 95 guest Status in QEMU: Expired Bug description: - QEMU 4.2.93 (v5.0.0-rc3) built from latest git master 20038cd7a8412feeb49c01f6ede89e36c8995472 using MSYS2 on Windows 10 and launched on same Windows 10 - Launched using "qemu-system-i386.exe -drive format=raw,file=hdd- 2gb.img -soundhw pcspk,sb16 -m 16 -cpu pentium -vga std -cdrom Windows_95.iso -boot c" - I have attached video screen capture of the issue --- I decided to make my first ever QEMU build after encountering the dsound issues using the latest 4.2.0 binary from https://qemu.weilnetz.de/w64/. In my 5.0.0-rc3 build the sound playback is working correctly, however the whole Windows 95 UI freezes while sound is playing. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1873769/+subscriptions
[Bug 1878501] Re: qemu-i386 does not define AT_SYSINFO
[Expired for QEMU because there has been no activity for 60 days.]
** Changed in: qemu
Status: Incomplete => Expired
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1878501
Title:
qemu-i386 does not define AT_SYSINFO
Status in QEMU:
Expired
Bug description:
qemu-i386 does not define the AT_SYSINFO auxval when running i386
Linux binaries.
On most libcs, this is properly handled, but this is mandatory for the
i686 Bionic (Android) libc or it will segfault.
This is due to a blind assumption that getauxval(AT_SYSINFO) will
return a valid function pointer:
The code varies from version to version, but it looks like this:
void *__libc_sysinfo;
// mangled as _Z19__libc_init_sysinfov
void __libc_init_sysinfo() {
bool dummy;
// __bionic_getauxval = getauxval
__libc_sysinfo = reinterpret_cast(__bionic_getauxval(AT_SYSINFO,
dummy));
}
A simple way to reproduce is to compile a basic C program against the
NDK:
int main(void) { return 0; }
$ i686-linux-android-clang -static empty.c -o empty
$ qemu-i386 -cpu max ./empty
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault
The place where it segfaults is misleading: It will, at least on the
current NDK, crash on __set_thread_area, this is due to it calling a
function pointer to __libc_sysinfo returned by __kernel_syscall.
QEMU 4.1.1 (aarch64)
Pixel 2 XL via Termux
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1878501/+subscriptions
Re: [PATCH v3 0/8] dp8393x: fixes and improvements
On Sat, 10 Jul 2021, Philippe Mathieu-Daudé wrote:
>
> The last 2 patches are included for Mark, but I don't plan to merge
>
> them without Finn's Ack, and apparently they require more work.
>
I tested the patch series both with and without the last 2 patches. Both
builds worked fine with my NetBSD/arc, Linux/mipsel and Linux/m68k guests.
Tested-by: Finn Thain
I have no objection to patch 8/8 ("dp8393x: don't force 32-bit register
access"). I asked Mark to explain why it was a bug fix (since it didn't
change QEMU behaviour in my tests) but when I looked into it I found that
he is quite right, the patch does fix a theoretical bug.
My only objection to patch 7/8 ("dp8393x: Rewrite dp8393x_get() /
dp8393x_put()") was that it could be churn.
If I'm right that the big_endian flag should go away, commit b1600ff195
("hw/mips/jazz: specify correct endian for dp8393x device") has already
taken mainline in the wrong direction and amounts to churn.
I have the same reservations about patch 6/8 ("dp8393x: Store CRC using
device configured endianess"). Perhaps that should be NOTFORMERGE too
(even though it too a theoretical bug fix).
Is there a good way to avoid using big_endian for storing the CRC and the
other DMA operations?
BTW, if you see "sn0: receive buffers exhausted" occasionally logged by
the NetBSD 9.2 kernel, accompanied by packet loss, it's not a regression
in QEMU. I first observed it last year when stress testing dp8393x with
NetBSD 5.1. I believe this to be an old NetBSD sn driver bug because Linux
is unaffected.
Re: [PATCH 00/41] tcg patch queue
Oops, yes. r~ On Sat, 10 Jul 2021, 09:24 Peter Maydell, wrote: > On Sat, 10 Jul 2021 at 16:33, Richard Henderson > wrote: > > > > The following changes since commit > 05de778b5b8ab0b402996769117b88c7ea5c7c61: > > > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into > staging (2021-07-09 14:30:01 +0100) > > > > are available in the Git repository at: > > > > https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210710 > > > > for you to fetch changes up to ad1a706f386c2281adb0b09257d892735e405834: > > > > cpu: Add breakpoint tracepoints (2021-07-09 21:31:11 -0700) > > > > > > Add translator_use_goto_tb. > > Cleanups in prep of breakpoint fixes. > > Misc fixes. > > > > > > Is this intended as a pullreq despite the "PATCH" in the subject? > > thanks > -- PMM >
Re: [NOTFORMERGE PATCH v3 8/8] dp8393x: don't force 32-bit register access
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
From: Mark Cave-Ayland
Commit 3fe9a838ec "dp8393x: Always use 32-bit accesses" set
.impl.min_access_size
and .impl.max_access_size to 4 to try and fix the Linux jazzsonic driver which
uses
32-bit accesses.
The problem with forcing the register access to 32-bit in this way is that
since the
dp8393x uses 16-bit registers, a manual endian swap is required for devices on
big
endian machines with 32-bit accesses.
For both access sizes and machine endians the QEMU memory API can do the right
thing
automatically: all that is needed is to set .impl.min_access_size to 2 to
declare that
the dp8393x implements 16-bit registers.
Normally .impl.max_access_size should also be set to 2, however that doesn't
quite
work in this case since the register stride is specified using a (dynamic)
it_shift
property which is applied during the MMIO access itself. The effect of this is
that
for a 32-bit access the memory API performs 2 x 16-bit accesses, but the use of
it_shift within the MMIO access itself causes the register value to be repeated
in both
the top 16-bits and bottom 16-bits. The Linux jazzsonic driver expects the
stride to be
zero-extended up to access size and therefore fails to correctly detect the
dp8393x
device due to the extra data in the top 16-bits.
The solution here is to remove .impl.max_access_size so that the memory API will
correctly zero-extend the 16-bit registers to the access size up to and
including
it_shift. Since it_shift is never greater than 2 than this will always do the
right
thing for both 16-bit and 32-bit accesses regardless of the machine endian,
allowing
the manual endian swap code to be removed.
Signed-off-by: Mark Cave-Ayland
Fixes: 3fe9a838ec ("dp8393x: Always use 32-bit accesses")
Message-Id: <20210705214929.17222-2-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index a9224a5bc88..71c82a07c23 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -588,15 +588,14 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr,
unsigned int size)
trace_dp8393x_read(reg, reg_names[reg], val, size);
-return s->big_endian ? val << 16 : val;
+return val;
}
-static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
+static void dp8393x_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int size)
{
dp8393xState *s = opaque;
int reg = addr >> s->it_shift;
-uint32_t val = s->big_endian ? data >> 16 : data;
trace_dp8393x_write(reg, reg_names[reg], val, size);
@@ -677,11 +676,16 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
}
}
+/*
+ * Since .impl.max_access_size is effectively controlled by the it_shift
+ * property, leave it unspecified for now to allow the memory API to
+ * correctly zero extend the 16-bit register values to the access size up to
and
+ * including it_shift.
+ */
static const MemoryRegionOps dp8393x_ops = {
.read = dp8393x_read,
.write = dp8393x_write,
-.impl.min_access_size = 4,
-.impl.max_access_size = 4,
+.impl.min_access_size = 2,
.endianness = DEVICE_NATIVE_ENDIAN,
};
Why has this been demoted to NOTFORMERGE? Removing the explicit .impl.max_access_size
= 4 means that we fall back to the default .impl.max_access_size = 4 which already
has a T-b tag from Finn for the series at
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg07167.html which includes
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06814.html.
This patch with the explicit .impl.max_access_size = 4 was the primary motivation for
posting the original series nearly a month ago: see the original version at
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg03199.html.
ATB,
Mark.
Re: [NOTFORMERGE PATCH v3 7/8] dp8393x: Rewrite dp8393x_get() / dp8393x_put()
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
Instead of accessing N registers via a single address_space API
call using a temporary buffer (stored in the device state) and
updating each register, move the address_space call in the
register put/get. The load/store and word size checks are moved
to put/get too. This simplifies a bit, making the code easier
to read.
Co-developed-by: Philippe Mathieu-Daudé
Co-developed-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
Missing:
Signed-off-by: Mark Cave-Ayland
---
hw/net/dp8393x.c | 160 +++
1 file changed, 63 insertions(+), 97 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index ac93412f70b..a9224a5bc88 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -163,7 +163,6 @@ struct dp8393xState {
/* Temporaries */
uint8_t tx_buffer[0x1];
-uint16_t data[12];
int loopback_packet;
/* Memory access */
@@ -220,34 +219,48 @@ static uint32_t dp8393x_wt(dp8393xState *s)
return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0];
}
-static uint16_t dp8393x_get(dp8393xState *s, int width, int offset)
+static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, int offset)
{
+const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
uint16_t val;
-if (s->big_endian) {
-val = be16_to_cpu(s->data[offset * width + width - 1]);
+if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
+addr += offset << 2;
+if (s->big_endian) {
+val = address_space_ldl_be(>as, addr, attrs, NULL);
+} else {
+val = address_space_ldl_le(>as, addr, attrs, NULL);
+}
} else {
-val = le16_to_cpu(s->data[offset * width]);
+addr += offset << 1;
+if (s->big_endian) {
+val = address_space_lduw_be(>as, addr, attrs, NULL);
+} else {
+val = address_space_lduw_le(>as, addr, attrs, NULL);
+}
}
+
return val;
}
-static void dp8393x_put(dp8393xState *s, int width, int offset,
-uint16_t val)
+static void dp8393x_put(dp8393xState *s,
+hwaddr addr, int offset, uint16_t val)
{
-if (s->big_endian) {
-if (width == 2) {
-s->data[offset * 2] = 0;
-s->data[offset * 2 + 1] = cpu_to_be16(val);
+const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+
+if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
+addr += offset << 2;
+if (s->big_endian) {
+address_space_stl_be(>as, addr, val, attrs, NULL);
} else {
-s->data[offset] = cpu_to_be16(val);
+address_space_stl_le(>as, addr, val, attrs, NULL);
}
} else {
-if (width == 2) {
-s->data[offset * 2] = cpu_to_le16(val);
-s->data[offset * 2 + 1] = 0;
+addr += offset << 1;
+if (s->big_endian) {
+address_space_stw_be(>as, addr, val, attrs, NULL);
} else {
-s->data[offset] = cpu_to_le16(val);
+address_space_stw_le(>as, addr, val, attrs, NULL);
}
}
}
@@ -278,12 +291,10 @@ static void dp8393x_do_load_cam(dp8393xState *s)
while (s->regs[SONIC_CDC] & 0x1f) {
/* Fill current entry */
-address_space_read(>as, dp8393x_cdp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
-index = dp8393x_get(s, width, 0) & 0xf;
-s->cam[index][0] = dp8393x_get(s, width, 1);
-s->cam[index][1] = dp8393x_get(s, width, 2);
-s->cam[index][2] = dp8393x_get(s, width, 3);
+index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf;
+s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1);
+s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2);
+s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3);
trace_dp8393x_load_cam(index,
s->cam[index][0] >> 8, s->cam[index][0] & 0xff,
s->cam[index][1] >> 8, s->cam[index][1] & 0xff,
@@ -294,9 +305,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
}
/* Read CAM enable */
-address_space_read(>as, dp8393x_cdp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
-s->regs[SONIC_CE] = dp8393x_get(s, width, 0);
+s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0);
trace_dp8393x_load_cam_done(s->regs[SONIC_CE]);
/* Done */
@@ -312,14 +321,12 @@ static void dp8393x_do_read_rra(dp8393xState *s)
/* Read memory */
width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
size = sizeof(uint16_t) * 4 * width;
-address_space_read(>as, dp8393x_rrp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
/* Update SONIC registers */
-s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0);
-s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1);
-s->regs[SONIC_RBWC0] =
Re: [PATCH v3 6/8] dp8393x: Store CRC using device configured endianess
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
Little-Endian CRC is dubious. The datasheet does not
specify it being little-endian. Use big-endian access
when the device is configured in such endianess.
(This is a theoretical bug fix.)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 68516241a1f..ac93412f70b 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -827,7 +827,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
/* Calculate the ethernet checksum */
-checksum = cpu_to_le32(crc32(0, buf, pkt_size));
+checksum = crc32(0, buf, pkt_size);
/* Put packet into RBA */
trace_dp8393x_receive_packet(dp8393x_crba(s));
@@ -837,8 +837,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
address += pkt_size;
/* Put frame checksum into RBA */
-address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
-, sizeof(checksum));
+if (s->big_endian) {
+address_space_stl_be(>as, address, checksum,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+} else {
+address_space_stl_le(>as, address, checksum,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+}
address += sizeof(checksum);
/* Pad short packets to keep pointers aligned */
This is obviously new to the series: I can test this on big endian m68k but are you
sure that this won't break big endian MIPS? Or do we not care for now since we don't
have a working test image?
ATB,
Mark.
Re: [PATCH v3 5/8] dp8393x: Migrate registers as array of uint16
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
The CAM registers are now arrays of 3 uint16_t. We can avoid using
the VMSTATE_BUFFER_UNSAFE() macro by using VMSTATE_UINT16_2DARRAY()
which is more appropriate.
Suggested-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 2c3047c8688..68516241a1f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -983,10 +983,10 @@ static void dp8393x_realize(DeviceState *dev, Error
**errp)
static const VMStateDescription vmstate_dp8393x = {
.name = "dp8393x",
-.version_id = 0,
-.minimum_version_id = 0,
+.version_id = 1,
+.minimum_version_id = 1,
.fields = (VMStateField []) {
-VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
+VMSTATE_UINT16_2DARRAY(cam, dp8393xState, 16, 3),
VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
I'm not convinced that this needs an extra patch and couldn't be squashed into the
previous patch, but still:
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: [PATCH v3 4/8] dp8393x: Store CAM registers as 16-bit
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
Per the DP83932C datasheet from July 1995:
4.0 SONIC Registers
4.1 THE CAM UNIT
The Content Addressable Memory (CAM) consists of sixteen
48-bit entries for complete address filtering of network
packets. Each entry corresponds to a 48-bit destination
address that is user programmable and can contain any
combination of Multicast or Physical addresses. Each entry
is partitioned into three 16-bit CAM cells accessible
through CAM Address Ports (CAP 2, CAP 1 and CAP 0) with
CAP0 corresponding to the least significant 16 bits of
the Destination Address and CAP2 corresponding to the
most significant bits.
Store the CAM registers as 16-bit as it simplifies the code.
There is no change in the migration stream.
Co-developed-by: Philippe Mathieu-Daudé
Co-developed-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
Missing:
Signed-off-by: Mark Cave-Ayland
---
hw/net/dp8393x.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 64f3b0fc3ea..2c3047c8688 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -158,7 +158,7 @@ struct dp8393xState {
MemoryRegion mmio;
/* Registers */
-uint8_t cam[16][6];
+uint16_t cam[16][3];
uint16_t regs[SONIC_REG_COUNT];
/* Temporaries */
@@ -281,15 +281,13 @@ static void dp8393x_do_load_cam(dp8393xState *s)
address_space_read(>as, dp8393x_cdp(s),
MEMTXATTRS_UNSPECIFIED, s->data, size);
index = dp8393x_get(s, width, 0) & 0xf;
-s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff;
-s->cam[index][1] = dp8393x_get(s, width, 1) >> 8;
-s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff;
-s->cam[index][3] = dp8393x_get(s, width, 2) >> 8;
-s->cam[index][4] = dp8393x_get(s, width, 3) & 0xff;
-s->cam[index][5] = dp8393x_get(s, width, 3) >> 8;
-trace_dp8393x_load_cam(index, s->cam[index][0], s->cam[index][1],
- s->cam[index][2], s->cam[index][3],
- s->cam[index][4], s->cam[index][5]);
+s->cam[index][0] = dp8393x_get(s, width, 1);
+s->cam[index][1] = dp8393x_get(s, width, 2);
+s->cam[index][2] = dp8393x_get(s, width, 3);
+trace_dp8393x_load_cam(index,
+ s->cam[index][0] >> 8, s->cam[index][0] & 0xff,
+ s->cam[index][1] >> 8, s->cam[index][1] & 0xff,
+ s->cam[index][2] >> 8, s->cam[index][2] & 0xff);
/* Move to next entry */
s->regs[SONIC_CDC]--;
s->regs[SONIC_CDP] += size;
@@ -592,8 +590,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr,
unsigned int size)
case SONIC_CAP1:
case SONIC_CAP0:
if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-val = s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg) + 1]
<< 8;
-val |= s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg)];
+val = s->cam[s->regs[SONIC_CEP] & 0xf][SONIC_CAP0 - reg];
}
break;
/* All other registers have no special contraints */
@@ -989,7 +986,7 @@ static const VMStateDescription vmstate_dp8393x = {
.version_id = 0,
.minimum_version_id = 0,
.fields = (VMStateField []) {
-VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
+VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: [PATCH v3 3/8] dp8393x: Only shift the device registers mapping by 1 bit
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
The SONIC device only allows 16/32-bit accesses. From the machine
view (from the bus), it is only shifted by 1 bit. Another bit is
shifted, but it is an implementation detail of the QEMU model.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/m68k/q800.c | 2 +-
hw/mips/jazz.c | 2 +-
hw/net/dp8393x.c | 2 ++
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6817c8b5d1a..d78edfe40e8 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -316,7 +316,7 @@ static void q800_init(MachineState *machine)
dev = qdev_new("dp8393x");
qdev_set_nic_properties(dev, _table[0]);
-qdev_prop_set_uint8(dev, "it_shift", 2);
+qdev_prop_set_uint8(dev, "it_shift", 1);
qdev_prop_set_bit(dev, "big_endian", true);
object_property_set_link(OBJECT(dev), "dma_mr",
OBJECT(get_system_memory()), _abort);
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index d6183e18821..7f8680a189d 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -295,7 +295,7 @@ static void mips_jazz_init(MachineState *machine,
dev = qdev_new("dp8393x");
qdev_set_nic_properties(dev, nd);
-qdev_prop_set_uint8(dev, "it_shift", 2);
+qdev_prop_set_uint8(dev, "it_shift", 1);
qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
object_property_set_link(OBJECT(dev), "dma_mr",
OBJECT(rc4030_dma_mr), _abort);
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index d1e147a82a6..64f3b0fc3ea 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -971,6 +971,8 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
{
dp8393xState *s = DP8393X(dev);
+s->it_shift += 1; /* Registers are 16-bit wide */
+
address_space_init(>as, s->dma_mr, "dp8393x");
memory_region_init_io(>mmio, OBJECT(dev), _ops, s,
"dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
This patch looks wrong to me: by convention it_shift is used to implement the
register stride, so by reducing this from 2 to 1 you've immediately broken the POLA
regarding the it_shift property. There could be an argument that there is an
intermediate bus that could be modelled for 16-bit accesses here, but this is worked
around (and clearly commented) in patch 8.
ATB,
Mark.
Re: [PATCH v3 2/8] dp8393x: Replace 0x40 magic value by SONIC_REG16_COUNT definition
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 9118364aa33..d1e147a82a6 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -85,6 +85,7 @@ static const char *reg_names[] = {
#define SONIC_MPT0x2e
#define SONIC_MDT0x2f
#define SONIC_DCR2 0x3f
+#define SONIC_REG_COUNT 0x40
#define SONIC_CR_HTX 0x0001
#define SONIC_CR_TXP 0x0002
@@ -158,7 +159,7 @@ struct dp8393xState {
/* Registers */
uint8_t cam[16][6];
-uint16_t regs[0x40];
+uint16_t regs[SONIC_REG_COUNT];
/* Temporaries */
uint8_t tx_buffer[0x1];
@@ -972,7 +973,7 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
address_space_init(>as, s->dma_mr, "dp8393x");
memory_region_init_io(>mmio, OBJECT(dev), _ops, s,
- "dp8393x-regs", 0x40 << s->it_shift);
+ "dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
s->nic = qemu_new_nic(_dp83932_info, >conf,
object_get_typename(OBJECT(dev)), dev->id, s);
@@ -987,7 +988,7 @@ static const VMStateDescription vmstate_dp8393x = {
.minimum_version_id = 0,
.fields = (VMStateField []) {
VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
-VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
+VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
};
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: [PATCH v3 1/8] dp8393x: Replace address_space_rw(is_write=1) by address_space_write()
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
Replace address_space_rw(is_write=1) by address_space_write()
and remove pointless cast.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 11810c9b600..9118364aa33 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -816,8 +816,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
size = sizeof(uint16_t) * width;
address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width;
dp8393x_put(s, width, 0, 0);
-address_space_rw(>as, address, MEMTXATTRS_UNSPECIFIED,
- (uint8_t *)s->data, size, 1);
+address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
+s->data, size);
/* Move to next descriptor */
s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
@@ -846,8 +846,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
/* Pad short packets to keep pointers aligned */
if (rx_len < padded_len) {
size = padded_len - rx_len;
-address_space_rw(>as, address, MEMTXATTRS_UNSPECIFIED,
-(uint8_t *)"\xFF\xFF\xFF", size, 1);
+address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
+"\xFF\xFF\xFF", size);
address += size;
}
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: [RFC PATCH 1/6] linux-header: add the SNP specific command
On Fri, Jul 09, 2021 at 04:55:45PM -0500, Brijesh Singh wrote:
> Sync the kvm.h with the kernel to include the SNP specific commands.
>
> Signed-off-by: Brijesh Singh
Pls specify which kernel version you used for the sync.
> ---
> linux-headers/linux/kvm.h | 47 +++
> 1 file changed, 47 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 20d6a263bb..c17ace1ece 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1679,6 +1679,12 @@ enum sev_cmd_id {
> /* Guest Migration Extension */
> KVM_SEV_SEND_CANCEL,
>
> + /* SNP specific commands */
> + KVM_SEV_SNP_INIT = 256,
> + KVM_SEV_SNP_LAUNCH_START,
> + KVM_SEV_SNP_LAUNCH_UPDATE,
> + KVM_SEV_SNP_LAUNCH_FINISH,
> +
> KVM_SEV_NR_MAX,
> };
>
> @@ -1775,6 +1781,47 @@ struct kvm_sev_receive_update_data {
> __u32 trans_len;
> };
>
> +struct kvm_snp_init {
> + __u64 flags;
> +};
> +
> +struct kvm_sev_snp_launch_start {
> + __u64 policy;
> + __u64 ma_uaddr;
> + __u8 ma_en;
> + __u8 imi_en;
> + __u8 gosvw[16];
> +};
> +
> +#define KVM_SEV_SNP_PAGE_TYPE_NORMAL 0x1
> +#define KVM_SEV_SNP_PAGE_TYPE_VMSA 0x2
> +#define KVM_SEV_SNP_PAGE_TYPE_ZERO 0x3
> +#define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED 0x4
> +#define KVM_SEV_SNP_PAGE_TYPE_SECRETS0x5
> +#define KVM_SEV_SNP_PAGE_TYPE_CPUID 0x6
> +
> +struct kvm_sev_snp_launch_update {
> + __u64 uaddr;
> + __u32 len;
> + __u8 imi_page;
> + __u8 page_type;
> + __u8 vmpl3_perms;
> + __u8 vmpl2_perms;
> + __u8 vmpl1_perms;
> +};
> +
> +#define KVM_SEV_SNP_ID_BLOCK_SIZE96
> +#define KVM_SEV_SNP_ID_AUTH_SIZE 4096
> +#define KVM_SEV_SNP_FINISH_DATA_SIZE 32
> +
> +struct kvm_sev_snp_launch_finish {
> + __u64 id_block_uaddr;
> + __u64 id_auth_uaddr;
> + __u8 id_block_en;
> + __u8 auth_key_en;
> + __u8 host_data[KVM_SEV_SNP_FINISH_DATA_SIZE];
> +};
> +
> #define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)
> #define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
> #define KVM_DEV_ASSIGN_MASK_INTX (1 << 2)
> --
> 2.17.1
Re: [PULL 00/28] Block layer patches
On Fri, 9 Jul 2021 at 13:50, Kevin Wolf wrote: > > The following changes since commit 9db3065c62a983286d06c207f4981408cf42184d: > > Merge remote-tracking branch > 'remotes/vivier2/tags/linux-user-for-6.1-pull-request' into staging > (2021-07-08 16:30:18 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to e60edf69e2f64e818466019313517a2e6d6b63f4: > > block: Make blockdev-reopen stable API (2021-07-09 13:19:11 +0200) > > > Block layer patches > > - Make blockdev-reopen stable > - Remove deprecated qemu-img backing file without format > - rbd: Convert to coroutines and add write zeroes support > - rbd: Updated MAINTAINERS > - export/fuse: Allow other users access to the export > - vhost-user: Fix backends without multiqueue support > - Fix drive-backup transaction endless drained section Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
[PULL v2 48/48] meson: Use input/output for entitlements target
From: Akihiko Odaki
input/output parameters respect dependencies.
Signed-off-by: Akihiko Odaki
Message-Id: <20210709012533.58262-1-akihiko.od...@gmail.com>
Signed-off-by: Paolo Bonzini
---
meson.build| 30 +-
scripts/entitlement.sh | 10 +-
2 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/meson.build b/meson.build
index 651c3b114b..b2e8731410 100644
--- a/meson.build
+++ b/meson.build
@@ -2609,28 +2609,32 @@ foreach target : target_dirs
link_args: link_args,
gui_app: exe['gui'])
-if 'CONFIG_HVF' in config_target
- entitlements = meson.current_source_dir() /
'accel/hvf/entitlements.plist'
-else
- entitlements = '/dev/null'
-endif
if targetos == 'darwin'
- icon = meson.current_source_dir() / 'pc-bios/qemu.rsrc'
+ icon = 'pc-bios/qemu.rsrc'
+ build_input = [emulator, files(icon)]
+ install_input = [
+get_option('bindir') / exe_name,
+meson.current_source_dir() / icon
+ ]
+ if 'CONFIG_HVF' in config_target
+entitlements = 'accel/hvf/entitlements.plist'
+build_input += files(entitlements)
+install_input += meson.current_source_dir() / entitlements
+ endif
+
emulators += {exe['name'] : custom_target(exe['name'],
- depends: emulator,
+ input: build_input,
output: exe['name'],
command: [
- meson.current_source_dir() / 'scripts/entitlement.sh',
- meson.current_build_dir() / exe_name,
- meson.current_build_dir() / exe['name'],
- entitlements, icon
+ files('scripts/entitlement.sh'),
+ '@OUTPUT@',
+ '@INPUT@'
])
}
meson.add_install_script('scripts/entitlement.sh', '--install',
- get_option('bindir') / exe_name,
get_option('bindir') / exe['name'],
- entitlements, icon)
+ install_input)
else
emulators += {exe['name']: emulator}
endif
diff --git a/scripts/entitlement.sh b/scripts/entitlement.sh
index d2a7079ce3..e2c956a3ac 100755
--- a/scripts/entitlement.sh
+++ b/scripts/entitlement.sh
@@ -8,10 +8,10 @@ if [ "$1" = --install ]; then
in_place=false
fi
-SRC="$1"
-DST="$2"
-ENTITLEMENT="$3"
-ICON="$4"
+DST="$1"
+SRC="$2"
+ICON="$3"
+ENTITLEMENT="$4"
if $in_place; then
trap 'rm "$DST.tmp"' exit
@@ -21,7 +21,7 @@ else
cd "$MESON_INSTALL_DESTDIR_PREFIX"
fi
-if test "$ENTITLEMENT" != '/dev/null'; then
+if test -n "$ENTITLEMENT"; then
codesign --entitlements "$ENTITLEMENT" --force -s - "$SRC"
fi
--
2.31.1
[PULL v2 05/48] modules: add modinfo macros
From: Gerd Hoffmann Add macros for module info annotations. Instead of having that module meta-data stored in lists in util/module.c place directly in the module source code. Signed-off-by: Gerd Hoffmann Reviewed-by: Jose R. Ziviani Message-Id: <20210624103836.2382472-2-kra...@redhat.com> Signed-off-by: Paolo Bonzini --- include/qemu/module.h | 61 +++ 1 file changed, 61 insertions(+) diff --git a/include/qemu/module.h b/include/qemu/module.h index 944d403cbd..b595f15975 100644 --- a/include/qemu/module.h +++ b/include/qemu/module.h @@ -73,4 +73,65 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail); void module_load_qom_one(const char *type); void module_load_qom_all(void); +/** + * DOC: module info annotation macros + * + * `scripts/modinfo-collect.py` will collect module info, + * using the preprocessor and -DQEMU_MODINFO. + * + * `scripts/modinfo-generate.py` will create a module meta-data database + * from the collected information so qemu knows about module + * dependencies and QOM objects implemented by modules. + * + * See `*.modinfo` and `modinfo.c` in the build directory to check the + * script results. + */ +#ifdef QEMU_MODINFO +# define modinfo(kind, value) \ +MODINFO_START kind value MODINFO_END +#else +# define modinfo(kind, value) +#endif + +/** + * module_obj + * + * @name: QOM type. + * + * This module implements QOM type @name. + */ +#define module_obj(name) modinfo(obj, name) + +/** + * module_dep + * + * @name: module name + * + * This module depends on module @name. + */ +#define module_dep(name) modinfo(dep, name) + +/** + * module_arch + * + * @name: target architecture + * + * This module is for target architecture @arch. + * + * Note that target-dependent modules are tagged automatically, so + * this is only needed in case target-independent modules should be + * restricted. Use case example: the ccw bus is implemented by s390x + * only. + */ +#define module_arch(name) modinfo(arch, name) + +/** + * module_opts + * + * @name: QemuOpts name + * + * This module registers QemuOpts @name. + */ +#define module_opts(name) modinfo(opts, name) + #endif -- 2.31.1
[PULL v2 00/48] Misc patches for QEMU 6.1 soft freeze
The following changes since commit 05de778b5b8ab0b402996769117b88c7ea5c7c61: Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-07-09 14:30:01 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 411ad8dd80077e98ed465775b044caf1a9482f6c: meson: Use input/output for entitlements target (2021-07-09 18:21:34 +0200) v1->v2: fix docs build, add final patch for entitlements target * More SVM fixes (Lara) * Module annotation database (Gerd) * Memory leak fixes (myself) * Build fixes (myself) * --with-devices-* support (Alex) Akihiko Odaki (1): meson: Use input/output for entitlements target Alex Bennée (4): hw/arm: add dependency on OR_IRQ for XLNX_VERSAL hw/arm: move CONFIG_V7M out of default-devices configs: rename default-configs to configs and reorganise configure: allow the selection of alternate config in the build Gerd Hoffmann (32): modules: add modinfo macros modules: collect module meta-data modules: generate modinfo.c modules: add qxl module annotations modules: add virtio-gpu module annotations modules: add chardev module annotations modules: add audio module annotations modules: add usb-redir module annotations modules: add ccid module annotations modules: add ui module annotations modules: add s390x module annotations modules: add block module annotations modules: use modinfo for dependencies modules: use modinfo for qom load modules: use modinfo for qemu opts load modules: add tracepoints modules: check arch and block load on mismatch modules: check arch on qom lookup modules: target-specific module build infrastructure modules: add documentation for module sourcesets modules: add module_obj() note to QOM docs modules: hook up modules.h to docs build accel: autoload modules accel: add qtest module annotations accel: build qtest modular accel: add tcg module annotations accel: build tcg modular monitor: allow register hmp commands usb: drop usb_host_dev_is_scsi_storage hook monitor/usb: register 'info usbhost' dynamically usb: build usb-host as module monitor/tcg: move tcg hmp commands to accel/tcg, register them dynamically Jose R. Ziviani (1): modules: check if all dependencies can be satisfied Lara Lazier (2): target/i386: Added MSRPM and IOPM size check target/i386: Added DR6 and DR7 consistency checks Miroslav Rezanina (2): configure: fix libdaxctl options configure: fix libpmem configuration option Paolo Bonzini (5): meson: fix missing preprocessor symbols osdep: fix HAVE_BROKEN_SIZE_MAX case target/i386: fix exceptions for MOV to DR vl: fix leak of qdict_crumple return value meson: switch function tests from compilation to linking Philippe Mathieu-Daudé (1): meson: Introduce target-specific Kconfig Kconfig| 1 + MAINTAINERS| 22 ++- accel/accel-common.c | 2 +- accel/accel-softmmu.c | 2 +- accel/qtest/meson.build| 8 +- accel/qtest/qtest.c| 2 + accel/tcg/hmp.c| 29 +++ accel/tcg/meson.build | 6 +- accel/tcg/tcg-accel-ops.c | 1 + accel/tcg/tcg-all.c| 1 + audio/spiceaudio.c | 2 + block/iscsi-opts.c | 1 + block/meson.build | 2 +- chardev/baum.c | 1 + chardev/spice.c| 4 + .../devices/aarch64-softmmu/default.mak| 2 +- configs/devices/aarch64-softmmu/minimal.mak| 9 + .../devices/alpha-softmmu/default.mak | 0 .../devices/arm-softmmu/default.mak| 3 - .../devices/avr-softmmu/default.mak| 0 .../devices/cris-softmmu/default.mak | 0 .../devices/hppa-softmmu/default.mak | 0 .../devices/i386-softmmu/default.mak | 0 .../devices/m68k-softmmu/default.mak | 0 .../devices/microblaze-softmmu/default.mak | 0 .../devices/microblazeel-softmmu/default.mak | 2 +- .../devices/mips-softmmu/common.mak| 0 .../devices/mips-softmmu/default.mak | 2 +- .../devices/mips64-softmmu/default.mak | 2 +- .../devices/mips64el-softmmu/default.mak
Re: [PULL 00/33] ppc-for-6.1 queue 20210709
On Fri, 9 Jul 2021 at 06:17, David Gibson wrote: > > The following changes since commit 9db3065c62a983286d06c207f4981408cf42184d: > > Merge remote-tracking branch > 'remotes/vivier2/tags/linux-user-for-6.1-pull-request' into staging > (2021-07-08 16:30:18 +0100) > > are available in the Git repository at: > > https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.1-20210709 > > for you to fetch changes up to 82123b756a1a2f1965350e5794aaa7b5c6a15282: > > target/ppc: Support for H_RPT_INVALIDATE hcall (2021-07-09 11:01:06 +1000) > > > ppc patch queue 2021-07-09 > > Here's a (probably) final pull request before the qemu-6.1 soft > freeze. Includes: > * Implementation of the new H_RPT_INVALIDATE hypercall > * Virtual Open Firmware for pSeries and pegasos2 machine types. > This is an experimental minimal Open Firmware implementation which > works by delegating nearly everything to qemu itself via a special > hypercall. > * A number of cleanups to the ppc soft MMU code > * Fix to handling of two-level radix mode translations for the > powernv machine type > * Update the H_GET_CPU_CHARACTERISTICS call with newly defined bits. > This will allow more flexible handling of possible future CPU > Spectre-like flaws > * Correctly treat mtmsrd as an illegal instruction on BookE cpus > * Firmware update for the ppce500 machine type > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [PATCH v2 3/3] qemu-img: Add --skip-broken-bitmaps for 'convert --bitmaps'
On 7/9/21 6:39 PM, Eric Blake wrote:
The point of 'qemu-img convert --bitmaps' is to be a convenience for
actions that are already possible through a string of smaller
'qemu-img bitmap' sub-commands. One situation not accounted for
already is that if a source image contains an inconsistent bitmap (for
example, because a qemu process died abruptly before flushing bitmap
state), the user MUST delete those inconsistent bitmaps before
anything else useful can be done with the image.
The only thing affected by inconsistent bitmap is creating incremental
backup, and taking some space on storage. Anything else should not be
affected by having such bitmap so the user does not need to remove it.
In oVirt we don't check or repair images after unclean guest shutdown.
Maybe this is a good idea for future version. Inconsistent bitmaps are
removed only when the user ask to remove the related checkpoint.
We don't want to delete inconsistent bitmaps by default: although a
corrupt bitmap is only a loss of optimization rather than a corruption
of user-visible data, it is still nice to require the user to opt in
to the fact that they are aware of the loss of the bitmap. Still,
requiring the user to check 'qemu-img info' to see whether bitmaps are
consistent, then use 'qemu-img bitmap --remove' to remove offenders,
all before using 'qemu-img convert', is a lot more work than just
adding a knob 'qemu-img convert --bitmaps --skip-broken-bitmaps' which
opts in to skipping the broken bitmaps.
I think this is more than convenience. During live storage migration in
oVirt, we mirror the top layer to the destination using libvirt
blockCopy, and copy the rest of the chain using qemu-img convert with
the --bitmaps option.
If we have to remove inconsistent bitmaps at this point we need to
modify images opened for reading by qemu, which is likely not possible
and even if it is possible, sounds like a bad idea.
After testing the new option, also demonstrate the way to manually fix
things (either deleting bad bitmaps, or re-creating them as empty) so
that it is possible to convert without the option.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1946084
Signed-off-by: Eric Blake
---
docs/tools/qemu-img.rst | 8 -
qemu-img.c| 26 +---
tests/qemu-iotests/tests/qemu-img-bitmaps | 10 ++
tests/qemu-iotests/tests/qemu-img-bitmaps.out | 31 +++
4 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index cfe11478791f..4d407b180450 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -414,7 +414,7 @@ Command description:
4
Error on reading data
-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts]
[--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t
CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l
SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W]
FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts]
[--target-is-zero] [--bitmaps [--skip-broken-bitmaps]] [-U] [-C] [-c] [-p] [-q]
[-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o
OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m
NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
I liked --skip-broken more, but Vladimir is right that this is not
really a sub-option.
Convert the disk image *FILENAME* or a snapshot *SNAPSHOT_PARAM*
to disk image *OUTPUT_FILENAME* using format *OUTPUT_FMT*. It can
@@ -456,6 +456,12 @@ Command description:
*NUM_COROUTINES* specifies how many coroutines work in parallel during
the convert process (defaults to 8).
+ Use of ``--bitmaps`` requests that any persistent bitmaps present in
+ the original are also copied to the destination. If any bitmap is
+ inconsistent in the source, the conversion will fail unless
+ ``--skip-broken-bitmaps`` is also specified to copy only the
+ consistent bitmaps.
+
.. option:: create [--object OBJECTDEF] [-q] [-f FMT] [-b BACKING_FILE] [-F
BACKING_FMT] [-u] [-o OPTIONS] FILENAME [SIZE]
Create the new disk image *FILENAME* of size *SIZE* and format
diff --git a/qemu-img.c b/qemu-img.c
index e84b3c530155..661538edd785 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -82,6 +82,7 @@ enum {
OPTION_MERGE = 274,
OPTION_BITMAPS = 275,
OPTION_FORCE = 276,
+OPTION_SKIP_BROKEN = 277,
};
typedef enum OutputFormat {
@@ -2102,7 +2103,7 @@ static int convert_do_copy(ImgConvertState *s)
}
/* Check that bitmaps can be copied, or output an error */
-static int convert_check_bitmaps(BlockDriverState *src)
+static int convert_check_bitmaps(BlockDriverState *src, bool skip_broken)
{
BdrvDirtyBitmap *bm;
@@ -2117,7 +2118,7 @@ static int
Re: [PATCH v2 2/3] qemu-img: Fail fast on convert --bitmaps with inconsistent bitmap
On 7/9/21 6:39 PM, Eric Blake wrote:
Waiting until the end of the convert operation (a potentially
time-consuming task) to finally detect that we can't copy a bitmap is
bad, comparing to failing fast up front. Furthermore, this prevents
us from leaving a file behind with a bitmap that is not marked as
inconsistent even though it does not have sane contents.
I don't think this is an issue since qemu-img terminate with non-zero
exit code, and we cannot ensure that image is complete if we fail in
the middle of the operation for all image formats and protocols.
For files we could use a temporary file and rename after successful
conversion for for raw format on block device we don't have any way
to mark the contents as temporary.
But failing fast is very important.
This fixes the problems exposed in the previous patch to the iotest.
Signed-off-by: Eric Blake
---
qemu-img.c| 30 +--
tests/qemu-iotests/tests/qemu-img-bitmaps | 2 --
tests/qemu-iotests/tests/qemu-img-bitmaps.out | 20 ++---
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7956a8996512..e84b3c530155 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2101,6 +2101,30 @@ static int convert_do_copy(ImgConvertState *s)
return s->ret;
}
+/* Check that bitmaps can be copied, or output an error */
+static int convert_check_bitmaps(BlockDriverState *src)
+{
+BdrvDirtyBitmap *bm;
+
+if (!bdrv_supports_persistent_dirty_bitmap(src)) {
+error_report("Source lacks bitmap support");
+return -1;
+}
+FOR_EACH_DIRTY_BITMAP(src, bm) {
+const char *name;
+
+if (!bdrv_dirty_bitmap_get_persistence(bm)) {
+continue;
+}
+name = bdrv_dirty_bitmap_name(bm);
+if (bdrv_dirty_bitmap_inconsistent(bm)) {
+error_report("Cannot copy inconsistent bitmap '%s'", name);
We can add a useful hint:
Try "qemu-img bitmap --remove" to delete this bitmap from disk.
+return -1;
+}
+}
+return 0;
+}
+
static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)
{
BdrvDirtyBitmap *bm;
@@ -2127,6 +2151,7 @@ static int convert_copy_bitmaps(BlockDriverState *src,
BlockDriverState *dst)
);
if (err) {
error_reportf_err(err, "Failed to populate bitmap %s: ", name);
+qmp_block_dirty_bitmap_remove(dst->node_name, name, NULL);
This may fail for the same reason populate failed (e.g. storage became
inaccessibel in the middle of the copy). Since we fail the convert, I
don't think it worth to try to do this kind of cleanup.
If we have a way to disable the bitmap before merge, and enable it after
successful merge it make more sense, since if the operation fails we are
left with disabled bitmap.
return -1;
}
}
@@ -2552,9 +2577,8 @@ static int img_convert(int argc, char **argv)
ret = -1;
goto out;
}
-if (!bdrv_supports_persistent_dirty_bitmap(blk_bs(s.src[0]))) {
-error_report("Source lacks bitmap support");
-ret = -1;
+ret = convert_check_bitmaps(blk_bs(s.src[0]));
+if (ret < 0) {
goto out;
}
}
diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps
b/tests/qemu-iotests/tests/qemu-img-bitmaps
index 2f51651d0ce5..3fde95907515 100755
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps
@@ -141,8 +141,6 @@ $QEMU_IMG bitmap --remove "$TEST_IMG" b1
_img_info --format-specific | _filter_irrelevant_img_info
$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" &&
echo "unexpected success"
-# Bug - even though we failed at conversion, we left a file around with
-# a bitmap marked as not corrupt
TEST_IMG=$TEST_IMG.copy _img_info --format-specific \
| _filter_irrelevant_img_info
diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps.out
b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
index b762362075d1..546aaa404bba 100644
--- a/tests/qemu-iotests/tests/qemu-img-bitmaps.out
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
@@ -143,22 +143,6 @@ Format specific information:
name: b4
granularity: 65536
corrupt: false
-qemu-img: Failed to populate bitmap b0: Bitmap 'b0' is inconsistent and cannot
be used
-Try block-dirty-bitmap-remove to delete this bitmap from disk
-image: TEST_DIR/t.IMGFMT.copy
-file format: IMGFMT
-virtual size: 10 MiB (10485760 bytes)
-cluster_size: 65536
-Format specific information:
-bitmaps:
-[0]:
-flags:
-name: b0
-granularity: 65536
-[1]:
-flags:
-[0]: auto
-name: b4
-granularity: 65536
-corrupt: false
+qemu-img: Cannot copy inconsistent bitmap 'b0'
+qemu-img: Could not open
[NOTFORMERGE PATCH v3 7/8] dp8393x: Rewrite dp8393x_get() / dp8393x_put()
Instead of accessing N registers via a single address_space API
call using a temporary buffer (stored in the device state) and
updating each register, move the address_space call in the
register put/get. The load/store and word size checks are moved
to put/get too. This simplifies a bit, making the code easier
to read.
Co-developed-by: Philippe Mathieu-Daudé
Co-developed-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
Missing:
Signed-off-by: Mark Cave-Ayland
---
hw/net/dp8393x.c | 160 +++
1 file changed, 63 insertions(+), 97 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index ac93412f70b..a9224a5bc88 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -163,7 +163,6 @@ struct dp8393xState {
/* Temporaries */
uint8_t tx_buffer[0x1];
-uint16_t data[12];
int loopback_packet;
/* Memory access */
@@ -220,34 +219,48 @@ static uint32_t dp8393x_wt(dp8393xState *s)
return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0];
}
-static uint16_t dp8393x_get(dp8393xState *s, int width, int offset)
+static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, int offset)
{
+const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
uint16_t val;
-if (s->big_endian) {
-val = be16_to_cpu(s->data[offset * width + width - 1]);
+if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
+addr += offset << 2;
+if (s->big_endian) {
+val = address_space_ldl_be(>as, addr, attrs, NULL);
+} else {
+val = address_space_ldl_le(>as, addr, attrs, NULL);
+}
} else {
-val = le16_to_cpu(s->data[offset * width]);
+addr += offset << 1;
+if (s->big_endian) {
+val = address_space_lduw_be(>as, addr, attrs, NULL);
+} else {
+val = address_space_lduw_le(>as, addr, attrs, NULL);
+}
}
+
return val;
}
-static void dp8393x_put(dp8393xState *s, int width, int offset,
-uint16_t val)
+static void dp8393x_put(dp8393xState *s,
+hwaddr addr, int offset, uint16_t val)
{
-if (s->big_endian) {
-if (width == 2) {
-s->data[offset * 2] = 0;
-s->data[offset * 2 + 1] = cpu_to_be16(val);
+const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+
+if (s->regs[SONIC_DCR] & SONIC_DCR_DW) {
+addr += offset << 2;
+if (s->big_endian) {
+address_space_stl_be(>as, addr, val, attrs, NULL);
} else {
-s->data[offset] = cpu_to_be16(val);
+address_space_stl_le(>as, addr, val, attrs, NULL);
}
} else {
-if (width == 2) {
-s->data[offset * 2] = cpu_to_le16(val);
-s->data[offset * 2 + 1] = 0;
+addr += offset << 1;
+if (s->big_endian) {
+address_space_stw_be(>as, addr, val, attrs, NULL);
} else {
-s->data[offset] = cpu_to_le16(val);
+address_space_stw_le(>as, addr, val, attrs, NULL);
}
}
}
@@ -278,12 +291,10 @@ static void dp8393x_do_load_cam(dp8393xState *s)
while (s->regs[SONIC_CDC] & 0x1f) {
/* Fill current entry */
-address_space_read(>as, dp8393x_cdp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
-index = dp8393x_get(s, width, 0) & 0xf;
-s->cam[index][0] = dp8393x_get(s, width, 1);
-s->cam[index][1] = dp8393x_get(s, width, 2);
-s->cam[index][2] = dp8393x_get(s, width, 3);
+index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf;
+s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1);
+s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2);
+s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3);
trace_dp8393x_load_cam(index,
s->cam[index][0] >> 8, s->cam[index][0] & 0xff,
s->cam[index][1] >> 8, s->cam[index][1] & 0xff,
@@ -294,9 +305,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
}
/* Read CAM enable */
-address_space_read(>as, dp8393x_cdp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
-s->regs[SONIC_CE] = dp8393x_get(s, width, 0);
+s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0);
trace_dp8393x_load_cam_done(s->regs[SONIC_CE]);
/* Done */
@@ -312,14 +321,12 @@ static void dp8393x_do_read_rra(dp8393xState *s)
/* Read memory */
width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
size = sizeof(uint16_t) * 4 * width;
-address_space_read(>as, dp8393x_rrp(s),
- MEMTXATTRS_UNSPECIFIED, s->data, size);
/* Update SONIC registers */
-s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0);
-s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1);
-s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2);
-s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3);
+s->regs[SONIC_CRBA0] =
[PATCH v3 5/8] dp8393x: Migrate registers as array of uint16
The CAM registers are now arrays of 3 uint16_t. We can avoid using
the VMSTATE_BUFFER_UNSAFE() macro by using VMSTATE_UINT16_2DARRAY()
which is more appropriate.
Suggested-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 2c3047c8688..68516241a1f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -983,10 +983,10 @@ static void dp8393x_realize(DeviceState *dev, Error
**errp)
static const VMStateDescription vmstate_dp8393x = {
.name = "dp8393x",
-.version_id = 0,
-.minimum_version_id = 0,
+.version_id = 1,
+.minimum_version_id = 1,
.fields = (VMStateField []) {
-VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
+VMSTATE_UINT16_2DARRAY(cam, dp8393xState, 16, 3),
VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
--
2.31.1
[NOTFORMERGE PATCH v3 8/8] dp8393x: don't force 32-bit register access
From: Mark Cave-Ayland
Commit 3fe9a838ec "dp8393x: Always use 32-bit accesses" set
.impl.min_access_size
and .impl.max_access_size to 4 to try and fix the Linux jazzsonic driver which
uses
32-bit accesses.
The problem with forcing the register access to 32-bit in this way is that
since the
dp8393x uses 16-bit registers, a manual endian swap is required for devices on
big
endian machines with 32-bit accesses.
For both access sizes and machine endians the QEMU memory API can do the right
thing
automatically: all that is needed is to set .impl.min_access_size to 2 to
declare that
the dp8393x implements 16-bit registers.
Normally .impl.max_access_size should also be set to 2, however that doesn't
quite
work in this case since the register stride is specified using a (dynamic)
it_shift
property which is applied during the MMIO access itself. The effect of this is
that
for a 32-bit access the memory API performs 2 x 16-bit accesses, but the use of
it_shift within the MMIO access itself causes the register value to be repeated
in both
the top 16-bits and bottom 16-bits. The Linux jazzsonic driver expects the
stride to be
zero-extended up to access size and therefore fails to correctly detect the
dp8393x
device due to the extra data in the top 16-bits.
The solution here is to remove .impl.max_access_size so that the memory API will
correctly zero-extend the 16-bit registers to the access size up to and
including
it_shift. Since it_shift is never greater than 2 than this will always do the
right
thing for both 16-bit and 32-bit accesses regardless of the machine endian,
allowing
the manual endian swap code to be removed.
Signed-off-by: Mark Cave-Ayland
Fixes: 3fe9a838ec ("dp8393x: Always use 32-bit accesses")
Message-Id: <20210705214929.17222-2-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index a9224a5bc88..71c82a07c23 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -588,15 +588,14 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr,
unsigned int size)
trace_dp8393x_read(reg, reg_names[reg], val, size);
-return s->big_endian ? val << 16 : val;
+return val;
}
-static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
+static void dp8393x_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int size)
{
dp8393xState *s = opaque;
int reg = addr >> s->it_shift;
-uint32_t val = s->big_endian ? data >> 16 : data;
trace_dp8393x_write(reg, reg_names[reg], val, size);
@@ -677,11 +676,16 @@ static void dp8393x_write(void *opaque, hwaddr addr,
uint64_t data,
}
}
+/*
+ * Since .impl.max_access_size is effectively controlled by the it_shift
+ * property, leave it unspecified for now to allow the memory API to
+ * correctly zero extend the 16-bit register values to the access size up to
and
+ * including it_shift.
+ */
static const MemoryRegionOps dp8393x_ops = {
.read = dp8393x_read,
.write = dp8393x_write,
-.impl.min_access_size = 4,
-.impl.max_access_size = 4,
+.impl.min_access_size = 2,
.endianness = DEVICE_NATIVE_ENDIAN,
};
--
2.31.1
Re: [PATCH v3 2/8] dp8393x: Replace 0x40 magic value by SONIC_REG16_COUNT definition
Typo 'SONIC_REG_COUNT' in subject.
On Sat, Jul 10, 2021 at 7:50 PM Philippe Mathieu-Daudé wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/net/dp8393x.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 9118364aa33..d1e147a82a6 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -85,6 +85,7 @@ static const char *reg_names[] = {
> #define SONIC_MPT0x2e
> #define SONIC_MDT0x2f
> #define SONIC_DCR2 0x3f
> +#define SONIC_REG_COUNT 0x40
>
> #define SONIC_CR_HTX 0x0001
> #define SONIC_CR_TXP 0x0002
> @@ -158,7 +159,7 @@ struct dp8393xState {
>
> /* Registers */
> uint8_t cam[16][6];
> -uint16_t regs[0x40];
> +uint16_t regs[SONIC_REG_COUNT];
>
> /* Temporaries */
> uint8_t tx_buffer[0x1];
> @@ -972,7 +973,7 @@ static void dp8393x_realize(DeviceState *dev, Error
> **errp)
>
> address_space_init(>as, s->dma_mr, "dp8393x");
> memory_region_init_io(>mmio, OBJECT(dev), _ops, s,
> - "dp8393x-regs", 0x40 << s->it_shift);
> + "dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
>
> s->nic = qemu_new_nic(_dp83932_info, >conf,
>object_get_typename(OBJECT(dev)), dev->id, s);
> @@ -987,7 +988,7 @@ static const VMStateDescription vmstate_dp8393x = {
> .minimum_version_id = 0,
> .fields = (VMStateField []) {
> VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
> -VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
> +VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
> VMSTATE_END_OF_LIST()
> }
> };
> --
> 2.31.1
>
[PATCH v3 4/8] dp8393x: Store CAM registers as 16-bit
Per the DP83932C datasheet from July 1995:
4.0 SONIC Registers
4.1 THE CAM UNIT
The Content Addressable Memory (CAM) consists of sixteen
48-bit entries for complete address filtering of network
packets. Each entry corresponds to a 48-bit destination
address that is user programmable and can contain any
combination of Multicast or Physical addresses. Each entry
is partitioned into three 16-bit CAM cells accessible
through CAM Address Ports (CAP 2, CAP 1 and CAP 0) with
CAP0 corresponding to the least significant 16 bits of
the Destination Address and CAP2 corresponding to the
most significant bits.
Store the CAM registers as 16-bit as it simplifies the code.
There is no change in the migration stream.
Co-developed-by: Philippe Mathieu-Daudé
Co-developed-by: Mark Cave-Ayland
Signed-off-by: Philippe Mathieu-Daudé
---
Missing:
Signed-off-by: Mark Cave-Ayland
---
hw/net/dp8393x.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 64f3b0fc3ea..2c3047c8688 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -158,7 +158,7 @@ struct dp8393xState {
MemoryRegion mmio;
/* Registers */
-uint8_t cam[16][6];
+uint16_t cam[16][3];
uint16_t regs[SONIC_REG_COUNT];
/* Temporaries */
@@ -281,15 +281,13 @@ static void dp8393x_do_load_cam(dp8393xState *s)
address_space_read(>as, dp8393x_cdp(s),
MEMTXATTRS_UNSPECIFIED, s->data, size);
index = dp8393x_get(s, width, 0) & 0xf;
-s->cam[index][0] = dp8393x_get(s, width, 1) & 0xff;
-s->cam[index][1] = dp8393x_get(s, width, 1) >> 8;
-s->cam[index][2] = dp8393x_get(s, width, 2) & 0xff;
-s->cam[index][3] = dp8393x_get(s, width, 2) >> 8;
-s->cam[index][4] = dp8393x_get(s, width, 3) & 0xff;
-s->cam[index][5] = dp8393x_get(s, width, 3) >> 8;
-trace_dp8393x_load_cam(index, s->cam[index][0], s->cam[index][1],
- s->cam[index][2], s->cam[index][3],
- s->cam[index][4], s->cam[index][5]);
+s->cam[index][0] = dp8393x_get(s, width, 1);
+s->cam[index][1] = dp8393x_get(s, width, 2);
+s->cam[index][2] = dp8393x_get(s, width, 3);
+trace_dp8393x_load_cam(index,
+ s->cam[index][0] >> 8, s->cam[index][0] & 0xff,
+ s->cam[index][1] >> 8, s->cam[index][1] & 0xff,
+ s->cam[index][2] >> 8, s->cam[index][2] & 0xff);
/* Move to next entry */
s->regs[SONIC_CDC]--;
s->regs[SONIC_CDP] += size;
@@ -592,8 +590,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr,
unsigned int size)
case SONIC_CAP1:
case SONIC_CAP0:
if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-val = s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg) + 1]
<< 8;
-val |= s->cam[s->regs[SONIC_CEP] & 0xf][2 * (SONIC_CAP0 - reg)];
+val = s->cam[s->regs[SONIC_CEP] & 0xf][SONIC_CAP0 - reg];
}
break;
/* All other registers have no special contraints */
@@ -989,7 +986,7 @@ static const VMStateDescription vmstate_dp8393x = {
.version_id = 0,
.minimum_version_id = 0,
.fields = (VMStateField []) {
-VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
+VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 3 * 2),
VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
--
2.31.1
[PATCH v3 6/8] dp8393x: Store CRC using device configured endianess
Little-Endian CRC is dubious. The datasheet does not
specify it being little-endian. Use big-endian access
when the device is configured in such endianess.
(This is a theoretical bug fix.)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 68516241a1f..ac93412f70b 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -827,7 +827,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
/* Calculate the ethernet checksum */
-checksum = cpu_to_le32(crc32(0, buf, pkt_size));
+checksum = crc32(0, buf, pkt_size);
/* Put packet into RBA */
trace_dp8393x_receive_packet(dp8393x_crba(s));
@@ -837,8 +837,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
address += pkt_size;
/* Put frame checksum into RBA */
-address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
-, sizeof(checksum));
+if (s->big_endian) {
+address_space_stl_be(>as, address, checksum,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+} else {
+address_space_stl_le(>as, address, checksum,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+}
address += sizeof(checksum);
/* Pad short packets to keep pointers aligned */
--
2.31.1
[PATCH v3 3/8] dp8393x: Only shift the device registers mapping by 1 bit
The SONIC device only allows 16/32-bit accesses. From the machine
view (from the bus), it is only shifted by 1 bit. Another bit is
shifted, but it is an implementation detail of the QEMU model.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/m68k/q800.c | 2 +-
hw/mips/jazz.c | 2 +-
hw/net/dp8393x.c | 2 ++
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6817c8b5d1a..d78edfe40e8 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -316,7 +316,7 @@ static void q800_init(MachineState *machine)
dev = qdev_new("dp8393x");
qdev_set_nic_properties(dev, _table[0]);
-qdev_prop_set_uint8(dev, "it_shift", 2);
+qdev_prop_set_uint8(dev, "it_shift", 1);
qdev_prop_set_bit(dev, "big_endian", true);
object_property_set_link(OBJECT(dev), "dma_mr",
OBJECT(get_system_memory()), _abort);
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index d6183e18821..7f8680a189d 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -295,7 +295,7 @@ static void mips_jazz_init(MachineState *machine,
dev = qdev_new("dp8393x");
qdev_set_nic_properties(dev, nd);
-qdev_prop_set_uint8(dev, "it_shift", 2);
+qdev_prop_set_uint8(dev, "it_shift", 1);
qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
object_property_set_link(OBJECT(dev), "dma_mr",
OBJECT(rc4030_dma_mr), _abort);
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index d1e147a82a6..64f3b0fc3ea 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -971,6 +971,8 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
{
dp8393xState *s = DP8393X(dev);
+s->it_shift += 1; /* Registers are 16-bit wide */
+
address_space_init(>as, s->dma_mr, "dp8393x");
memory_region_init_io(>mmio, OBJECT(dev), _ops, s,
"dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
--
2.31.1
[PATCH v3 2/8] dp8393x: Replace 0x40 magic value by SONIC_REG16_COUNT definition
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 9118364aa33..d1e147a82a6 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -85,6 +85,7 @@ static const char *reg_names[] = {
#define SONIC_MPT0x2e
#define SONIC_MDT0x2f
#define SONIC_DCR2 0x3f
+#define SONIC_REG_COUNT 0x40
#define SONIC_CR_HTX 0x0001
#define SONIC_CR_TXP 0x0002
@@ -158,7 +159,7 @@ struct dp8393xState {
/* Registers */
uint8_t cam[16][6];
-uint16_t regs[0x40];
+uint16_t regs[SONIC_REG_COUNT];
/* Temporaries */
uint8_t tx_buffer[0x1];
@@ -972,7 +973,7 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
address_space_init(>as, s->dma_mr, "dp8393x");
memory_region_init_io(>mmio, OBJECT(dev), _ops, s,
- "dp8393x-regs", 0x40 << s->it_shift);
+ "dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
s->nic = qemu_new_nic(_dp83932_info, >conf,
object_get_typename(OBJECT(dev)), dev->id, s);
@@ -987,7 +988,7 @@ static const VMStateDescription vmstate_dp8393x = {
.minimum_version_id = 0,
.fields = (VMStateField []) {
VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
-VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
+VMSTATE_UINT16_ARRAY(regs, dp8393xState, SONIC_REG_COUNT),
VMSTATE_END_OF_LIST()
}
};
--
2.31.1
[PATCH v3 1/8] dp8393x: Replace address_space_rw(is_write=1) by address_space_write()
Replace address_space_rw(is_write=1) by address_space_write()
and remove pointless cast.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/dp8393x.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 11810c9b600..9118364aa33 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -816,8 +816,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
size = sizeof(uint16_t) * width;
address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width;
dp8393x_put(s, width, 0, 0);
-address_space_rw(>as, address, MEMTXATTRS_UNSPECIFIED,
- (uint8_t *)s->data, size, 1);
+address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
+s->data, size);
/* Move to next descriptor */
s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
@@ -846,8 +846,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const
uint8_t * buf,
/* Pad short packets to keep pointers aligned */
if (rx_len < padded_len) {
size = padded_len - rx_len;
-address_space_rw(>as, address, MEMTXATTRS_UNSPECIFIED,
-(uint8_t *)"\xFF\xFF\xFF", size, 1);
+address_space_write(>as, address, MEMTXATTRS_UNSPECIFIED,
+"\xFF\xFF\xFF", size);
address += size;
}
--
2.31.1
[PATCH v3 0/8] dp8393x: fixes and improvements
Hi Mark, Finn. This respin aims go group all the fixes sent/suggested on the list the last weeks around the dp8393x device. Mark, can you send your S-o-b for patches 4 & 6? The last 2 patches are included for Mark, but I don't plan to merge them without Finn's Ack, and apparently they require more work. Tested with NetBSD guest on Magnum Jazz. Based-on mips-next. Mark Cave-Ayland (1): NOTFORMERGE dp8393x: don't force 32-bit register access Philippe Mathieu-Daudé (7): dp8393x: Replace address_space_rw(is_write=1) by address_space_write() dp8393x: Replace 0x40 magic value by SONIC_REG16_COUNT definition dp8393x: Only shift the device registers mapping by 1 bit dp8393x: Store CAM registers as 16-bit dp8393x: Migrate registers as array of uint16 dp8393x: Store CRC using device configured endianess NOTFORMERGE dp8393x: Rewrite dp8393x_get() / dp8393x_put() hw/m68k/q800.c | 2 +- hw/mips/jazz.c | 2 +- hw/net/dp8393x.c | 219 +-- 3 files changed, 99 insertions(+), 124 deletions(-) -- 2.31.1
Re: [PATCH v2 1/3] iotests: Improve and rename test 291 to qemu-img-bitmap
On 7/9/21 6:39 PM, Eric Blake wrote:
Enhance the test to demonstrate existing less-than-stellar behavior of
qemu-img with a qcow2 image containing an inconsistent bitmap: we
don't diagnose the problem until after copying the entire image (a
potentially long time), and when we do diagnose the failure, we still
end up leaving an empty bitmap in the destination. This mess will be
cleaned up in the next patch.
While at it, rename the test now that we support useful iotest names,
and fix a missing newline in the error message thus exposed.
Much nicer with a meaningful name!
Signed-off-by: Eric Blake
---
block/dirty-bitmap.c | 2 +-
.../{291 => tests/qemu-img-bitmaps} | 19 +++-
.../{291.out => tests/qemu-img-bitmaps.out} | 48 ++-
3 files changed, 66 insertions(+), 3 deletions(-)
rename tests/qemu-iotests/{291 => tests/qemu-img-bitmaps} (88%)
rename tests/qemu-iotests/{291.out => tests/qemu-img-bitmaps.out} (75%)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 68d295d6e3ed..0ef46163e3ea 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -193,7 +193,7 @@ int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap,
uint32_t flags,
error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used",
bitmap->name);
error_append_hint(errp, "Try block-dirty-bitmap-remove to delete"
- " this bitmap from disk");
+ " this bitmap from disk\n");
return -1;
}
diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/tests/qemu-img-bitmaps
similarity index 88%
rename from tests/qemu-iotests/291
rename to tests/qemu-iotests/tests/qemu-img-bitmaps
index 20efb080a6c0..2f51651d0ce5 100755
--- a/tests/qemu-iotests/291
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps
@@ -3,7 +3,7 @@
#
# Test qemu-img bitmap handling
#
-# Copyright (C) 2018-2020 Red Hat, Inc.
+# Copyright (C) 2018-2021 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -27,11 +27,13 @@ status=1 # failure is the default!
_cleanup()
{
_cleanup_test_img
+_rm_test_img "$TEST_IMG.copy"
nbd_server_stop
}
trap "_cleanup; exit \$status" 0 1 2 3 15
# get standard environment, filters and checks
+cd ..
. ./common.rc
. ./common.filter
. ./common.nbd
@@ -129,6 +131,21 @@ $QEMU_IMG map --output=json --image-opts \
nbd_server_stop
+echo
+echo "=== Check handling of inconsistent bitmap ==="
+echo
+
+$QEMU_IO -c abort "$TEST_IMG" 2>/dev/null
+$QEMU_IMG bitmap --add "$TEST_IMG" b4
+$QEMU_IMG bitmap --remove "$TEST_IMG" b1
+_img_info --format-specific | _filter_irrelevant_img_info
+$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" &&
+echo "unexpected success"
+# Bug - even though we failed at conversion, we left a file around with
+# a bitmap marked as not corrupt
+TEST_IMG=$TEST_IMG.copy _img_info --format-specific \
+| _filter_irrelevant_img_info
+
# success, all done
echo '*** done'
rm -f $seq.full
diff --git a/tests/qemu-iotests/291.out
b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
similarity index 75%
rename from tests/qemu-iotests/291.out
rename to tests/qemu-iotests/tests/qemu-img-bitmaps.out
index 23411c0ff4d9..b762362075d1 100644
--- a/tests/qemu-iotests/291.out
+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out
@@ -1,4 +1,4 @@
-QA output created by 291
+QA output created by qemu-img-bitmaps
=== Initial image setup ===
@@ -115,4 +115,50 @@ Format specific information:
[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true,
"offset": OFFSET},
{ "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data":
false},
{ "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true,
"offset": OFFSET}]
+
+=== Check handling of inconsistent bitmap ===
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
+cluster_size: 65536
+backing file: TEST_DIR/t.IMGFMT.base
+backing file format: IMGFMT
+Format specific information:
+bitmaps:
+[0]:
+flags:
+[0]: in-use
+[1]: auto
+name: b2
+granularity: 65536
+[1]:
+flags:
+[0]: in-use
+name: b0
+granularity: 65536
+[2]:
+flags:
+[0]: auto
+name: b4
+granularity: 65536
+corrupt: false
+qemu-img: Failed to populate bitmap b0: Bitmap 'b0' is inconsistent and cannot
be used
+Try block-dirty-bitmap-remove to delete this bitmap from disk
In this context a more useful error message would be:
Try "qemu-img bitmap --remove" ...
but this is not a new issue.
+image: TEST_DIR/t.IMGFMT.copy
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
Re: retrying failed gitlab CI external jobs (travis)
On Sat, 10 Jul 2021 at 14:34, Peter Maydell wrote: > > Hi; we now have travis's CI hooked into gitlab, which is nice. However, > unlike the gitlab native CI jobs, there's no UI for saying "retry this" > when the "travis CI" part of the overall gitlab pipeline fails. > This is awkward because travis seems to be prone to intermittent failures. > Is there any way we can make the jobs retryable? Also on the subject of the external travis job, what determines when it runs? I would expect it to be run always, but if you look at https://gitlab.com/qemu-project/qemu/-/pipelines you can see that it didn't get run for the pipeline for staging commit fc32b91a. It's not just "doesn't run for staging" because it did run in the pipeline for staging ebd1f710. -- PMM
Re: [PATCH 00/41] tcg patch queue
On Sat, 10 Jul 2021 at 16:33, Richard Henderson wrote: > > The following changes since commit 05de778b5b8ab0b402996769117b88c7ea5c7c61: > > Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging > (2021-07-09 14:30:01 +0100) > > are available in the Git repository at: > > https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210710 > > for you to fetch changes up to ad1a706f386c2281adb0b09257d892735e405834: > > cpu: Add breakpoint tracepoints (2021-07-09 21:31:11 -0700) > > > Add translator_use_goto_tb. > Cleanups in prep of breakpoint fixes. > Misc fixes. > > Is this intended as a pullreq despite the "PATCH" in the subject? thanks -- PMM
[Bug 1905297] Re: Zynq7000 UART clock reset initialization
Any update? ** Changed in: qemu Status: Expired => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1905297 Title: Zynq7000 UART clock reset initialization Status in QEMU: Incomplete Bug description: Hello, we have come across a strange behavior in the Zynq7000 model of Qemu that seems to have been introduced between 5.0.0 and 5.1.0. The reset values of the SLCR register, in particular those for UART_CLK_CTRL, are such that the UARTs should find functional clocks. Up to 5.0.0 this was also the behavior that was implemented in QEMU. Starting in 5.1.0, we found that - despite correct reset values [1] - the UARTs are non-functional upon reset. Some investigation revealed that the cause for that is that the corresponding clocks are not properly initialized. Between 5.0.0 and 5.1.0, there are three commits that touch the Zynq UART clocks [2]. The last of them [3] triggers the faulty behavior. Attached is a patch that applies 5.2.0-rc2 and yields a functional UART. We surmise that the underlying device release issue runs much deeper, so it is only meant to identify the issue. [1] hw/misc/zynq_slcr.c static void zynq_slcr_reset_init(Object *obj, ResetType type) s->regs[R_UART_CLK_CTRL] = 0x3F03; [2] 38867cb7ec90..5b49a34c6800 [3] commit 5b49a34c6800d0cb917f959d8e75e5775f0fac3f (refs/bisect/bad) Author: Damien Hedde Date: Mon Apr 6 15:52:50 2020 +0200 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1905297/+subscriptions
[PATCH 38/41] accel/tcg: Split out log_cpu_exec
Split out CPU_LOG_EXEC and CPU_LOG_TB_CPU logging from
cpu_tb_exec to a new function. Perform only one pc
range check after a combined mask check.
Use the new function in lookup_tb_ptr. This enables
CPU_LOG_TB_CPU between indirectly chained tbs.
Signed-off-by: Richard Henderson
---
accel/tcg/cpu-exec.c | 61
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 0d92698030..67ed25beb9 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -175,6 +175,36 @@ static inline TranslationBlock *tb_lookup(CPUState *cpu,
target_ulong pc,
return tb;
}
+static inline void log_cpu_exec(target_ulong pc, CPUState *cpu,
+const TranslationBlock *tb)
+{
+if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_CPU | CPU_LOG_EXEC))
+&& qemu_log_in_addr_range(pc)) {
+
+qemu_log_mask(CPU_LOG_EXEC,
+ "Trace %d: %p [" TARGET_FMT_lx
+ "/" TARGET_FMT_lx "/%#x] %s\n",
+ cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc, tb->flags,
+ lookup_symbol(pc));
+
+#if defined(DEBUG_DISAS)
+if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
+FILE *logfile = qemu_log_lock();
+int flags = 0;
+
+if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
+flags |= CPU_DUMP_FPU;
+}
+#if defined(TARGET_I386)
+flags |= CPU_DUMP_CCOP;
+#endif
+log_cpu_state(cpu, flags);
+qemu_log_unlock(logfile);
+}
+#endif /* DEBUG_DISAS */
+}
+}
+
/**
* helper_lookup_tb_ptr: quick check for next tb
* @env: current cpu state
@@ -196,11 +226,9 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
if (tb == NULL) {
return tcg_code_gen_epilogue;
}
-qemu_log_mask_and_addr(CPU_LOG_EXEC, pc,
- "Chain %d: %p ["
- TARGET_FMT_lx "/" TARGET_FMT_lx "/%#x] %s\n",
- cpu->cpu_index, tb->tc.ptr, cs_base, pc, flags,
- lookup_symbol(pc));
+
+log_cpu_exec(pc, cpu, tb);
+
return tb->tc.ptr;
}
@@ -222,28 +250,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int
*tb_exit)
TranslationBlock *last_tb;
const void *tb_ptr = itb->tc.ptr;
-qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc,
- "Trace %d: %p ["
- TARGET_FMT_lx "/" TARGET_FMT_lx "/%#x] %s\n",
- cpu->cpu_index, itb->tc.ptr,
- itb->cs_base, itb->pc, itb->flags,
- lookup_symbol(itb->pc));
-
-#if defined(DEBUG_DISAS)
-if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
-&& qemu_log_in_addr_range(itb->pc)) {
-FILE *logfile = qemu_log_lock();
-int flags = 0;
-if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
-flags |= CPU_DUMP_FPU;
-}
-#if defined(TARGET_I386)
-flags |= CPU_DUMP_CCOP;
-#endif
-log_cpu_state(cpu, flags);
-qemu_log_unlock(logfile);
-}
-#endif /* DEBUG_DISAS */
+log_cpu_exec(itb->pc, cpu, itb);
qemu_thread_jit_execute();
ret = tcg_qemu_tb_exec(env, tb_ptr);
--
2.25.1
[PATCH 37/41] accel/tcg: Move tb_lookup to cpu-exec.c
Now that we've moved helper_lookup_tb_ptr, the only user
of tb-lookup.h is cpu-exec.c; merge the contents in.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
accel/tcg/tb-lookup.h | 49 ---
accel/tcg/cpu-exec.c | 31 ++-
2 files changed, 30 insertions(+), 50 deletions(-)
delete mode 100644 accel/tcg/tb-lookup.h
diff --git a/accel/tcg/tb-lookup.h b/accel/tcg/tb-lookup.h
deleted file mode 100644
index 9c9e0079da..00
--- a/accel/tcg/tb-lookup.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Copyright (C) 2017, Emilio G. Cota
- *
- * License: GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-#ifndef EXEC_TB_LOOKUP_H
-#define EXEC_TB_LOOKUP_H
-
-#ifdef NEED_CPU_H
-#include "cpu.h"
-#else
-#include "exec/poison.h"
-#endif
-
-#include "exec/exec-all.h"
-#include "tb-hash.h"
-
-/* Might cause an exception, so have a longjmp destination ready */
-static inline TranslationBlock *tb_lookup(CPUState *cpu, target_ulong pc,
- target_ulong cs_base,
- uint32_t flags, uint32_t cflags)
-{
-TranslationBlock *tb;
-uint32_t hash;
-
-/* we should never be trying to look up an INVALID tb */
-tcg_debug_assert(!(cflags & CF_INVALID));
-
-hash = tb_jmp_cache_hash_func(pc);
-tb = qatomic_rcu_read(>tb_jmp_cache[hash]);
-
-if (likely(tb &&
- tb->pc == pc &&
- tb->cs_base == cs_base &&
- tb->flags == flags &&
- tb->trace_vcpu_dstate == *cpu->trace_dstate &&
- tb_cflags(tb) == cflags)) {
-return tb;
-}
-tb = tb_htable_lookup(cpu, pc, cs_base, flags, cflags);
-if (tb == NULL) {
-return NULL;
-}
-qatomic_set(>tb_jmp_cache[hash], tb);
-return tb;
-}
-
-#endif /* EXEC_TB_LOOKUP_H */
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index fb6668606f..0d92698030 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -40,7 +40,6 @@
#include "sysemu/replay.h"
#include "exec/helper-proto.h"
#include "tb-hash.h"
-#include "tb-lookup.h"
#include "tb-context.h"
#include "internal.h"
@@ -146,6 +145,36 @@ static void init_delay_params(SyncClocks *sc, const
CPUState *cpu)
}
#endif /* CONFIG USER ONLY */
+/* Might cause an exception, so have a longjmp destination ready */
+static inline TranslationBlock *tb_lookup(CPUState *cpu, target_ulong pc,
+ target_ulong cs_base,
+ uint32_t flags, uint32_t cflags)
+{
+TranslationBlock *tb;
+uint32_t hash;
+
+/* we should never be trying to look up an INVALID tb */
+tcg_debug_assert(!(cflags & CF_INVALID));
+
+hash = tb_jmp_cache_hash_func(pc);
+tb = qatomic_rcu_read(>tb_jmp_cache[hash]);
+
+if (likely(tb &&
+ tb->pc == pc &&
+ tb->cs_base == cs_base &&
+ tb->flags == flags &&
+ tb->trace_vcpu_dstate == *cpu->trace_dstate &&
+ tb_cflags(tb) == cflags)) {
+return tb;
+}
+tb = tb_htable_lookup(cpu, pc, cs_base, flags, cflags);
+if (tb == NULL) {
+return NULL;
+}
+qatomic_set(>tb_jmp_cache[hash], tb);
+return tb;
+}
+
/**
* helper_lookup_tb_ptr: quick check for next tb
* @env: current cpu state
--
2.25.1
[PATCH 36/41] accel/tcg: Move helper_lookup_tb_ptr to cpu-exec.c
This will allow additional code sharing.
No functional change.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
accel/tcg/cpu-exec.c| 30 ++
accel/tcg/tcg-runtime.c | 22 --
2 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index ad1279d2ed..fb6668606f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -38,6 +38,7 @@
#include "exec/cpu-all.h"
#include "sysemu/cpu-timers.h"
#include "sysemu/replay.h"
+#include "exec/helper-proto.h"
#include "tb-hash.h"
#include "tb-lookup.h"
#include "tb-context.h"
@@ -145,6 +146,35 @@ static void init_delay_params(SyncClocks *sc, const
CPUState *cpu)
}
#endif /* CONFIG USER ONLY */
+/**
+ * helper_lookup_tb_ptr: quick check for next tb
+ * @env: current cpu state
+ *
+ * Look for an existing TB matching the current cpu state.
+ * If found, return the code pointer. If not found, return
+ * the tcg epilogue so that we return into cpu_tb_exec.
+ */
+const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
+{
+CPUState *cpu = env_cpu(env);
+TranslationBlock *tb;
+target_ulong cs_base, pc;
+uint32_t flags;
+
+cpu_get_tb_cpu_state(env, , _base, );
+
+tb = tb_lookup(cpu, pc, cs_base, flags, curr_cflags(cpu));
+if (tb == NULL) {
+return tcg_code_gen_epilogue;
+}
+qemu_log_mask_and_addr(CPU_LOG_EXEC, pc,
+ "Chain %d: %p ["
+ TARGET_FMT_lx "/" TARGET_FMT_lx "/%#x] %s\n",
+ cpu->cpu_index, tb->tc.ptr, cs_base, pc, flags,
+ lookup_symbol(pc));
+return tb->tc.ptr;
+}
+
/* Execute a TB, and fix up the CPU state afterwards if necessary */
/*
* Disable CFI checks.
diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c
index 66ac830e2f..e4e030043f 100644
--- a/accel/tcg/tcg-runtime.c
+++ b/accel/tcg/tcg-runtime.c
@@ -30,7 +30,6 @@
#include "disas/disas.h"
#include "exec/log.h"
#include "tcg/tcg.h"
-#include "tb-lookup.h"
/* 32-bit helpers */
@@ -145,27 +144,6 @@ uint64_t HELPER(ctpop_i64)(uint64_t arg)
return ctpop64(arg);
}
-const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
-{
-CPUState *cpu = env_cpu(env);
-TranslationBlock *tb;
-target_ulong cs_base, pc;
-uint32_t flags;
-
-cpu_get_tb_cpu_state(env, , _base, );
-
-tb = tb_lookup(cpu, pc, cs_base, flags, curr_cflags(cpu));
-if (tb == NULL) {
-return tcg_code_gen_epilogue;
-}
-qemu_log_mask_and_addr(CPU_LOG_EXEC, pc,
- "Chain %d: %p ["
- TARGET_FMT_lx "/" TARGET_FMT_lx "/%#x] %s\n",
- cpu->cpu_index, tb->tc.ptr, cs_base, pc, flags,
- lookup_symbol(pc));
-return tb->tc.ptr;
-}
-
void HELPER(exit_atomic)(CPUArchState *env)
{
cpu_loop_exit_atomic(env_cpu(env), GETPC());
--
2.25.1
[PATCH 34/41] tcg: Fix prologue disassembly
In tcg_region_prologue_set, we reset TCGContext.code_gen_ptr.
So do that after we've used it to dump the prologue contents.
Fixes: b0a0794a0f16
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 4dd4084419..ed86a70b79 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -752,8 +752,6 @@ void tcg_prologue_init(TCGContext *s)
(uintptr_t)s->code_buf, prologue_size);
#endif
-tcg_region_prologue_set(s);
-
#ifdef DEBUG_DISAS
if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
FILE *logfile = qemu_log_lock();
@@ -795,6 +793,8 @@ void tcg_prologue_init(TCGContext *s)
tcg_debug_assert(tcg_code_gen_epilogue != NULL);
}
#endif
+
+tcg_region_prologue_set(s);
}
void tcg_func_start(TCGContext *s)
--
2.25.1
[PATCH 31/41] target/tricore: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Bastian Koppelmann
Signed-off-by: Richard Henderson
---
target/tricore/translate.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 2a814263de..09465ea013 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3225,19 +3225,6 @@ static inline void gen_save_pc(target_ulong pc)
tcg_gen_movi_tl(cpu_PC, pc);
}
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
-{
-if (unlikely(ctx->base.singlestep_enabled)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
static void generate_qemu_excp(DisasContext *ctx, int excp)
{
TCGv_i32 tmp = tcg_const_i32(excp);
@@ -3246,9 +3233,9 @@ static void generate_qemu_excp(DisasContext *ctx, int
excp)
tcg_temp_free(tmp);
}
-static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
+static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
{
-if (use_goto_tb(ctx, dest)) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
gen_save_pc(dest);
tcg_gen_exit_tb(ctx->base.tb, n);
--
2.25.1
[PATCH 40/41] tcg: Remove TCG_TARGET_HAS_goto_ptr
Since 6eea04347eb6, all tcg backends support goto_ptr.
Remove the conditional, making support mandatory.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
include/tcg/tcg-opc.h| 3 +--
tcg/aarch64/tcg-target.h | 1 -
tcg/arm/tcg-target.h | 1 -
tcg/i386/tcg-target.h| 1 -
tcg/mips/tcg-target.h| 1 -
tcg/ppc/tcg-target.h | 1 -
tcg/riscv/tcg-target.h | 1 -
tcg/s390/tcg-target.h| 1 -
tcg/sparc/tcg-target.h | 1 -
tcg/tci/tcg-target.h | 1 -
tcg/tcg-op.c | 2 +-
tcg/tcg.c| 8 ++--
12 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/include/tcg/tcg-opc.h b/include/tcg/tcg-opc.h
index 993992373e..675873e200 100644
--- a/include/tcg/tcg-opc.h
+++ b/include/tcg/tcg-opc.h
@@ -194,8 +194,7 @@ DEF(insn_start, 0, 0, TLADDR_ARGS * TARGET_INSN_START_WORDS,
TCG_OPF_NOT_PRESENT)
DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
-DEF(goto_ptr, 0, 1, 0,
-TCG_OPF_BB_EXIT | TCG_OPF_BB_END | IMPL(TCG_TARGET_HAS_goto_ptr))
+DEF(goto_ptr, 0, 1, 0, TCG_OPF_BB_EXIT | TCG_OPF_BB_END)
DEF(plugin_cb_start, 0, 0, 3, TCG_OPF_NOT_PRESENT)
DEF(plugin_cb_end, 0, 0, 0, TCG_OPF_NOT_PRESENT)
diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index 551baf8da3..7a93ac8023 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -88,7 +88,6 @@ typedef enum {
#define TCG_TARGET_HAS_mulsh_i320
#define TCG_TARGET_HAS_extrl_i64_i320
#define TCG_TARGET_HAS_extrh_i64_i320
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_qemu_st8_i32 0
#define TCG_TARGET_HAS_div_i64 1
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index 95fcef33bc..d113b7f8db 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -148,7 +148,6 @@ extern bool use_neon_instructions;
#define TCG_TARGET_HAS_mulsh_i320
#define TCG_TARGET_HAS_div_i32 use_idiv_instructions
#define TCG_TARGET_HAS_rem_i32 0
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump 0
#define TCG_TARGET_HAS_qemu_st8_i32 0
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index ac10066c3e..b00a6da293 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -135,7 +135,6 @@ extern bool have_movbe;
#define TCG_TARGET_HAS_muls2_i321
#define TCG_TARGET_HAS_muluh_i320
#define TCG_TARGET_HAS_mulsh_i320
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump 1
#if TCG_TARGET_REG_BITS == 64
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index e81e824cab..3a62055f04 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -136,7 +136,6 @@ extern bool use_mips32r2_instructions;
#define TCG_TARGET_HAS_muluh_i321
#define TCG_TARGET_HAS_mulsh_i321
#define TCG_TARGET_HAS_bswap32_i32 1
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump 1
#if TCG_TARGET_REG_BITS == 64
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index c13ed5640a..0943192cde 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -108,7 +108,6 @@ extern bool have_vsx;
#define TCG_TARGET_HAS_muls2_i320
#define TCG_TARGET_HAS_muluh_i321
#define TCG_TARGET_HAS_mulsh_i321
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump 1
#define TCG_TARGET_HAS_qemu_st8_i32 0
diff --git a/tcg/riscv/tcg-target.h b/tcg/riscv/tcg-target.h
index 87ea94666b..ef78b99e98 100644
--- a/tcg/riscv/tcg-target.h
+++ b/tcg/riscv/tcg-target.h
@@ -85,7 +85,6 @@ typedef enum {
#define TCG_TARGET_CALL_STACK_OFFSET0
/* optional instructions */
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_movcond_i32 0
#define TCG_TARGET_HAS_div_i32 1
#define TCG_TARGET_HAS_rem_i32 1
diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
index b04b72b7eb..2e4ede2ea2 100644
--- a/tcg/s390/tcg-target.h
+++ b/tcg/s390/tcg-target.h
@@ -98,7 +98,6 @@ extern uint64_t s390_facilities;
#define TCG_TARGET_HAS_mulsh_i32 0
#define TCG_TARGET_HAS_extrl_i64_i32 0
#define TCG_TARGET_HAS_extrh_i64_i32 0
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump(s390_facilities & FACILITY_GEN_INST_EXT)
#define TCG_TARGET_HAS_qemu_st8_i32 0
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 86bb9a2d39..c050763049 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -121,7 +121,6 @@ extern bool use_vis3_instructions;
#define TCG_TARGET_HAS_muls2_i321
#define TCG_TARGET_HAS_muluh_i320
#define TCG_TARGET_HAS_mulsh_i320
-#define TCG_TARGET_HAS_goto_ptr 1
#define TCG_TARGET_HAS_direct_jump 1
#define TCG_TARGET_HAS_qemu_st8_i32 0
diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
[PATCH 28/41] target/s390x: Remove use_exit_tb
We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block").
In use_goto_tb, the check for singlestep_enabled is in the
generic translator_use_goto_tb. In s390x_tr_tb_stop, the
check for singlestep_enabled is in the preceding do_debug test.
Which leaves only FLAG_MASK_PER: fold that test alone into
the two callers of use_exit tb.
Reviewed-by: David Hildenbrand
Signed-off-by: Richard Henderson
---
target/s390x/translate.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 767e77ca19..0cfe29d227 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -684,16 +684,9 @@ static void gen_op_calc_cc(DisasContext *s)
set_cc_static(s);
}
-static bool use_exit_tb(DisasContext *s)
-{
-return s->base.singlestep_enabled ||
-(tb_cflags(s->base.tb) & CF_LAST_IO) ||
-(s->base.tb->flags & FLAG_MASK_PER);
-}
-
static bool use_goto_tb(DisasContext *s, uint64_t dest)
{
-if (unlikely(use_exit_tb(s))) {
+if (unlikely(s->base.tb->flags & FLAG_MASK_PER)) {
return false;
}
return translator_use_goto_tb(>base, dest);
@@ -6633,7 +6626,7 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase,
CPUState *cs)
/* Exit the TB, either by raising a debug exception or by return. */
if (dc->do_debug) {
gen_exception(EXCP_DEBUG);
-} else if (use_exit_tb(dc) ||
+} else if ((dc->base.tb->flags & FLAG_MASK_PER) ||
dc->base.is_jmp == DISAS_PC_STALE_NOCHAIN) {
tcg_gen_exit_tb(NULL, 0);
} else {
--
2.25.1
[PATCH 25/41] target/riscv: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Alistair Francis
Signed-off-by: Richard Henderson
---
target/riscv/translate.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 62a7d7e4c7..deda0c8a44 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -168,29 +168,11 @@ static void gen_exception_inst_addr_mis(DisasContext *ctx)
generate_exception_mtval(ctx, RISCV_EXCP_INST_ADDR_MIS);
}
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
-{
-if (unlikely(ctx->base.singlestep_enabled)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
{
-if (use_goto_tb(ctx, dest)) {
-/* chaining is only allowed when the jump is to the same page */
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_tl(cpu_pc, dest);
-
-/* No need to check for single stepping here as use_goto_tb() will
- * return false in case of single stepping.
- */
tcg_gen_exit_tb(ctx->base.tb, n);
} else {
tcg_gen_movi_tl(cpu_pc, dest);
--
2.25.1
[PATCH 39/41] accel/tcg: Log tb->cflags with -d exec
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
accel/tcg/cpu-exec.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 67ed25beb9..e22bcb99f7 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -183,9 +183,9 @@ static inline void log_cpu_exec(target_ulong pc, CPUState
*cpu,
qemu_log_mask(CPU_LOG_EXEC,
"Trace %d: %p [" TARGET_FMT_lx
- "/" TARGET_FMT_lx "/%#x] %s\n",
- cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc, tb->flags,
- lookup_symbol(pc));
+ "/" TARGET_FMT_lx "/%08x/%08x] %s\n",
+ cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc,
+ tb->flags, tb->cflags, lookup_symbol(pc));
#if defined(DEBUG_DISAS)
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
--
2.25.1
[PATCH 23/41] target/openrisc: Use translator_use_goto_tb
Reorder the control statements to allow using the page boundary
check from translator_use_goto_tb().
Reviewed-by: Stafford Horne
Signed-off-by: Richard Henderson
---
target/openrisc/translate.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 5db63d7609..37c3e3e0a3 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1719,16 +1719,17 @@ static void openrisc_tr_tb_stop(DisasContextBase
*dcbase, CPUState *cs)
/* fallthru */
case DISAS_TOO_MANY:
-if (unlikely(dc->base.singlestep_enabled)) {
-tcg_gen_movi_tl(cpu_pc, jmp_dest);
-gen_exception(dc, EXCP_DEBUG);
-} else if ((dc->base.pc_first ^ jmp_dest) & TARGET_PAGE_MASK) {
-tcg_gen_movi_tl(cpu_pc, jmp_dest);
-tcg_gen_lookup_and_goto_ptr();
-} else {
+if (translator_use_goto_tb(>base, jmp_dest)) {
tcg_gen_goto_tb(0);
tcg_gen_movi_tl(cpu_pc, jmp_dest);
tcg_gen_exit_tb(dc->base.tb, 0);
+break;
+}
+tcg_gen_movi_tl(cpu_pc, jmp_dest);
+if (unlikely(dc->base.singlestep_enabled)) {
+gen_exception(dc, EXCP_DEBUG);
+} else {
+tcg_gen_lookup_and_goto_ptr();
}
break;
--
2.25.1
[PATCH 32/41] target/tricore: Use tcg_gen_lookup_and_goto_ptr
The non-single-step case of gen_goto_tb may use
tcg_gen_lookup_and_goto_ptr to indirectly chain.
Reviewed-by: Bastian Koppelmann
Signed-off-by: Richard Henderson
---
target/tricore/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 09465ea013..865020754d 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3243,8 +3243,9 @@ static void gen_goto_tb(DisasContext *ctx, int n,
target_ulong dest)
gen_save_pc(dest);
if (ctx->base.singlestep_enabled) {
generate_qemu_excp(ctx, EXCP_DEBUG);
+} else {
+tcg_gen_lookup_and_goto_ptr();
}
-tcg_gen_exit_tb(NULL, 0);
}
}
--
2.25.1
[PATCH 22/41] target/nios2: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/nios2/translate.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 930f3d3395..17742cebc7 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -150,24 +150,11 @@ static void t_gen_helper_raise_exception(DisasContext *dc,
dc->base.is_jmp = DISAS_NORETURN;
}
-static bool use_goto_tb(DisasContext *dc, uint32_t dest)
-{
-if (unlikely(dc->base.singlestep_enabled)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (dc->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
static void gen_goto_tb(DisasContext *dc, int n, uint32_t dest)
{
const TranslationBlock *tb = dc->base.tb;
-if (use_goto_tb(dc, dest)) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_tl(cpu_R[R_PC], dest);
tcg_gen_exit_tb(tb, n);
--
2.25.1
[PATCH 24/41] target/ppc: Use translator_use_goto_tb
Reviewed-by: Luis Pires
Signed-off-by: Richard Henderson
---
target/ppc/translate.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 07d79acc08..0ad601793c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4301,15 +4301,7 @@ static inline void gen_update_cfar(DisasContext *ctx,
target_ulong nip)
static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
{
-if (unlikely(ctx->singlestep_enabled)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
+return translator_use_goto_tb(>base, dest);
}
static void gen_lookup_and_goto_ptr(DisasContext *ctx)
--
2.25.1
[PATCH 18/41] target/m68k: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Acked-by: Laurent Vivier
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/m68k/translate.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 348fc6e844..1fee04b8dd 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1519,16 +1519,6 @@ static void gen_exit_tb(DisasContext *s)
} \
} while (0)
-static inline bool use_goto_tb(DisasContext *s, uint32_t dest)
-{
-#ifndef CONFIG_USER_ONLY
-return (s->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)
-|| (s->base.pc_next & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
/* Generate a jump to an immediate address. */
static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
{
@@ -1536,7 +1526,7 @@ static void gen_jmp_tb(DisasContext *s, int n, uint32_t
dest)
update_cc_op(s);
tcg_gen_movi_i32(QREG_PC, dest);
gen_singlestep_exception(s);
-} else if (use_goto_tb(s, dest)) {
+} else if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_i32(QREG_PC, dest);
tcg_gen_exit_tb(s->base.tb, n);
--
2.25.1
[PATCH 35/41] target/i386: Use cpu_breakpoint_test in breakpoint_handler
The loop is performing a simple boolean test for the existence
of a BP_CPU breakpoint at EIP. Plus it gets the iteration wrong,
if we happen to have a BP_GDB breakpoint at the same address.
We have a function for this: cpu_breakpoint_test.
Signed-off-by: Richard Henderson
Reviewed-by: Eduardo Habkost
Message-Id: <20210620062317.1399034-1-richard.hender...@linaro.org>
---
target/i386/tcg/sysemu/bpt_helper.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/sysemu/bpt_helper.c
b/target/i386/tcg/sysemu/bpt_helper.c
index 9bdf7e170b..f1fb479ad9 100644
--- a/target/i386/tcg/sysemu/bpt_helper.c
+++ b/target/i386/tcg/sysemu/bpt_helper.c
@@ -210,7 +210,6 @@ void breakpoint_handler(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
CPUX86State *env = >env;
-CPUBreakpoint *bp;
if (cs->watchpoint_hit) {
if (cs->watchpoint_hit->flags & BP_CPU) {
@@ -222,14 +221,9 @@ void breakpoint_handler(CPUState *cs)
}
}
} else {
-QTAILQ_FOREACH(bp, >breakpoints, entry) {
-if (bp->pc == env->eip) {
-if (bp->flags & BP_CPU) {
-check_hw_breakpoints(env, true);
-raise_exception(env, EXCP01_DB);
-}
-break;
-}
+if (cpu_breakpoint_test(cs, env->eip, BP_CPU)) {
+check_hw_breakpoints(env, true);
+raise_exception(env, EXCP01_DB);
}
}
}
--
2.25.1
[PATCH 20/41] target/mips: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/mips/tcg/translate.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index cb82426f66..5cd3e7d8dd 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4947,22 +4947,9 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
tcg_temp_free(t1);
}
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
+static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
{
-if (unlikely(ctx->base.singlestep_enabled)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
-static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
-{
-if (use_goto_tb(ctx, dest)) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
gen_save_pc(dest);
tcg_gen_exit_tb(ctx->base.tb, n);
--
2.25.1
[PATCH 17/41] target/i386: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/i386/tcg/translate.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 85b00a6945..37a66b4097 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2313,21 +2313,11 @@ static inline int insn_const_size(MemOp ot)
}
}
-static inline bool use_goto_tb(DisasContext *s, target_ulong pc)
-{
-#ifndef CONFIG_USER_ONLY
-return (pc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) ||
- (pc & TARGET_PAGE_MASK) == (s->pc_start & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
-static inline void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
+static void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
{
target_ulong pc = s->cs_base + eip;
-if (use_goto_tb(s, pc)) {
+if (translator_use_goto_tb(>base, pc)) {
/* jump to same page: we can use a direct jump */
tcg_gen_goto_tb(tb_num);
gen_jmp_im(s, eip);
--
2.25.1
[PATCH 19/41] target/microblaze: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/microblaze/translate.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 5dfb08d49f..c68a84a219 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -124,15 +124,6 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t
esr_ec)
gen_raise_exception_sync(dc, EXCP_HW_EXCP);
}
-static inline bool use_goto_tb(DisasContext *dc, target_ulong dest)
-{
-#ifndef CONFIG_USER_ONLY
-return (dc->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
{
if (dc->base.singlestep_enabled) {
@@ -140,7 +131,7 @@ static void gen_goto_tb(DisasContext *dc, int n,
target_ulong dest)
tcg_gen_movi_i32(cpu_pc, dest);
gen_helper_raise_exception(cpu_env, tmp);
tcg_temp_free_i32(tmp);
-} else if (use_goto_tb(dc, dest)) {
+} else if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_i32(cpu_pc, dest);
tcg_gen_exit_tb(dc->base.tb, n);
--
2.25.1
[PATCH 30/41] target/sparc: Use translator_use_goto_tb
Reviewed-by: Mark Cave-Ayland
Signed-off-by: Richard Henderson
---
target/sparc/translate.c | 19 +--
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index f3fe7a0369..e530cb4aa8 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -338,23 +338,14 @@ static inline TCGv gen_dest_gpr(DisasContext *dc, int reg)
}
}
-static inline bool use_goto_tb(DisasContext *s, target_ulong pc,
- target_ulong npc)
+static bool use_goto_tb(DisasContext *s, target_ulong pc, target_ulong npc)
{
-if (unlikely(s->base.singlestep_enabled || singlestep)) {
-return false;
-}
-
-#ifndef CONFIG_USER_ONLY
-return (pc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) &&
- (npc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
+return translator_use_goto_tb(>base, pc) &&
+ translator_use_goto_tb(>base, npc);
}
-static inline void gen_goto_tb(DisasContext *s, int tb_num,
- target_ulong pc, target_ulong npc)
+static void gen_goto_tb(DisasContext *s, int tb_num,
+target_ulong pc, target_ulong npc)
{
if (use_goto_tb(s, pc, npc)) {
/* jump to same page: we can use a direct jump */
--
2.25.1
[PATCH 16/41] target/hppa: Use translator_use_goto_tb
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/hppa/translate.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 424ec3252e..835120c038 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -816,10 +816,7 @@ static bool gen_illegal(DisasContext *ctx)
static bool use_goto_tb(DisasContext *ctx, target_ureg dest)
{
-/* Suppress goto_tb for page crossing, IO, or single-steping. */
-return !(((ctx->base.pc_first ^ dest) & TARGET_PAGE_MASK)
- || (tb_cflags(ctx->base.tb) & CF_LAST_IO)
- || ctx->base.singlestep_enabled);
+return translator_use_goto_tb(>base, dest);
}
/* If the next insn is to be nullified, and it's on the same page,
--
2.25.1
[PATCH 26/41] target/rx: Use translator_use_goto_tb
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/rx/translate.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/target/rx/translate.c b/target/rx/translate.c
index 22a15ee11d..23a626438a 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -142,18 +142,9 @@ void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
}
}
-static bool use_goto_tb(DisasContext *dc, target_ulong dest)
-{
-if (unlikely(dc->base.singlestep_enabled)) {
-return false;
-} else {
-return true;
-}
-}
-
static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
{
-if (use_goto_tb(dc, dest)) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_i32(cpu_pc, dest);
tcg_gen_exit_tb(dc->base.tb, n);
--
2.25.1
[PATCH 14/41] target/avr: Mark some helpers noreturn
All of these helpers end with cpu_loop_exit. Reviewed-by: Michael Rolnik Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/avr/helper.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/avr/helper.h b/target/avr/helper.h index 8e1ae7fda0..4d02e648fa 100644 --- a/target/avr/helper.h +++ b/target/avr/helper.h @@ -19,10 +19,10 @@ */ DEF_HELPER_1(wdr, void, env) -DEF_HELPER_1(debug, void, env) -DEF_HELPER_1(break, void, env) -DEF_HELPER_1(sleep, void, env) -DEF_HELPER_1(unsupported, void, env) +DEF_HELPER_1(debug, noreturn, env) +DEF_HELPER_1(break, noreturn, env) +DEF_HELPER_1(sleep, noreturn, env) +DEF_HELPER_1(unsupported, noreturn, env) DEF_HELPER_3(outb, void, env, i32, i32) DEF_HELPER_2(inb, tl, env, i32) DEF_HELPER_3(fullwr, void, env, i32, i32) -- 2.25.1
[PATCH 41/41] cpu: Add breakpoint tracepoints
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
cpu.c| 13 +
trace-events | 5 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/cpu.c b/cpu.c
index 164fefeaa3..83059537d7 100644
--- a/cpu.c
+++ b/cpu.c
@@ -38,6 +38,7 @@
#include "exec/translate-all.h"
#include "exec/log.h"
#include "hw/core/accel-cpu.h"
+#include "trace/trace-root.h"
uintptr_t qemu_host_page_size;
intptr_t qemu_host_page_mask;
@@ -285,6 +286,8 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int
flags,
if (breakpoint) {
*breakpoint = bp;
}
+
+trace_breakpoint_insert(cpu->cpu_index, pc, flags);
return 0;
}
@@ -303,13 +306,14 @@ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int
flags)
}
/* Remove a specific breakpoint by reference. */
-void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint)
+void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *bp)
{
-QTAILQ_REMOVE(>breakpoints, breakpoint, entry);
+QTAILQ_REMOVE(>breakpoints, bp, entry);
-breakpoint_invalidate(cpu, breakpoint->pc);
+breakpoint_invalidate(cpu, bp->pc);
-g_free(breakpoint);
+trace_breakpoint_remove(cpu->cpu_index, bp->pc, bp->flags);
+g_free(bp);
}
/* Remove all matching breakpoints. */
@@ -337,6 +341,7 @@ void cpu_single_step(CPUState *cpu, int enabled)
/* XXX: only flush what is necessary */
tb_flush(cpu);
}
+trace_breakpoint_singlestep(cpu->cpu_index, enabled);
}
}
diff --git a/trace-events b/trace-events
index 765fe251e6..c4cca29939 100644
--- a/trace-events
+++ b/trace-events
@@ -25,6 +25,11 @@
#
# The should be a sprintf()-compatible format string.
+# cpu.c
+breakpoint_insert(int cpu_index, uint64_t pc, int flags) "cpu=%d pc=0x%"
PRIx64 " flags=0x%x"
+breakpoint_remove(int cpu_index, uint64_t pc, int flags) "cpu=%d pc=0x%"
PRIx64 " flags=0x%x"
+breakpoint_singlestep(int cpu_index, int enabled) "cpu=%d enable=%d"
+
# dma-helpers.c
dma_blk_io(void *dbs, void *bs, int64_t offset, bool to_dev) "dbs=%p bs=%p
offset=%" PRId64 " to_dev=%d"
dma_aio_cancel(void *dbs) "dbs=%p"
--
2.25.1
[PATCH 11/41] target/arm: Use translator_use_goto_tb for aarch64
We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block"),
and gdbstub singlestep is handled by the generic function.
Drop the unused 'n' argument to use_goto_tb.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/translate-a64.c | 25 +
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index a6dd9ec701..ca11a5fecd 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -385,35 +385,20 @@ static void gen_step_complete_exception(DisasContext *s)
s->base.is_jmp = DISAS_NORETURN;
}
-static inline bool use_goto_tb(DisasContext *s, int n, uint64_t dest)
+static inline bool use_goto_tb(DisasContext *s, uint64_t dest)
{
-/* No direct tb linking with singlestep (either QEMU's or the ARM
- * debug architecture kind) or deterministic io
- */
-if (s->base.singlestep_enabled || s->ss_active ||
-(tb_cflags(s->base.tb) & CF_LAST_IO)) {
+if (s->ss_active) {
return false;
}
-
-#ifndef CONFIG_USER_ONLY
-/* Only link tbs from inside the same guest page */
-if ((s->base.tb->pc & TARGET_PAGE_MASK) != (dest & TARGET_PAGE_MASK)) {
-return false;
-}
-#endif
-
-return true;
+return translator_use_goto_tb(>base, dest);
}
static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
{
-const TranslationBlock *tb;
-
-tb = s->base.tb;
-if (use_goto_tb(s, n, dest)) {
+if (use_goto_tb(s, dest)) {
tcg_gen_goto_tb(n);
gen_a64_set_pc_im(dest);
-tcg_gen_exit_tb(tb, n);
+tcg_gen_exit_tb(s->base.tb, n);
s->base.is_jmp = DISAS_NORETURN;
} else {
gen_a64_set_pc_im(dest);
--
2.25.1
[PATCH 21/41] target/mips: Fix missing else in gen_goto_tb
Do not emit dead code for the singlestep_enabled case,
after having exited the TB with a debug exception.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/mips/tcg/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 5cd3e7d8dd..47c967acbf 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4958,8 +4958,9 @@ static void gen_goto_tb(DisasContext *ctx, int n,
target_ulong dest)
if (ctx->base.singlestep_enabled) {
save_cpu_state(ctx, 0);
gen_helper_raise_exception_debug(cpu_env);
+} else {
+tcg_gen_lookup_and_goto_ptr();
}
-tcg_gen_lookup_and_goto_ptr();
}
}
--
2.25.1
[PATCH 12/41] target/arm: Use translator_use_goto_tb for aarch32
Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/translate.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 6d2867be1d..e1a8152598 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -2578,16 +2578,6 @@ static int disas_dsp_insn(DisasContext *s, uint32_t insn)
return 1;
}
-static inline bool use_goto_tb(DisasContext *s, target_ulong dest)
-{
-#ifndef CONFIG_USER_ONLY
-return (s->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) ||
- ((s->base.pc_next - 1) & TARGET_PAGE_MASK) == (dest &
TARGET_PAGE_MASK);
-#else
-return true;
-#endif
-}
-
static void gen_goto_ptr(void)
{
tcg_gen_lookup_and_goto_ptr();
@@ -2599,7 +2589,7 @@ static void gen_goto_ptr(void)
*/
static void gen_goto_tb(DisasContext *s, int n, target_ulong dest)
{
-if (use_goto_tb(s, dest)) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
gen_set_pc_im(s, dest);
tcg_gen_exit_tb(s->base.tb, n);
--
2.25.1
[PATCH 33/41] target/xtensa: Use translator_use_goto_tb
Reviewed-by: Max Filippov
Signed-off-by: Richard Henderson
---
target/xtensa/translate.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index d5da35f4fc..7094cfcf1d 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -405,11 +405,7 @@ static void gen_jump(DisasContext *dc, TCGv dest)
static int adjust_jump_slot(DisasContext *dc, uint32_t dest, int slot)
{
-if (((dc->base.pc_first ^ dest) & TARGET_PAGE_MASK) != 0) {
-return -1;
-} else {
-return slot;
-}
+return translator_use_goto_tb(>base, dest) ? slot : -1;
}
static void gen_jumpi(DisasContext *dc, uint32_t dest, int slot)
--
2.25.1
[PATCH 07/41] target/alpha: Remove use_exit_tb
We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block").
We do not need to use exit_tb for singlestep, which only
means generate one insn per TB.
Which leaves only singlestep_enabled, which means raise a
debug trap after every TB, which does not use exit_tb,
which would leave the function mis-named.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/alpha/translate.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index a607c898f4..cb2cb2de6b 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -449,19 +449,8 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
#endif
}
-static bool use_exit_tb(DisasContext *ctx)
-{
-return ((tb_cflags(ctx->base.tb) & CF_LAST_IO)
-|| ctx->base.singlestep_enabled
-|| singlestep);
-}
-
static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
{
-/* Suppress goto_tb in the case of single-steping and IO. */
-if (unlikely(use_exit_tb(ctx))) {
-return false;
-}
#ifndef CONFIG_USER_ONLY
/* If the destination is in the superpage, the page perms can't change. */
if (in_superpage(ctx, dest)) {
@@ -1270,7 +1259,7 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int
palcode)
need the page permissions check. We'll see the existence of
the page when we create the TB, and we'll flush all TBs if
we change the PAL base register. */
-if (!use_exit_tb(ctx)) {
+if (!ctx->base.singlestep_enabled) {
tcg_gen_goto_tb(0);
tcg_gen_movi_i64(cpu_pc, entry);
tcg_gen_exit_tb(ctx->base.tb, 0);
@@ -3094,7 +3083,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase,
CPUState *cpu)
tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
/* FALLTHRU */
case DISAS_PC_UPDATED:
-if (!use_exit_tb(ctx)) {
+if (!ctx->base.singlestep_enabled) {
tcg_gen_lookup_and_goto_ptr();
break;
}
--
2.25.1
[PATCH 15/41] target/cris: Use translator_use_goto_tb
The test for singlestepping is done in translator_use_goto_tb,
so we may elide it from cris_tr_tb_stop.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/cris/translate.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index a6796c83b9..9258c13e9f 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -481,7 +481,7 @@ static void t_gen_swapr(TCGv d, TCGv s)
static bool use_goto_tb(DisasContext *dc, target_ulong dest)
{
-return ((dest ^ dc->base.pc_first) & TARGET_PAGE_MASK) == 0;
+return translator_use_goto_tb(>base, dest);
}
static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
@@ -3234,8 +3234,7 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase,
CPUState *cpu)
* Use a conditional branch if either taken or not-taken path
* can use goto_tb. If neither can, then treat it as indirect.
*/
-if (likely(!dc->base.singlestep_enabled)
-&& likely(!dc->cpustate_changed)
+if (likely(!dc->cpustate_changed)
&& (use_goto_tb(dc, dc->jmp_pc) || use_goto_tb(dc, npc))) {
TCGLabel *not_taken = gen_new_label();
--
2.25.1
[PATCH 08/41] target/alpha: Remove in_superpage
The number of links across (normal) pages using this is low,
and it will shortly violate the contract for breakpoints.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/alpha/translate.c | 24 ++--
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index cb2cb2de6b..bb7b5ce994 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -438,24 +438,9 @@ static DisasJumpType gen_store_conditional(DisasContext
*ctx, int ra, int rb,
return DISAS_NEXT;
}
-static bool in_superpage(DisasContext *ctx, int64_t addr)
-{
-#ifndef CONFIG_USER_ONLY
-return ((ctx->tbflags & ENV_FLAG_PS_USER) == 0
-&& addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
-&& ((addr >> 41) & 3) == 2);
-#else
-return false;
-#endif
-}
-
static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
{
#ifndef CONFIG_USER_ONLY
-/* If the destination is in the superpage, the page perms can't change. */
-if (in_superpage(ctx, dest)) {
-return true;
-}
/* Check for the dest on the same page as the start of the TB. */
return ((ctx->base.tb->pc ^ dest) & TARGET_PAGE_MASK) == 0;
#else
@@ -2990,7 +2975,7 @@ static void alpha_tr_init_disas_context(DisasContextBase
*dcbase, CPUState *cpu)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
CPUAlphaState *env = cpu->env_ptr;
-int64_t bound, mask;
+int64_t bound;
ctx->tbflags = ctx->base.tb->flags;
ctx->mem_idx = cpu_mmu_index(env, false);
@@ -3019,12 +3004,7 @@ static void alpha_tr_init_disas_context(DisasContextBase
*dcbase, CPUState *cpu)
ctx->lit = NULL;
/* Bound the number of insns to execute to those left on the page. */
-if (in_superpage(ctx, ctx->base.pc_first)) {
-mask = -1ULL << 41;
-} else {
-mask = TARGET_PAGE_MASK;
-}
-bound = -(ctx->base.pc_first | mask) / 4;
+bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
}
--
2.25.1
[PATCH 29/41] target/sh4: Use translator_use_goto_tb
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/sh4/translate.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 8a25a4362e..40898e2393 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -224,17 +224,12 @@ static inline bool use_exit_tb(DisasContext *ctx)
return (ctx->tbflags & GUSA_EXCLUSIVE) != 0;
}
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
+static bool use_goto_tb(DisasContext *ctx, target_ulong dest)
{
-/* Use a direct jump if in same page and singlestep not enabled */
-if (unlikely(ctx->base.singlestep_enabled || use_exit_tb(ctx))) {
+if (use_exit_tb(ctx)) {
return false;
}
-#ifndef CONFIG_USER_ONLY
-return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
+return translator_use_goto_tb(>base, dest);
}
static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
--
2.25.1
[PATCH 13/41] target/avr: Use translator_use_goto_tb
Single stepping is not the only reason not to use goto_tb.
If goto_tb is disallowed, and single-stepping is not enabled,
then use tcg_gen_lookup_and_goto_tb to indirectly chain.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/avr/translate.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index c06ce45bc7..8237a03c23 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1083,14 +1083,17 @@ static void gen_goto_tb(DisasContext *ctx, int n,
target_ulong dest)
{
const TranslationBlock *tb = ctx->base.tb;
-if (!ctx->base.singlestep_enabled) {
+if (translator_use_goto_tb(>base, dest)) {
tcg_gen_goto_tb(n);
tcg_gen_movi_i32(cpu_pc, dest);
tcg_gen_exit_tb(tb, n);
} else {
tcg_gen_movi_i32(cpu_pc, dest);
-gen_helper_debug(cpu_env);
-tcg_gen_exit_tb(NULL, 0);
+if (ctx->base.singlestep_enabled) {
+gen_helper_debug(cpu_env);
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
}
ctx->base.is_jmp = DISAS_NORETURN;
}
--
2.25.1
[PATCH 05/41] tcg: Move tb_phys_invalidate_count to tb_ctx
We can call do_tb_phys_invalidate from an iocontext, which has
no per-thread tcg_ctx. Move this to tb_ctx, which is global.
The actual update still takes place with a lock held, so only
an atomic set is required, not an atomic increment.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/457
Tested-by: Viktor Ashirov
Signed-off-by: Richard Henderson
---
accel/tcg/tb-context.h| 1 +
include/tcg/tcg.h | 3 ---
accel/tcg/translate-all.c | 8
tcg/region.c | 14 --
4 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/accel/tcg/tb-context.h b/accel/tcg/tb-context.h
index cc33979113..cac62d9749 100644
--- a/accel/tcg/tb-context.h
+++ b/accel/tcg/tb-context.h
@@ -34,6 +34,7 @@ struct TBContext {
/* statistics */
unsigned tb_flush_count;
+unsigned tb_phys_invalidate_count;
};
extern TBContext tb_ctx;
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index dedb86939a..25dd19d6e1 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -579,8 +579,6 @@ struct TCGContext {
/* Threshold to flush the translated code buffer. */
void *code_gen_highwater;
-size_t tb_phys_invalidate_count;
-
/* Track which vCPU triggers events */
CPUState *cpu; /* *_trans */
@@ -815,7 +813,6 @@ size_t tcg_code_capacity(void);
void tcg_tb_insert(TranslationBlock *tb);
void tcg_tb_remove(TranslationBlock *tb);
-size_t tcg_tb_phys_invalidate_count(void);
TranslationBlock *tcg_tb_lookup(uintptr_t tc_ptr);
void tcg_tb_foreach(GTraverseFunc func, gpointer user_data);
size_t tcg_nb_tbs(void);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 57455d8639..4df26de858 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1219,8 +1219,8 @@ static void do_tb_phys_invalidate(TranslationBlock *tb,
bool rm_from_page_list)
/* suppress any remaining jumps to this TB */
tb_jmp_unlink(tb);
-qatomic_set(_ctx->tb_phys_invalidate_count,
- tcg_ctx->tb_phys_invalidate_count + 1);
+qatomic_set(_ctx.tb_phys_invalidate_count,
+tb_ctx.tb_phys_invalidate_count + 1);
}
static void tb_phys_invalidate__locked(TranslationBlock *tb)
@@ -2128,8 +2128,8 @@ void dump_exec_info(void)
qemu_printf("\nStatistics:\n");
qemu_printf("TB flush count %u\n",
qatomic_read(_ctx.tb_flush_count));
-qemu_printf("TB invalidate count %zu\n",
-tcg_tb_phys_invalidate_count());
+qemu_printf("TB invalidate count %u\n",
+qatomic_read(_ctx.tb_phys_invalidate_count));
tlb_flush_counts(_full, _part, _elide);
qemu_printf("TLB full flushes%zu\n", flush_full);
diff --git a/tcg/region.c b/tcg/region.c
index d3a3658e81..e64c3ea230 100644
--- a/tcg/region.c
+++ b/tcg/region.c
@@ -980,17 +980,3 @@ size_t tcg_code_capacity(void)
return capacity;
}
-
-size_t tcg_tb_phys_invalidate_count(void)
-{
-unsigned int n_ctxs = qatomic_read(_cur_ctxs);
-unsigned int i;
-size_t total = 0;
-
-for (i = 0; i < n_ctxs; i++) {
-const TCGContext *s = qatomic_read(_ctxs[i]);
-
-total += qatomic_read(>tb_phys_invalidate_count);
-}
-return total;
-}
--
2.25.1
[PATCH 02/41] tcg: Avoid including 'trace-tcg.h' in target translate.c
From: Philippe Mathieu-Daudé The root trace-events only declares a single TCG event: $ git grep -w tcg trace-events trace-events:115:# tcg/tcg-op.c trace-events:137:vcpu tcg guest_mem_before(TCGv vaddr, uint16_t info) "info=%d", "vaddr=0x%016"PRIx64" info=%d" and only a tcg/tcg-op.c uses it: $ git grep -l trace_guest_mem_before_tcg tcg/tcg-op.c therefore it is pointless to include "trace-tcg.h" in each target (because it is not used). Remove it. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20210629050935.2570721-1-f4...@amsat.org> Signed-off-by: Richard Henderson --- target/alpha/translate.c | 1 - target/arm/translate-a64.c| 1 - target/arm/translate-sve.c| 1 - target/arm/translate.c| 1 - target/cris/translate.c | 1 - target/hppa/translate.c | 1 - target/i386/tcg/translate.c | 1 - target/m68k/translate.c | 1 - target/microblaze/translate.c | 1 - target/mips/tcg/translate.c | 1 - target/openrisc/translate.c | 1 - target/ppc/translate.c| 1 - target/rx/translate.c | 1 - target/s390x/translate.c | 1 - target/sh4/translate.c| 1 - target/sparc/translate.c | 1 - target/xtensa/translate.c | 1 - 17 files changed, 17 deletions(-) diff --git a/target/alpha/translate.c b/target/alpha/translate.c index f2922f5f8c..a607c898f4 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -28,7 +28,6 @@ #include "exec/cpu_ldst.h" #include "exec/helper-proto.h" #include "exec/helper-gen.h" -#include "trace-tcg.h" #include "exec/translator.h" #include "exec/log.h" diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index e81cc20d04..a6dd9ec701 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -35,7 +35,6 @@ #include "exec/helper-gen.h" #include "exec/log.h" -#include "trace-tcg.h" #include "translate-a64.h" #include "qemu/atomic128.h" diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c index 46210eb696..35d838aa06 100644 --- a/target/arm/translate-sve.c +++ b/target/arm/translate-sve.c @@ -30,7 +30,6 @@ #include "exec/helper-proto.h" #include "exec/helper-gen.h" #include "exec/log.h" -#include "trace-tcg.h" #include "translate-a64.h" #include "fpu/softfloat.h" diff --git a/target/arm/translate.c b/target/arm/translate.c index 28e478927d..fdf2b3d1c8 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -34,7 +34,6 @@ #include "exec/helper-proto.h" #include "exec/helper-gen.h" -#include "trace-tcg.h" #include "exec/log.h" diff --git a/target/cris/translate.c b/target/cris/translate.c index 4cfe5c86d9..a6796c83b9 100644 --- a/target/cris/translate.c +++ b/target/cris/translate.c @@ -37,7 +37,6 @@ #include "exec/helper-gen.h" -#include "trace-tcg.h" #include "exec/log.h" diff --git a/target/hppa/translate.c b/target/hppa/translate.c index 64af1e0d5c..424ec3252e 100644 --- a/target/hppa/translate.c +++ b/target/hppa/translate.c @@ -27,7 +27,6 @@ #include "exec/helper-proto.h" #include "exec/helper-gen.h" #include "exec/translator.h" -#include "trace-tcg.h" #include "exec/log.h" /* Since we have a distinction between register size and address size, diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index b21873ed23..85b00a6945 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -30,7 +30,6 @@ #include "exec/helper-gen.h" #include "helper-tcg.h" -#include "trace-tcg.h" #include "exec/log.h" #define PREFIX_REPZ 0x01 diff --git a/target/m68k/translate.c b/target/m68k/translate.c index f0c5bf9154..348fc6e844 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -31,7 +31,6 @@ #include "exec/helper-proto.h" #include "exec/helper-gen.h" -#include "trace-tcg.h" #include "exec/log.h" #include "fpu/softfloat.h" diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c index c1b13f4c7d..5dfb08d49f 100644 --- a/target/microblaze/translate.c +++ b/target/microblaze/translate.c @@ -29,7 +29,6 @@ #include "exec/translator.h" #include "qemu/qemu-print.h" -#include "trace-tcg.h" #include "exec/log.h" #define EXTRACT_FIELD(src, start, end) \ diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c index ae33c75f08..cb82426f66 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -32,7 +32,6 @@ #include "semihosting/semihost.h" #include "trace.h" -#include "trace-tcg.h" #include "exec/translator.h" #include "exec/log.h" #include "qemu/qemu-print.h" diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index a9c81f8bd5..5db63d7609 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -33,7 +33,6 @@ #include "exec/helper-gen.h" #include "exec/gen-icount.h" -#include "trace-tcg.h" #include "exec/log.h" /* is_jmp field values */ diff --git a/target/ppc/translate.c b/target/ppc/translate.c index
[PATCH 27/41] target/s390x: Use translator_use_goto_tb
Reviewed-by: David Hildenbrand
Signed-off-by: Richard Henderson
---
target/s390x/translate.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5af68e01c6..767e77ca19 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -696,12 +696,7 @@ static bool use_goto_tb(DisasContext *s, uint64_t dest)
if (unlikely(use_exit_tb(s))) {
return false;
}
-#ifndef CONFIG_USER_ONLY
-return (dest & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) ||
- (dest & TARGET_PAGE_MASK) == (s->base.pc_next & TARGET_PAGE_MASK);
-#else
-return true;
-#endif
+return translator_use_goto_tb(>base, dest);
}
static void account_noninline_branch(DisasContext *s, int cc_op)
--
2.25.1
[PATCH 09/41] target/alpha: Use translator_use_goto_tb
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/alpha/translate.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index bb7b5ce994..833d3baa7b 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -440,12 +440,7 @@ static DisasJumpType gen_store_conditional(DisasContext
*ctx, int ra, int rb,
static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
{
-#ifndef CONFIG_USER_ONLY
-/* Check for the dest on the same page as the start of the TB. */
-return ((ctx->base.tb->pc ^ dest) & TARGET_PAGE_MASK) == 0;
-#else
-return true;
-#endif
+return translator_use_goto_tb(>base, dest);
}
static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, int32_t disp)
--
2.25.1
[PATCH 03/41] accel/tcg: Hoist tcg_tb_insert() up above tb_link_page()
From: Liren Wei TranslationBlocks not inserted into the corresponding region tree shall be regarded as partially initialized objects, and needs to be finalized first before inserting into QHT. Signed-off-by: Liren Wei Message-Id: Signed-off-by: Richard Henderson --- accel/tcg/translate-all.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 7929a7e320..75e4d06557 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1657,6 +1657,13 @@ TranslationBlock *tb_gen_code(CPUState *cpu, return tb; } +/* + * Insert TB into the corresponding region tree before publishing it + * through QHT. Otherwise rewinding happened in the TB might fail to + * lookup itself using host PC. + */ +tcg_tb_insert(tb); + /* check next page if needed */ virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK; phys_page2 = -1; @@ -1675,9 +1682,9 @@ TranslationBlock *tb_gen_code(CPUState *cpu, orig_aligned -= ROUND_UP(sizeof(*tb), qemu_icache_linesize); qatomic_set(_ctx->code_gen_ptr, (void *)orig_aligned); tb_destroy(tb); +tcg_tb_remove(tb); return existing_tb; } -tcg_tb_insert(tb); return tb; } -- 2.25.1
[PATCH 04/41] tcg: Bake tb_destroy() into tcg_region_tree
From: Liren Wei
The function is called only at tcg_gen_code() when duplicated TBs
are translated by different threads, and when the tcg_region_tree
is reset. Bake it into the underlying GTree as its value destroy
function to unite these situations.
Also remove tcg_region_tree_traverse() which now becomes useless.
Signed-off-by: Liren Wei
Message-Id:
<8dc352f08d038c4e7a1f5f56962398cdc700c3aa.1625404483.git.lr...@bupt.edu.cn>
[rth: Name the new tb_tc_cmp parameter correctly.]
Signed-off-by: Richard Henderson
---
include/tcg/tcg.h | 1 -
accel/tcg/translate-all.c | 6 --
tcg/region.c | 19 ---
3 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 899493701c..dedb86939a 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -808,7 +808,6 @@ void *tcg_malloc_internal(TCGContext *s, int size);
void tcg_pool_reset(TCGContext *s);
TranslationBlock *tcg_tb_alloc(TCGContext *s);
-void tb_destroy(TranslationBlock *tb);
void tcg_region_reset_all(void);
size_t tcg_code_size(void);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 75e4d06557..57455d8639 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -378,11 +378,6 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
TranslationBlock *tb,
return 0;
}
-void tb_destroy(TranslationBlock *tb)
-{
-qemu_spin_destroy(>jmp_lock);
-}
-
bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
{
/*
@@ -1681,7 +1676,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
orig_aligned -= ROUND_UP(sizeof(*tb), qemu_icache_linesize);
qatomic_set(_ctx->code_gen_ptr, (void *)orig_aligned);
-tb_destroy(tb);
tcg_tb_remove(tb);
return existing_tb;
}
diff --git a/tcg/region.c b/tcg/region.c
index 00b0c3b091..d3a3658e81 100644
--- a/tcg/region.c
+++ b/tcg/region.c
@@ -112,7 +112,7 @@ static int ptr_cmp_tb_tc(const void *ptr, const struct
tb_tc *s)
return 0;
}
-static gint tb_tc_cmp(gconstpointer ap, gconstpointer bp)
+static gint tb_tc_cmp(gconstpointer ap, gconstpointer bp, gpointer userdata)
{
const struct tb_tc *a = ap;
const struct tb_tc *b = bp;
@@ -143,6 +143,12 @@ static gint tb_tc_cmp(gconstpointer ap, gconstpointer bp)
return ptr_cmp_tb_tc(b->ptr, a);
}
+static void tb_destroy(gpointer value)
+{
+TranslationBlock *tb = value;
+qemu_spin_destroy(>jmp_lock);
+}
+
static void tcg_region_trees_init(void)
{
size_t i;
@@ -153,7 +159,7 @@ static void tcg_region_trees_init(void)
struct tcg_region_tree *rt = region_trees + i * tree_size;
qemu_mutex_init(>lock);
-rt->tree = g_tree_new(tb_tc_cmp);
+rt->tree = g_tree_new_full(tb_tc_cmp, NULL, NULL, tb_destroy);
}
}
@@ -277,14 +283,6 @@ size_t tcg_nb_tbs(void)
return nb_tbs;
}
-static gboolean tcg_region_tree_traverse(gpointer k, gpointer v, gpointer data)
-{
-TranslationBlock *tb = v;
-
-tb_destroy(tb);
-return FALSE;
-}
-
static void tcg_region_tree_reset_all(void)
{
size_t i;
@@ -293,7 +291,6 @@ static void tcg_region_tree_reset_all(void)
for (i = 0; i < region.n; i++) {
struct tcg_region_tree *rt = region_trees + i * tree_size;
-g_tree_foreach(rt->tree, tcg_region_tree_traverse, NULL);
/* Increment the refcount first so that destroy acts as a reset */
g_tree_ref(rt->tree);
g_tree_destroy(rt->tree);
--
2.25.1
[PATCH 10/41] target/arm: Use DISAS_TOO_MANY for ISB and SB
Using gen_goto_tb directly misses the single-step check. Let the branch or debug exception be emitted by arm_tr_tb_stop. Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- target/arm/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index fdf2b3d1c8..6d2867be1d 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -8904,7 +8904,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a) * self-modifying code correctly and also to take * any pending interrupts immediately. */ -gen_goto_tb(s, 0, s->base.pc_next); +s->base.is_jmp = DISAS_TOO_MANY; return true; } @@ -8918,7 +8918,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a) * for TCG; MB and end the TB instead. */ tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); -gen_goto_tb(s, 0, s->base.pc_next); +s->base.is_jmp = DISAS_TOO_MANY; return true; } -- 2.25.1
[PATCH 06/41] accel/tcg: Introduce translator_use_goto_tb
Add a generic version of the common use_goto_tb test.
Various targets avoid the page crossing test for CONFIG_USER_ONLY,
but that is wrong: mmap and mprotect can change page permissions.
Reviewed-by: Max Filippov
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
include/exec/translator.h | 10 ++
accel/tcg/translator.c| 11 +++
2 files changed, 21 insertions(+)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 24232ead41..dd9c06d40d 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -145,6 +145,16 @@ void translator_loop(const TranslatorOps *ops,
DisasContextBase *db,
void translator_loop_temp_check(DisasContextBase *db);
+/**
+ * translator_use_goto_tb
+ * @db: Disassembly context
+ * @dest: target pc of the goto
+ *
+ * Return true if goto_tb is allowed between the current TB
+ * and the destination PC.
+ */
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
+
/*
* Translator Load Functions
*
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 1d32732198..59804af37b 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -31,6 +31,17 @@ void translator_loop_temp_check(DisasContextBase *db)
}
}
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
+{
+/* Suppress goto_tb in the case of single-steping. */
+if (db->singlestep_enabled || singlestep) {
+return false;
+}
+
+/* Check for the dest on the same page as the start of the TB. */
+return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
+}
+
void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
CPUState *cpu, TranslationBlock *tb, int max_insns)
{
--
2.25.1
[PATCH 00/41] tcg patch queue
The following changes since commit 05de778b5b8ab0b402996769117b88c7ea5c7c61: Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-07-09 14:30:01 +0100) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210710 for you to fetch changes up to ad1a706f386c2281adb0b09257d892735e405834: cpu: Add breakpoint tracepoints (2021-07-09 21:31:11 -0700) Add translator_use_goto_tb. Cleanups in prep of breakpoint fixes. Misc fixes. Liren Wei (2): accel/tcg: Hoist tcg_tb_insert() up above tb_link_page() tcg: Bake tb_destroy() into tcg_region_tree Philippe Mathieu-Daudé (1): tcg: Avoid including 'trace-tcg.h' in target translate.c Richard Henderson (38): tcg: Add separator in INDEX_op_call dump tcg: Move tb_phys_invalidate_count to tb_ctx accel/tcg: Introduce translator_use_goto_tb target/alpha: Remove use_exit_tb target/alpha: Remove in_superpage target/alpha: Use translator_use_goto_tb target/arm: Use DISAS_TOO_MANY for ISB and SB target/arm: Use translator_use_goto_tb for aarch64 target/arm: Use translator_use_goto_tb for aarch32 target/avr: Use translator_use_goto_tb target/avr: Mark some helpers noreturn target/cris: Use translator_use_goto_tb target/hppa: Use translator_use_goto_tb target/i386: Use translator_use_goto_tb target/m68k: Use translator_use_goto_tb target/microblaze: Use translator_use_goto_tb target/mips: Use translator_use_goto_tb target/mips: Fix missing else in gen_goto_tb target/nios2: Use translator_use_goto_tb target/openrisc: Use translator_use_goto_tb target/ppc: Use translator_use_goto_tb target/riscv: Use translator_use_goto_tb target/rx: Use translator_use_goto_tb target/s390x: Use translator_use_goto_tb target/s390x: Remove use_exit_tb target/sh4: Use translator_use_goto_tb target/sparc: Use translator_use_goto_tb target/tricore: Use translator_use_goto_tb target/tricore: Use tcg_gen_lookup_and_goto_ptr target/xtensa: Use translator_use_goto_tb tcg: Fix prologue disassembly target/i386: Use cpu_breakpoint_test in breakpoint_handler accel/tcg: Move helper_lookup_tb_ptr to cpu-exec.c accel/tcg: Move tb_lookup to cpu-exec.c accel/tcg: Split out log_cpu_exec accel/tcg: Log tb->cflags with -d exec tcg: Remove TCG_TARGET_HAS_goto_ptr cpu: Add breakpoint tracepoints accel/tcg/tb-context.h | 1 + accel/tcg/tb-lookup.h | 49 include/exec/translator.h | 10 include/tcg/tcg-opc.h | 3 +- include/tcg/tcg.h | 4 -- target/avr/helper.h | 8 +-- tcg/aarch64/tcg-target.h| 1 - tcg/arm/tcg-target.h| 1 - tcg/i386/tcg-target.h | 1 - tcg/mips/tcg-target.h | 1 - tcg/ppc/tcg-target.h| 1 - tcg/riscv/tcg-target.h | 1 - tcg/s390/tcg-target.h | 1 - tcg/sparc/tcg-target.h | 1 - tcg/tci/tcg-target.h| 1 - accel/tcg/cpu-exec.c| 112 accel/tcg/tcg-runtime.c | 22 --- accel/tcg/translate-all.c | 23 accel/tcg/translator.c | 11 cpu.c | 13 +++-- target/alpha/translate.c| 47 ++- target/arm/translate-a64.c | 26 ++--- target/arm/translate-sve.c | 1 - target/arm/translate.c | 17 +- target/avr/translate.c | 9 ++- target/cris/translate.c | 6 +- target/hppa/translate.c | 6 +- target/i386/tcg/sysemu/bpt_helper.c | 12 +--- target/i386/tcg/translate.c | 15 + target/m68k/translate.c | 13 + target/microblaze/translate.c | 12 +--- target/mips/tcg/translate.c | 21 ++- target/nios2/translate.c| 15 + target/openrisc/translate.c | 16 +++--- target/ppc/translate.c | 11 +--- target/riscv/translate.c| 20 +-- target/rx/translate.c | 12 +--- target/s390x/translate.c| 19 +- target/sh4/translate.c | 12 +--- target/sparc/translate.c| 20 ++- target/tricore/translate.c | 20 ++- target/xtensa/translate.c | 7 +-- tcg/region.c| 33 +++ tcg/tcg-op.c| 2 +- tcg/tcg.c | 14 ++--- trace-events| 5 ++ 46 files changed, 217 insertions(+), 439 deletions(-) delete m
[PATCH 01/41] tcg: Add separator in INDEX_op_call dump
We lost the ',' following the called function name.
Fixes: 3e92aa34434
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 5150ed700e..4dd4084419 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1849,7 +1849,7 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs)
col += qemu_log("plugin(%p)", func);
}
-col += qemu_log("$0x%x,$%d", info->flags, nb_oargs);
+col += qemu_log(",$0x%x,$%d", info->flags, nb_oargs);
for (i = 0; i < nb_oargs; i++) {
col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf),
op->args[i]));
--
2.25.1
Re: [PULL v2 00/15] Machine queue, 2021-07-07
On Thu, 8 Jul 2021 at 20:55, Eduardo Habkost wrote: > > Changes v2: > * Fix doc build warning > > The following changes since commit 9aef0954195cc592e86846dbbe7f3c2c5603690a: > > Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' > into staging (2021-07-06 11:24:58 +0100) > > are available in the Git repository at: > > https://gitlab.com/ehabkost/qemu.git tags/machine-next-pull-request > > for you to fetch changes up to 53d1b5fcfb40c47da4c060dc913df0e9f62894bd: > > vfio: Disable only uncoordinated discards for VFIO_TYPE1 iommus (2021-07-08 > 15:54:45 -0400) > > > Machine queue, 2021-07-07 > > Deprecation: > * Deprecate pmem=on with non-DAX capable backend file > (Igor Mammedov) > > Feature: > * virtio-mem: vfio support (David Hildenbrand) > > Cleanup: > * vmbus: Don't make QOM property registration conditional > (Eduardo Habkost) > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [RFC PATCH 06/11] target/riscv: Update CSR xtvec in CLIC mode
LIU Zhiwei 於 2021年4月9日 週五 下午3:51寫道：
> The new CLIC interrupt-handling mode is encoded as a new state in the
> existing WARL xtvec register, where the low two bits of are 11.
>
> Signed-off-by: LIU Zhiwei
> ---
> target/riscv/csr.c | 22 --
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index f6c84b9fe4..39ff72041a 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -637,9 +637,18 @@ static int read_mtvec(CPURISCVState *env, int csrno,
> target_ulong *val)
>
> static int write_mtvec(CPURISCVState *env, int csrno, target_ulong val)
> {
> -/* bits [1:0] encode mode; 0 = direct, 1 = vectored, 2 >= reserved */
> +/*
> + * bits [1:0] encode mode; 0 = direct, 1 = vectored, 3 = CLIC,
> + * others reserved
> + */
> if ((val & 3) < 2) {
> env->mtvec = val;
>
We might need to zero new fields in mcause when set back to basic mode:
Section 4.7:
When basic mode is written to xtvec,
the new xcause state fields (mhinv and mpil) are zeroed.
The other new xcause fields, mpp and mpie, appear as zero in the xcause CSR
but the corresponding state bits in the mstatus register are not cleared.
> +} else if ((val & 1) && env->clic) {
> +/*
> + * If only CLIC mode is supported, writes to bit 1 are also
> ignored and
> + * it is always set to one. CLIC mode hardwires xtvec bits 2-5 to
> zero.
> + */
> +env->mtvec = ((val & ~0x3f) << 6) | (0b11);
> } else {
> qemu_log_mask(LOG_UNIMP, "CSR_MTVEC: reserved mode not
> supported\n");
> }
> @@ -837,9 +846,18 @@ static int read_stvec(CPURISCVState *env, int csrno,
> target_ulong *val)
>
> static int write_stvec(CPURISCVState *env, int csrno, target_ulong val)
> {
> -/* bits [1:0] encode mode; 0 = direct, 1 = vectored, 2 >= reserved */
> +/*
> + * bits [1:0] encode mode; 0 = direct, 1 = vectored, 3 = CLIC,
> + * others reserved
> + */
> if ((val & 3) < 2) {
> env->stvec = val;
>
Same to write_mtvec()
Regards,
Frank Chang
> +} else if ((val & 1) && env->clic) {
> +/*
> + * If only CLIC mode is supported, writes to bit 1 are also
> ignored and
> + * it is always set to one. CLIC mode hardwires xtvec bits 2-5 to
> zero.
> + */
> +env->stvec = ((val & ~0x3f) << 6) | (0b11);
> } else {
> qemu_log_mask(LOG_UNIMP, "CSR_STVEC: reserved mode not
> supported\n");
> }
> --
> 2.25.1
>
>
>
Re: [RFC PATCH 08/11] target/riscv: Update CSR xnxti in CLIC mode
LIU Zhiwei 於 2021年4月9日 週五 下午3:52寫道：
> The CSR can be used by software to service the next horizontal interrupt
> when it has greater level than the saved interrupt context
> (held in xcause`.pil`) and greater level than the interrupt threshold of
> the corresponding privilege mode,
>
> Signed-off-by: LIU Zhiwei
> ---
> target/riscv/cpu_bits.h | 16 ++
> target/riscv/csr.c | 114
> 2 files changed, 130 insertions(+)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 7922097776..494e41edc9 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -166,6 +166,7 @@
> #define CSR_MCAUSE 0x342
> #define CSR_MTVAL 0x343
> #define CSR_MIP 0x344
> +#define CSR_MNXTI 0x345 /* clic-spec-draft */
> #define CSR_MINTSTATUS 0x346 /* clic-spec-draft */
> #define CSR_MINTTHRESH 0x347 /* clic-spec-draft */
>
> @@ -187,6 +188,7 @@
> #define CSR_SCAUSE 0x142
> #define CSR_STVAL 0x143
> #define CSR_SIP 0x144
> +#define CSR_SNXTI 0x145 /* clic-spec-draft */
> #define CSR_SINTSTATUS 0x146 /* clic-spec-draft */
> #define CSR_SINTTHRESH 0x147 /* clic-spec-draft */
>
> @@ -596,10 +598,24 @@
> #define MINTSTATUS_SIL 0xff00 /* sil[7:0] */
> #define MINTSTATUS_UIL 0x00ff /* uil[7:0] */
>
> +/* mcause */
> +#define MCAUSE_MINHV 0x4000 /* minhv */
> +#define MCAUSE_MPP 0x3000 /* mpp[1:0] */
> +#define MCAUSE_MPIE0x0800 /* mpie */
> +#define MCAUSE_MPIL0x00ff /* mpil[7:0] */
> +#define MCAUSE_EXCCODE 0x0fff /* exccode[11:0] */
> +
> /* sintstatus */
> #define SINTSTATUS_SIL 0xff00 /* sil[7:0] */
> #define SINTSTATUS_UIL 0x00ff /* uil[7:0] */
>
> +/* scause */
> +#define SCAUSE_SINHV 0x4000 /* sinhv */
> +#define SCAUSE_SPP 0x1000 /* spp */
> +#define SCAUSE_SPIE0x0800 /* spie */
> +#define SCAUSE_SPIL0x00ff /* spil[7:0] */
> +#define SCAUSE_EXCCODE 0x0fff /* exccode[11:0] */
> +
> /* MIE masks */
> #define MIE_SEIE (1 << IRQ_S_EXT)
> #define MIE_UEIE (1 << IRQ_U_EXT)
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e1b77f..72cba080bf 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -774,6 +774,80 @@ static int rmw_mip(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> return 0;
> }
>
> +static bool get_xnxti_status(CPURISCVState *env)
> +{
> +CPUState *cs = env_cpu(env);
> +int clic_irq, clic_priv, clic_il, pil;
> +
> +if (!env->exccode) { /* No interrupt */
> +return false;
> +}
> +/* The system is not in a CLIC mode */
> +if (!riscv_clic_is_clic_mode(env)) {
> +return false;
> +} else {
> +riscv_clic_decode_exccode(env->exccode, _priv, _il,
> + _irq);
> +
> +if (env->priv == PRV_M) {
> +pil = MAX(get_field(env->mcause, MCAUSE_MPIL),
> env->mintthresh);
> +} else if (env->priv == PRV_S) {
> +pil = MAX(get_field(env->scause, SCAUSE_SPIL),
> env->sintthresh);
> +} else {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "CSR: rmw xnxti with unsupported mode\n");
> +exit(1);
> +}
> +
> +if ((clic_priv != env->priv) || /* No horizontal interrupt */
> +(clic_il <= pil) || /* No higher level interrupt */
> +(riscv_clic_shv_interrupt(env->clic, clic_priv, cs->cpu_index,
> + clic_irq))) { /* CLIC vector mode */
> +return false;
> +} else {
> +return true;
> +}
> +}
> +}
> +
> +static int rmw_mnxti(CPURISCVState *env, int csrno, target_ulong
> *ret_value,
> + target_ulong new_value, target_ulong write_mask)
> +{
> +int clic_priv, clic_il, clic_irq;
> +bool ready;
> +CPUState *cs = env_cpu(env);
> +if (write_mask) {
> +env->mstatus |= new_value & (write_mask & 0b1);
>
This won't work for CSRRCI instruction.
new_value is assigned to 0 for CSRRCI operation.
Should be:
target_ulong mask = write_mask & 0b1;
env->mstatus = (env->mstatus & ~mask) | (new_value & mask);
> +}
> +
> +qemu_mutex_lock_iothread();
> +ready = get_xnxti_status(env);
> +if (ready) {
> +riscv_clic_decode_exccode(env->exccode, _priv, _il,
> + _irq);
> +if (write_mask) {
> +bool edge = riscv_clic_edge_triggered(env->clic, clic_priv,
> +
Re: [PATCH v1 1/5] target/riscv: Expose interrupt pending bits as GPIO lines
On Fri, Jul 9, 2021 at 11:30 AM Alistair Francis wrote: > > Expose the 12 interrupt pending bits in MIP as GPIO lines. > > Signed-off-by: Alistair Francis > --- > target/riscv/cpu.c | 30 ++ > 1 file changed, 30 insertions(+) > Reviewed-by: Bin Meng
retrying failed gitlab CI external jobs (travis)
Hi; we now have travis's CI hooked into gitlab, which is nice. However, unlike the gitlab native CI jobs, there's no UI for saying "retry this" when the "travis CI" part of the overall gitlab pipeline fails. This is awkward because travis seems to be prone to intermittent failures. Is there any way we can make the jobs retryable? thanks -- PMM
intermittent hang in qos-test for qemu-system-i386 on 32-bit arm host
I've noticed recently that intermittently 'make check' will hang on
my aarch32 test system (really an aarch64 box with an aarch32 chroot).
I think from grep that this must be the vhost-user-blk test.
Here's the process tree:
pmaydell 13126 0.0 0.0 8988 6416 ?SJul09 0:01 make
-C build/all-a32 check V=1 GCC_COLORS= -j9
pmaydell 19632 0.0 0.0 4432 2096 ?SJul09 0:00 \_
bash -o pipefail -c echo 'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((
${RANDOM:-0} % 255 + 1))} QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/peter.maydell/qemu/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-i386
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
tests/qtest/qos-test --tap -k' &&
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/peter.maydell/qemu/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-i386
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
tests/qtest/qos-test --tap -k -m quick < /dev/null |
./scripts/tap-driver.pl --test-name="qtest-i386/qos-test"
pmaydell 19634 0.0 0.0 13608 3076 ?Sl Jul09 0:02
\_ tests/qtest/qos-test --tap -k -m quick
pmaydell 20679 0.0 0.0 109076 16100 ?Sl Jul09 0:00
| \_ ./storage-daemon/qemu-storage-daemon --blockdev
driver=file,node-name=disk0,filename=qtest.X7RL2X --export
type=vhost-user-blk,id=disk0,addr.type=unix,addr.path=/tmp/qtest-19634-sock.9LJoHn,node-name=disk0,writable=on,num-queues=1
pmaydell 20681 0.0 0.2 447828 46544 ?Sl Jul09 0:00
| \_ ./qemu-system-i386 -qtest unix:/tmp/qtest-19634.sock -qtest-log
/dev/null -chardev socket,path=/tmp/qtest-19634.qmp,id=char0 -mon
chardev=char0,mode=control -display none -M pc -device
vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 -object
memory-backend-memfd,id=mem,size=256M,share=on -M memory-backend=mem
-m 256M -chardev socket,id=char1,path=/tmp/qtest-19634-sock.9LJoHn
-accel qtest
pmaydell 19635 0.0 0.0 10256 7176 ?SJul09 0:00
\_ perl ./scripts/tap-driver.pl --test-name=qtest-i386/qos-test
Backtrace from tests/qtest/qos-test (not as helpful as it could
be since this is an optimized build):
(gdb) thread apply all bt
Thread 2 (Thread 0xf76ff240 (LWP 19636)):
#0 syscall () at ../sysdeps/unix/sysv/linux/arm/syscall.S:37
#1 0x005206de in qemu_futex_wait (val=, f=) at /home/peter.maydell/qemu/include/qemu/futex.h:29
#2 qemu_event_wait (ev=ev@entry=0x5816fc ) at
../../util/qemu-thread-posix.c:480
#3 0x005469c2 in call_rcu_thread (opaque=) at
../../util/rcu.c:258
#4 0x0051fbc2 in qemu_thread_start (args=) at
../../util/qemu-thread-posix.c:541
#5 0xf785a614 in start_thread (arg=0xf6ce711c) at pthread_create.c:463
#6 0xf77f57ec in ?? () at ../sysdeps/unix/sysv/linux/arm/clone.S:73
from /lib/arm-linux-gnueabihf/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Thread 1 (Thread 0xf7a04010 (LWP 19634)):
#0 __libc_do_syscall () at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:46
#1 0xf7861d8c in __libc_read (fd=12, buf=buf@entry=0xff9ce8e4,
nbytes=nbytes@entry=1024) at ../sysdeps/unix/sysv/linux/read.c:27
#2 0x004ebc5a in read (__nbytes=1024, __buf=0xff9ce8e4,
__fd=) at
/usr/include/arm-linux-gnueabihf/bits/unistd.h:44
#3 qtest_client_socket_recv_line (s=0x1a46cb8) at
../../tests/qtest/libqtest.c:494
#4 0x004ebd4e in qtest_rsp_args (s=s@entry=0x1a46cb8,
expected_args=expected_args@entry=1) at
../../tests/qtest/libqtest.c:521
#5 0x004ec1ee in qtest_query_target_endianness (s=0x1a46cb8) at
../../tests/qtest/libqtest.c:570
#6 0x004ec94a in qtest_init_without_qmp_handshake
(extra_args=) at ../../tests/qtest/libqtest.c:332
#7 0x004ecd7a in qtest_init (extra_args=) at
../../tests/qtest/libqtest.c:339
#8 0x004ded10 in qtest_start (
args=0x1a63710 "-M pc -device
vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 -object
memory-backend-memfd,id=mem,size=256M,share=on -M memory-backend=mem
-m 256M -chardev socket,id=char1,path=/tmp/qtest-19634-so"...) at
../../tests/qtest/libqtest-single.h:29
#9 restart_qemu_or_continue (
path=0x1a63710 "-M pc -device
vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 -object
memory-backend-memfd,id=mem,size=256M,share=on -M memory-backend=mem
-m 256M -chardev socket,id=char1,path=/tmp/qtest-19634-so"...) at
../../tests/qtest/qos-test.c:105
#10 run_one_test (arg=) at ../../tests/qtest/qos-test.c:178
#11 0xf794ee74 in ?? () from /usr/lib/arm-linux-gnueabihf/libglib-2.0.so.0
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Backtrace from qemu-system-i386:
(gdb) thread apply all bt
Thread 4 (Thread 0xdfd0cb90 (LWP 20734)):
#0 0xf6f85206 in __libc_do_syscall () at
../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47
#1 0xf6f93492 in __GI___sigtimedwait (set=,
set@entry=0xdfd0c3c4, info=info@entry=0xdfd0c324,
timeout=timeout@entry=0x0) at
../sysdeps/unix/sysv/linux/sigtimedwait.c:42
#2 0xf7073e6c
Re: [PATCH 2/4] target/openrisc: Use tcg_constant_tl for dc->R0
On Thu, Jul 08, 2021 at 02:37:52PM -0700, Richard Henderson wrote: > The temp allocated for tcg_const_tl is auto-freed at branches, > but pure constants are not. So we can remove the extra hoop > jumping in trans_l_swa. This is nice. > Signed-off-by: Richard Henderson Reviewed-by: Stafford Horne
Re: [PATCH v1 1/1] vfio: Make migration support non experimental by default.
On 3/8/21 5:09 PM, Tarun Gupta wrote:
> VFIO migration support in QEMU is experimental as of now, which was done to
> provide soak time and resolve concerns regarding bit-stream.
> But, with the patches discussed in
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg784931.html , we have
> corrected ordering of saving PCI config space and bit-stream.
>
> So, this patch proposes to make vfio migration support in QEMU to be enabled
> by default. Tested by successfully migrating mdev device.
>
> Signed-off-by: Tarun Gupta
> Signed-off-by: Kirti Wankhede
> ---
> hw/vfio/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index f74be78209..15e26f460b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3199,7 +3199,7 @@ static Property vfio_pci_dev_properties[] = {
> DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
> VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
> DEFINE_PROP_BOOL("x-enable-migration", VFIOPCIDevice,
> - vbasedev.enable_migration, false),
> + vbasedev.enable_migration, true),
> DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
> DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
> vbasedev.ram_block_discard_allowed, false),
>
Hello,
has plain snapshot been tested?
If I issue the HMP command "savevm", and then "loadvm", will things work fine?
Thanks,
CLaudio
Re: [PATCH v4 3/3] memory_hotplug.c: send DEVICE_UNPLUG_ERROR in acpi_memory_hotplug_write()
Igor Mammedov writes:
> On Fri, 09 Jul 2021 13:25:43 +0200
> Markus Armbruster wrote:
>
>> Igor Mammedov writes:
>>
>> > On Thu, 08 Jul 2021 15:08:57 +0200
>> > Markus Armbruster wrote:
>> >
>> >> Daniel Henrique Barboza writes:
>> >>
>> >> > MEM_UNPLUG_ERROR is deprecated since the introduction of
>> >> > DEVICE_UNPLUG_ERROR. Keep emitting both while the deprecation of
>> >> > MEM_UNPLUG_ERROR is pending.
>> >> >
>> >> > CC: Michael S. Tsirkin
>> >> > CC: Igor Mammedov
>> >> > Reviewed-by: David Gibson
>> >> > Signed-off-by: Daniel Henrique Barboza
>> >> > ---
>> >> > hw/acpi/memory_hotplug.c | 13 +++--
>> >> > 1 file changed, 11 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
>> >> > index af37889423..fb9f4d2de7 100644
>> >> > --- a/hw/acpi/memory_hotplug.c
>> >> > +++ b/hw/acpi/memory_hotplug.c
>> >> > @@ -8,6 +8,7 @@
>> >> > #include "qapi/error.h"
>> >> > #include "qapi/qapi-events-acpi.h"
>> >> > #include "qapi/qapi-events-machine.h"
>> >> > +#include "qapi/qapi-events-qdev.h"
>> >> >
>> >> > #define MEMORY_SLOTS_NUMBER "MDNR"
>> >> > #define MEMORY_HOTPLUG_IO_REGION "HPMR"
>> >> > @@ -177,9 +178,17 @@ static void acpi_memory_hotplug_write(void
>> >> > *opaque, hwaddr addr, uint64_t data,
>> >> > /* call pc-dimm unplug cb */
>> >> > hotplug_handler_unplug(hotplug_ctrl, dev, _err);
>> >> > if (local_err) {
>> >> > +const char *error_pretty = error_get_pretty(local_err);
>> >> > +
>> >> > trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
>> >> > -qapi_event_send_mem_unplug_error(dev->id,
>> >> > -
>> >> > error_get_pretty(local_err));
>> >> > +
>> >> > +/*
>> >> > + * Send both MEM_UNPLUG_ERROR and DEVICE_UNPLUG_ERROR
>> >> > + * while the deprecation of MEM_UNPLUG_ERROR is
>> >> > + * pending.
>> >> > + */
>> >> > +qapi_event_send_mem_unplug_error(dev->id,
>> >> > error_pretty);
>> >> > +qapi_event_send_device_unplug_error(dev->id,
>> >> > error_pretty);
>> >> > error_free(local_err);
>> >> > break;
>> >> > }
>> >>
>> >> Same question as for PATCH 2: can dev->id be null?
>> > only theoretically (if memory device were created directly without
>> > using device_add), which as far as I know is not the case as all
>> > memory devices are created using -device/device_add so far.
>> >
>> > ( for device_add case see qdev_device_add->qdev_set_id where
>> > 'id' is set to user provided or to generated "device[%d]" value)
>>
>> Something is set to a generated value, but it's not dev->id :)
>>
>> void qdev_set_id(DeviceState *dev, const char *id)
>>
>> @id is the value of id=... It may be null.
>>
>> dev->id still is null here.
>>
>> {
>> if (id) {
>> dev->id = id;
>> }
>>
>> dev->id is now the value of id=... It may be null.
>>
>> if (dev->id) {
>> object_property_add_child(qdev_get_peripheral(), dev->id,
>> OBJECT(dev));
>>
>> If the user specified id=..., add @dev as child of /peripheral. The
>> child's name is the (non-null) value of id=...
>>
>> } else {
>> static int anon_count;
>> gchar *name = g_strdup_printf("device[%d]", anon_count++);
>> object_property_add_child(qdev_get_peripheral_anon(), name,
>> OBJECT(dev));
>> g_free(name);
>>
>> Else, add @dev as child of /peripheral-anon. The child's name is made
>> up.
>>
>>
>> }
>> }
>>
>> dev->id is still the value of id=..., i.e. it may be null.
> yep, I was wrong and confused it child name in QOM tree.
>
>> Sure dereferencing dev->id in acpi_memory_hotplug_write() is safe?
>
> it aren't safe since guest may trigger this error when
> memory-device is created without id.
Thanks!
Daniel, the issue predates your series, but your series adds instances.
We need a patch fixing the existing instances before your series, and
fix up your series. Can you take care of that?
Use of migrate_add_blocker() in qxl.c
migrate_add_blocker() fails when running with --only-migratable, and
when migration is in progress.
qxl.c continues after migrate_add_blocker() fails:
{
/*
* Windows 8 drivers place qxl commands in the vram
* (instead of the ram) bar. We can't live migrate such a
* guest, so add a migration blocker in case we detect
* this, to avoid triggering the assert in pre_save().
*
*
https://cgit.freedesktop.org/spice/win32/qxl-wddm-dod/commit/?id=f6e099db39e7d0787f294d5fd0dce328b5210faa
*/
void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
if (msg != NULL && (
msg < (void *)qxl->vga.vram_ptr ||
msg > ((void *)qxl->vga.vram_ptr + qxl->vga.vram_size))) {
if (!qxl->migration_blocker) {
Error *local_err = NULL;
error_setg(>migration_blocker,
"qxl: guest bug: command not in ram bar");
migrate_add_blocker(qxl->migration_blocker, _err);
if (local_err) {
error_report_err(local_err);
}
}
}
}
Why is this safe? If the blocker is needed to prevent a crash, as the
comment says, then what prevents the crash when adding the blocker
fails?
Code goes back to
commit 86dbcdd9c7590d06db89ca256c5eaf0b4aba8858
Author: Gerd Hoffmann
Date: Mon Apr 10 13:31:31 2017 +0200
qxl: add migration blocker to avoid pre-save assert
Cc: 1635...@bugs.launchpad.net
Signed-off-by: Gerd Hoffmann
Reviewed-by: Marc-André Lureau
Message-id: 20170410113131.2585-1-kra...@redhat.com
