This creates a new test group 'quick' for some test case that take at
most a couple of seconds each, so that the group can be run during a
quick 'make check'
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
qemu-img resize has some limitations with qcow2, but the user is only
told that "this image format does not support resize". Quite confusing,
so add some more detailed error_report() calls and change "this image
format" into "this image".
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Monitor operations that manipulate image files must not execute while a
background job (like image streaming) is in progress. This prevents
corruptions from happening when two pieces of code are manipulating the
image file without knowledge of each other.
The monitor "commit" command raises QERR_DEVICE_IN_USE when
bdrv_commit() returns -EBUSY but "commit all" has no error handling.
This is easy to fix, although note that we do not deliver a detailed
error about which device was busy in the "commit all" case.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The L2 table cache reduces QED metadata reads that would be required
when translating LBAs to offsets into the image file. Since requests
execute in parallel it is possible to share an L2 table between multiple
requests.
There is a potential data corruption issue when an in-use L2 table is
evicted from the cache because the following situation occurs:
1. An allocating write performs an update to L2 table "A".
2. Another request needs L2 table "B" and causes table "A" to be
evicted.
3. A new read request needs L2 table "A" but it is not cached.
As a result the L2 update from #1 can overlap with the L2 fetch from #3.
We must avoid doing overlapping I/O requests here since the worst case
outcome is that the L2 fetch completes before the L2 update and yields
stale data. In that case we would effectively discard the L2 update and
lose data clusters!
Thanks to Benoît Canet <benoit.canet@gmail.com> for extensive testing
and debugging which lead to discovery of this bug.
Reported-by: Benoît Canet <benoit.canet@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Tested-by: Benoît Canet <benoit.canet@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Most MemoryRegionOps already had the const attribute.
This patch adds it to the remaining ones.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
tcg_out_label is always called with a third argument of pointer type
which was casted to tcg_target_long.
These casts can be avoided by changing the prototype of tcg_out_label.
There was also a cast to long. For most hosts with
sizeof(long) == sizeof(tcg_target_long) == sizeof(void *) this did not
matter, but for w64 it was wrong. This is fixed now.
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
MinGW-w64 and some versions of MinGW32 don't provide libiberty.a,
so add this library only if it was found.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
MinGW-w64 already defines lseek and ftruncate (and uses the 64 bit
variants). The conditional compilation avoids redefinitions
(which would be wrong) and compiler warnings.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Commit 021ecd8b9d breaks the build for
PPC hosts because it uses uintptr_t without the necessary include file.
uintptr_t is defined in stdint.h, so add this include.
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Current code depends on variables defined in config-host.mak before it is
actually included.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paul Brook <paul@codesourcery.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Too many VM kittens were killed since 7d03f82f81. Another one just died
under my fat fingers.
When you quit a kgdb session, does the Linux kernel power off? Or when
you terminate gdb attached to a hardware debugger, does your board
vanish in space? No.
So let's stop terminating QEMU when the gdbstub receives a kill commando
in system emulation mode. Real termination can still be achieved via
"monitor quit". We keep the behavior for user mode emulation which is
arguably more like a gdbserver scenario.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
This was a long pending bug, now revealed by the assert in
phys_page_find that stumbled over the large page index returned by
cpu_get_phys_page_debug for NX-marked pages: We need to mask out NX and
all user-definable bits 52..62 from PDEs and the final PTE to avoid
corrupting physical addresses.
Reviewed-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
* stefanha/trivial-patches:
configure: Quote the configure args printed in config.log
osdep: Remove local definition of macro offsetof
libcacard: Spelling and grammar fixes in documentation
Spelling fixes in comments (it's -> its)
vnc: Add break statement
libcacard: Use format specifier %u instead of %d for unsigned values
Fix sign of sscanf format specifiers
block/vmdk: Fix warning from splint (comparision of unsigned value)
qmp: Fix spelling fourty -> forty
qom: Fix spelling in documentation
sh7750: Remove redundant 'struct' from MemoryRegionOps
* qemu-kvm/uq/master:
kvm: fill in padding to help valgrind
kvm: x86: Add user space part for in-kernel i8254
kvm: Add kvm_has_pit_state2 helper
i8254: Open-code timer restore
i8254: Factor out base class for KVM reuse
* kraxel/usb.42:
xhci: fix port status
xhci: fix control xfers
usb: add shortcut for control transfers
usb-host: enable pipelineing for bulk endpoints.
usb: add pipelining option to usb endpoints
usb: queue can have async packets
uhci_fill_queue: zap debug printf
usb: add USB_RET_IOERROR
usb: return BABBLE rather then NAK when we receive too much data
usb-ehci: Cleanup itd error handling
usb-ehci: Fix and simplify nakcnt handling
usb-ehci: Remove dead nakcnt code
usb-ehci: Fix cerr tracking
usb-ehci: Any packet completion except for NAK should set the interrupt
usb-ehci: Rip the queues when the async or period schedule is halted
usb-ehci: Drop cached qhs when the doorbell gets rung
usb-ehci: always call ehci_queues_rip_unused for period queues
usb-ehci: split our qh queue into async and periodic queues
usb-ehci: Never follow table entries with the T-bit set
usb-redir: Set ep type and interface
VCARD_ATR_PREFIX is used as part of an array initializer so it should
not have () around it, so far this happened to work, but gcc-4.7 does
not like it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Use the same mechanism we use for printing the configure command
line to config-host.mak to print it to config.log. This fixes a
bug where the config.log version didn't quote arguments with spaces.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
The macro offsetof is defined in stddef.h. It is conforming to
the standards C89, C99 and POSIX.1-2001 (see man page), so it
is a sufficiently old standard.
Therefore chances are very high that QEMU never needs a local
definition of this macro.
osdep.h already includes stddef.h, so this patch simply removes
the unneeded code from the files configure and osdep.h.
If we ever need the local definition again, it should be added
to compiler.h (the macro is usually provided with the compiler,
it is not OS specific).
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
* it's -> its
* it's -> it is (that's no fix, but makes future checks easier)
* this functions -> this function
* replacable -> replaceable
* reader's -> readers
* logins into -> logs into
v2:
Also replace 'aid' by 'AID' (thanks to Peter Maydell for this hint).
v3:
Fix sentence (contributed by Alon Levy / Robert Relyea).
Cc: Alon Levy <alevy@redhat.com>
Cc: Robert Relyea <rrelyea@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
* it's -> its (fixed for all files)
* dont -> don't (only fixed in a line which was touched by the previous fix)
* distrub -> disturb (fixed in the same line)
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
This was not a bug, but it is not common practice to omit the break statement
from the last case statement before an empty default case.
Any change of the default case would introduce a bug.
This was reported as a warning by splint.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
splint reported warnings for those code statements.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
All values read by sscanf are unsigned, so replace %d by %u.
This signed / unsigned mismatch was detected by splint.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
This was found by codespell.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
This fixes a new spelling issue which was detected by codespell.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
The 'struct' is not needed, and all other MemoryRegionOps don't use it.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Don't signal port status change if the usb device isn't in attached
state. Happens with usb-host devices with the pass-through device
being plugged out at the host.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Use the new, direct control transfer submission method instead of
bypassing the usb core by calling usb_device_handle_control directly.
The later fails for async control transfers.
This patch gets xhci + usb-host combo going.
Add a more direct code path to submit control transfers. Instead of
feeding three usb packets (setup, data, ack) to usb_handle_packet and
have the do_token_* functions in usb.c poke the control transfer
parameters out of it just submit a single packet carrying the actual
data with the control xfer parameters filled into USBPacket->parameters.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
We really don't want to wait for packets finish before submitting the
next, we want keep the data flow running.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
With this patch applied USB drivers can enable pipelining per endpoint.
With pipelining enabled the usb core will continue submitting packets
even when there are still async transfers in flight instead of passing
them on one by one.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This can happen today in case the ->complete() callback queues up the
next packet. Also we'll support pipelining soon, which allows to have
multiple packets per queue in flight (aka ASYNC) state.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
We already have USB_RET_NAK, but that means that a device does not want
to send/receive right now. But with host / network redirection we can
actually have a transaction fail due to some io error, rather then ie
the device just not having any data atm.
This patch adds a new error code named USB_RET_IOERROR for this, and uses
it were appropriate.
Notes:
-Currently all usb-controllers handle this the same as NODEV, but that
may change in the future, OHCI could indicate a CRC error instead for example.
-This patch does not touch hw/usb-musb.c, that is because the code in there
handles STALL and NAK specially and has a if status < 0 generic catch all
for all other errors
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
All error statuses except for NAK are handled in a switch case, move the
handling of NAK into the same switch case.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The nakcnt code in ehci_execute_complete() marked transactions as finished
when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK
means that the device cannot receive / send data at that time and that
the transaction should be retried later, which is also what the usb-uhci
and usb-ohci code does.
Note that there already was some special code in place to handle this
for interrupt endpoints in the form of doing a return from
ehci_execute_complete() when reload == 0, but that for bulk transactions
this was not handled correctly (where as for example the usb-ccid device does
return USB_RET_NAK for bulk packets).
Besides that the code in ehci_execute_complete() decrement nakcnt by 1
on a packet result of USB_RET_NAK, but
-since the transaction got marked as finished,
nakcnt would never be decremented again
-there is no code checking for nakcnt becoming 0
-there is no use in re-trying the transaction within the same usb frame /
usb-ehci frame-timer call, since the status of emulated devices won't change
as long as the usb-ehci frame-timer is running
So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus
claiming that we've tried reload times (or as many times as possible if
reload is 0).
Besides the code in ehci_execute_complete() handling USB_RET_NAK there
was also code handling it in ehci_state_executing(), which calls
ehci_execute_complete(), and then does its own handling on top of the handling
in ehci_execute_complete(), this code would decrement nakcnt *again* (if not
already 0), or restore the reload value (which was never changed) on success.
Since the double decrement was wrong to begin with, and is no longer needed
now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload
is not needed either, this patch simply removes all nakcnt handling from
ehci_state_executing().
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This patch removes 2 bits of dead nakcnt code:
1) usb_ehci_execute calls ehci_qh_do_overlay which does:
nakcnt = reload;
and then has a block of code which is conditional on:
if (reload && !nakcnt) {
which ofcourse is never true now as nakcnt == reload.
2) ehci_state_fetchqh does:
nakcnt = reload;
but before nakcnt is ever used ehci_state_fetchqh is always followed
by a ehci_qh_do_overlay call which also does:
nakcnt = reload;
So doing this from ehci_state_fetchqh is redundant.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
cerr should only be decremented on errors which cause XactErr to be set, and
when that happens the failing transaction should be retried until cerr reaches
0 and only then should USBSTS_ERRINT be set (and inactive cleared and
USBSTS_INT set if requested).
Since we don't have any hardware level errors (and in case of redirection
the real hardware has already retried), re-trying makes no sense, so
immediately set cerr to 0 on errors which set XactErr.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
As clearly stated in the 2.3.2 of the EHCI spec, any time USBERRINT get
sets then if the td has its IOC bit set USBINT should be set as well.
This means that for any status except for USB_RET_NAK we should set
USBINT if the IOC bit is set.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The purpose of the IAAD bit / the doorbell is to make the ehci controller
forget about cached qhs, this is mainly used when cancelling transactions,
the qh is unlinked from the async schedule and then the doorbell gets rung,
once the doorbell is acked by the controller the hcd knows that the qh is
no longer in use and that it can do something else with the memory, such
as re-use it for a new qh! But we keep our struct representing this qh around
for circa 250 ms. This allows for a (mightily large) race window where the
following could happen:
-hcd submits a qh at address 0xdeadbeef
-our ehci code sees the qh, sends a request to a usb-device, gets a result
of USB_RET_ASYNC, sets the async_state of the qh to EHCI_ASYNC_INFLIGHT
-hcd unlinks the qh at address 0xdeadbeef
-hcd rings the doorbell, wait for us to ack it
-hcd re-uses the qh at address 0xdeadbeef
-our ehci code sees the qh, looks in the async_queue, sees there already is
a qh at address 0xdeadbeef there with async_state of EHCI_ASYNC_INFLIGHT,
does nothing
-the *original* (which the hcd thinks it has cancelled) transaction finishes
-our ehci code sees the qh on yet another pass through the async list,
looks in the async_queue, sees there already is a qh at address 0xdeadbeef
there with async_state of EHCI_ASYNC_COMPLETED, and finished the transaction
with the results of the *original* transaction.
Not good (tm), this patch fixes this race by removing all qhs which have not
been seen during the last cycle through the async list immidiately when the
doorbell is rung.
Note this patch does not fix any actually observed problem, but upon
reading of the EHCI spec it became apparent to me that the above race could
happen and the usb-ehci behavior from before this patch is not good.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
