Tracewrap 8.1 #32

Rot127 · 2023-09-11T22:04:34Z

Should be merge to tracerap-8.1 though., not stable-8.1

Currently the emulation of VSTRS recognizes partial matches in presence of \0 in the haystack, which, according to PoP, is not correct: If the ZS flag is one and a zero byte was detected in the second operand, then there can not be a partial match ... Add a check for this. While at it, fold a number of explicitly handled special cases into the generic logic. Cc: [email protected] Reported-by: Claudio Fontana <[email protected]> Closes: https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg00633.html Fixes: 1d706f3 ("target/s390x: vxeh2: vector string search") Signed-off-by: Ilya Leoshkevich <[email protected]> Message-Id: <[email protected]> Tested-by: Claudio Fontana <[email protected]> Acked-by: David Hildenbrand <[email protected]> Signed-off-by: Thomas Huth <[email protected]> (cherry picked from commit 791b2b6) Signed-off-by: Michael Tokarev <[email protected]>

Unlike most other instructions that contain an immediate element index, VREP's one is 16-bit, and not 4-bit. The code uses only 8 bits, so using, e.g., 0x101 does not lead to a specification exception. Fix by checking all 16 bits. Cc: [email protected] Fixes: 28d0873 ("s390x/tcg: Implement VECTOR REPLICATE") Signed-off-by: Ilya Leoshkevich <[email protected]> Message-Id: <[email protected]> Reviewed-by: David Hildenbrand <[email protected]> Signed-off-by: Thomas Huth <[email protected]> (cherry picked from commit 23e87d4) Signed-off-by: Michael Tokarev <[email protected]>

The length is always truncated to 16 bytes. Do not probe more than that. Cc: [email protected] Fixes: 0e0a5b4 ("s390x/tcg: Implement VECTOR STORE WITH LENGTH") Signed-off-by: Ilya Leoshkevich <[email protected]> Message-Id: <[email protected]> Reviewed-by: David Hildenbrand <[email protected]> Signed-off-by: Thomas Huth <[email protected]> (cherry picked from commit 6db3518) Signed-off-by: Michael Tokarev <[email protected]>

VFMIN and VFMAX should raise a specification exceptions when bits 1-3 of M5 are set. Cc: [email protected] Fixes: da48075 ("s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)") Signed-off-by: Ilya Leoshkevich <[email protected]> Message-Id: <[email protected]> Reviewed-by: David Hildenbrand <[email protected]> Signed-off-by: Thomas Huth <[email protected]> (cherry picked from commit 6a2ea61) Signed-off-by: Michael Tokarev <[email protected]>

…hosts Using "-device virtio-gpu,blob=true" currently does not work on big endian hosts (like s390x). The guest kernel prints an error message like: [drm:virtio_gpu_dequeue_ctrl_func [virtio_gpu]] *ERROR* response 0x1200 (command 0x10c) and the display stays black. When running QEMU with "-d guest_errors", it shows an error message like this: virtio_gpu_create_mapping_iov: nr_entries is too big (83886080 > 16384) which indicates that this value has not been properly byte-swapped. And indeed, the virtio_gpu_create_blob_bswap() function (that should swap the fields in the related structure) fails to swap some of the entries. After correctly swapping all missing values here, too, the virtio-gpu device is now also working with blob=true on s390x hosts. Fixes: e0933d9 ("virtio-gpu: Add virtio_gpu_resource_create_blob") Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2230469 Message-Id: <[email protected]> Reviewed-by: Marc-André Lureau <[email protected]> Signed-off-by: Thomas Huth <[email protected]> (cherry picked from commit d194362) Signed-off-by: Michael Tokarev <[email protected]>

kvm_arch_get_default_type() returns the default KVM type. This hook is particularly useful to derive a KVM type that is valid for "none" machine model, which is used by libvirt to probe the availability of KVM. For MIPS, the existing mips_kvm_type() is reused. This function ensures the availability of VZ which is mandatory to use KVM on the current QEMU. Cc: [email protected] Signed-off-by: Akihiko Odaki <[email protected]> Message-id: [email protected] Reviewed-by: Peter Maydell <[email protected]> [PMM: added doc comment for new function] Signed-off-by: Peter Maydell <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit 5e0d659) Signed-off-by: Michael Tokarev <[email protected]>

