Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
On Thu, Dec 17, 2020 at 10:10 AM Warner Losh wrote: > > > On Thu, Dec 17, 2020 at 9:21 AM Peter Maydell > wrote: > >> On Thu, 17 Dec 2020 at 16:03, Warner Losh wrote: >> > On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé >> wrote: >> >> I don't recall what happened after that initial discussion about >> >> merging the new impl. Did Sean simply not have the time to invest >> >> in the merge ? I'll CC him here to see what opinion he has on the >> >> future of bsd-user in QEMU. >> > >> > >> > I've actually taken over for Sean Bruno managing this. >> >> > I'd love to hear from people ways that I can speed things up. >> >> There was a bit of discussion about this on #qemu IRC the other >> day, coincidentally. I think the conclusion we (upstream QEMU) >> came to was that we'd be happy with a "delete all of bsd-user >> and reinstate" approach, assuming that the "reinstate" part is >> in reasonably logical chunks and not one big "here's what we >> have all in one lump" patch. >> >> AIUI from IRC this is being primarily driven by FreeBSD and >> NetBSD/OpenBSD support is merely "we hope it is not broken >> by the delete-and-reinstate but it was probably broken anyway" ? >> > > Yea, I don't think it actually works for anything non-trivial on the other > BSDs. > Looking at the changes, it may be possible to get the first dozen or so into a recent tree. It's not until after that that the changes touch areas that have the high churn rate, but it may mean things like threaded apps may have issues... I'll see what I can do over the holidays. Warner
Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
On Thu, Dec 17, 2020 at 9:21 AM Peter Maydell wrote: > On Thu, 17 Dec 2020 at 16:03, Warner Losh wrote: > > On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé > wrote: > >> I don't recall what happened after that initial discussion about > >> merging the new impl. Did Sean simply not have the time to invest > >> in the merge ? I'll CC him here to see what opinion he has on the > >> future of bsd-user in QEMU. > > > > > > I've actually taken over for Sean Bruno managing this. > > > I'd love to hear from people ways that I can speed things up. > > There was a bit of discussion about this on #qemu IRC the other > day, coincidentally. I think the conclusion we (upstream QEMU) > came to was that we'd be happy with a "delete all of bsd-user > and reinstate" approach, assuming that the "reinstate" part is > in reasonably logical chunks and not one big "here's what we > have all in one lump" patch. > > AIUI from IRC this is being primarily driven by FreeBSD and > NetBSD/OpenBSD support is merely "we hope it is not broken > by the delete-and-reinstate but it was probably broken anyway" ? > Yea, I don't think it actually works for anything non-trivial on the other BSDs. Warner
Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
On Thu, 17 Dec 2020 at 16:03, Warner Losh wrote: > On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé > wrote: >> I don't recall what happened after that initial discussion about >> merging the new impl. Did Sean simply not have the time to invest >> in the merge ? I'll CC him here to see what opinion he has on the >> future of bsd-user in QEMU. > > > I've actually taken over for Sean Bruno managing this. > I'd love to hear from people ways that I can speed things up. There was a bit of discussion about this on #qemu IRC the other day, coincidentally. I think the conclusion we (upstream QEMU) came to was that we'd be happy with a "delete all of bsd-user and reinstate" approach, assuming that the "reinstate" part is in reasonably logical chunks and not one big "here's what we have all in one lump" patch. AIUI from IRC this is being primarily driven by FreeBSD and NetBSD/OpenBSD support is merely "we hope it is not broken by the delete-and-reinstate but it was probably broken anyway" ? thanks -- PMM
Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé wrote: > On Thu, Dec 17, 2020 at 02:03:47PM +0100, Thomas Huth wrote: > > On 17/12/2020 13.51, Peter Maydell wrote: > > > On Wed, 16 Dec 2020 at 17:29, Thomas Huth wrote: > > >> > > >> Hi! > > >> > > >> The following changes since commit > af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f: > > >> > > >> Merge remote-tracking branch > 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-12-15 > 21:24:31 +) > > >> > > >> are available in the Git repository at: > > >> > > >> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16 > > >> > > >> for you to fetch changes up to > cbbedfeeb77e25b065f8a2b0c33e81403edaf728: > > >> > > >> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 > 12:52:20 +0100) > > >> > > >> > > >> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in > > >> switch-case statements > > >> > > > > > > Hi; this generates a new warning on the NetBSD build: > > > > > > ../src/bsd-user/main.c: In function 'cpu_loop': > > > ../src/bsd-user/main.c:513:16: warning: this statement may fall > > > through [-Wimplicit-fallthrough=] > > > if (bsd_type != target_freebsd) > > > ^ > > > ../src/bsd-user/main.c:515:9: note: here > > > case 0x100: > > > ^~~~ > > > > Oh man, can't we just ditch the bsd-user folder now? It's known to be > broken > > since many releases, so it's currently only causing additional effort to > > keep this code compilable (also with regards to the automatic code scan > tool > > reports that we've seen during the past months), without real benefit. > Even > > if the BSD folks finally upstream their fixed version again, it's more > > likely that they will start from scratch again instead of fixing the old > > folder, I guess? > > Yeah, it has been a while since we last discussed this: > > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html > > Meanwhile their out of free bsd-user impl continues to be developed > until Dec 2019 at least: > > https://github.com/seanbruno/qemu-bsd-user/commits/bsd-user You should check out the bsd-user-rebase-3.1 branch. The most recent commit was this month... > > I don't recall what happened after that initial discussion about > merging the new impl. Did Sean simply not have the time to invest > in the merge ? I'll CC him here to see what opinion he has on the > future of bsd-user in QEMU. > I've actually taken over for Sean Bruno managing this. I spent some time and rebased our extensive work up through QEMU 3.1, but I hit some snags with changes in the underlying QEMU, and we had a few bugs we had to sort out to make it stable, which took some time. There were other people doing the sorting out since it affected them, but not me directly. By the time they were sorted out, 4.1 was being released. Plus I blew about a week trying to make every single commit compile rather than just the tip of the branch. I spent quite a bit of time curating the changes into more logical bits, which also chewed up a few weeks and in the end was only partially complete. In the end, The main problem with the merge, which I've done a couple of times, is that this is so tied to the structure of QEMU, a part that has a high velocity is that it takes so long to do the merge that by the time you're done, things have changed again in the code base and you have to do a lot more work to catch up. The code that's in the tree is so bit-rotted I'd be surprised if it could run anything beyond trivial programs. I'd love to hear from people ways that I can speed things up. I'd love to get this back into the tree since the version we have in the Warner
Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
On Thu, Dec 17, 2020 at 02:03:47PM +0100, Thomas Huth wrote: > On 17/12/2020 13.51, Peter Maydell wrote: > > On Wed, 16 Dec 2020 at 17:29, Thomas Huth wrote: > >> > >> Hi! > >> > >> The following changes since commit > >> af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f: > >> > >> Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' > >> into staging (2020-12-15 21:24:31 +) > >> > >> are available in the Git repository at: > >> > >> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16 > >> > >> for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728: > >> > >> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 > >> +0100) > >> > >> > >> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in > >> switch-case statements > >> > > > > Hi; this generates a new warning on the NetBSD build: > > > > ../src/bsd-user/main.c: In function 'cpu_loop': > > ../src/bsd-user/main.c:513:16: warning: this statement may fall > > through [-Wimplicit-fallthrough=] > > if (bsd_type != target_freebsd) > > ^ > > ../src/bsd-user/main.c:515:9: note: here > > case 0x100: > > ^~~~ > > Oh man, can't we just ditch the bsd-user folder now? It's known to be broken > since many releases, so it's currently only causing additional effort to > keep this code compilable (also with regards to the automatic code scan tool > reports that we've seen during the past months), without real benefit. Even > if the BSD folks finally upstream their fixed version again, it's more > likely that they will start from scratch again instead of fixing the old > folder, I guess? Yeah, it has been a while since we last discussed this: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html Meanwhile their out of free bsd-user impl continues to be developed until Dec 2019 at least: https://github.com/seanbruno/qemu-bsd-user/commits/bsd-user I don't recall what happened after that initial discussion about merging the new impl. Did Sean simply not have the time to invest in the merge ? I'll CC him here to see what opinion he has on the future of bsd-user in QEMU. 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 :|
Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
On 17/12/2020 13.51, Peter Maydell wrote: > On Wed, 16 Dec 2020 at 17:29, Thomas Huth wrote: >> >> Hi! >> >> The following changes since commit af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f: >> >> Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' >> into staging (2020-12-15 21:24:31 +) >> >> are available in the Git repository at: >> >> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16 >> >> for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728: >> >> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 >> +0100) >> >> >> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in >> switch-case statements >> > > Hi; this generates a new warning on the NetBSD build: > > ../src/bsd-user/main.c: In function 'cpu_loop': > ../src/bsd-user/main.c:513:16: warning: this statement may fall > through [-Wimplicit-fallthrough=] > if (bsd_type != target_freebsd) > ^ > ../src/bsd-user/main.c:515:9: note: here > case 0x100: > ^~~~ Oh man, can't we just ditch the bsd-user folder now? It's known to be broken since many releases, so it's currently only causing additional effort to keep this code compilable (also with regards to the automatic code scan tool reports that we've seen during the past months), without real benefit. Even if the BSD folks finally upstream their fixed version again, it's more likely that they will start from scratch again instead of fixing the old folder, I guess? Thomas
Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
On Wed, 16 Dec 2020 at 17:29, Thomas Huth wrote: > > Hi! > > The following changes since commit af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f: > > Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' > into staging (2020-12-15 21:24:31 +) > > are available in the Git repository at: > > https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16 > > for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728: > > configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 +0100) > > > * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in > switch-case statements > Hi; this generates a new warning on the NetBSD build: ../src/bsd-user/main.c: In function 'cpu_loop': ../src/bsd-user/main.c:513:16: warning: this statement may fall through [-Wimplicit-fallthrough=] if (bsd_type != target_freebsd) ^ ../src/bsd-user/main.c:515:9: note: here case 0x100: ^~~~ thanks -- PMM
Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
On 17/12/2020 00.01, no-re...@patchew.org wrote: > Patchew URL: https://patchew.org/QEMU/20201216172949.57380-1-th...@redhat.com/ > > > > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Type: series > Message-id: 20201216172949.57380-1-th...@redhat.com > Subject: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough > > === TEST SCRIPT BEGIN === > #!/bin/bash > git rev-parse base > /dev/null || exit 0 > git config --local diff.renamelimit 0 > git config --local diff.renames True > git config --local diff.algorithm histogram > ./scripts/checkpatch.pl --mailback base.. > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > From https://github.com/patchew-project/qemu > * [new tag] patchew/20201216172949.57380-1-th...@redhat.com -> > patchew/20201216172949.57380-1-th...@redhat.com > Switched to a new branch 'test' > 7bedbc8 configure: Compile with -Wimplicit-fallthrough=2 > e14bb9d tests/fp: Do not emit implicit-fallthrough warnings in the softfloat > tests > ebd3c45 tcg/optimize: Add fallthrough annotations > cfe5662 target/sparc/win_helper: silence the compiler warnings > 8ef9335 target/sparc/translate: silence the compiler warnings > 4588bf9 accel/tcg/user-exec: silence the compiler warnings > be2108e hw/intc/arm_gicv3_kvm: silence the compiler warnings > 7d033d0 target/i386: silence the compiler warnings in gen_shiftd_rm_T1 > 284b00a hw/timer/renesas_tmr: silence the compiler warnings > c3d2957 hw/rtc/twl92230: Silence warnings about missing fallthrough statements > 1b1609c target/unicore32/translate: Add missing fallthrough annotations > 99bc0f0 disas/libvixl: Fix fall-through annotation for GCC >= 7 > > === OUTPUT BEGIN === > 1/12 Checking commit 99bc0f0e92b7 (disas/libvixl: Fix fall-through annotation > for GCC >= 7) > ERROR: do not use C99 // comments > #49: FILE: disas/libvixl/vixl/globals.h:111: > +// Fallthrough annotation for Clang and C++11(201103L). > > ERROR: do not use C99 // comments > #52: FILE: disas/libvixl/vixl/globals.h:114: > +// Fallthrough annotation for GCC >= 7. Well, libvixl is C++ code and the upstream patch used these comments, too, so this error can be ignored. > total: 2 errors, 0 warnings, 24 lines checked > > Patch 1/12 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > 2/12 Checking commit 1b1609c7573a (target/unicore32/translate: Add missing > fallthrough annotations) > 3/12 Checking commit c3d2957383b8 (hw/rtc/twl92230: Silence warnings about > missing fallthrough statements) > 4/12 Checking commit 284b00aef566 (hw/timer/renesas_tmr: silence the compiler > warnings) > 5/12 Checking commit 7d033d02b90d (target/i386: silence the compiler warnings > in gen_shiftd_rm_T1) > 6/12 Checking commit be2108e641c9 (hw/intc/arm_gicv3_kvm: silence the > compiler warnings) > 7/12 Checking commit 4588bf97482b (accel/tcg/user-exec: silence the compiler > warnings) > 8/12 Checking commit 8ef9335f2838 (target/sparc/translate: silence the > compiler warnings) > 9/12 Checking commit cfe56623ece8 (target/sparc/win_helper: silence the > compiler warnings) > 10/12 Checking commit ebd3c45fd052 (tcg/optimize: Add fallthrough annotations) > WARNING: architecture specific defines should be avoided > #35: FILE: include/qemu/compiler.h:230: > +#if __has_attribute(fallthrough) Maybe we should teach checkpatch.pl to allow these in compiler.h? Thomas
Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
Patchew URL: https://patchew.org/QEMU/20201216172949.57380-1-th...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20201216172949.57380-1-th...@redhat.com Subject: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20201216172949.57380-1-th...@redhat.com -> patchew/20201216172949.57380-1-th...@redhat.com Switched to a new branch 'test' 7bedbc8 configure: Compile with -Wimplicit-fallthrough=2 e14bb9d tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests ebd3c45 tcg/optimize: Add fallthrough annotations cfe5662 target/sparc/win_helper: silence the compiler warnings 8ef9335 target/sparc/translate: silence the compiler warnings 4588bf9 accel/tcg/user-exec: silence the compiler warnings be2108e hw/intc/arm_gicv3_kvm: silence the compiler warnings 7d033d0 target/i386: silence the compiler warnings in gen_shiftd_rm_T1 284b00a hw/timer/renesas_tmr: silence the compiler warnings c3d2957 hw/rtc/twl92230: Silence warnings about missing fallthrough statements 1b1609c target/unicore32/translate: Add missing fallthrough annotations 99bc0f0 disas/libvixl: Fix fall-through annotation for GCC >= 7 === OUTPUT BEGIN === 1/12 Checking commit 99bc0f0e92b7 (disas/libvixl: Fix fall-through annotation for GCC >= 7) ERROR: do not use C99 // comments #49: FILE: disas/libvixl/vixl/globals.h:111: +// Fallthrough annotation for Clang and C++11(201103L). ERROR: do not use C99 // comments #52: FILE: disas/libvixl/vixl/globals.h:114: +// Fallthrough annotation for GCC >= 7. total: 2 errors, 0 warnings, 24 lines checked Patch 1/12 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/12 Checking commit 1b1609c7573a (target/unicore32/translate: Add missing fallthrough annotations) 3/12 Checking commit c3d2957383b8 (hw/rtc/twl92230: Silence warnings about missing fallthrough statements) 4/12 Checking commit 284b00aef566 (hw/timer/renesas_tmr: silence the compiler warnings) 5/12 Checking commit 7d033d02b90d (target/i386: silence the compiler warnings in gen_shiftd_rm_T1) 6/12 Checking commit be2108e641c9 (hw/intc/arm_gicv3_kvm: silence the compiler warnings) 7/12 Checking commit 4588bf97482b (accel/tcg/user-exec: silence the compiler warnings) 8/12 Checking commit 8ef9335f2838 (target/sparc/translate: silence the compiler warnings) 9/12 Checking commit cfe56623ece8 (target/sparc/win_helper: silence the compiler warnings) 10/12 Checking commit ebd3c45fd052 (tcg/optimize: Add fallthrough annotations) WARNING: architecture specific defines should be avoided #35: FILE: include/qemu/compiler.h:230: +#if __has_attribute(fallthrough) total: 0 errors, 1 warnings, 43 lines checked Patch 10/12 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 11/12 Checking commit e14bb9ddd6f3 (tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests) 12/12 Checking commit 7bedbc83bcfa (configure: Compile with -Wimplicit-fallthrough=2) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20201216172949.57380-1-th...@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
