Just CC'ing Richard to make sure it catches his attention. On Sat, Jun 24, 2023 at 12:40:33AM -0600, Warner Losh wrote: > This change: > > commit f00506aeca2f6d92318967693f8da8c713c163f3 > Merge: d37158bb242 87e303de70f > Author: Peter Maydell <peter.mayd...@linaro.org> > Date: Wed Mar 29 11:19:19 2023 +0100 > > Merge tag 'pull-tcg-20230328' of https://gitlab.com/rth7680/qemu into > staging > > Use a local version of GTree [#285] > Fix page_set_flags vs the last page of the address space [#1528] > Re-enable gdbstub breakpoints under KVM > > # -----BEGIN PGP SIGNATURE----- > # > # iQFRBAABCgA7FiEEekgeeIaLTbaoWgXAZN846K9+IV8FAmQjcLIdHHJpY2hhcmQu > # aGVuZGVyc29uQGxpbmFyby5vcmcACgkQZN846K9+IV8rkgf/ZazodovRKxfaO622 > # mGW7ywIm+hIZYmKC7ObiMKFrBoCyeXH9yOLSx42T70QstWvBMukjovLMz1+Ttbo1 > # VOvpGH2B5W76l3i+muAlKxFRbBH2kMLTaL+BXtkmkL4FJ9bS8WiPApsL3lEX/q2E > # 3kqaT3N3C09sWO5oVAPGTUHL0EutKhOar2VZL0+PVPFzL3BNPhnQH9QcbNvDBV3n > # cx3GSXZyL7Plyi+qwsKf/3Jo+F2wr2NVf3Dqscu9T1N1kI5hSjRpwqUEJzJZ5rei > # ly/gBXC/J7+WN+x+w2JlN0kWXWqC0QbDfZnj96Pd3owWZ7j4sT9zR5fcNenecxlR > # 38Bo0w== > # =ysF7 > # -----END PGP SIGNATURE----- > # gpg: Signature made Tue 28 Mar 2023 23:56:50 BST > # gpg: using RSA key > 7A481E78868B4DB6A85A05C064DF38E8AF7E215F > # gpg: issuer "richard.hender...@linaro.org" > # gpg: Good signature from "Richard Henderson < > richard.hender...@linaro.org>" [full] > # Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A 05C0 64DF 38E8 > AF7E 215F > > * tag 'pull-tcg-20230328' of https://gitlab.com/rth7680/qemu: > softmmu: Restore use of CPU watchpoint for all accelerators > softmmu/watchpoint: Add missing 'qemu/error-report.h' include > softmmu: Restrict cpu_check_watchpoint / address_matches to TCG accel > linux-user/arm: Take more care allocating commpage > include/exec: Change reserved_va semantics to last byte > linux-user: Pass last not end to probe_guest_base > accel/tcg: Pass last not end to tb_invalidate_phys_range > accel/tcg: Pass last not end to tb_invalidate_phys_page_range__locked > accel/tcg: Pass last not end to page_collection_lock > accel/tcg: Pass last not end to PAGE_FOR_EACH_TB > accel/tcg: Pass last not end to page_reset_target_data > accel/tcg: Pass last not end to page_set_flags > linux-user: Diagnose misaligned -R size > tcg: use QTree instead of GTree > util: import GTree as QTree > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > breaks bsd-user. when I merge it to the bsd-user upstream blitz branch I > get memory allocation errors on startup. At least for armv7. > > specifically, if I back out the bsd-user part of both > commit 95059f9c313a7fbd7f22e4cdc1977c0393addc7b > Author: Richard Henderson <richard.hender...@linaro.org> > Date: Mon Mar 6 01:26:29 2023 +0300 > > include/exec: Change reserved_va semantics to last byte > > Change the semantics to be the last byte of the guest va, rather > than the following byte. This avoids some overflow conditions. > > Reviewed-by: Philippe Mathieu-Daud<C3><A9> <phi...@linaro.org> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > > and > > commit 49840a4a098149067789255bca6894645f411036 > Author: Richard Henderson <richard.hender...@linaro.org> > Date: Mon Mar 6 01:51:09 2023 +0300 > > accel/tcg: Pass last not end to page_set_flags > > Pass the address of the last byte to be changed, rather than > the first address past the last byte. This avoids overflow > when the last page of the address space is involved. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1528 > Reviewed-by: Philippe Mathieu-Daud<C3><A9> <phi...@linaro.org> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > > things work again. If I backout parts, it fails still. If I back out only > one of > the two, but not both, then it fails. > > What's happening is that we're picking a reserved_va that's overflowing when > we add 1 to it. this overflow goes away if I make the overflows not > possible: > diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c > index a88251f8705..bd86c0a8689 100644 > --- a/bsd-user/mmap.c > +++ b/bsd-user/mmap.c > @@ -237,8 +237,8 @@ unsigned long last_brk; > static abi_ulong mmap_find_vma_reserved(abi_ulong start, abi_ulong size, > abi_ulong alignment) > { > - abi_ulong addr; > - abi_ulong end_addr; > + uint64_t addr; > + uint64_t end_addr; > int prot; > int looped = 0; > > My question is, is this the right fix? The old code avoided the overflow in > two ways. 1 it set reserve_va to a page short (which if I fix that, it > works better, but not quite right). and it never computes an address that > may overflow (which the new code does without the above patch). > > It seems to work, but it looks super weird. > > Comments? > > Warrner
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
