char: serial: check divider value against baud base

16550A UART device uses an oscillator to generate frequencies
(baud base), which decide communication speed. This speed could
be changed by dividi

char: serial: check divider value against baud base

16550A UART device uses an oscillator to generate frequencies
(baud base), which decide communication speed. This speed could
be changed by dividing it by a divider. If the divider is
greater than the baud base, speed is set to zero, leading to a
divide by zero error. Add check to avoid it.

Reported-by: Huawei PSIRT <psirt@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-Id: <1476251888-20238-1-git-send-email-ppandit@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: reinstate watch after migration

Otherwise, a serial port can get stuck if it is migrated while flow control
is in effect.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. Davi

serial: reinstate watch after migration

Otherwise, a serial port can get stuck if it is migrated while flow control
is in effect.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: remove watch on reset

Otherwise, this can cause serial_xmit to be entered with LSR.TEMT=0,
which is invalid and causes an assertion failure.

Reported-by: Bret Ketchum <bcketchum@gmail.com>

serial: remove watch on reset

Otherwise, this can cause serial_xmit to be entered with LSR.TEMT=0,
which is invalid and causes an assertion failure.

Reported-by: Bret Ketchum <bcketchum@gmail.com>
Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: separate serial_xmit and serial_watch_cb

serial_xmit starts transmission of whatever is in the transmitter
register, THR or FIFO; serial_watch_cb is a wrapper around it and is
only used as a

serial: separate serial_xmit and serial_watch_cb

serial_xmit starts transmission of whatever is in the transmitter
register, THR or FIFO; serial_watch_cb is a wrapper around it and is
only used as a qemu_chr_fe_add_watch callback.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: simplify tsr_retry reset

Move common code outside the if, and reset tsr_retry even in loopback mode.
Right now it cannot become non-zero, but it will be possible as soon as
we start respecti

serial: simplify tsr_retry reset

Move common code outside the if, and reset tsr_retry even in loopback mode.
Right now it cannot become non-zero, but it will be possible as soon as
we start respecting the baud rate.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: make tsr_retry unsigned

It can never become negative; reflect this in the type of the field
and simplify the conditions.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David

serial: make tsr_retry unsigned

It can never become negative; reflect this in the type of the field
and simplify the conditions.

Tested-by: Bret Ketchum <bcketchum@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

Replaced get_tick_per_sec() by NANOSECONDS_PER_SECOND

This patch replaces get_ticks_per_sec() calls with the macro
NANOSECONDS_PER_SECOND. Also, as there are no callers, get_ticks_per_sec()
is then

Replaced get_tick_per_sec() by NANOSECONDS_PER_SECOND

This patch replaces get_ticks_per_sec() calls with the macro
NANOSECONDS_PER_SECOND. Also, as there are no callers, get_ticks_per_sec()
is then removed. This replacement improves the readability and
understandability of code.

For example,

timer_mod(fdctrl->result_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (get_ticks_per_sec() / 50));

NANOSECONDS_PER_SECOND makes it obvious that qemu_clock_get_ns
matches the unit of the expression on the right side of the plus.

Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

include/qemu/osdep.h: Don't include qapi/error.h

Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its fi

include/qemu/osdep.h: Don't include qapi/error.h

Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.

Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.

Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.

This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

x86: Clean up includes

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-b

x86: Clean up includes

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-11-git-send-email-peter.maydell@linaro.org

show more ...

Use error_fatal to simplify obvious fatal errors

Done with this Coccinelle semantic patch:

@@
type T;
identifier FUN, RET;
expression list ARGS;
expression ERR, EC;
@@
(

Use error_fatal to simplify obvious fatal errors

Done with this Coccinelle semantic patch:

@@
type T;
identifier FUN, RET;
expression list ARGS;
expression ERR, EC;
@@
(
- T RET = FUN(ARGS, &ERR);
+ T RET = FUN(ARGS, &error_fatal);
|
- RET = FUN(ARGS, &ERR);
+ RET = FUN(ARGS, &error_fatal);
|
- FUN(ARGS, &ERR);
+ FUN(ARGS, &error_fatal);
)
- if (ERR != NULL) {
- error_report_err(ERR);
- exit(EC);
- }

This is actually a more elegant version of my initial semantic patch
by courtesy of Eduardo.

It leaves dead Error * variables behind, cleaned up manually.

Cc: qemu-arm@nongnu.org
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

show more ...

migration: Use normal VMStateDescriptions for Subsections

We create optional sections with this patch. But we already have
optional subsections. Instead of having two mechanism that do the
same, w

migration: Use normal VMStateDescriptions for Subsections

We create optional sections with this patch. But we already have
optional subsections. Instead of having two mechanism that do the
same, we can just generalize it.

For subsections we just change:

- Add a needed function to VMStateDescription
- Remove VMStateSubsection (after removal of the needed function
it is just a VMStateDescription)
- Adjust the whole tree, moving the needed function to the corresponding
VMStateDescription

Signed-off-by: Juan Quintela <quintela@redhat.com>

show more ...

error: Use error_report_err() where appropriate

Coccinelle semantic patch:

@@
expression E;
@@
- error_report("%s", error_get_pretty(E));
- error_free(E);
+ error_r

error: Use error_report_err() where appropriate

Coccinelle semantic patch:

@@
expression E;
@@
- error_report("%s", error_get_pretty(E));
- error_free(E);
+ error_report_err(E);
@@
expression E, S;
@@
- error_report("%s", error_get_pretty(E));
+ error_report_err(E);
(
exit(S);
|
abort();
)

Trivial manual touch-ups in block/sheepdog.c.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

show more ...

serial: Fix warnings caused by missing 'static' attribute

Warnings from the Sparse static analysis tool:

hw/char/serial.c:630:26: warning: symbol
'vmstate_serial_thr_ipending' was not declared. Sh

serial: Fix warnings caused by missing 'static' attribute

Warnings from the Sparse static analysis tool:

hw/char/serial.c:630:26: warning: symbol
'vmstate_serial_thr_ipending' was not declared. Should it be static?
hw/char/serial.c:646:26: warning: symbol
'vmstate_serial_tsr' was not declared. Should it be static?
hw/char/serial.c:665:26: warning: symbol
'vmstate_serial_recv_fifo' was not declared. Should it be static?
hw/char/serial.c:681:26: warning: symbol
'vmstate_serial_xmit_fifo' was not declared. Should it be static?
hw/char/serial.c:697:26: warning: symbol
'vmstate_serial_fifo_timeout_timer' was not declared. Should it be static?
hw/char/serial.c:713:26: warning: symbol
'vmstate_serial_timeout_ipending' was not declared. Should it be static?
hw/char/serial.c:729:26: warning: symbol
'vmstate_serial_poll' was not declared. Should it be static?

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

show more ...

vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*

Old users of VMSTATE_TIMER* are mechanically changed to VMSTATE_TIMER_PTR
variants.

Signed-off-by: Paolo Bonzini <pbonzini@redhat

vmstate: accept QEMUTimer in VMSTATE_TIMER*, add VMSTATE_TIMER_PTR*

Old users of VMSTATE_TIMER* are mechanically changed to VMSTATE_TIMER_PTR
variants.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: refine serial_thr_ipending_needed

If the THR interrupt is disabled, there is no need to migrate thr_ipending
because LSR.THRE will be sampled again when the interrupt is enabled.
(This is th

serial: refine serial_thr_ipending_needed

If the THR interrupt is disabled, there is no need to migrate thr_ipending
because LSR.THRE will be sampled again when the interrupt is enabled.
(This is the behavior that is not documented in the datasheet, but
relied on by Windows!)

Note that in this case IIR will never be 0x2 so, if thr_ipending were
to be one, QEMU would produce the subsection.

Reported-by: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: only resample THR interrupt on rising edge of IER.THRI

There is disagreement on whether LSR.THRE should be resampled when
IER.THRI goes from 1 to 1. Bochs only does it if IER.THRI goes from

serial: only resample THR interrupt on rising edge of IER.THRI

There is disagreement on whether LSR.THRE should be resampled when
IER.THRI goes from 1 to 1. Bochs only does it if IER.THRI goes from 0
to 1; PCE does it even if IER.THRI is unchanged. But the Windows driver
seems to always go from 1 to 0 and back to 1, so do things in agreement
with Bochs, because the handling of thr_ipending was reported in 2010
(https://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01914.html)
as breaking DR-DOS Plus.

Reported-by: Roy Tam <roytam@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: update LSR on enabling/disabling FIFOs

When the transmit FIFO is emptied or enabled, the transmitter
hold register is empty. When it is disabled, it is also emptied and
in addition the prev

serial: update LSR on enabling/disabling FIFOs

When the transmit FIFO is emptied or enabled, the transmitter
hold register is empty. When it is disabled, it is also emptied and
in addition the previous contents of the transmitter hold register
are discarded. In either case, the THRE bit in LSR must be set and
THRI raised.

When the receive FIFO is emptied or enabled, the data ready and break
bits must be cleared in LSR. Likewise when the receive FIFO is disabled.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: clean up THRE/TEMT handling

- assert TEMT is cleared before sending a character; we'll get one from
TSR if tsr_retry > 0, from the FIFO or THR otherwise

- assert THRE cleared and FIFO not e

serial: clean up THRE/TEMT handling

- assert TEMT is cleared before sending a character; we'll get one from
TSR if tsr_retry > 0, from the FIFO or THR otherwise

- assert THRE cleared and FIFO not empty (if enabled) before fetching a
character to send. This effectively reverts dffacd46, but the check
makes no sense and commit f702e62 (serial: change retry logic to avoid
concurrency, 2014-07-11) must have made it unnecessary. The commit
message for f702e62 talks about multiple calls to qemu_chr_fe_add_watch
triggering s->tsr_retry >= MAX_XMIT_RETRY, but other failures were
possible. For example, if you have multiple calls, the subsequent ones
will see s->tsr_retry == 0 and will find THRE and/or TEMT on entry.

- for clarity, raise THRI immediately after the code sets THRE

- check THRE to see if another character has to be sent. This makes
the assertions more obvious and also means TEMT has to be set as soon as
the loop ends. It makes the loop send both TSR and THR if flow-control
happens in non-FIFO mode. Previously, THR would be lost.

- clear TEMT together with THRE even in the non-FIFO case

The last two items are bugfixes, but they were just found by inspection
and do not squash known bugs.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: reset thri_pending on IER writes with THRI=0

This is responsible for failure of migration from 2.2 to 2.1, because
thr_ipending is always one in practice.

serial.c is setting thr_ipending u

serial: reset thri_pending on IER writes with THRI=0

This is responsible for failure of migration from 2.2 to 2.1, because
thr_ipending is always one in practice.

serial.c is setting thr_ipending unconditionally. However, thr_ipending
is not used at all if THRI=0, and it will be overwritten again the next
time THRE or THRI changes. For that reason, we can set thr_ipending to
zero every time THRI is reset.

There is disagreement on whether LSR.THRE should be resampled when IER.THRI
goes from 1 to 1. This patch does not touch the code, leaving that for
QEMU 2.3+.

This has no semantic change and is enough to fix migration in the common
case where the interrupt is not pending or is reported in IIR. It does not
change the migration format, so 2.2.0 -> 2.1 will remain broken but we
can fix 2.2.1 -> 2.1 without breaking 2.2.1 <-> 2.2.0.

The case that remains broken (the one in which the subsection is strictly
necessary) is when THRE=1, the THRI interrupt has *not* been acknowledged
yet, and a higher-priority interrupt comes. In this case, you need the
subsection to tell the source that the lower-priority THRI interrupt is
pending. The subsection's breakage of migration, in this case, prevents
continuing the VM on the destination with an invalid state.

Cc: qemu-stable@nongnu.org
Reported-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: check if backed by a physical serial port at realize time

Right now, s->poll_msl may linger at "0" value for an arbitrarily long
time, until serial_update_msl is called for the first time.

serial: check if backed by a physical serial port at realize time

Right now, s->poll_msl may linger at "0" value for an arbitrarily long
time, until serial_update_msl is called for the first time. This is
unnecessary, and will lead to the s->poll_msl field being unnecessarily
migrated.

We can call serial_update_msl immediately at realize time (via
serial_reset) and be done with it. The memory-mapped UART was already
doing that, but not the ISA and PCI variants.

Regarding the delta bits, be consistent with what serial_reset does when
the serial port is not backed by a physical serial port, and always clear
them at reset time.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: reset state at startup

When a serial port is started, its initial state is all zero. Make
it consistent with reset state instead.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

serial: fixing vmstate for save/restore

Some fields were added to VMState by this patch to preserve correct
loading of the serial port controller state.
Updating FCR value while loading was also mod

serial: fixing vmstate for save/restore

Some fields were added to VMState by this patch to preserve correct
loading of the serial port controller state.
Updating FCR value while loading was also modified to disable generating
an interrupt by loadvm.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: change retry logic to avoid concurrency

Whenever serial_xmit fails to transmit a byte it adds a watch that would
call it again when the "line" becomes ready. This results in a retry
chain:

serial: change retry logic to avoid concurrency

Whenever serial_xmit fails to transmit a byte it adds a watch that would
call it again when the "line" becomes ready. This results in a retry
chain:
serial_xmit -> add_watch -> serial_xmit
Each chain is able to transmit one character, and for every character
passed to serial by the guest driver a new chain is spawned.

The problem lays with the fact that a new chain is spawned even when
there is one already waiting on the watch. So there can be several retry
chains waiting concurrently on one "line". Every chain tries to transmit
current character, so character order is not messed up. But also every
chain increases retry counter (tsr_retry). If there are enough
concurrent chains this counter will hit MAX_XMIT_RETRY value and
the character will be dropped.

To reproduce this bug you need to feed serial output to some program
consuming it slowly enough. A python script from bug #1335444
description is an example of such program.

This commit changes retry logic in the following way to avoid
concurrency: instead of spawning a new chain for each character being
transmitted spawn only one and make it transmit characters until FIFO is
empty.

The change consists of two parts:
- add a do {} while () loop in serial_xmit (diff is a bit erratic
for this part, diff -w will show actual change),
- do not call serial_xmit from serial_ioport_write if there is one
waiting on the watch already.

This should fix another issue causing bug #1335444.

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

serial: poll the serial console with G_IO_HUP

On FreeBSD polling a master pty while the other end is not connected
with G_IO_OUT only results in an endless wait. This is different from
the Linux beh

serial: poll the serial console with G_IO_HUP

On FreeBSD polling a master pty while the other end is not connected
with G_IO_OUT only results in an endless wait. This is different from
the Linux behaviour, that returns immediately. In order to demonstrate
this, I have the following example code:

http://xenbits.xen.org/people/royger/test_poll.c

When executed on Linux:

$ ./test_poll
In callback

On FreeBSD instead, the callback never gets called:

$ ./test_poll

So, in order to workaround this, poll the source with G_IO_HUP (which
makes the code behave the same way on both Linux and FreeBSD).

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: xen-devel@lists.xenproject.org
[Add hw/char/cadence_uart.c too. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

show more ...

savevm: Remove all the unneeded version_minimum_id_old (x86)

After previous Peter patch, they are redundant. This way we don't
assign them except when needed. Once there, there were lots of case
w

savevm: Remove all the unneeded version_minimum_id_old (x86)

After previous Peter patch, they are redundant. This way we don't
assign them except when needed. Once there, there were lots of case
where the ".fields" indentation was wrong:

.fields = (VMStateField []) {
and
.fields = (VMStateField []) {

Change all the combinations to:

.fields = (VMStateField[]){

The biggest problem (appart from aesthetics) was that checkpatch complained
when we copy&pasted the code from one place to another.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

show more ...