Before this change, the default KVM type, which is used for non-virt machine models, was 0. The kernel documentation says: > On arm64, the physical address size for a VM (IPA Size limit) is > limited to 40bits by default. The limit can be configured if the host > supports the extension KVM_CAP_ARM_VM_IPA_SIZE. When supported, use > KVM_VM_TYPE_ARM_IPA_SIZE(IPA_Bits) to set the size in the machine type > identifier, where IPA_Bits is the maximum width of any physical > address used by the VM. The IPA_Bits is encoded in bits[7-0] of the > machine type identifier. > > e.g, to configure a guest to use 48bit physical address size:: > > vm_fd = ioctl(dev_fd, KVM_CREATE_VM, KVM_VM_TYPE_ARM_IPA_SIZE(48)); > > The requested size (IPA_Bits) must be: > > == ========================================================= > 0 Implies default size, 40bits (for backward compatibility) > N Implies N bits, where N is a positive integer such that, > 32 <= N <= Host_IPA_Limit > == ========================================================= > Host_IPA_Limit is the maximum possible value for IPA_Bits on the host > and is dependent on the CPU capability and the kernel configuration. > The limit can be retrieved using KVM_CAP_ARM_VM_IPA_SIZE of the > KVM_CHECK_EXTENSION ioctl() at run-time. > > Creation of the VM will fail if the requested IPA size (whether it is > implicit or explicit) is unsupported on the host. https://docs.kernel.org/virt/kvm/api.html#kvm-create-vm So if Host_IPA_Limit < 40, specifying 0 as the type will fail. This actually confused libvirt, which uses "none" machine model to probe the KVM availability, on M2 MacBook Air. Fix this by using Host_IPA_Limit as the default type when KVM_CAP_ARM_VM_IPA_SIZE is available. Cc: [email protected] Signed-off-by: Akihiko Odaki <[email protected]> Message-id: [email protected] Reviewed-by: Peter Maydell <[email protected]> Signed-off-by: Peter Maydell <[email protected]> (cherry picked from commit 1ab445a) Signed-off-by: Michael Tokarev <[email protected]>

A typo, noted in the bug report, resulting in an incorrect write offset. Cc: [email protected] Fixes: 7390e0e ("target/arm: Implement SME LD1, ST1") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1833 Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-id: [email protected] Signed-off-by: Peter Maydell <[email protected]> (cherry picked from commit 4b3520f) Signed-off-by: Michael Tokarev <[email protected]>

Typo applied byte-wise shift instead of double-word shift. Cc: [email protected] Fixes: 631e565 ("target/arm: Create gen_gvec_[us]sra") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1737 Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Message-id: [email protected] Signed-off-by: Peter Maydell <[email protected]> (cherry picked from commit cd1e4db) Signed-off-by: Michael Tokarev <[email protected]>

In early 2021 (see commit 2ad7843 "docs: update README to use GitLab repo URLs") almost all of the code base was converted to point to GitLab instead of git.qemu.org. During 2023, git.qemu.org switched from a git mirror to a http redirect to GitLab (see [1]). Update the LICENSE URL to match its previous content, displaying the file raw content similarly to gitweb 'blob_plain' format ([2]). [1] https://lore.kernel.org/qemu-devel/CABgObfZu3mFc8tM20K-yXdt7F-7eV-uKZN4sKDarSeu7DYoRbA@mail.gmail.com/ [2] https://git-scm.com/docs/gitweb#Documentation/gitweb.txt-blobplain Reviewed-by: Daniel P. Berrangé <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Thomas Huth <[email protected]> Reviewed-by: Stefan Hajnoczi <[email protected]> Signed-off-by: Stefan Hajnoczi <[email protected]> Message-ID: <[email protected]> (cherry picked from commit 09a3fff) Signed-off-by: Michael Tokarev <[email protected]>

Acked-by: Alex Bennée <[email protected]> Suggested-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 86e4f93) Signed-off-by: Michael Tokarev <[email protected]>

XVilka

