[PATCH] util: fix build with musl libc on ppc64le
Use PPC_FEATURE2_ISEL and PPC_FEATURE2_VEC_CRYPTO from linux headers
instead of the GNU specific PPC_FEATURE2_HAS_ISEL and
PPC_FEATURE2_HAS_VEC_CRYPTO. This fixes build with musl libc.
Fixes: 623d7e3551a6 (util: Add cpuinfo-ppc.c)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1861
Signed-off-by: Natanael Copa
---
util/cpuinfo-ppc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/util/cpuinfo-ppc.c b/util/cpuinfo-ppc.c
index 1ea3db0ac8..b2d8893a06 100644
--- a/util/cpuinfo-ppc.c
+++ b/util/cpuinfo-ppc.c
@@ -6,10 +6,10 @@
#include "qemu/osdep.h"
#include "host/cpuinfo.h"
+#include
#ifdef CONFIG_GETAUXVAL
# include
#else
-# include
# include "elf.h"
#endif
@@ -40,7 +40,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
info |= CPUINFO_V2_06;
}
-if (hwcap2 & PPC_FEATURE2_HAS_ISEL) {
+if (hwcap2 & PPC_FEATURE2_ISEL) {
info |= CPUINFO_ISEL;
}
if (hwcap & PPC_FEATURE_HAS_ALTIVEC) {
@@ -53,7 +53,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
* always have both anyway, since VSX came with Power7
* and crypto came with Power8.
*/
-if (hwcap2 & PPC_FEATURE2_HAS_VEC_CRYPTO) {
+if (hwcap2 & PPC_FEATURE2_VEC_CRYPTO) {
info |= CPUINFO_CRYPTO;
}
}
--
2.43.0
[PATCH] target/riscv/kvm: do not use non-portable strerrorname_np()
strerrorname_np is non-portable and breaks building with musl libc.
Use strerror(errno) instead, like we do other places.
Cc: qemu-sta...@nongnu.org
Fixes: commit 082e9e4a58ba (target/riscv/kvm: improve 'init_multiext_cfg' error
msg)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2041
Buglink: https://gitlab.alpinelinux.org/alpine/aports/-/issues/15541
Signed-off-by: Natanael Copa
---
target/riscv/kvm/kvm-cpu.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 45b6cf1cfa..117e33cf90 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -832,9 +832,8 @@ static void kvm_riscv_read_multiext_legacy(RISCVCPU *cpu,
multi_ext_cfg->supported = false;
val = false;
} else {
-error_report("Unable to read ISA_EXT KVM register %s, "
- "error code: %s", multi_ext_cfg->name,
- strerrorname_np(errno));
+error_report("Unable to read ISA_EXT KVM register %s: %s",
+ multi_ext_cfg->name, strerror(errno));
exit(EXIT_FAILURE);
}
} else {
@@ -895,8 +894,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu,
KVMScratchCPU *kvmcpu)
*
* Error out if we get any other errno.
*/
-error_report("Error when accessing get-reg-list, code: %s",
- strerrorname_np(errno));
+error_report("Error when accessing get-reg-list: %s",
+ strerror(errno));
exit(EXIT_FAILURE);
}
@@ -905,8 +904,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu,
KVMScratchCPU *kvmcpu)
reglist->n = rl_struct.n;
ret = ioctl(kvmcpu->cpufd, KVM_GET_REG_LIST, reglist);
if (ret) {
-error_report("Error when reading KVM_GET_REG_LIST, code %s ",
- strerrorname_np(errno));
+error_report("Error when reading KVM_GET_REG_LIST: %s",
+ strerror(errno));
exit(EXIT_FAILURE);
}
@@ -927,9 +926,8 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu,
KVMScratchCPU *kvmcpu)
reg.addr = (uint64_t)
ret = ioctl(kvmcpu->cpufd, KVM_GET_ONE_REG, );
if (ret != 0) {
-error_report("Unable to read ISA_EXT KVM register %s, "
- "error code: %s", multi_ext_cfg->name,
- strerrorname_np(errno));
+error_report("Unable to read ISA_EXT KVM register %s: %s",
+ multi_ext_cfg->name, strerror(errno));
exit(EXIT_FAILURE);
}
--
2.43.0
[PATCH] edk2: fix TPM 2.0 regression
Fix a regression that was introduced with the edk2 202202 upgrade (commit e2f3137c7344). Upstream changed TPM_ENABLE to TPM1_ENABLE and TPM2_ENABLE. This led to TPM 2.0 (which is needed for windows 11) silently got disabled. Ref: https://github.com/tianocore/edk2/commit/4de8d61bcec02a13ceed84f92b0cf3ea58adf9c5 Signed-off-by: Natanael Copa --- I have not really been able to test this, due to issues with building the OVMF_CODE.fd from the qemu source tree. I do have tested building OVMF_CODE.fd for the Alpine Linux package, and adding -D TPM2_ENABLE dis solve the issue in windows guests. So I believe it is fine. I don't think an explicit -D TPM1_ENABLE is needed as I believe it is the default. https://github.com/tianocore/edk2/blob/8fc06b6e19e3df93cc989b4f85877d8a7783e5bf/OvmfPkg/OvmfTpmDefines.dsc.inc#L8 The -D TPM_CONFIG_ENABLE can probably be removed. It was not found anywhere when grepping the sources. roms/Makefile.edk2 | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2 index 485f2244b1..eb47f008c6 100644 --- a/roms/Makefile.edk2 +++ b/roms/Makefile.edk2 @@ -101,7 +101,7 @@ submodules: -D NETWORK_IP6_ENABLE \ -D NETWORK_HTTP_BOOT_ENABLE \ -D NETWORK_TLS_ENABLE \ - -D TPM_ENABLE \ + -D TPM2_ENABLE \ -D TPM_CONFIG_ENABLE cp edk2/Build/OvmfIa32/$(target)_$(call toolchain,i386)/FV/OVMF_CODE.fd $@ @@ -113,7 +113,7 @@ submodules: -D NETWORK_IP6_ENABLE \ -D NETWORK_HTTP_BOOT_ENABLE \ -D NETWORK_TLS_ENABLE \ - -D TPM_ENABLE \ + -D TPM2_ENABLE \ -D TPM_CONFIG_ENABLE \ -D SECURE_BOOT_ENABLE \ -D SMM_REQUIRE @@ -127,7 +127,7 @@ submodules: -D NETWORK_IP6_ENABLE \ -D NETWORK_HTTP_BOOT_ENABLE \ -D NETWORK_TLS_ENABLE \ - -D TPM_ENABLE \ + -D TPM2_ENABLE \ -D TPM_CONFIG_ENABLE cp edk2/Build/OvmfX64/$(target)_$(call toolchain,x86_64)/FV/OVMF_CODE.fd $@ @@ -140,7 +140,7 @@ submodules: -D NETWORK_IP6_ENABLE \ -D NETWORK_HTTP_BOOT_ENABLE \ -D NETWORK_TLS_ENABLE \ - -D TPM_ENABLE \ + -D TPM2_ENABLE \ -D TPM_CONFIG_ENABLE \ -D SECURE_BOOT_ENABLE \ -D SMM_REQUIRE -- 2.38.0
Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
On Mon, 25 Feb 2019 13:06:16 + Peter Maydell wrote: > On Mon, 25 Feb 2019 at 12:22, Natanael Copa wrote: > > > > On Mon, 25 Feb 2019 10:34:23 + > > Peter Maydell wrote: > > > The short term fix is to fix your toolchain/compilation > > > environment options so that it isn't trying to override > > > the definition of memcpy(). > > > > The easiest workaround is to simply disable FORTIY_SOURCE, but that > > will weaken the security for all implemented string functions, strcpy, > > memmove etc, so I don't want to do that. > > > > Is it only lduw_he_p that needs to be atomic or are the other functions > > in include/qemu/bswap.h using memcpy also required to be atomic? > > Hard to say, since we haven't done the "audit all the callers" > step that Stefan mentioned. If you're going to replace memcpy > with __builtin_memcpy then the safest thing is to do it for > all those uses (this will also give you much better generated > code for performance purposes). I figured that and that is exactly what I did. Fernando: Can you please test the binary from qemu-system-x86_64-3.1.0-r3 from alpine edge? I will backport the fix if you can confirm it fixes the problem. Thanks! -nc PS. Those issues are pretty hard to track down, so big thanks to everyone who helped find the exact issue here. You have done a great work!
Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
On Mon, 25 Feb 2019 10:34:23 +
Peter Maydell wrote:
> On Mon, 25 Feb 2019 at 10:24, Natanael Copa wrote:
> >
> > On Sat, 23 Feb 2019 16:18:15 +
> > Peter Maydell wrote:
> >
> > > On Sat, 23 Feb 2019 at 16:05, Natanael Copa
> > > wrote:
> > > > I was thinking of something in the lines of:
> > > >
> > > > typedef volatile uint16_t __attribute__((__may_alias__))
> > > > volatile_uint16_t;
> > > > static inline int lduw_he_p(const void *ptr)
> > > > {
> > > > volatile_uint16_t r = *(volatile_uint16_t*)ptr;
> > > > return r;
> > > > }
> > >
> > > This won't correctly handle accesses with unaligned pointers,
> > > I'm afraid. We rely on these functions correctly working
> > > with pointers that are potentially unaligned.
> >
> > Well, current code does not even handle access with aligned pointers,
> > depending on FORTIFY_SOURCE implementation.
>
> It correctly handles aligned and unaligned pointers for the
> API guarantees that the function in QEMU provides, which is
> to say "definitely works on any kind of aligned or unaligned
> pointer, not guaranteed to be atomic". Unfortunately some
> code in QEMU is implicitly assuming it is atomic, which this
> QEMU function does not guarantee -- it just happens to provide
> that most of the time.
>
> > My thinking here is that we depend on assumption that compiler will
> > remove the memcpy call, so maybe find other way to generate same
> > assembly, while still depend on compiler optimization.
>
> > I did some tests and compared the assembly output. The compiler will
> > generate same assembly if volatile is not used. The attribute
> > __may_alias__ does not seem to make any difference. So the following
> > function generates exactly same assembly:
> >
> > static inline int lduw_he_p(const void *ptr)
> > {
> > uint16_t r = *(uint16_t*)ptr;
> > return r;
> > }
>
> This still is not guaranteed to work on unaligned pointers.
> (For instance it probably won't work on SPARC hosts.)
> More generally there is no way to have a single function
> that is guaranteed to handle unaligned pointers and also
> guaranteed to produce an atomic access on all host architectures
> that we support, because not all host architectures allow
> you to do both with the same instruction sequence. So
> you can't just change this function to make it provide
> atomic access without breaking the other guarantee it is
> providing.
Right, so it is possible that there are other architectures (like
SPARC) that suffer from the same race here as Alpine, because memcpy is
not guaranteed to be atomic.
> The long term fix for this is that we need to separate out
> our APIs so we can have a family of functions that guarantee
> to work on unaligned pointers, and a family that guarantee
> to work atomically, and calling code can use the one that
> provides the semantics it requires.
>
> The short term fix is to fix your toolchain/compilation
> environment options so that it isn't trying to override
> the definition of memcpy().
The easiest workaround is to simply disable FORTIY_SOURCE, but that
will weaken the security for all implemented string functions, strcpy,
memmove etc, so I don't want to do that.
Is it only lduw_he_p that needs to be atomic or are the other functions
in include/qemu/bswap.h using memcpy also required to be atomic?
I intend to replace memcpy with __builtin_memcpy there.
>
> thanks
> -- PMM
Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
On Sat, 23 Feb 2019 16:18:15 +
Peter Maydell wrote:
> On Sat, 23 Feb 2019 at 16:05, Natanael Copa wrote:
> > I was thinking of something in the lines of:
> >
> > typedef volatile uint16_t __attribute__((__may_alias__)) volatile_uint16_t;
> > static inline int lduw_he_p(const void *ptr)
> > {
> > volatile_uint16_t r = *(volatile_uint16_t*)ptr;
> > return r;
> > }
>
> This won't correctly handle accesses with unaligned pointers,
> I'm afraid. We rely on these functions correctly working
> with pointers that are potentially unaligned.
Well, current code does not even handle access with aligned pointers,
depending on FORTIFY_SOURCE implementation.
My thinking here is that we depend on assumption that compiler will
remove the memcpy call, so maybe find other way to generate same
assembly, while still depend on compiler optimization.
I did some tests and compared the assembly output. The compiler will
generate same assembly if volatile is not used. The attribute
__may_alias__ does not seem to make any difference. So the following
function generates exactly same assembly:
static inline int lduw_he_p(const void *ptr)
{
uint16_t r = *(uint16_t*)ptr;
return r;
}
I created a script to compare on different Linux distros and
architectures. My conclusion is:
- Alpine fortify implementation is non-optimal, but technically
correct.
- `uint16_t r = *(uint16_t*)ptr;` generates the same assembly code,
regardless of fortify implementation, and is clearer on the
intention, than use of memcpy.
test.sh:
---[CUT HERE]---8<--
#!/bin/sh -x
cat > testcase.c <
#ifdef WITH_MEMCPY
#include
static inline int lduw_he_p(const void *ptr)
{
uint16_t r;
memcpy(, ptr, sizeof(r));
return r;
}
#else
static inline int lduw_he_p(const void *ptr)
{
uint16_t r = *(uint16_t*)ptr;
return r;
}
#endif
int main(int argc, char *argv[])
{
void *p = argv;
int i;
p++; // make sure we are unaligned
i=lduw_he_p(p);
return i;
}
EOF
: ${CC:=gcc}
$CC --version
uname -m
for fortify in "-D_FORTIFY_SOURCE=2" "-U_FORTIFY_SOURCE"; do
for cflag in "-DWITH_MEMCPY" "-DWITHOUT_MEMCPY"; do
$CC $cflag $fortify $@ -o testcase$cflag testcase.c
gdb --batch -ex "disas ${func:-main}" ./testcase$cflag
done
done
---[CUT HERE]---8<--
Output on Alpine:
$ sh -x test.sh -O2
+ cat
+ : gcc
+ gcc --version
gcc (Alpine 8.2.0) 8.2.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ uname -m
x86_64
+ gcc -DWITH_MEMCPY '-D_FORTIFY_SOURCE=2' -O2 -o testcase-DWITH_MEMCPY
testcase.c
+ gdb --batch -ex 'disas main' ./testcase-DWITH_MEMCPY
Dump of assembler code for function main:
0x1070 <+0>: sub$0x18,%rsp
0x1074 <+4>: mov%fs:0x28,%rax
0x107d <+13>:mov%rax,0x8(%rsp)
0x1082 <+18>:xor%eax,%eax
0x1084 <+20>:lea0x6(%rsp),%rdi
0x1089 <+25>:lea0x1(%rsi),%rax
0x108d <+29>:lea0x2(%rdi),%rdx
0x1091 <+33>:cmp%rdx,%rax
0x1094 <+36>:jae0x109b
0x1096 <+38>:cmp%rdi,%rax
0x1099 <+41>:ja 0x10d0
0x109b <+43>:cmp%rdi,%rax
0x109e <+46>:jb 0x10c7
0x10a0 <+48>:mov$0x2,%edx
0x10a5 <+53>:mov%rax,%rsi
0x10a8 <+56>:callq 0x1020
0x10ad <+61>:movzwl 0x6(%rsp),%eax
0x10b2 <+66>:mov0x8(%rsp),%rcx
0x10b7 <+71>:xor%fs:0x28,%rcx
0x10c0 <+80>:jne0x10d2
0x10c2 <+82>:add$0x18,%rsp
0x10c6 <+86>:retq
0x10c7 <+87>:add$0x3,%rsi
0x10cb <+91>:cmp%rsi,%rdi
0x10ce <+94>:jae0x10a0
0x10d0 <+96>:ud2
0x10d2 <+98>:callq 0x1030 <__stack_chk_fail@plt>
End of assembler dump.
+ gcc -DWITHOUT_MEMCPY '-D_FORTIFY_SOURCE=2' -O2 -o testcase-DWITHOUT_MEMCPY
testcase.c
+ gdb --batch -ex 'disas main' ./testcase-DWITHOUT_MEMCPY
Dump of assembler code for function main:
0x1050 <+0>: movzwl 0x1(%rsi),%eax
0x1054 <+4>: retq
End of assembler dump.
+ gcc -DWITH_MEMCPY -U_FORTIFY_SOU
Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
On Fri, 22 Feb 2019 14:04:20 +
Stefan Hajnoczi wrote:
> On Fri, Feb 22, 2019 at 12:57 PM Fernando Casas Schössow
> wrote:
>
> I have CCed Natanael Copa, qemu package maintainer in Alpine Linux.
>
> Fernando: Can you confirm that the bug occurs with an unmodified
> Alpine Linux qemu binary?
I wonder exactly which binary it is. What arcitecture (x86 or x86_64)
and what binary. Is it ddynamic or statically linked qemu-system-x86_64?
> Richard: Commit 7db2145a6826b14efceb8dd64bfe6ad8647072eb ("bswap: Add
> host endian unaligned access functions") introduced the unaligned
> memory access functions in question here. Please see below for
> details on the bug - basically QEMU code assumes they are atomic, but
> that is not guaranteed :(. Any ideas for how to fix this?
>
> Natanael: It seems likely that the qemu package in Alpine Linux
> suffers from a compilation issue resulting in a broken QEMU. It may
> be necessary to leave the compiler optimization flag alone in APKBUILD
> to work around this problem.
>
> Here are the details. QEMU relies on the compiler turning
> memcpy(, , 2) turning into a load instruction in
> include/qemu/bswap.h:lduw_he_p() (explanation below):
>
> /* 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. */
>
> static inline int lduw_he_p(const void *ptr)
> {
> uint16_t r;
> memcpy(, ptr, sizeof(r));
> return r;
> }
>
> Here is the disassembly snippet of virtqueue_pop() from Fedora 29 that
> shows the load instruction:
>
> 398166: 0f b7 42 02 movzwl 0x2(%rdx),%eax
> 39816a: 66 89 43 32 mov%ax,0x32(%rbx)
>
> Here is the instruction sequence in the Alpine Linux binary:
>
> 455562:ba 02 00 00 00 mov$0x2,%edx
> 455567:e8 74 24 f3 ff callq 3879e0
> 45556c:0f b7 44 24 42 movzwl 0x42(%rsp),%eax
> 455571:66 41 89 47 32 mov%ax,0x32(%r15)
>
> It's calling memcpy instead of using a load instruction.
I tried to find this section. How do you get the assembly listing of
relevant secion? I tried to do "disas virtio_pop" from
`gdb /usr/bin/qemu-system-x86_64` from the binary in alpine edge. I
could find 2 memcpy but none of them look like a 16 bit operation after:
0x004551f1 <+353>: mov0x10(%rsp),%rdi
0x004551f6 <+358>: mov$0x10,%edx
0x004551fb <+363>: callq 0x3879e0
0x00455200 <+368>: movzwl 0x5c(%rsp),%eax
0x00455205 <+373>: test $0x4,%al
0x00455207 <+375>: je 0x4552aa
0x00455291 <+513>: mov0x10(%rsp),%rdi
0x00455296 <+518>: mov$0x10,%edx
0x0045529b <+523>: callq 0x3879e0
0x004552a0 <+528>: mov%rbp,0x20(%rsp)
0x004552a5 <+533>: movzwl 0x5c(%rsp),%eax
0x004552aa <+538>: lea0x20e0(%rsp),%rdi
0x004552b2 <+546>: xor%r11d,%r11d
0x004552b5 <+549>: mov%r15,0x38(%rsp)
>
> Fernando found that QEMU's virtqueue_pop() function sees bogus values
> when loading a 16-bit guest RAM location. Paolo figured out that the
> bogus value can be produced by memcpy() when another thread is
> updating the 16-bit memory location simultaneously. This is a race
> condition between one thread loading the 16-bit value and another
> thread storing it (in this case a guest vcpu thread). Sometimes
> memcpy() may load one old byte and one new byte, resulting in a bogus
> value.
>
> The symptom that Fernando experienced is a "Virtqueue size exceeded"
> error message from QEMU and then the virtio-blk or virtio-scsi device
> stops working. This issue potentially affects other device emulation
> code in QEMU as well, not just virtio devices.
>
> For the time being, I suggest tweaking the APKBUILD so the memcpy() is
> not generated. Hopefully QEMU can make the code more portable in the
> future so the compiler always does the expected thing, but this may
> not be easily possible.
I was thinking of something in the lines of:
typedef volatile uint16_t __attribute__((__may_alias__)) volatile_uint16_t;
static inline int lduw_he_p(const void *ptr)
{
volatile_uint16_t r = *(volatile_uint16_t*)ptr;
return r;
}
I can test different CFLAGS with and without the _FORTIFY_SOURCE and
with different variants of memcpy (like __builtint_memcpy etc) but i
need find a way to get the correct assembly output so I know if/when I
have found something that works.
Thanks!
-nc
>
> Stefan
Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error
On Fri, 22 Feb 2019 14:04:20 +
Stefan Hajnoczi wrote:
> On Fri, Feb 22, 2019 at 12:57 PM Fernando Casas Schössow
> wrote:
>
> I have CCed Natanael Copa, qemu package maintainer in Alpine Linux.
Hi!
...
> Richard: Commit 7db2145a6826b14efceb8dd64bfe6ad8647072eb ("bswap: Add
> host endian unaligned access functions") introduced the unaligned
> memory access functions in question here. Please see below for
> details on the bug - basically QEMU code assumes they are atomic, but
> that is not guaranteed :(. Any ideas for how to fix this?
>
> Natanael: It seems likely that the qemu package in Alpine Linux
> suffers from a compilation issue resulting in a broken QEMU. It may
> be necessary to leave the compiler optimization flag alone in APKBUILD
> to work around this problem.
>
> Here are the details. QEMU relies on the compiler turning
> memcpy(, , 2) turning into a load instruction in
> include/qemu/bswap.h:lduw_he_p() (explanation below):
>
> /* 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. */
>
> static inline int lduw_he_p(const void *ptr)
> {
> uint16_t r;
> memcpy(, ptr, sizeof(r));
> return r;
> }
>
> Here is the disassembly snippet of virtqueue_pop() from Fedora 29 that
> shows the load instruction:
>
> 398166: 0f b7 42 02 movzwl 0x2(%rdx),%eax
> 39816a: 66 89 43 32 mov%ax,0x32(%rbx)
>
> Here is the instruction sequence in the Alpine Linux binary:
>
> 455562:ba 02 00 00 00 mov$0x2,%edx
> 455567:e8 74 24 f3 ff callq 3879e0
> 45556c:0f b7 44 24 42 movzwl 0x42(%rsp),%eax
> 455571:66 41 89 47 32 mov%ax,0x32(%r15)
>
> It's calling memcpy instead of using a load instruction.
My first reaction to this is: If the intention is to not actually call
memcpy function, then maybe memcpy should not be used in the C code in
first place?
> Fernando found that QEMU's virtqueue_pop() function sees bogus values
> when loading a 16-bit guest RAM location. Paolo figured out that the
> bogus value can be produced by memcpy() when another thread is
> updating the 16-bit memory location simultaneously. This is a race
> condition between one thread loading the 16-bit value and another
> thread storing it (in this case a guest vcpu thread). Sometimes
> memcpy() may load one old byte and one new byte, resulting in a bogus
> value.
>
> The symptom that Fernando experienced is a "Virtqueue size exceeded"
> error message from QEMU and then the virtio-blk or virtio-scsi device
> stops working. This issue potentially affects other device emulation
> code in QEMU as well, not just virtio devices.
>
> For the time being, I suggest tweaking the APKBUILD so the memcpy() is
> not generated. Hopefully QEMU can make the code more portable in the
> future so the compiler always does the expected thing, but this may
> not be easily possible.
I suspect this happens due to the Alpine toolchain will enable
_FORTIFY_SOURCE=2 by default and the way this is implemented via
fortify-headers:
http://git.2f30.org/fortify-headers/file/include/string.h.html#l39
Try build with -U_FORTIFY_SOURCE
>
> Stefan
[Qemu-devel] [PATCH] linux-user: fix build with musl on aarch64
Use the standard uint64_t instead of internal __u64.
This fixes compiler error with musl libc on aarch64:
.../qemu-2.7.0/linux-user/host/aarch64/hostdep.h:28:5:
error: unknown type name '__u64'
__u64 *pcreg = >uc_mcontext.pc;
^
Signed-off-by: Natanael Copa <nc...@alpinelinux.org>
---
linux-user/host/aarch64/hostdep.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/host/aarch64/hostdep.h
b/linux-user/host/aarch64/hostdep.h
index 64f75cef49..6fd6e36b2a 100644
--- a/linux-user/host/aarch64/hostdep.h
+++ b/linux-user/host/aarch64/hostdep.h
@@ -25,7 +25,7 @@ extern char safe_syscall_end[];
static inline void rewind_if_in_safe_syscall(void *puc)
{
struct ucontext *uc = puc;
-__u64 *pcreg = >uc_mcontext.pc;
+uint64_t *pcreg = >uc_mcontext.pc;
if (*pcreg > (uintptr_t)safe_syscall_start
&& *pcreg < (uintptr_t)safe_syscall_end) {
--
2.11.0
[Qemu-devel] [PATCH v2] linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms
The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available
on all platforms, so we define those if they are missing. We also check
that those corresponds with the posix variables SIGRTMIN/SIGRTMAX which
may only be available during runtime.
This is needed for musl libc.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes v1 - v2:
- replace NSIG with _NSIG since thats use everywhere else in the code.
- add runtime asserts.
linux-user/signal.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 5b8a01f..67771ad 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -32,6 +32,13 @@
//#define DEBUG_SIGNAL
+#ifndef __SIGRTMIN
+#define __SIGRTMIN 32
+#endif
+#ifndef __SIGRTMAX
+#define __SIGRTMAX (_NSIG-1)
+#endif
+
static struct target_sigaltstack target_sigaltstack_used = {
.ss_sp = 0,
.ss_size = 0,
@@ -379,6 +386,13 @@ void signal_init(void)
int i, j;
int host_sig;
+/* SIGRTMIN/SIGRTMAX might be runtime variables so we cannot use them
+ to declare the host_to_target_signal table. But we are interacting
+ with a given kernel where the values will be fixed. Check that the
+ runtime values actually corresponds. */
+assert(__SIGRTMIN == SIGRTMIN);
+assert(__SIGRTMAX == SIGRTMAX);
+
/* generate signal conversion tables */
for(i = 1; i _NSIG; i++) {
if (host_to_target_signal_table[i] == 0)
--
2.0.0
Re: [Qemu-devel] [PATCH 0/5] fix building with musl libc
On Wed, 4 Jun 2014 00:37:44 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 29 April 2014 15:17, Natanael Copa nc...@alpinelinux.org wrote: In addition to the previoiusly sent linux-user: avoid using glibc internals in _syscall5 and in definition of target_sigevent struct, those are needed for making qemu build with musl libc on Alpine Linux. There is still 2 missing fcntl.h definitions (F_EXLCK and F_SHLCK) but those should probably be defined in libc so patch for that is not included here. Natanael Copa (5): util/qemu-openpty: fix build with musl libc by include termios.h as fallback xen: replace ffsl with ctzl vhost: replace ffsl with ctzl exec: replace ffsl with ctzl linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms Hmm. I just noticed these ffsl patches didn't get committed (the BSDs don't like ffsl either). There were issues with patch 5( the signal.c one) but 1..4 are good (and get my Reviewed-by: as well as Paolo's). I'm currently building up a tree with various bsd-user fixes in it, so I'll take these through that, I think. (Patch 1 is arguably slightly out of scope for that but better to keep it together with 2..4.) Natanael: you might want to update the signal.c patch to accommodate review comments and resubmit it. thanks I will try get it done today. Thanks! -nc -- PMM
Re: [Qemu-devel] [PATCH] linux-user: avoid using glibc internals
On Wed, 23 Apr 2014 19:00:41 +0100
Peter Maydell peter.mayd...@linaro.org wrote:
On 23 April 2014 15:59, Natanael Copa nc...@alpinelinux.org wrote:
Avoid using glibc specific internals.
Calculate the sigevent pad size is calculated in similar way as kernel
does it.
This is needed for building with musl libc.
Thanks for this patch -- is this the only fix that was needed, or
are there more to come?
There are more patches needed to make it build and run with musl libc.
Those were not mine, but I can try clean them up and submit those if
here is interest for it.
The problem this patch resolves was introduced with qemu 2.0.
It would be nice to be a little more specific in the patch summary
line about the change, like:
linux-user: avoid using glibc internals in definition of
target_sigevent struct
Agree.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
linux-user/syscall.c | 2 +-
linux-user/syscall_defs.h | 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9864813..c8989b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -406,7 +406,7 @@ static int sys_inotify_init1(int flags)
#endif
#define __NR_sys_ppoll __NR_ppoll
_syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
- struct timespec *, timeout, const __sigset_t *, sigmask,
+ struct timespec *, timeout, const sigset_t *, sigmask,
size_t, sigsetsize)
#endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fdf9a47..450f71b 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2552,12 +2552,16 @@ struct target_timer_t {
abi_ulong ptr;
};
+#define TARGET_SIGEV_MAX_SIZE 64
+#define TARGET_SIGEV_PREAMABLE_SIZE (sizeof(int32_t) * 2 +
sizeof(target_sigval_t))
This is wrong for 64 bit MIPS, I think; compare the kernel's
MIPS-specific override:
http://lxr.linux.no/#linux+v3.14.1/arch/mips/include/uapi/asm/siginfo.h#L13
I suggest
/* This is architecture-specific but most architectures use the default */
#ifdef TARGET_MIPS
#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(abi_long))
#else
TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(target_sigval_t))
#endif
I'm not entirely sure this is required -- our target_sigval_t looks
like it ought to be sizeof(abi_long) for MIPS so I don't know
why the kernel needs this override and we apparently don't.
Perhaps our target_sigval_t definition is slightly wrong?
Anyway, putting in the override can't hurt and might avoid subtle
issues later on if target_sigval_t does turn out to be broken and
need changing...
Ok. No problem.
(Note also that 'PREAMBLE' only has one 'A'.)
whoops.
+#define TARGET_SIGEV_PAD_SIZE ((TARGET_SIGEV_MAX_SIZE -
TARGET_SIGEV_PREAMABLE_SIZE) / sizeof(int32_t))
This line looks like it has more than 80 chars; if so, it should be
folded. (You can check using scripts/checkpatch.pl.)
I wasn't sure of the folding rules.
struct target_sigevent {
target_sigval_t sigev_value;
int32_t sigev_signo;
int32_t sigev_notify;
union {
-int32_t _pad[ARRAY_SIZE(((struct sigevent *)0)-_sigev_un._pad)];
+int32_t _pad[TARGET_SIGEV_PAD_SIZE];
int32_t _tid;
struct {
--
1.9.2
Looks good overall though.
thanks
-- PMM
I'll fix and resend. Thanks for feedback.
-nc
[Qemu-devel] [PATCH] linux-user: avoid using glibc internals in _syscall5 and in definition of target_sigevent struct
Use the public sigset_t instead of the glibc specific internal
__sigset_t in _syscall.
Calculate the sigevent pad size is calculated in similar way as kernel
does it instead of using glibc internal field _pad.
This is needed for building with musl libc.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes since v1:
- Be more specific in commit message.
- Special handling for target MIPS 64
- Fix typo of PREAMBLE
- Fold lines longer than 80 chars to make scripts/checkpatch.pl happy
- Replace tabs with spaces to make scripts/checkpatch.pl happy
linux-user/syscall.c | 2 +-
linux-user/syscall_defs.h | 16 +++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9864813..c8989b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -406,7 +406,7 @@ static int sys_inotify_init1(int flags)
#endif
#define __NR_sys_ppoll __NR_ppoll
_syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
- struct timespec *, timeout, const __sigset_t *, sigmask,
+ struct timespec *, timeout, const sigset_t *, sigmask,
size_t, sigsetsize)
#endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fdf9a47..5dd96b2 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2552,12 +2552,26 @@ struct target_timer_t {
abi_ulong ptr;
};
+#define TARGET_SIGEV_MAX_SIZE 64
+
+/* This is architecture-specific but most architectures use the default */
+#ifdef TARGET_MIPS
+#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(abi_long))
+#else
+#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 \
++ sizeof(target_sigval_t))
+#endif
+
+#define TARGET_SIGEV_PAD_SIZE ((TARGET_SIGEV_MAX_SIZE \
+- TARGET_SIGEV_PREAMABLE_SIZE) \
+ / sizeof(int32_t))
+
struct target_sigevent {
target_sigval_t sigev_value;
int32_t sigev_signo;
int32_t sigev_notify;
union {
-int32_t _pad[ARRAY_SIZE(((struct sigevent *)0)-_sigev_un._pad)];
+int32_t _pad[TARGET_SIGEV_PAD_SIZE];
int32_t _tid;
struct {
--
1.9.2
Re: [Qemu-devel] [PATCH] linux-user: avoid using glibc internals
On Wed, 23 Apr 2014 19:00:41 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 23 April 2014 15:59, Natanael Copa nc...@alpinelinux.org wrote: Avoid using glibc specific internals. Calculate the sigevent pad size is calculated in similar way as kernel does it. This is needed for building with musl libc. Thanks for this patch -- is this the only fix that was needed, or are there more to come? Alpine Linux actually also needs the previously posted patch: http://lists.gnu.org/archive/html/qemu-devel/2013-07/msg00774.html Thanks! -nc
[Qemu-devel] [PATCH v3] linux-user: avoid using glibc internals in _syscall5 and in definition of target_sigevent struct
Use the public sigset_t instead of the glibc specific internal
__sigset_t in _syscall.
Calculate the sigevent pad size is calculated in similar way as kernel
does it instead of using glibc internal field _pad.
This is needed for building with musl libc.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes v2 - v3:
- Fix another typ of PREAMBLE
linux-user/syscall.c | 2 +-
linux-user/syscall_defs.h | 16 +++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9864813..c8989b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -406,7 +406,7 @@ static int sys_inotify_init1(int flags)
#endif
#define __NR_sys_ppoll __NR_ppoll
_syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
- struct timespec *, timeout, const __sigset_t *, sigmask,
+ struct timespec *, timeout, const sigset_t *, sigmask,
size_t, sigsetsize)
#endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fdf9a47..69c3982 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2552,12 +2552,26 @@ struct target_timer_t {
abi_ulong ptr;
};
+#define TARGET_SIGEV_MAX_SIZE 64
+
+/* This is architecture-specific but most architectures use the default */
+#ifdef TARGET_MIPS
+#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 + sizeof(abi_long))
+#else
+#define TARGET_SIGEV_PREAMBLE_SIZE (sizeof(int32_t) * 2 \
++ sizeof(target_sigval_t))
+#endif
+
+#define TARGET_SIGEV_PAD_SIZE ((TARGET_SIGEV_MAX_SIZE \
+- TARGET_SIGEV_PREAMBLE_SIZE) \
+ / sizeof(int32_t))
+
struct target_sigevent {
target_sigval_t sigev_value;
int32_t sigev_signo;
int32_t sigev_notify;
union {
-int32_t _pad[ARRAY_SIZE(((struct sigevent *)0)-_sigev_un._pad)];
+int32_t _pad[TARGET_SIGEV_PAD_SIZE];
int32_t _tid;
struct {
--
1.9.2
[Qemu-devel] [PATCH 1/5] util/qemu-openpty: fix build with musl libc by include termios.h as fallback
Include termios.h as POSIX fallback when not glibc, bsd or solaris. POSIX says that termios.h should define struct termios and TCAFLUSH. http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html This fixes the following compile errors with musl libc: util/qemu-openpty.c: In function 'qemu_openpty_raw': util/qemu-openpty.c:112:20: error: storage size of 'tty' isn't known struct termios tty; ^ ... util/qemu-openpty.c:128:24: error: 'TCSAFLUSH' undeclared (first use in this function) tcsetattr(*aslave, TCSAFLUSH, tty); ^ Signed-off-by: Natanael Copa nc...@alpinelinux.org --- util/qemu-openpty.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c index 4febfe9..4c53211 100644 --- a/util/qemu-openpty.c +++ b/util/qemu-openpty.c @@ -47,6 +47,8 @@ #elif defined CONFIG_SOLARIS # include termios.h # include stropts.h +#else +# include termios.h #endif #ifdef __sun__ -- 1.9.2
[Qemu-devel] [PATCH 0/5] fix building with musl libc
In addition to the previoiusly sent linux-user: avoid using glibc internals in _syscall5 and in definition of target_sigevent struct, those are needed for making qemu build with musl libc on Alpine Linux. There is still 2 missing fcntl.h definitions (F_EXLCK and F_SHLCK) but those should probably be defined in libc so patch for that is not included here. Natanael Copa (5): util/qemu-openpty: fix build with musl libc by include termios.h as fallback xen: replace ffsl with ctzl vhost: replace ffsl with ctzl exec: replace ffsl with ctzl linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms hw/virtio/vhost.c | 6 ++ include/exec/ram_addr.h | 2 +- linux-user/signal.c | 7 +++ util/qemu-openpty.c | 2 ++ xen-all.c | 2 +- 5 files changed, 13 insertions(+), 6 deletions(-) -- 1.9.2
[Qemu-devel] [PATCH 2/5] xen: replace ffsl with ctzl
ffsl is a GNU extension and not available in musl libc.
See also commit fbeadf50 (bitops: unify bitops_ffsl with the one in
host-utils.h, call it bitops_ctzl) on why ctzl should be used instead
of ffsl.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
xen-all.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen-all.c b/xen-all.c
index ba34739..3a0e9e5 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -511,7 +511,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
for (i = 0; i ARRAY_SIZE(bitmap); i++) {
unsigned long map = bitmap[i];
while (map != 0) {
-j = ffsl(map) - 1;
+j = ctzl(map);
map = ~(1ul j);
memory_region_set_dirty(framebuffer,
(i * width + j) * TARGET_PAGE_SIZE,
--
1.9.2
[Qemu-devel] [PATCH 4/5] exec: replace ffsl with ctzl
See commit fbeadf50 (bitops: unify bitops_ffsl with the one in
host-utils.h, call it bitops_ctzl) on why ctzl should be used instead
of ffsl.
This is also needed for musl libc which does not implement ffsl.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
include/exec/ram_addr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2edfa96..b94de02 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -117,7 +117,7 @@ static inline void
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
if (bitmap[i] != 0) {
c = leul_to_cpu(bitmap[i]);
do {
-j = ffsl(c) - 1;
+j = ctzl(c);
c = ~(1ul j);
page_number = (i * HOST_LONG_BITS + j) * hpratio;
addr = page_number * TARGET_PAGE_SIZE;
--
1.9.2
[Qemu-devel] [PATCH 3/5] vhost: replace ffsl with ctzl
Avoid using the GNU extesion ffsl which is not implemented in musl libc.
The atomic_xchg() means we know that vhost_log_chunk_t will never be
larger than the 'long' type, so ctzl() is always sufficient.
See also commit fbeadf50 (bitops: unify bitops_ffsl with the one in
host-utils.h, call it bitops_ctzl) on why ctzl should be used instead
of ffsl.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
hw/virtio/vhost.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9e336ad..f62cfaf 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -41,7 +41,6 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
for (;from to; ++from) {
vhost_log_chunk_t log;
-int bit;
/* We first check with non-atomic: much cheaper,
* and we expect non-dirty to be the common case. */
if (!*from) {
@@ -51,12 +50,11 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
/* Data must be read atomically. We don't really need barrier semantics
* but it's easier to use atomic_* than roll our own. */
log = atomic_xchg(from, 0);
-while ((bit = sizeof(log) sizeof(int) ?
-ffsll(log) : ffs(log))) {
+while (log) {
+int bit = ctzl(log);
hwaddr page_addr;
hwaddr section_offset;
hwaddr mr_offset;
-bit -= 1;
page_addr = addr + bit * VHOST_LOG_PAGE;
section_offset = page_addr - section-offset_within_address_space;
mr_offset = section_offset + section-offset_within_region;
--
1.9.2
[Qemu-devel] [PATCH 5/5] linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms
The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available
on all platforms, so we define those if they are missing.
This is needed for musl libc.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
linux-user/signal.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7d6246f..6019dbb 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -32,6 +32,13 @@
//#define DEBUG_SIGNAL
+#ifndef __SIGRTMIN
+#define __SIGRTMIN 32
+#endif
+#ifndef __SIGRTMAX
+#define __SIGRTMAX (NSIG-1)
+#endif
+
static struct target_sigaltstack target_sigaltstack_used = {
.ss_sp = 0,
.ss_size = 0,
--
1.9.2
Re: [Qemu-devel] [PATCH 5/5] linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms
On Tue, 29 Apr 2014 08:28:29 -0600 Eric Blake ebl...@redhat.com wrote: On 04/29/2014 08:17 AM, Natanael Copa wrote: The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available on all platforms, so we define those if they are missing. This is needed for musl libc. Signed-off-by: Natanael Copa nc...@alpinelinux.org --- linux-user/signal.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/linux-user/signal.c b/linux-user/signal.c index 7d6246f..6019dbb 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -32,6 +32,13 @@ //#define DEBUG_SIGNAL +#ifndef __SIGRTMIN +#define __SIGRTMIN 32 Rather than defining the implementation-specific __SIGRTMIN to a magic number that is liable to be wrong, why not instead fix the code to use the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? Those seems to be runtime values: /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) so it gives: /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer [SIGRTMIN] = __SIGRTMAX, I could have used (NSIG-1) but are not sure if NSIG is a runtime macro in glibc. The array itself is using _NSIG instead of NSIG for some reason. -nc signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 5/5] linux-user/signal.c: define __SIGRTMIN/MAX for non-GNU platforms
On Tue, 29 Apr 2014 09:02:13 -0600 Eric Blake ebl...@redhat.com wrote: On 04/29/2014 08:53 AM, Natanael Copa wrote: On Tue, 29 Apr 2014 08:28:29 -0600 Eric Blake ebl...@redhat.com wrote: On 04/29/2014 08:17 AM, Natanael Copa wrote: The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available on all platforms, so we define those if they are missing. +#define __SIGRTMIN 32 Rather than defining the implementation-specific __SIGRTMIN to a magic number that is liable to be wrong, why not instead fix the code to use the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? Those seems to be runtime values: /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) Oh right - POSIX allows them to be runtime variable. But we are interacting with a given kernel, where the values will be fixed. Maybe you have to define __SIGRTMIN after all, but can we at least have an assert() that the value you picked matches SIGRTMIN at runtime? Yeah, that might be an idea. /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) so it gives: /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer [SIGRTMIN] = __SIGRTMAX, I could have used (NSIG-1) but are not sure if NSIG is a runtime macro in glibc. The array itself is using _NSIG instead of NSIG for some reason. NSIG is not any more portable; nor does POSIX require that the RT signals occur at the tail end of NSIG (in other words, NSIG-1 need not be SIGRTMAX). Unless someone knows of a kernel define, it sounds like we're stuck hard-coding in some knowledge of Linux. Since we already use _NSIG to define the size of the array, and we want to use the last element of the array, maybe we should just use _NSIG-1? -nc
[Qemu-devel] [PATCH] linux-user: avoid using glibc internals
Avoid using glibc specific internals.
Calculate the sigevent pad size is calculated in similar way as kernel
does it.
This is needed for building with musl libc.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
linux-user/syscall.c | 2 +-
linux-user/syscall_defs.h | 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9864813..c8989b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -406,7 +406,7 @@ static int sys_inotify_init1(int flags)
#endif
#define __NR_sys_ppoll __NR_ppoll
_syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
- struct timespec *, timeout, const __sigset_t *, sigmask,
+ struct timespec *, timeout, const sigset_t *, sigmask,
size_t, sigsetsize)
#endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index fdf9a47..450f71b 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2552,12 +2552,16 @@ struct target_timer_t {
abi_ulong ptr;
};
+#define TARGET_SIGEV_MAX_SIZE 64
+#define TARGET_SIGEV_PREAMABLE_SIZE (sizeof(int32_t) * 2 +
sizeof(target_sigval_t))
+#define TARGET_SIGEV_PAD_SIZE ((TARGET_SIGEV_MAX_SIZE -
TARGET_SIGEV_PREAMABLE_SIZE) / sizeof(int32_t))
+
struct target_sigevent {
target_sigval_t sigev_value;
int32_t sigev_signo;
int32_t sigev_notify;
union {
-int32_t _pad[ARRAY_SIZE(((struct sigevent *)0)-_sigev_un._pad)];
+int32_t _pad[TARGET_SIGEV_PAD_SIZE];
int32_t _tid;
struct {
--
1.9.2
[Qemu-devel] [PATCH v5] configure: properly check if -lrt and -lm is needed
Fixes build against uClibc.
uClibc provides 2 versions of clock_gettime(), one with realtime
support and one without (this is so you can avoid linking in -lrt
unless actually needed). This means that the clock_gettime() don't
need -lrt. We still need it for timer_create() so we check for this
function in addition.
We also need check if -lm is needed for isnan().
Both -lm and -lrt are needed for libs_qga.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes v4-v5:
- Do not exit with error if librt fails.
Apparently, mingw32 does not use those functions at all so we
should not exit with error.
This is how it originally worked.
configure | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index edf9da4..c1ed856 100755
--- a/configure
+++ b/configure
@@ -2624,17 +2624,44 @@ fi
##
+# Do we need libm
+cat $TMPC EOF
+#include math.h
+int main(void) { return isnan(sin(0.0)); }
+EOF
+if compile_prog ; then
+ :
+elif compile_prog -lm ; then
+ LIBS=-lm $LIBS
+ libs_qga=-lm $libs_qga
+else
+ echo
+ echo Error: libm check failed
+ echo
+ exit 1
+fi
+
+##
# Do we need librt
+# uClibc provides 2 versions of clock_gettime(), one with realtime
+# support and one without. This means that the clock_gettime() don't
+# need -lrt. We still need it for timer_create() so we check for this
+# function in addition.
cat $TMPC EOF
#include signal.h
#include time.h
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) {
+ timer_create(CLOCK_REALTIME, NULL, NULL);
+ return clock_gettime(CLOCK_REALTIME, NULL);
+}
EOF
if compile_prog ; then
:
-elif compile_prog -lrt ; then
+# we need pthread for static linking. use previous pthread test result
+elif compile_prog -lrt $pthread_lib ; then
LIBS=-lrt $LIBS
+ libs_qga=-lrt $libs_qga
fi
if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
--
1.7.12
Re: [Qemu-devel] [PATCH v4] configure: properly check if -lrt and -lm is needed
On Tue, 28 Aug 2012 17:16:18 + Blue Swirl blauwir...@gmail.com wrote: On Tue, Aug 28, 2012 at 7:33 AM, Natanael Copa nc...@alpinelinux.org wrote: On Tue, 21 Aug 2012 18:12:05 + Blue Swirl blauwir...@gmail.com wrote: Now I get this on mingw32: config-host.mak is out-of-date, running configure Error: librt check failed Any news on the v4 patch, which should fix this? No change: config-host.mak is out-of-date, running configure Error: librt check failed I have run out of guesses. Could I please have the last lines from your config.log? Thanks! -nc
Re: [Qemu-devel] [PATCH v4] configure: properly check if -lrt and -lm is needed
On Tue, 21 Aug 2012 18:12:05 + Blue Swirl blauwir...@gmail.com wrote: Now I get this on mingw32: config-host.mak is out-of-date, running configure Error: librt check failed Any news on the v4 patch, which should fix this? Thanks!
[Qemu-devel] [PATCH v4] configure: properly check if -lrt and -lm is needed
Fixes build against uClibc.
uClibc provides 2 versions of clock_gettime(), one with realtime
support and one without (this is so you can avoid linking in -lrt
unless actually needed). This means that the clock_gettime() don't
need -lrt. We still need it for timer_create() so we check for this
function in addition.
We also need check if -lm is needed for isnan().
Both -lm and -lrt are needed for libs_qga.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes v3-v4:
- Use $pthread_lib from previous pthread test
We don't need to add it to $LIBS since it should be there already
configure | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index edf9da4..31eee91 100755
--- a/configure
+++ b/configure
@@ -2624,17 +2624,49 @@ fi
##
+# Do we need libm
+cat $TMPC EOF
+#include math.h
+int main(void) { return isnan(sin(0.0)); }
+EOF
+if compile_prog ; then
+ :
+elif compile_prog -lm ; then
+ LIBS=-lm $LIBS
+ libs_qga=-lm $libs_qga
+else
+ echo
+ echo Error: libm check failed
+ echo
+ exit 1
+fi
+
+##
# Do we need librt
+# uClibc provides 2 versions of clock_gettime(), one with realtime
+# support and one without. This means that the clock_gettime() don't
+# need -lrt. We still need it for timer_create() so we check for this
+# function in addition.
cat $TMPC EOF
#include signal.h
#include time.h
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) {
+ timer_create(CLOCK_REALTIME, NULL, NULL);
+ return clock_gettime(CLOCK_REALTIME, NULL);
+}
EOF
if compile_prog ; then
:
-elif compile_prog -lrt ; then
+# we need pthread for static linking. use previous pthread test result
+elif compile_prog -lrt $pthread_lib ; then
LIBS=-lrt $LIBS
+ libs_qga=-lrt $libs_qga
+else
+ echo
+ echo Error: librt check failed
+ echo
+ exit 1
fi
if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
--
1.7.12
Re: [Qemu-devel] [PATCH v2] configure: properly check if -lrt and -lm is needed
On Mon, 20 Aug 2012 19:53:22 + Blue Swirl blauwir...@gmail.com wrote: - Do not remove the explicit add of -lm unless Haiku. This was due to http://www.mail-archive.com/qemu-devel@nongnu.org/msg102965.html I am not sure if this is valid, though. Certainly building a linux-user target works for me even without that explicit 'LIBS+=-lm' in Makefile.target. Blue, can you remember what you had in mind when you wrote that email? Build failed since math library functions (sin() etc.) were not found. The v2 and v3 patches I sent tests for sin() in addition to isnan (which normally is a macro). It might explain why it works now. Adding a second -lm does not hurt, but omitting it when needed breaks build on uclibc for sure, so it would be nice if the v3 patch could be applied. Thanks! -nc
Re: [Qemu-devel] [PATCH v2] configure: properly check if -lrt and -lm is needed
On Mon, Aug 20, 2012 at 3:27 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 16 August 2012 14:22, Natanael Copa natanael.c...@gmail.com wrote: Fixes build against uClibc. uClibc provides 2 versions of clock_gettime(), one with realtime support and one without (this is so you can avoid linking in -lrt unless actually needed). This means that the clock_gettime() don't need -lrt. We still need it for timer_create() so we check for this function in addition. We also need check if -lm is needed for isnan(). Both -lm and -lrt are needed for libs_qga. This patch breaks building the linux-user targets with --static: cam-vm-266:precise:qemu$ ./configure --target-list=arm-linux-user --static Error: librt check failed The test program is failing to link with this /usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/librt.a(timer_create.o): In function `timer_create': (.text+0x121): undefined reference to `pthread_once' and a lot of similar errors. smells missing -lpthread. (This seems to me like a glibc bug but since it's out there we rather have to deal with it I think.) Agree. Will configure pass if you: --- a/configure +++ b/configure @@ -2661,6 +2661,10 @@ if compile_prog ; then elif compile_prog -lrt ; then LIBS=-lrt $LIBS libs_qga=-lrt $libs_qga +# we might need -lpthread in case static linking on glibc +elif compile_prog -lrt -lpthread ; then + LIBS=-lrt -lpthread $LIBS + libs_qga=-lrt -lpthread $libs_qga else echo echo Error: librt check failed Thanks for feedback! -- Natanael Copa
[Qemu-devel] [PATCH v3] configure: properly check if -lrt and -lm is needed
Fixes build against uClibc.
uClibc provides 2 versions of clock_gettime(), one with realtime
support and one without (this is so you can avoid linking in -lrt
unless actually needed). This means that the clock_gettime() don't
need -lrt. We still need it for timer_create() so we check for this
function in addition.
We also need check if -lm is needed for isnan().
Both -lm and -lrt are needed for libs_qga.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Changes v2-v3:
- Check if -lpthread is needed with static -lrt
configure | 37 -
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index edf9da4..3d7fe69 100755
--- a/configure
+++ b/configure
@@ -2624,17 +2624,52 @@ fi
##
+# Do we need libm
+cat $TMPC EOF
+#include math.h
+int main(void) { return isnan(sin(0.0)); }
+EOF
+if compile_prog ; then
+ :
+elif compile_prog -lm ; then
+ LIBS=-lm $LIBS
+ libs_qga=-lm $libs_qga
+else
+ echo
+ echo Error: libm check failed
+ echo
+ exit 1
+fi
+
+##
# Do we need librt
+# uClibc provides 2 versions of clock_gettime(), one with realtime
+# support and one without. This means that the clock_gettime() don't
+# need -lrt. We still need it for timer_create() so we check for this
+# function in addition.
cat $TMPC EOF
#include signal.h
#include time.h
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) {
+ timer_create(CLOCK_REALTIME, NULL, NULL);
+ return clock_gettime(CLOCK_REALTIME, NULL);
+}
EOF
if compile_prog ; then
:
elif compile_prog -lrt ; then
LIBS=-lrt $LIBS
+ libs_qga=-lrt $libs_qga
+# we might need -lpthread in case static linking
+elif compile_prog -lrt -lpthread ; then
+ LIBS=-lrt -lpthread $LIBS
+ libs_qga=-lrt -lpthread $libs_qga
+else
+ echo
+ echo Error: librt check failed
+ echo
+ exit 1
fi
if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
--
1.7.12
[Qemu-devel] [PATCH v2] configure: properly check if -lrt and -lm is needed
Fixes build against uClibc.
uClibc provides 2 versions of clock_gettime(), one with realtime
support and one without (this is so you can avoid linking in -lrt
unless actually needed). This means that the clock_gettime() don't
need -lrt. We still need it for timer_create() so we check for this
function in addition.
We also need check if -lm is needed for isnan().
Both -lm and -lrt are needed for libs_qga.
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
The Xen people have nagged me to get this patch upstream so I have come
up with a rebased v2 patch after consulting with pm215 on IRC.
Please consider include this.
Changes v1-v2:
- Check for sin() in addition to isnan()
- Add comment on why we also check for timer_create
- Use $LIBS and $libs_qga instead of $libm and $librt, based on
feedback from pm215 on IRC
- Do not remove the explicit add of -lm unless Haiku. This was due
to http://www.mail-archive.com/qemu-devel@nongnu.org/msg102965.html
I am not sure if this is valid, though.
configure | 33 -
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index edf9da4..a351f9b 100755
--- a/configure
+++ b/configure
@@ -2624,17 +2624,48 @@ fi
##
+# Do we need libm
+cat $TMPC EOF
+#include math.h
+int main(void) { return isnan(sin(0.0)); }
+EOF
+if compile_prog ; then
+ :
+elif compile_prog -lm ; then
+ LIBS=-lm $LIBS
+ libs_qga=-lm $libs_qga
+else
+ echo
+ echo Error: libm check failed
+ echo
+ exit 1
+fi
+
+##
# Do we need librt
+# uClibc provides 2 versions of clock_gettime(), one with realtime
+# support and one without. This means that the clock_gettime() don't
+# need -lrt. We still need it for timer_create() so we check for this
+# function in addition.
cat $TMPC EOF
#include signal.h
#include time.h
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) {
+ timer_create(CLOCK_REALTIME, NULL, NULL);
+ return clock_gettime(CLOCK_REALTIME, NULL);
+}
EOF
if compile_prog ; then
:
elif compile_prog -lrt ; then
LIBS=-lrt $LIBS
+ libs_qga=-lrt $libs_qga
+else
+ echo
+ echo Error: librt check failed
+ echo
+ exit 1
fi
if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
--
1.7.11.4
[Qemu-devel] Release Engineering: qemu-1.1.1.tar.bz2 release name and filetype mismatch
Hi, The qemu-1.1.1 release tarball[1] has file extention .tar.bz2 while the real file format is gzip. $ file Downloads/qemu-1.1.1.tar.bz2 Downloads/qemu-1.1.1.tar.bz2: gzip compressed data, from Unix, last modified: Thu Jul 12 21:52:36 2012 Would it be possible to add a tarball which has a file suffix that matches the real file format? (eg mv qemu-1.1.1.tar.bz2 qemu-1.1.1.tar.gz) Thanks! -nc
[Qemu-devel] [PATCH] configure: properly check if -lrt and -lm is needed
Fixes build against uClibc.
uClibc provides 2 versions of clock_gettime(), one with realtime
support and one without (this is so you can avoid linking in -lrt
unless actually needed). This means that the clock_gettime() don't
need -lrt. We still need it for timer_create() so we check for this
function in addition.
We also need check if -lm is needed for isnan().
Signed-off-by: Natanael Copa nc...@alpinelinux.org
---
Makefile|4 ++--
Makefile.target |4 +---
configure | 33 +++--
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 32550cb..9dfa01a 100644
--- a/Makefile
+++ b/Makefile
@@ -35,7 +35,7 @@ configure: ;
$(call set-vpath, $(SRC_PATH))
-LIBS+=-lz $(LIBS_TOOLS)
+LIBS+=-lz $(LIBS_TOOLS) $(LIBM) $(LIBRT)
HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
@@ -172,7 +172,7 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h $ $@, GEN
$@)
qapi-dir := $(BUILD_DIR)/qapi-generated
-qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
+qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) $(LIBRT) $(LIBM)
qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
gen-out-type = $(subst .,-,$(suffix $@))
diff --git a/Makefile.target b/Makefile.target
index 2907aad..d214d2c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -34,9 +34,7 @@ PROGS+=$(QEMU_PROGW)
endif
STPFILES=
-ifndef CONFIG_HAIKU
-LIBS+=-lm
-endif
+LIBS+=$(LIBM) $(LIBRT)
config-target.h: config-target.h-timestamp
config-target.h-timestamp: config-target.mak
diff --git a/configure b/configure
index c2366ee..f925973 100755
--- a/configure
+++ b/configure
@@ -102,6 +102,8 @@ audio_win_int=
cc_i386=i386-pc-linux-gnu-gcc
libs_qga=
debug_info=yes
+libm=
+librt=
target_list=
@@ -2568,17 +2570,42 @@ fi
##
+# Do we need libm
+cat $TMPC EOF
+#include math.h
+int main(void) { return isnan(0.0); }
+EOF
+if compile_prog ; then
+ libm=
+elif compile_prog -lm ; then
+ libm=-lm
+else
+ echo
+ echo Error: libm check failed
+ echo
+ exit 1
+fi
+
+##
# Do we need librt
cat $TMPC EOF
#include signal.h
#include time.h
-int main(void) { return clock_gettime(CLOCK_REALTIME, NULL); }
+int main(void) {
+ timer_create(CLOCK_REALTIME, NULL, NULL);
+ return clock_gettime(CLOCK_REALTIME, NULL);
+}
EOF
if compile_prog ; then
:
elif compile_prog -lrt ; then
- LIBS=-lrt $LIBS
+ librt=-lrt
+else
+ echo
+ echo Error: librt check failed
+ echo
+ exit 1
fi
if test $darwin != yes -a $mingw32 != yes -a $solaris != yes -a \
@@ -3442,6 +3469,8 @@ echo LIBS_TOOLS+=$libs_tools $config_host_mak
echo EXESUF=$EXESUF $config_host_mak
echo LIBS_QGA+=$libs_qga $config_host_mak
echo POD2MAN=$POD2MAN $config_host_mak
+echo LIBM=$libm $config_host_mak
+echo LIBRT=$librt $config_host_mak
# generate list of library paths for linker script
--
1.7.10.4
