linux-user: correct msgrcv()

All parameters must be swapped before the call of do_msgrcv().

Allow faked (debian fakeroot daemon) to work properly.

WITHOUT this patch:

$ faked-sysv --foreground --debug
using 1723744788 as msg key
msg_key=1723744788
1723744788:431
FAKEROOT: msg=131072, key=1723744788
FAKEROOT: r=-1, received message type=-150996052, message=-160219330
FAKEROOT, get_msg: Bad address
r=14, EINTR=4
fakeroot: clearing up message queues and semaphores, signal=-1
fakeroot: database save FAILED

WITH this patch:

$ faked-sysv --foreground --debug
using 1569385744 as msg key
msg_key=1569385744
1569385744:424
FAKEROOT: msg=0, key=1569385744
^C
fakeroot: clearing up message queues and semaphores, signal=2
fakeroot: database save FAILED

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
linux-user: correct print_timeval() swap tv_sec and tv_usec

Signed-off-by: Laurent Vivier <Laurent@Vivier.EU>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
linux-user: correct setsockopt()

SO_SNDTIMEO and SO_RCVTIMEO take a struct timeval, not an int

To test this, you can use :

QEMU_STRACE= ping localhost 2>&1 |grep TIMEO
568 setsockopt(3,SOL_SOCKET,SO_SNDTIMEO,{1,0},8) = 0
568 setsockopt(3,SOL_SOCKET,SO_RCVTIMEO,{1,0},8) = 0

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
linux-user: correct reboot()

According to man reboot(2), the 4th argument is only used with
LINUX_REBOOT_CMD_RESTART2. In other cases, trying to convert
the value can generate EFAULT.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
target-cris: Build fix for debug output

Around r3361 (81fdc5f8d2d681da8d255baf0713144f8656bac9) env->debug1 used
to contain the address of an MMU fault. This is now written into
env->pregs[PR_EDA] instead.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
cutils: unsigned int parsing functions

There are lots of duplicate parsing code using strto*() in QEMU, and
most of that code is broken in one way or another. Even the visitors
code have duplicate integer parsing code[1]. This introduces functions
to help parsing unsigned int values: parse_uint() and parse_uint_full().

Parsing functions for signed ints and floats will be submitted later.

parse_uint_full() has all the checks made by opts_type_uint64() at
opts-visitor.c:

 - Check for NULL (returns -EINVAL)
 - Check for negative numbers (returns -EINVAL)
 - Check for empty string (returns -EINVAL)
 - Check for overflow or other errno values set by strtoll() (returns
   -errno)
 - Check for end of string (reject invalid characters after number)
   (returns -EINVAL)

parse_uint() does everything above except checking for the end of the
string, so callers can continue parsing the remainder of string after
the number.

Unit tests included.

[1] string-input-visitor.c:parse_int() could use the same parsing code
    used by opts-visitor.c:opts_type_int(), instead of duplicating that
    logic.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: Fix off-by-one bug when handling "-numa node" argument

The numa_add() code was unconditionally adding 1 to the get_opt_name()
return value, making it point after the end of the string if no ','
separator is present.

Example of weird behavior caused by the bug:

  $ qemu-img create -f qcow2 this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2 5G
  Formatting 'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2', fmt=qcow2 size=5368709120 encryption=off cluster_size=65536
  $ ./x86_64-softmmu/qemu-system-x86_64 -S -monitor stdio -numa node 'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2'
  QEMU 1.3.50 monitor - type 'help' for more information
  (qemu) info numa
  1 nodes
  node 0 cpus: 0
  node 0 size: 1000 MB
  (qemu)

This changes the code to nove the pointer only if ',' is found.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: Abort on unknown -numa option type

Abort in case an invalid -numa option is provided, instead of silently
ignoring it.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: Check for NUMA node limit inside numa_add()

Instead of checking the limit before calling numa_add(), check the limit
only when we already know we're going to add a new node.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: numa_add(): Validate nodeid before using it

Without this check, QEMU will corrupt memory if a too-large nodeid is
provided in the command-line. e.g.:

  -numa node,mem=...,cpus=...,nodeid=65