LGTM. But I suggest to do at least the basic CI first, which ensures it still compiles and runs (prints version, for example): #30

DMaroo · 2023-09-12T08:17:02Z

Should be merge to tracerap-8.1 though, not stable-8.1

I think you can do that. There is an option to change it on GitHub UI.

DMaroo · 2023-09-12T08:21:16Z

protobuf/fix_proto_src.py

+This just does:
+sed -i 's/->base/->__base/g' <file>
+sed -i 's/ProtobufCMessage base;/ProtobufCMessage __base;/g' <file>
+"""


Why can't we just have a shell script which does this? Why do we need Python? Other than the fact that it is easy to write, I don't see any other reason. But again it's just a one time effort, so we might as well not force people to run Python when this can be also done with bash.

I don't know. I did just copy the files in this case.

Rot127 · 2023-09-12T15:02:15Z

I think you can do that. There is an option to change it on GitHub UI.

Only have the option to select an already present branch.

Why can't we just have a shell script which does this? Why do we need Python? Other than the fact that it is easy to write, I don't see any other reason. But again it's just a one time effort, so we might as well not force people to run Python when this can be also done with bash.

Guess this is run during build. But nonetheless we should do it in another PR anyways.

Rot127 · 2023-09-12T15:12:54Z

I suggest to do at least the basic CI first

@XVilka Just added one. Could it be that you need to approve the run?

XVilka · 2023-09-12T15:16:32Z

I suggest to do at least the basic CI first

@XVilka Just added one. Could it be that you need to approve the run?

Probably Actions need to be enabled first. @ivg could you please do that?

Rot127 · 2023-09-12T23:02:36Z

Just figured that we won't catch build errors like the one fixed in d1fe736 if there is no code which uses tracewrap.
So the CI build is only useful with an arch.

We can fail the blk_insert_bs() at init_blk_migration(), leaving the BlkMigDevState without a dirty_bitmap and BlockDriverState. Account for the possibly missing elements when doing cleanup. Fix the following crashes: Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 359 BlockDriverState *bs = bitmap->bs; #0 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 qemu#1 0x0000555555bba331 in unset_dirty_tracking () at ../migration/block.c:371 qemu#2 0x0000555555bbad98 in block_migration_cleanup_bmds () at ../migration/block.c:681 Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 7073 QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) { #0 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 qemu#1 0x0000555555e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at ../block.c:7095 qemu#2 0x0000555555bbae13 in block_migration_cleanup_bmds () at ../migration/block.c:690 Signed-off-by: Fabiano Rosas <[email protected]> Message-id: [email protected] Signed-off-by: Stefan Hajnoczi <[email protected]> (cherry picked from commit f187609) Signed-off-by: Michael Tokarev <[email protected]>

This is a mandatory feature for Armv8.1 architectures but we don't state the feature clearly in our emulation list. Also include FEAT_CRC32 comment in aarch64_max_tcg_initfn for ease of grepping. Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Alex Bennée <[email protected]> Message-id: [email protected] Cc: [email protected] Message-Id: <[email protected]> [PMM: pluralize 'instructions' in docs] Signed-off-by: Peter Maydell <[email protected]> (cherry picked from commit 9e771a2) Signed-off-by: Michael Tokarev <[email protected]>

PIE executables are usually linked at offset 0 and are relocated somewhere during load. The hiaddr needs to be adjusted to keep the brk next to the executable. Cc: [email protected] Fixes: 1f356e8 ("linux-user: Adjust initial brk when interpreter is close to executable") Tested-by: Helge Deller <[email protected]> Reviewed-by: Ilya Leoshkevich <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit aec338d) Signed-off-by: Michael Tokarev <[email protected]>

Otherwise tcg_handle_interrupt() triggers an assertion failure: qemu#5 0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83 qemu#6 tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81 qemu#7 0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555 qemu#8 0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611 qemu#9 0x00007fffa42bde14 in code_gen_buffer () qemu#10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457 Cc: [email protected] Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808 Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328> Co-developed-by: Richard Henderson <[email protected]>' Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]> (cherry picked from commit c1f27a0) Signed-off-by: Michael Tokarev <[email protected]>

Fixes: 142ca62 ("ui: add a D-Bus display backend") Fixes: de9f844 ("ui/dbus: Expose a touch device interface") Signed-off-by: Bilal Elmoussaoui <[email protected]> Reviewed-by: Marc-André Lureau <[email protected]> Message-Id: <[email protected]> (cherry picked from commit cb6ccdc) Signed-off-by: Michael Tokarev <[email protected]>

Failing to reset the of_instance_last makes ihandle allocation continue to increase, which causes record-replay replay fail to match the recorded trace. Not resetting claimed_base makes VOF eventually run out of memory after some resets. Cc: Alexey Kardashevskiy <[email protected]> Fixes: fc8c745 ("spapr: Implement Open Firmware client interface") Signed-off-by: Nicholas Piggin <[email protected]> Reviewed-by: Alexey Kardashevskiy <[email protected]> Signed-off-by: Cédric Le Goater <[email protected]> (cherry picked from commit 7b8589d) Signed-off-by: Michael Tokarev <[email protected]>

ppce500_reset_device_tree is registered for system reset, but after c4b0753 this function rerandomizes rng-seed via qemu_guest_getrandom_nofail. And when loading a snapshot, it tries to read EVENT_RANDOM that doesn't exist, so we have an error: qemu-system-ppc: Missing random event in the replay log To fix this, use qemu_register_reset_nosnapshotload instead of qemu_register_reset. Reported-by: Vitaly Cheptsov <[email protected]> Fixes: c4b0753 ("hw/ppc: pass random seed to fdt ") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1634 Signed-off-by: Maksim Kostin <[email protected]> Reviewed-by: Nicholas Piggin <[email protected]> Signed-off-by: Cédric Le Goater <[email protected]> (cherry picked from commit 6ec65b6) Signed-off-by: Michael Tokarev <[email protected]>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1779 Signed-off-by: Richard Henderson <[email protected]> Reviewed-by: Nicholas Piggin <[email protected]> Signed-off-by: Cédric Le Goater <[email protected]> (cherry picked from commit af03aeb) Signed-off-by: Michael Tokarev <[email protected]>

Rot127 · 2023-11-21T16:07:32Z

@ivg Please take a look again. Especially at the changes to the workflow.
If you merge it, please don't forget to merge it into tracewrap-8.1

.github/workflows/build.yaml

ivg · 2023-11-21T17:16:43Z

.github/workflows/build.yaml

+        echo "NOTE:"
+        echo "Please update this workflow so it builds the target you are working on."
+        echo "See the commented lines for an example."
+        exit 1


Okay, I am not getting it, what's the purpose of a workflow that doesn't build the code? Let's at least build it without any targets, or with a target but that is not tracewrapped.

My thinking was, that we only want to test the build for targets which have tracing implemented. Since tracewrap-8.1 has no such target, we just skip it and notify the user to add the build step in the CI for their target.
So for tracewrap-8.1-hexagon we only build Hexagon and so forth.

The reason I didn't added a normal build here, was that I did not see the point in wasting CI time, if we do not care about building qemu without targets with tracing.
Would you agree with that?

This branch contains changes to TCG and other core parts of qemu, so we would like to be at least sure that they do compile. Building with any target, e.g., x86, or maybe without any targets at all (if it builds the core part will check that a change to the code base doesn't break at least the compilation part.

Otherwise, this CI will just waste time by only installing the prerequisites of the repository, in which I don't see any point.

ivg

We need a workflow that at least builds the code that is committed.

.github/workflows/build.yaml

ivg · 2023-11-22T18:16:11Z

As a final question, do you prefer it to be rebased, squashed, or just merged with a merge commit?

Rot127 · 2023-11-23T13:45:45Z

@ivg I think squashed into tracewrap-8.1-base is fine.

XVilka · 2023-11-27T15:54:17Z

@ivg apparently @Rot127 asked to squash by mistake. We have fixed the history in stable-8.1 and tracewrap-8.1 branches, but now there is protected branch tracewrap-8.1-base that I cannot remove, could you please remove it?

https://github.com/BinaryAnalysisPlatform/qemu/branches

ivg · 2023-11-27T16:56:27Z

I can't also remove the protected branch (we should have the same permission btw), so I need to remove the protection before deleting it. I will remove the protection, delete it, and re-instantiate the protection back.

iii-i and others added 11 commits August 23, 2023 16:55

softmmu: Assert data in bounds in iotlb_to_section

5691fbf

Acked-by: Alex Bennée <[email protected]> Suggested-by: Alex Bennée <[email protected]> Signed-off-by: Richard Henderson <[email protected]> (cherry picked from commit 86e4f93) Signed-off-by: Michael Tokarev <[email protected]>

XVilka approved these changes Sep 12, 2023

View reviewed changes

XVilka requested review from thestr4ng3r, ivg and DMaroo September 12, 2023 05:57

DMaroo approved these changes Sep 12, 2023

View reviewed changes

XVilka assigned ivg Sep 13, 2023

Fabiano Rosas and others added 8 commits September 21, 2023 19:35

Rot127 force-pushed the tracewrap-8.1 branch 8 times, most recently from f26434f to 65598fd Compare November 21, 2023 15:45

Rot127 force-pushed the tracewrap-8.1 branch from 65598fd to 3882417 Compare November 21, 2023 16:10

ivg reviewed Nov 21, 2023

View reviewed changes

.github/workflows/build.yaml Show resolved Hide resolved

ivg reviewed Nov 21, 2023

View reviewed changes

.github/workflows/build.yaml Outdated Show resolved Hide resolved

ivg reviewed Nov 21, 2023

View reviewed changes

ivg requested changes Nov 21, 2023

View reviewed changes

Rot127 force-pushed the tracewrap-8.1 branch 7 times, most recently from cafecdf to 403c9ea Compare November 21, 2023 18:51

Update CI build.yaml

67f77c8

Rot127 force-pushed the tracewrap-8.1 branch from 403c9ea to 67f77c8 Compare November 21, 2023 18:51

ivg reviewed Nov 21, 2023

View reviewed changes

.github/workflows/build.yaml Show resolved Hide resolved

ivg approved these changes Nov 22, 2023

View reviewed changes

ivg merged commit 885ca60 into BinaryAnalysisPlatform:stable-8.1 Nov 27, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracewrap 8.1 #32

Tracewrap 8.1 #32

Rot127 commented Sep 11, 2023

XVilka left a comment

DMaroo commented Sep 12, 2023

DMaroo Sep 12, 2023

Rot127 Nov 21, 2023

Rot127 commented Sep 12, 2023

Rot127 commented Sep 12, 2023

XVilka commented Sep 12, 2023

Rot127 commented Sep 12, 2023

Rot127 commented Nov 21, 2023 •

edited

Loading

ivg Nov 21, 2023

Rot127 Nov 21, 2023 •

edited

Loading

ivg Nov 21, 2023

ivg left a comment

ivg commented Nov 22, 2023

Rot127 commented Nov 23, 2023

XVilka commented Nov 27, 2023 •

edited

Loading

ivg commented Nov 27, 2023

Tracewrap 8.1 #32

Tracewrap 8.1 #32

Conversation

Rot127 commented Sep 11, 2023

XVilka left a comment

Choose a reason for hiding this comment

DMaroo commented Sep 12, 2023

DMaroo Sep 12, 2023

Choose a reason for hiding this comment

Rot127 Nov 21, 2023

Choose a reason for hiding this comment

Rot127 commented Sep 12, 2023

Rot127 commented Sep 12, 2023

XVilka commented Sep 12, 2023

Rot127 commented Sep 12, 2023

Rot127 commented Nov 21, 2023 • edited Loading

ivg Nov 21, 2023

Choose a reason for hiding this comment

Rot127 Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

ivg Nov 21, 2023

Choose a reason for hiding this comment

ivg left a comment

Choose a reason for hiding this comment

ivg commented Nov 22, 2023

Rot127 commented Nov 23, 2023

XVilka commented Nov 27, 2023 • edited Loading

ivg commented Nov 27, 2023

Rot127 commented Nov 21, 2023 •

edited

Loading

Rot127 Nov 21, 2023 •

edited

Loading

XVilka commented Nov 27, 2023 •

edited

Loading