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: NHuawei PSIRT <psirt@huawei.com>
Signed-off-by: NPrasad J Pandit <pjp@fedoraproject.org>
Message-Id: <1476251888-20238-1-git-send-email-ppandit@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Otherwise, a serial port can get stuck if it is migrated while flow control
is in effect.
Tested-by: NBret Ketchum <bcketchum@gmail.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Otherwise, this can cause serial_xmit to be entered with LSR.TEMT=0,
which is invalid and causes an assertion failure.
Reported-by: NBret Ketchum <bcketchum@gmail.com>
Tested-by: NBret Ketchum <bcketchum@gmail.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NBret Ketchum <bcketchum@gmail.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NBret Ketchum <bcketchum@gmail.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

It can never become negative; reflect this in the type of the field
and simplify the conditions.
Tested-by: NBret Ketchum <bcketchum@gmail.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NRutuja Shah <rutu.shah.26@gmail.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Commit 57cb38b3 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: NMarkus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NPeter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-11-git-send-email-peter.maydell@linaro.org

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: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEduardo Habkost <ehabkost@redhat.com>

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: NJuan Quintela <quintela@redhat.com>

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: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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: NStefan Weil <sw@weilnetz.de>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Old users of VMSTATE_TIMER* are mechanically changed to VMSTATE_TIMER_PTR
variants.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NIgor Mammedov <imammedo@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NRoy Tam <roytam@gmail.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

- 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 f702e62a (serial: change retry logic to avoid
concurrency, 2014-07-11) must have made it unnecessary.  The commit
message for f702e62a 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: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NIgor Mammedov <imammedo@redhat.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NPaolo Bonzini <pbonzini@redhat.com>

When a serial port is started, its initial state is all zero.  Make
it consistent with reset state instead.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NPavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NKirill Batuzov <batuzovk@ispras.ru>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

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: NRoger 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: NPaolo Bonzini <pbonzini@redhat.com>

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: NJuan Quintela <quintela@redhat.com>
Acked-by: NAlexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>

The commit 88c1ee73
char/serial: Fix emptyness check

Still causes extra NULL byte(s) to be sent.

So if the fifo is empty, do not send an extra NULL byte.
Reviewed-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: NDon Slutz <dslutz@verizon.com>
Message-id: 1395160174-16006-1-git-send-email-dslutz@verizon.com
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

This was guarding against a full fifo rather than an empty fifo when
popping. Fix.
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: NMartin Kletzander <mkletzan@redhat.com>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

This is an autogenerated patch using scripts/switch-timer-api.

Switch the entire code base to using the new timer API.

Note this patch may introduce some line length issues.
Signed-off-by: NAlex Bligh <alex@alex.org.uk>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Convert stderr messages calling error_get_pretty()
to error_report().

Timestamp is prepended by -msg timstamp option with it.

Per Markus's comment below, A conversion from fprintf() to
error_report() is always an improvement, regardless of
error_get_pretty().

http://marc.info/?l=qemu-devel&m=137513283408601&w=2

But, it is not reasonable to convert them at one time
because fprintf() is used everwhere in qemu.

So, it should be done step by step with avoiding regression.
Signed-off-by: NSeiji Aguchi <seiji.aguchi@hds.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

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

Copy&paste error in serial.c causes a crash when attempting
to read from UART (if there is no data to be read)
Signed-off-by: NVladimir Senkov <hangup@gmail.com>
Reviewed-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

These three lines are common to both FIFO and regular mode. Just factor
them out to outside the if rather than replicate the same lines inside
both if and else.

Cc: qemu-trivial@nongnu.org
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Use the generic Fifo8 helper provided by QEMU, rather than re-implement
privately.
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Some cosmetic fixes to char/serial fixing some checkpatch errors.

Cc: qemu-trivial@nongnu.org
Signed-off-by: NPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Drop ISADeviceClass::init and the resulting no-op initfn and let
children implement their own realizefn. Adapt error handling.
Split off an instance_init where sensible.
Signed-off-by: NAndreas Färber <afaerber@suse.de>

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

This patch tackles all files that are compiled once, moving
them to subdirectories of hw/.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Many of these should be cleaned up with proper qdev-/QOM-ification.
Right now there are many catch-all headers in include/hw/ARCH depending
on cpu.h, and this makes it necessary to compile these files per-target.
However, fixing this does not belong in these patches.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

This fixes the build of hw/serial.c with DEBUG_SERIAL enabled.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>
Signed-off-by: NAmit Shah <amit.shah@redhat.com>
Message-id: 2976f10d4e66ed4a34011f6f0d6937026d22be5f.1362505276.git.amit.shah@redhat.com
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>