This changes nodenr to unsigned long long, to avoid integer conversion
issues when converting the strtoull() result to int.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: Use parse_uint_full() for NUMA nodeid

This should catch many kinds of errors that the current code wasn't
checking for:

 - Values that can't be parsed as a number
 - Negative values
 - Overflow
 - Empty string

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: Extract -numa "cpus" parsing to separate function

This will make it easier to refactor that code later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
vl.c: validate -numa "cpus" parameter properly

- Accept empty strings without aborting
- Use parse_uint*() to parse numbers
- Abort if anything except '-' or end-of-string is found after the first
  number.
- Check for endvalue < value

Also change the MAX_CPUMASK_BITS warning message from "A max of %d CPUs
are supported in a guest" to "qemu: NUMA: A max of %d VCPUs are
supported".

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
acpi_piix4: fix segfault migrating from 1.2

b0b873a07872f7ab7f66f259c73fb9dd42aa66a9 bumped the vmstate version and
introduced an old-style load function to handle migration from prior
(<= 1.2) versions.

The load function passes the top-level PIIX4PMState pointer to
vmstate_load_state() to handle nested structs for APMState and
pci_status, which leads to corruption of the top-level PIIX4PMState,
since pointers to the nested structs are expected.

A segfault can be fairly reliably triggered by migrating from 1.2 and
issuing a reset, which will trigger a number of QOM operations which
rely on the now corrupted ObjectClass/Object members.

Fix this by passing in the expected pointers for vmstate_load_state().

Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
s390x: silence warning from GCC on uninitialized values

As best I can tell, this is a false positive.

  [aliguori@ccnode4 qemu-s390]$ make
    CC    s390x-softmmu/target-s390x/helper.o
  /home/aliguori/git/qemu/target-s390x/helper.c: In function ‘do_interrupt’:
  /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  /home/aliguori/git/qemu/target-s390x/helper.c:620:20: note: ‘addr’ was declared here
  /home/aliguori/git/qemu/target-s390x/helper.c:673:17: error: ‘mask’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  /home/aliguori/git/qemu/target-s390x/helper.c:620:14: note: ‘mask’ was declared here
  cc1: all warnings being treated as errors
  make[1]: *** [target-s390x/helper.o] Error 1
  make: *** [subdir-s390x-softmmu] Error 2

Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
target-s390x: Fix wrong comparison in interrupt handling

gcc with -Wextra complains about an ordered pointer comparison:

target-s390x/helper.c:660:27: warning:
 ordered comparison of pointer with integer zero [-Wextra]

Obviously the index was missing in the code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
trace: Fix simple trace dropped event record for big endian

We use atomic operations to keep track of dropped events.

Inconveniently, GLib supports only int and void * atomics, but the
counter dropped_events is uint64_t.  Can't stop commit 62bab732: a
quick (gint *)&dropped_events bludgeons the compiler into submission.

That cast is okay only when int is exactly 64 bits wide, which it
commonly isn't.

If int is even wider, we clobber whatever follows dropped_events.  Not
worth worrying about, as none of the machines that interest us have
such morbidly obese ints.

That leaves the common case: int narrower than 64 bits.

Harmless on little endian hosts: we just don't access the most
significant bits of dropped_events.  They remain zero.

On big endian hosts, we use only the most significant bits of
dropped_events as counter.  The least significant bits remain zero.
However, we write out the full value, which is the correct counter
shifted left a bunch of places.

Fix by changing the variables involved to int.

There's another, equally suspicious-looking (gint *)&trace_idx
argument to g_atomic_int_compare_and_exchange(), but that one casts
unsigned *, so it's okay.  But it's also superfluous, because GLib's
atomic int operations work just fine for unsigned.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
trace: Direct access of atomics is verboten, use the API

The GLib Reference Manual says:

    It is very important that all accesses to a particular integer or
    pointer be performed using only this API and that different sizes
    of operation are not mixed or used on overlapping memory
    regions. Never read or assign directly from or to a value --
    always use this API.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
trace: Clean up the "try to update atomic until it worked" loops

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
trace: Fix location of simpletrace.py in docs

Missed when commit 4c3b5a48 moved it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>