meson.build | 7 +++++- include/hw/xen/xen.h | 2 +- accel/stubs/hax-stub.c | 1 - contrib/plugins/lockstep.c | 3 +++ hw/i386/acpi-build.c | 41 ++++++++++++++++------------------ stubs/xen-hw-stub.c | 4 ---- .gitlab-ci.d/check-patch.py | 4 ++-- tests/acceptance/boot_linux_console.py | 2 ++ tests/acceptance/replay_kernel.py | 2 ++ 9 files changed, 35 insertions(+), 31 deletions(-)
The following changes since commit b50ea0d54bbca7d440315c3d0c0f7a4d6537b180: Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20201113-1' into staging (2020-11-14 11:22:07 +0000) are available in the Git repository at: https://github.com/stsquad/qemu.git tags/pull-testing-and-misc-161120-1 for you to fetch changes up to 7025111a199b97ae806817788bec50f456c47d85: .gitlab-ci.d/check-patch: tweak output for CI logs (2020-11-16 11:08:40 +0000) ---------------------------------------------------------------- Various fixes - fix resource leak in a couple of plugin - fix build of Xen enabled i386 image on Aarch64 - maybe unitialized warning fix - disable unstable Spartan-3A acceptance test - terser output of gitlab checkpatch check ---------------------------------------------------------------- Alex Bennée (5): meson.build: fix building of Xen support for aarch64 include/hw/xen.h: drop superfluous struct stubs/xen-hw-stub: drop xenstore_store_pv_console_info stub accel/stubs: drop unused cpu.h include .gitlab-ci.d/check-patch: tweak output for CI logs Alex Chen (2): plugins: Fix resource leak in connect_socket() plugins: Fix two resource leaks in setup_socket() Philippe Mathieu-Daudé (2): hw/i386/acpi-build: Fix maybe-uninitialized error when ACPI hotplug off tests/acceptance: Disable Spartan-3A DSP 1800A test meson.build | 7 +++++- include/hw/xen/xen.h | 2 +- accel/stubs/hax-stub.c | 1 - contrib/plugins/lockstep.c | 3 +++ hw/i386/acpi-build.c | 41 ++++++++++++++++------------------ stubs/xen-hw-stub.c | 4 ---- .gitlab-ci.d/check-patch.py | 4 ++-- tests/acceptance/boot_linux_console.py | 2 ++ tests/acceptance/replay_kernel.py | 2 ++ 9 files changed, 35 insertions(+), 31 deletions(-) -- 2.20.1
On Mon, 16 Nov 2020 at 12:24, Alex Bennée <alex.bennee@linaro.org> wrote: > > The following changes since commit b50ea0d54bbca7d440315c3d0c0f7a4d6537b180: > > Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20201113-1' into staging (2020-11-14 11:22:07 +0000) > > are available in the Git repository at: > > https://github.com/stsquad/qemu.git tags/pull-testing-and-misc-161120-1 > > for you to fetch changes up to 7025111a199b97ae806817788bec50f456c47d85: > > .gitlab-ci.d/check-patch: tweak output for CI logs (2020-11-16 11:08:40 +0000) > > ---------------------------------------------------------------- > Various fixes > > - fix resource leak in a couple of plugin > - fix build of Xen enabled i386 image on Aarch64 > - maybe unitialized warning fix > - disable unstable Spartan-3A acceptance test > - terser output of gitlab checkpatch check Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
From: Alex Chen <alex.chen@huawei.com> Close the fd when the connect() fails. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Alex Chen <alex.chen@huawei.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20201109082829.87496-2-alex.chen@huawei.com> Message-Id: <20201110192316.26397-2-alex.bennee@linaro.org> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index a696673dff..319bd44b83 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -292,6 +292,7 @@ static bool connect_socket(const char *path) if (connect(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr)) < 0) { perror("failed to connect"); + close(fd); return false; } -- 2.20.1
From: Alex Chen <alex.chen@huawei.com> Either accept() fails or exits normally, we need to close the fd. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Alex Chen <alex.chen@huawei.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20201109082829.87496-3-alex.chen@huawei.com> Message-Id: <20201110192316.26397-3-alex.bennee@linaro.org> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index 319bd44b83..5aad50869d 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -268,11 +268,13 @@ static bool setup_socket(const char *path) socket_fd = accept(fd, NULL, NULL); if (socket_fd < 0 && errno != EINTR) { perror("accept socket"); + close(fd); return false; } qemu_plugin_outs("setup_socket::ready\n"); + close(fd); return true; } -- 2.20.1
Xen is supported on ARM although weirdly using the i386-softmmu model. Checking based on the host CPU meant we never enabled Xen support. It would be nice to enable CONFIG_XEN for aarch64-softmmu to make it not seem weird but that will require further build surgery. Fixes: 8a19980e3f ("configure: move accelerator logic to meson") Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Cc: Masami Hiramatsu <masami.hiramatsu@linaro.org> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Anthony Perard <anthony.perard@citrix.com> Cc: Paul Durrant <paul@xen.org> Message-Id: <20201110192316.26397-4-alex.bennee@linaro.org> diff --git a/meson.build b/meson.build index 61d883bc07..132bc49782 100644 --- a/meson.build +++ b/meson.build @@ -74,10 +74,15 @@ else endif accelerator_targets = { 'CONFIG_KVM': kvm_targets } +if cpu in ['x86', 'x86_64', 'arm', 'aarch64'] + # i368 emulator provides xenpv machine type for multiple architectures + accelerator_targets += { + 'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu'], + } +endif if cpu in ['x86', 'x86_64'] accelerator_targets += { 'CONFIG_HAX': ['i386-softmmu', 'x86_64-softmmu'], - 'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu'], 'CONFIG_HVF': ['x86_64-softmmu'], 'CONFIG_WHPX': ['i386-softmmu', 'x86_64-softmmu'], } -- 2.20.1
Chardev is already a typedef'ed struct. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201110192316.26397-5-alex.bennee@linaro.org> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index 1406648ca5..0f9962b1c1 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -28,7 +28,7 @@ int xen_is_pirq_msi(uint32_t msi_data); qemu_irq *xen_interrupt_controller_init(void); -void xenstore_store_pv_console_info(int i, struct Chardev *chr); +void xenstore_store_pv_console_info(int i, Chardev *chr); void xen_register_framebuffer(struct MemoryRegion *mr); -- 2.20.1
We should never build something that calls this without having it. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201110192316.26397-6-alex.bennee@linaro.org> diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c index 2ea8190921..15f3921a76 100644 --- a/stubs/xen-hw-stub.c +++ b/stubs/xen-hw-stub.c @@ -10,10 +10,6 @@ #include "hw/xen/xen.h" #include "hw/xen/xen-x86.h" -void xenstore_store_pv_console_info(int i, Chardev *chr) -{ -} - int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) { return -1; -- 2.20.1
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201110192316.26397-7-alex.bennee@linaro.org> diff --git a/accel/stubs/hax-stub.c b/accel/stubs/hax-stub.c index 1a9da83185..49077f88e3 100644 --- a/accel/stubs/hax-stub.c +++ b/accel/stubs/hax-stub.c @@ -14,7 +14,6 @@ */ #include "qemu/osdep.h" -#include "cpu.h" #include "sysemu/hax.h" int hax_sync_vcpus(void) -- 2.20.1
From: Philippe Mathieu-Daudé <philmd@redhat.com> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code is already in the "if (bsel || pcihp_bridge_en)" block statement, but it isn't smart enough to figure it out. Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)" block statement to fix (on Ubuntu): ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized in this function [-Werror=maybe-uninitialized] 496 | aml_append(parent_scope, method); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally") Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20201108204535.2319870-4-philmd@redhat.com> Message-Id: <20201110192316.26397-8-alex.bennee@linaro.org> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 4f66642d88..1f5c211245 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -465,34 +465,31 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, */ if (bsel || pcihp_bridge_en) { method = aml_method("PCNT", 0, AML_NOTSERIALIZED); - } - /* If bus supports hotplug select it and notify about local events */ - if (bsel) { - uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel)); - aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM"))); - aml_append(method, - aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */) - ); - aml_append(method, - aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request */) - ); - } + /* If bus supports hotplug select it and notify about local events */ + if (bsel) { + uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel)); - /* Notify about child bus events in any case */ - if (pcihp_bridge_en) { - QLIST_FOREACH(sec, &bus->child, sibling) { - int32_t devfn = sec->parent_dev->devfn; + aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM"))); + aml_append(method, aml_call2("DVNT", aml_name("PCIU"), + aml_int(1))); /* Device Check */ + aml_append(method, aml_call2("DVNT", aml_name("PCID"), + aml_int(3))); /* Eject Request */ + } - if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) { - continue; - } + /* Notify about child bus events in any case */ + if (pcihp_bridge_en) { + QLIST_FOREACH(sec, &bus->child, sibling) { + int32_t devfn = sec->parent_dev->devfn; + + if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) { + continue; + } - aml_append(method, aml_name("^S%.02X.PCNT", devfn)); + aml_append(method, aml_name("^S%.02X.PCNT", devfn)); + } } - } - if (bsel || pcihp_bridge_en) { aml_append(parent_scope, method); } qobject_unref(bsel); -- 2.20.1
On Mon, Nov 16, 2020 at 12:24:15PM +0000, Alex Bennée wrote: > From: Philippe Mathieu-Daudé <philmd@redhat.com> > > GCC 9.3.0 thinks that 'method' can be left uninitialized. This code > is already in the "if (bsel || pcihp_bridge_en)" block statement, > but it isn't smart enough to figure it out. > > Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)" > block statement to fix (on Ubuntu): > > ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': > ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized > in this function [-Werror=maybe-uninitialized] > 496 | aml_append(parent_scope, method); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > Message-Id: <20201108204535.2319870-4-philmd@redhat.com> > Message-Id: <20201110192316.26397-8-alex.bennee@linaro.org> BTW it's in my pull request alredy. Not sure why you are merging it too ... > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 4f66642d88..1f5c211245 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -465,34 +465,31 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, > */ > if (bsel || pcihp_bridge_en) { > method = aml_method("PCNT", 0, AML_NOTSERIALIZED); > - } > - /* If bus supports hotplug select it and notify about local events */ > - if (bsel) { > - uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel)); > > - aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM"))); > - aml_append(method, > - aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */) > - ); > - aml_append(method, > - aml_call2("DVNT", aml_name("PCID"), aml_int(3)/* Eject Request */) > - ); > - } > + /* If bus supports hotplug select it and notify about local events */ > + if (bsel) { > + uint64_t bsel_val = qnum_get_uint(qobject_to(QNum, bsel)); > > - /* Notify about child bus events in any case */ > - if (pcihp_bridge_en) { > - QLIST_FOREACH(sec, &bus->child, sibling) { > - int32_t devfn = sec->parent_dev->devfn; > + aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM"))); > + aml_append(method, aml_call2("DVNT", aml_name("PCIU"), > + aml_int(1))); /* Device Check */ > + aml_append(method, aml_call2("DVNT", aml_name("PCID"), > + aml_int(3))); /* Eject Request */ > + } > > - if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) { > - continue; > - } > + /* Notify about child bus events in any case */ > + if (pcihp_bridge_en) { > + QLIST_FOREACH(sec, &bus->child, sibling) { > + int32_t devfn = sec->parent_dev->devfn; > + > + if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) { > + continue; > + } > > - aml_append(method, aml_name("^S%.02X.PCNT", devfn)); > + aml_append(method, aml_name("^S%.02X.PCNT", devfn)); > + } > } > - } > > - if (bsel || pcihp_bridge_en) { > aml_append(parent_scope, method); > } > qobject_unref(bsel); > -- > 2.20.1
On 11/16/20 1:27 PM, Michael S. Tsirkin wrote: > On Mon, Nov 16, 2020 at 12:24:15PM +0000, Alex Bennée wrote: >> From: Philippe Mathieu-Daudé <philmd@redhat.com> >> >> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code >> is already in the "if (bsel || pcihp_bridge_en)" block statement, >> but it isn't smart enough to figure it out. >> >> Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)" >> block statement to fix (on Ubuntu): >> >> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': >> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized >> in this function [-Werror=maybe-uninitialized] >> 496 | aml_append(parent_scope, method); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> cc1: all warnings being treated as errors >> >> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally") >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> Message-Id: <20201108204535.2319870-4-philmd@redhat.com> >> Message-Id: <20201110192316.26397-8-alex.bennee@linaro.org> > > BTW it's in my pull request alredy. > Not sure why you are merging it too ... I suppose to unbreak Gitlab-CI... There is no policy w.r.t. CI so maintainer don't have to use it, but this breaking it delay the workflow of others subsystems. I'm not asking you to use it, just explaining why this patch is in Alex's queue. Regards, Phil.
On Mon, Nov 16, 2020 at 02:11:35PM +0100, Philippe Mathieu-Daudé wrote: > On 11/16/20 1:27 PM, Michael S. Tsirkin wrote: > > On Mon, Nov 16, 2020 at 12:24:15PM +0000, Alex Bennée wrote: > >> From: Philippe Mathieu-Daudé <philmd@redhat.com> > >> > >> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code > >> is already in the "if (bsel || pcihp_bridge_en)" block statement, > >> but it isn't smart enough to figure it out. > >> > >> Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)" > >> block statement to fix (on Ubuntu): > >> > >> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': > >> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized > >> in this function [-Werror=maybe-uninitialized] > >> 496 | aml_append(parent_scope, method); > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> cc1: all warnings being treated as errors > >> > >> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally") > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > >> Message-Id: <20201108204535.2319870-4-philmd@redhat.com> > >> Message-Id: <20201110192316.26397-8-alex.bennee@linaro.org> > > > > BTW it's in my pull request alredy. > > Not sure why you are merging it too ... > > I suppose to unbreak Gitlab-CI... > > There is no policy w.r.t. CI so maintainer don't have to use it, > but this breaking it delay the workflow of others subsystems. > > I'm not asking you to use it, just explaining why this patch is > in Alex's queue. > > Regards, > > Phil. Not sure I understand. It's in my pull request from Nov 15. I'm not sure how does it help anyone to also have it in another request from Nov 16... -- MST
Michael S. Tsirkin <mst@redhat.com> writes: > On Mon, Nov 16, 2020 at 02:11:35PM +0100, Philippe Mathieu-Daudé wrote: >> On 11/16/20 1:27 PM, Michael S. Tsirkin wrote: >> > On Mon, Nov 16, 2020 at 12:24:15PM +0000, Alex Bennée wrote: >> >> From: Philippe Mathieu-Daudé <philmd@redhat.com> >> >> >> >> GCC 9.3.0 thinks that 'method' can be left uninitialized. This code >> >> is already in the "if (bsel || pcihp_bridge_en)" block statement, >> >> but it isn't smart enough to figure it out. >> >> >> >> Restrict the code to be used only in the "if (bsel || pcihp_bridge_en)" >> >> block statement to fix (on Ubuntu): >> >> >> >> ../hw/i386/acpi-build.c: In function 'build_append_pci_bus_devices': >> >> ../hw/i386/acpi-build.c:496:9: error: 'method' may be used uninitialized >> >> in this function [-Werror=maybe-uninitialized] >> >> 496 | aml_append(parent_scope, method); >> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> cc1: all warnings being treated as errors >> >> >> >> Fixes: df4008c9c59 ("piix4: don't reserve hw resources when hotplug is off globally") >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> >> Message-Id: <20201108204535.2319870-4-philmd@redhat.com> >> >> Message-Id: <20201110192316.26397-8-alex.bennee@linaro.org> >> > >> > BTW it's in my pull request alredy. >> > Not sure why you are merging it too ... >> >> I suppose to unbreak Gitlab-CI... >> >> There is no policy w.r.t. CI so maintainer don't have to use it, >> but this breaking it delay the workflow of others subsystems. >> >> I'm not asking you to use it, just explaining why this patch is >> in Alex's queue. >> >> Regards, >> >> Phil. > > Not sure I understand. > It's in my pull request from Nov 15. I'm not sure how does it > help anyone to also have it in another request from Nov 16... I didn't see your PR had gone in, it shouldn't matter though because git should be smart enough to deal with it being in two places at once. -- Alex Bennée
We don't need running commentary for the CI logs and by keeping it short we might just see the problem on the first page. While we are at it flush the previous line so order is maintained between script and sub process. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20201113174404.19608-1-alex.bennee@linaro.org> diff --git a/.gitlab-ci.d/check-patch.py b/.gitlab-ci.d/check-patch.py index 0ff30ee077..39e2b403c9 100755 --- a/.gitlab-ci.d/check-patch.py +++ b/.gitlab-ci.d/check-patch.py @@ -45,9 +45,9 @@ if log == "": errors = False -print("\nChecking all commits since %s...\n" % ancestor) +print("\nChecking all commits since %s...\n" % ancestor, flush=True) -ret = subprocess.run(["scripts/checkpatch.pl", ancestor + "..."]) +ret = subprocess.run(["scripts/checkpatch.pl", "--terse", ancestor + "..."]) if ret.returncode != 0: print(" ❌ FAIL one or more commits failed scripts/checkpatch.pl") -- 2.20.1
© 2016 - 2024 Red Hat, Inc.