From d912e795e029b6ff76e8b57c7d0254b791907559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 9 Apr 2019 15:45:35 +0200 Subject: [PATCH 1/5] roms: Rename the EFIROM variable to avoid clashing with iPXE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The iPXE's 'veryclean' recipe removes $(EFIROM) even if the EFIROM macro originates from elsewhere: $ git checkout f590a812c21~ $ make -C roms clean EFIROM=$(type -P EfiRom) make: Entering directory '/source/qemu/roms' [...] make -C ipxe/src veryclean make[1]: Entering directory '/source/qemu/roms/ipxe/src' rm -f bin{,-*}/*.* bin{,-*}/.certificate.* bin{,-*}/.certificates.* bin{,-*}/.private_key.* bin{,-*}/errors bin{,-*}/NIC ./util/zbin ./util/elf2efi32 ./util/elf2efi64 /usr/bin/EfiRom ./util/efifatbin ./util/iccfix ./util/einfo TAGS bin{,-*}/symtab rm: cannot remove '/usr/bin/EfiRom': Permission denied make[1]: *** [Makefile.housekeeping:1564: clean] Error 1 make[1]: Leaving directory '/source/qemu/roms/ipxe/src' make: *** [Makefile:152: clean] Error 2 make: Leaving directory '/source/qemu/roms' Before f590a812c21 this variable could be overridden or unset, and the 'veryclean' Makefile rule would not complain. Commit f590a812c21 enforces this variable to the Intel EfiRom tool provided by the EDK2 project. To avoid the name clash and make the difference between the projects obvious, rename the variable used by the EDK2 project as EDK2_EFIROM. Fixes: f590a812c21074e82228de3e1dfb57b75fc02b62 Reported-by: Olaf Hering Reviewed-by: Laszlo Ersek Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20190409134536.15548-2-philmd@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Paolo Bonzini --- roms/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/roms/Makefile b/roms/Makefile index 78d5dd18c3..d28252dafd 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -47,7 +47,7 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org" # We need that to combine multiple images (legacy bios, # efi ia32, efi x64) into a single rom binary. # -EFIROM = edk2/BaseTools/Source/C/bin/EfiRom +EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom default: @echo "nothing is build by default" @@ -102,8 +102,8 @@ pxe-rom-%: build-pxe-roms efirom: $(patsubst %,efi-rom-%,$(pxerom_variants)) -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM) - $(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \ +efi-rom-%: build-pxe-roms build-efi-roms $(EDK2_EFIROM) + $(EDK2_EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \ -b ipxe/src/bin/$(VID)$(DID).rom \ -ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \ -ec ipxe/src/bin-x86_64-efi/$(VID)$(DID).efidrv \ @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \ $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets)) -$(EFIROM): +$(EDK2_EFIROM): $(MAKE) -C edk2/BaseTools slof: From 1cab464136b424380dc9a0691147ccf909d5df31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 9 Apr 2019 15:45:36 +0200 Subject: [PATCH 2/5] roms: Allow passing configure options to the EDK2 build tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit f590a812c210 we build the EDK2 EfiRom utility unconditionally. Some distributions require to use extra compiler/linker flags, i.e. SUSE which enforces the PIE protection (see [*]). EDK2 build tools already provide a set of variables for that, use them to allow the caller to easily inject compiler/linker options.. Now build scripts can pass extra options, example: $ make -C roms \ EDK2_BASETOOLS_OPTFLAGS='-fPIE' \ efirom [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html Reported-by: Olaf Hering Suggested-by: Laszlo Ersek Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20190409134536.15548-3-philmd@redhat.com> Reviewed-by: Michael S. Tsirkin Reviewed-by: Laszlo Ersek Reviewed-by: Igor Mammedov Signed-off-by: Paolo Bonzini --- roms/Makefile | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/roms/Makefile b/roms/Makefile index d28252dafd..1ff78b63bb 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \ $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets)) +# Build scripts can pass compiler/linker flags to the EDK2 build tools +# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and +# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables. +# +# Example: +# +# make -C roms \ +# EDK2_BASETOOLS_OPTFLAGS='...' \ +# EDK2_BASETOOLS_LDFLAGS='...' \ +# efirom +# $(EDK2_EFIROM): - $(MAKE) -C edk2/BaseTools + $(MAKE) -C edk2/BaseTools \ + EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \ + EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)' slof: $(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu From 77b17570900fdfff32c5abb8d92fb527e4a0737a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 18 Mar 2019 11:29:38 +0000 Subject: [PATCH 3/5] include/qemu/bswap.h: Use __builtin_memcpy() in accessor functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the accessor functions ld*_he_p() and st*_he_p() we use memcpy() to perform a load or store to a pointer which might not be aligned for the size of the type. We rely on the compiler to optimize this memcpy() into an efficient load or store instruction where possible. This is required for good performance, but at the moment it is also required for correct operation, because some users of these functions require that the access is atomic if the pointer is aligned, which will only be the case if the compiler has optimized out the memcpy(). (The particular example where we discovered this is the virtio vring_avail_idx() which calls virtio_lduw_phys_cached() which eventually ends up calling lduw_he_p().) Unfortunately some compile environments, such as the fortify-source setup used in Alpine Linux, define memcpy() to a wrapper function in a way that inhibits this compiler optimization. The correct long-term fix here is to add a set of functions for doing atomic accesses into AddressSpaces (and to other relevant families of accessor functions like the virtio_*_phys_cached() ones), and make sure that callsites which want atomic behaviour use the correct functions. In the meantime, switch to using __builtin_memcpy() in the bswap.h accessor functions. This will make us robust against things like this fortify library in the short term. In the longer term it will mean that we don't end up with these functions being really badly-performing even if the semantics of the out-of-line memcpy() are correct. Reported-by: Fernando Casas Schössow Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-Id: <20190318112938.8298-1-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini --- include/qemu/bswap.h | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h index 5a70f78c0b..2a9f3fe783 100644 --- a/include/qemu/bswap.h +++ b/include/qemu/bswap.h @@ -316,51 +316,57 @@ static inline void stb_p(void *ptr, uint8_t v) *(uint8_t *)ptr = v; } -/* Any compiler worth its salt will turn these memcpy into native unaligned - operations. Thus we don't need to play games with packed attributes, or - inline byte-by-byte stores. */ +/* + * Any compiler worth its salt will turn these memcpy into native unaligned + * operations. Thus we don't need to play games with packed attributes, or + * inline byte-by-byte stores. + * Some compilation environments (eg some fortify-source implementations) + * may intercept memcpy() in a way that defeats the compiler optimization, + * though, so we use __builtin_memcpy() to give ourselves the best chance + * of good performance. + */ static inline int lduw_he_p(const void *ptr) { uint16_t r; - memcpy(&r, ptr, sizeof(r)); + __builtin_memcpy(&r, ptr, sizeof(r)); return r; } static inline int ldsw_he_p(const void *ptr) { int16_t r; - memcpy(&r, ptr, sizeof(r)); + __builtin_memcpy(&r, ptr, sizeof(r)); return r; } static inline void stw_he_p(void *ptr, uint16_t v) { - memcpy(ptr, &v, sizeof(v)); + __builtin_memcpy(ptr, &v, sizeof(v)); } static inline int ldl_he_p(const void *ptr) { int32_t r; - memcpy(&r, ptr, sizeof(r)); + __builtin_memcpy(&r, ptr, sizeof(r)); return r; } static inline void stl_he_p(void *ptr, uint32_t v) { - memcpy(ptr, &v, sizeof(v)); + __builtin_memcpy(ptr, &v, sizeof(v)); } static inline uint64_t ldq_he_p(const void *ptr) { uint64_t r; - memcpy(&r, ptr, sizeof(r)); + __builtin_memcpy(&r, ptr, sizeof(r)); return r; } static inline void stq_he_p(void *ptr, uint64_t v) { - memcpy(ptr, &v, sizeof(v)); + __builtin_memcpy(ptr, &v, sizeof(v)); } static inline int lduw_le_p(const void *ptr) From ae909496e9d33f8c074f1063597298ba1d183133 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Sun, 7 Apr 2019 11:23:14 +0200 Subject: [PATCH 4/5] hw/i386/pc: Fix crash when hot-plugging nvdimm on older machine types QEMU currently crashes when you try to hot-plug an "nvdimm" device on older machine types: $ qemu-system-x86_64 -monitor stdio -M pc-1.1 QEMU 3.1.92 monitor - type 'help' for more information (qemu) device_add nvdimm,id=nvdimmn1 qemu-system-x86_64: /home/thuth/devel/qemu/util/error.c:57: error_setv: Assertion `*errp == ((void *)0)' failed. Aborted (core dumped) The call to hotplug_handler_pre_plug() in pc_memory_pre_plug() has been added recently before the check whether nvdimm is enabled. It should be done after the check. And while we're at it, also check the errp after the hotplug_handler_pre_plug(), otherwise errors are silently ignored here. Fixes: 9040e6dfa8c3fed87695a3de555d2c775727bb51 Signed-off-by: Thomas Huth Message-Id: <20190407092314.11066-1-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- hw/i386/pc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 6077d27361..f2c15bf1f2 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2078,6 +2078,7 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, const MachineState *ms = MACHINE(hotplug_dev); const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); const uint64_t legacy_align = TARGET_PAGE_SIZE; + Error *local_err = NULL; /* * When -no-acpi is used with Q35 machine type, no ACPI is built, @@ -2090,13 +2091,17 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - hotplug_handler_pre_plug(pcms->acpi_dev, dev, errp); - if (is_nvdimm && !ms->nvdimms_state->is_enabled) { error_setg(errp, "nvdimm is not enabled: missing 'nvdimm' in '-M'"); return; } + hotplug_handler_pre_plug(pcms->acpi_dev, dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), pcmc->enforce_aligned_dimm ? NULL : &legacy_align, errp); } From 3e20c81ed87cb15cd04f929b075e244e0758641a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 19 Mar 2019 08:21:04 +0100 Subject: [PATCH 5/5] tests: Make check-block a phony target Fixes: b93b63f574c "test makefile overhaul" Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20190319072104.32591-1-armbru@redhat.com> Signed-off-by: Paolo Bonzini --- tests/Makefile.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 6b904d7430..36fc73fef5 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -1164,7 +1164,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) # Consolidated targets -.PHONY: check-qapi-schema check-qtest check-unit check check-clean +.PHONY: check-block check-qapi-schema check-qtest check-unit check check-clean check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS)) check-block: $(patsubst %,check-%, $(check-block-y))