Commit Graph

184 Commits

Author SHA1 Message Date
Alexander Bulekov
f63192b054 hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
This protects devices from bh->mmio reentrancy issues.

Thanks: Thomas Huth <thuth@redhat.com> for diagnosing OS X test failure.
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20230427211013.2994127-5-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-04-28 11:31:54 +02:00
Arnout Engelen
f471e8b060 hw/usb/hcd-ehci: fix writeback order
The 'active' bit passes control over a qTD between the guest and the
controller: set to 1 by guest to enable execution by the controller,
and the controller sets it to '0' to hand back control to the guest.

ehci_state_writeback write two dwords to main memory using DMA:
the third dword of the qTD (containing dt, total bytes to transfer,
cpage, cerr and status) and the fourth dword of the qTD (containing
the offset).

This commit makes sure the fourth dword is written before the third,
avoiding a race condition where a new offset written into the qTD
by the guest after it observed the status going to go to '0' gets
overwritten by a 'late' DMA writeback of the previous offset.

This race condition could lead to 'cpage out of range (5)' errors,
and reproduced by:

./qemu-system-x86_64 -enable-kvm -bios $SEABIOS/bios.bin -m 4096 -device usb-ehci -blockdev driver=file,read-only=on,filename=/home/aengelen/Downloads/openSUSE-Tumbleweed-DVD-i586-Snapshot20220428-Media.iso,node-name=iso -device usb-storage,drive=iso,bootindex=0 -chardev pipe,id=shell,path=/tmp/pipe -device virtio-serial -device virtconsole,chardev=shell -device virtio-rng-pci -serial mon:stdio -nographic

(press a key, select 'Installation' (2), and accept the default
values. On my machine the 'cpage out of range' is reproduced while
loading the Linux Kernel about once per 7 attempts. With the fix in
this commit it no longer fails)

This problem was previously reported as a seabios problem in
https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/OUTHT5ISSQJGXPNTUPY3O5E5EPZJCHM3/
and as a nixos CI build failure in
https://github.com/NixOS/nixpkgs/issues/170803

Signed-off-by: Arnout Engelen <arnout@bzzt.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2022-06-14 10:34:36 +02:00
Philippe Mathieu-Daudé
ba06fe8add dma: Let dma_memory_read/write() take MemTxAttrs argument
Let devices specify transaction attributes when calling
dma_memory_read() or dma_memory_write().

Patch created mechanically using spatch with this script:

  @@
  expression E1, E2, E3, E4;
  @@
  (
  - dma_memory_read(E1, E2, E3, E4)
  + dma_memory_read(E1, E2, E3, E4, MEMTXATTRS_UNSPECIFIED)
  |
  - dma_memory_write(E1, E2, E3, E4)
  + dma_memory_write(E1, E2, E3, E4, MEMTXATTRS_UNSPECIFIED)
  )

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20211223115554.3155328-6-philmd@redhat.com>
2021-12-30 17:16:32 +01:00
Thomas Huth
db0b034185 hw/usb/hcd-ehci: Fix crash when showing help of EHCI devices
QEMU crashes with certain targets when trying to show the help
output of EHCI devices:

$ ./qemu-system-aarch64 -device ich9-usb-ehci1,help
qemu-system-aarch64: ../../devel/qemu/softmmu/physmem.c:1154: phys_section_add:
 Assertion `map->sections_nb < TARGET_PAGE_SIZE' failed.
Aborted (core dumped)

This happens because the device is doing things at "instance_init" time
that should be done at "realize" time instead. So move the related code
to the realize() function instead. (NB: This now also matches the
memory_region_del_subregion() calls which are done in usb_ehci_unrealize(),
and not during finalize()).

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210326095155.1994604-1-thuth@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2021-03-26 11:10:49 +01:00
Philippe Mathieu-Daudé
538f049704 sysemu: Let VMChangeStateHandler take boolean 'running' argument
The 'running' argument from VMChangeStateHandler does not require
other value than 0 / 1. Make it a plain boolean.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20210111152020.1422021-3-philmd@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2021-03-09 23:13:57 +01:00
Alex Chen
268c02424b hw/usb: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-id: 20201119025751.45750-1-alex.chen@huawei.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2021-01-22 14:51:35 +01:00
Peter Maydell
729cc68373 Remove superfluous timer_del() calls
This commit is the result of running the timer-del-timer-free.cocci
script on the whole source tree.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Corey Minyard <cminyard@mvista.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20201215154107.3255-4-peter.maydell@linaro.org
2021-01-08 15:13:38 +00:00
Anthony PERARD
ccee80c68d usb/hcd-ehci: Fix error handling on missing device for iTD
The EHCI Host Controller emulation attempt to locate the device
associated with a periodic isochronous transfer description (iTD) and
when this fail the host controller is reset.

But according the EHCI spec 1.0 section 5.15.2.4 Host System
Error, the host controller is supposed to reset itself only when it
failed to communicate with the Host (Operating System), like when
there's an error on the PCI bus. If a transaction fails, there's
nothing in the spec that say to reset the host controller.

This patch rework the error path so that the host controller can keep
working when the OS setup a bogus transaction, it also revert to the
behavior of the EHCI emulation to before commits:
e94682f1fe ("ehci: check device is not NULL before calling usb_ep_get()")
7011baece2 ("usb: remove unnecessary NULL device check from usb_ep_get()")

The issue has been found while trying to passthrough a USB device to a
Windows Server 2012 Xen guest via "usb-ehci", which prevent the USB
device from working in Windows. ("usb-ehci" alone works, windows only
setup this weird periodic iTD to device 127 endpoint 15 when the USB
device is passthrough.)

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Message-id: 20201014104106.2962640-1-anthony.perard@citrix.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2020-10-19 09:17:08 +02:00
Gerd Hoffmann
10b2d90c94 ehci: drop pointless warn_report for guest bugs.
We have a tracepoint at the same place which can be enabled if needed.

Buglink: https://bugzilla.redhat.com//show_bug.cgi?id=1859236
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200722072613.10390-1-kraxel@redhat.com>
2020-08-31 08:10:47 +02:00
Li Qiang
2fdb42d840 hw: ehci: check return value of 'usb_packet_map'
If 'usb_packet_map' fails, we should stop to process the usb
request.

Signed-off-by: Li Qiang <liq3ea@163.com>
Message-Id: <20200812161727.29412-1-liq3ea@163.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2020-08-31 08:10:47 +02:00
Li Qiang
dd8525472a hw: ehci: destroy sglist in error path
This may cause resource leak.

Signed-off-by: Li Qiang <liq3ea@163.com>
Message-Id: <20200812161712.29361-1-liq3ea@163.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2020-08-31 08:10:47 +02:00
Markus Armbruster
b69c3c21a5 qdev: Unrealize must not fail
Devices may have component devices and buses.

Device realization may fail.  Realization is recursive: a device's
realize() method realizes its components, and device_set_realized()
realizes its buses (which should in turn realize the devices on that
bus, except bus_set_realized() doesn't implement that, yet).

When realization of a component or bus fails, we need to roll back:
unrealize everything we realized so far.  If any of these unrealizes
failed, the device would be left in an inconsistent state.  Must not
happen.

device_set_realized() lets it happen: it ignores errors in the roll
back code starting at label child_realize_fail.

Since realization is recursive, unrealization must be recursive, too.
But how could a partly failed unrealize be rolled back?  We'd have to
re-realize, which can fail.  This design is fundamentally broken.

device_set_realized() does not roll back at all.  Instead, it keeps
unrealizing, ignoring further errors.

It can screw up even for a device with no buses: if the lone
dc->unrealize() fails, it still unregisters vmstate, and calls
listeners' unrealize() callback.

bus_set_realized() does not roll back either.  Instead, it stops
unrealizing.

Fortunately, no unrealize method can fail, as we'll see below.

To fix the design error, drop parameter @errp from all the unrealize
methods.

Any unrealize method that uses @errp now needs an update.  This leads
us to unrealize() methods that can fail.  Merely passing it to another
unrealize method cannot cause failure, though.  Here are the ones that
do other things with @errp:

* virtio_serial_device_unrealize()

  Fails when qbus_set_hotplug_handler() fails, but still does all the
  other work.  On failure, the device would stay realized with its
  resources completely gone.  Oops.  Can't happen, because
  qbus_set_hotplug_handler() can't actually fail here.  Pass
  &error_abort to qbus_set_hotplug_handler() instead.

* hw/ppc/spapr_drc.c's unrealize()

  Fails when object_property_del() fails, but all the other work is
  already done.  On failure, the device would stay realized with its
  vmstate registration gone.  Oops.  Can't happen, because
  object_property_del() can't actually fail here.  Pass &error_abort
  to object_property_del() instead.

* spapr_phb_unrealize()

  Fails and bails out when remove_drcs() fails, but other work is
  already done.  On failure, the device would stay realized with some
  of its resources gone.  Oops.  remove_drcs() fails only when
  chassis_from_bus()'s object_property_get_uint() fails, and it can't
  here.  Pass &error_abort to remove_drcs() instead.

Therefore, no unrealize method can fail before this patch.

device_set_realized()'s recursive unrealization via bus uses
object_property_set_bool().  Can't drop @errp there, so pass
&error_abort.

We similarly unrealize with object_property_set_bool() elsewhere,
always ignoring errors.  Pass &error_abort instead.

Several unrealize methods no longer handle errors from other unrealize
methods: virtio_9p_device_unrealize(),
virtio_input_device_unrealize(), scsi_qdev_unrealize(), ...
Much of the deleted error handling looks wrong anyway.

One unrealize methods no longer ignore such errors:
usb_ehci_pci_exit().

Several realize methods no longer ignore errors when rolling back:
v9fs_device_realize_common(), pci_qdev_unrealize(),
spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(),
virtio_device_realize().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200505152926.18877-17-armbru@redhat.com>
2020-05-15 07:08:14 +02:00
Chen Qun
e13a22db0d usb/hcd-ehci: Remove redundant statements
The "again" assignment is meaningless before g_assert_not_reached.
In addition, the break statements no longer needs to be after
g_assert_not_reached.

Clang static code analyzer show warning:
hw/usb/hcd-ehci.c:2108:13: warning: Value stored to 'again' is never read
            again = -1;
            ^       ~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200226084647.20636-13-kuhn.chenqun@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2020-03-09 11:12:55 +01:00
Gerd Hoffmann
1be344b7ad ehci: fix queue->dev null ptr dereference
In case we don't have a device for an active queue, just skip
processing the queue (same we do for inactive queues) and log
a guest bug.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-id: 20190821085319.13711-1-kraxel@redhat.com
2019-08-22 06:55:29 +02:00
Markus Armbruster
54d31236b9 sysemu: Split sysemu/runstate.h off sysemu/sysemu.h
sysemu/sysemu.h is a rather unfocused dumping ground for stuff related
to the system-emulator.  Evidence:

* It's included widely: in my "build everything" tree, changing
  sysemu/sysemu.h still triggers a recompile of some 1100 out of 6600
  objects (not counting tests and objects that don't depend on
  qemu/osdep.h, down from 5400 due to the previous two commits).

* It pulls in more than a dozen additional headers.

Split stuff related to run state management into its own header
sysemu/runstate.h.

Touching sysemu/sysemu.h now recompiles some 850 objects.  qemu/uuid.h
also drops from 1100 to 850, and qapi/qapi-types-run-state.h from 4400
to 4200.  Touching new sysemu/runstate.h recompiles some 500 objects.

Since I'm touching MAINTAINERS to add sysemu/runstate.h anyway, also
add qemu/main-loop.h.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190812052359.30071-30-armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
[Unbreak OS-X build]
2019-08-16 13:37:36 +02:00
Markus Armbruster
2f780b6a91 sysemu: Move the VMChangeStateEntry typedef to qemu/typedefs.h
In my "build everything" tree, changing sysemu/sysemu.h triggers a
recompile of some 1800 out of 6600 objects (not counting tests and
objects that don't depend on qemu/osdep.h, down from 5400 due to the
previous commit).

Several headers include sysemu/sysemu.h just to get typedef
VMChangeStateEntry.  Move it from sysemu/sysemu.h to qemu/typedefs.h.
Spell its structure tag the same while there.  Drop the now
superfluous includes of sysemu/sysemu.h from headers.

Touching sysemu/sysemu.h now recompiles some 1100 objects.
qemu/uuid.h also drops from 1800 to 1100, and
qapi/qapi-types-run-state.h from 5000 to 4400.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190812052359.30071-29-armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2019-08-16 13:31:53 +02:00
Markus Armbruster
db72581598 Include qemu/main-loop.h less
In my "build everything" tree, changing qemu/main-loop.h triggers a
recompile of some 5600 out of 6600 objects (not counting tests and
objects that don't depend on qemu/osdep.h).  It includes block/aio.h,
which in turn includes qemu/event_notifier.h, qemu/notify.h,
qemu/processor.h, qemu/qsp.h, qemu/queue.h, qemu/thread-posix.h,
qemu/thread.h, qemu/timer.h, and a few more.

Include qemu/main-loop.h only where it's needed.  Touching it now
recompiles only some 1700 objects.  For block/aio.h and
qemu/event_notifier.h, these numbers drop from 5600 to 2800.  For the
others, they shrink only slightly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190812052359.30071-21-armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2019-08-16 13:31:52 +02:00
Markus Armbruster
d645427057 Include migration/vmstate.h less
In my "build everything" tree, changing migration/vmstate.h triggers a
recompile of some 2700 out of 6600 objects (not counting tests and
objects that don't depend on qemu/osdep.h).

hw/hw.h supposedly includes it for convenience.  Several other headers
include it just to get VMStateDescription.  The previous commit made
that unnecessary.

Include migration/vmstate.h only where it's still needed.  Touching it
now recompiles only some 1600 objects.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20190812052359.30071-16-armbru@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2019-08-16 13:31:52 +02:00
Markus Armbruster
64552b6be4 Include hw/irq.h a lot less
In my "build everything" tree, changing hw/irq.h triggers a recompile
of some 5400 out of 6600 objects (not counting tests and objects that
don't depend on qemu/osdep.h).

hw/hw.h supposedly includes it for convenience.  Several other headers
include it just to get qemu_irq and.or qemu_irq_handler.

Move the qemu_irq and qemu_irq_handler typedefs from hw/irq.h to
qemu/typedefs.h, and then include hw/irq.h only where it's still
needed.  Touching it now recompiles only some 500 objects.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190812052359.30071-13-armbru@redhat.com>
2019-08-16 13:31:52 +02:00
Liam Merwick
e94682f1fe ehci: check device is not NULL before calling usb_ep_get()
In ehci_process_itd(), the call to ehci_find_device() can return NULL
if it doesn't find a device matching 'devaddr' so explicitly check
the return value before passing it to usb_ep_get().

Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
Message-id: 1549460216-25808-5-git-send-email-liam.merwick@oracle.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2019-02-20 09:41:23 +01:00
Thomas Huth
75a49fc61a hw/usb: Fix LGPL information in the file headers
It's either "GNU *Library* General Public version 2" or "GNU Lesser
General Public version *2.1*", but there was no "version 2.0" of the
"Lesser" library. So assume that version 2.1 is meant here.
Additionally, suggest that the user should have received a copy of
the LGPL, and not the GPL here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1548254454-7659-1-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2019-01-30 06:47:52 +01:00
Paolo Bonzini
eae3eb3e18 qemu/queue.h: simplify reverse access to QTAILQ
The new definition of QTAILQ does not require passing the headname,
remove it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-01-11 15:46:55 +01:00
Gerd Hoffmann
b7d3a7e1a8 ehci: fix fetch qtd race
The token field contains the (guest-filled) state of the qtd, which
indicates whenever the other fields are valid or not.  So make sure
we read the token first, otherwise we may end up with an stale next
pointer:

  (1) ehci reads next
  (2) guest writes next
  (3) guest writes token
  (4) ehci reads token
  (5) ehci operates with stale next.

Typical effect is that qemu doesn't notice that the guest appends new
qtds to the end of the queue.  Looks like the usb device stopped
responding.  Linux can recover from that, but leaves a message in the
kernel log that it did reset the usb device in question.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20181126100836.8805-1-kraxel@redhat.com
2018-12-10 15:30:18 +01:00
Sebastian Bauer
8bb01b257f ehci: Don't fetch a NULL current qtd but advance the queue instead.
Fetching qtd with the NULL address most likely makes no sense so from now
on, we handle it this case similarly as if the terminate (T) bit is not
set, which is already an exception as according to section 3.6 of the EHCI
spec there is no T bit defined for the current_qtd field.

The spec is a bit vague on how an EHCI driver should initialize these
fields: "The general operational model is that the host controller can
detect whether the overlay area contains a description of an active
transfer" (p. 49). QEMU primarily uses the QTD_TOKEN_ACTIVE bit of the
queue header to infer the activity state but there are other ways
conceivable.

This change allows QEMU to boot further into AmigaOS. The public available
version of the EHCI driver recycles queue heads in some rare conditions but
only clears the current_qtd field but not the status field. This works with
many available EHCI PCI cards but e.g., not with the Freescale USB
controller's found on the P5040. On the emulated EHCI controller of QEMU
the consequence is that some garbage was read in, which resulted in a
reset of the controller. This change fixes the problem.

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Tested-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-id: 20180625222718.4488-1-mail@sebastianbauer.info
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2018-07-03 09:49:44 +02:00
Dr. David Alan Gilbert
44b1ff319c migration: pre_save return int
Modify the pre_save method on VMStateDescription to return an int
rather than void so that it potentially can fail.

Changed zillions of devices to make them return 0; the only
case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already
had an error_report/return case.

Note: If you add an error exit in your pre_save you must emit
an error_report to say why.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170925112917.21340-2-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2017-09-27 11:35:59 +01:00
Alistair Francis
2ab4b13563 Convert single line fprintf(.../n) to warn_report()
Convert all the single line uses of fprintf(stderr, "warning:"..."\n"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using this command:
  find ./* -type f -exec sed -i \
    's|fprintf(.*".*warning[,:] \(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig' \
    {} +

Some of the lines were manually edited to reduce the line length to below
80 charecters.

The #include lines were manually updated to allow the code to compile.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: James Hogan <james.hogan@imgtec.com> [mips]
Message-Id: <ae8f8a7f0a88ded61743dff2adade21f8122a9e7.1505158760.git.alistair.francis@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-09-19 14:09:34 +02:00
Gerd Hoffmann
2a7f263068 ehci: add sanity check for maxframes
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20170703111549.10924-1-kraxel@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2017-07-17 11:39:08 +02:00
Gerd Hoffmann
ad3c5412f2 ehci: stop recursive calls to ehci_work_bh
Can happen with usb-storage devices: ehci_work_bh calls usb-storage,
usb-storage calls into block layer, block layer may run BHs.

Add a simple bool and just do nothing in case we figure ehci_work_bh is
active.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170612073109.25930-1-kraxel@redhat.com
2017-06-13 12:17:33 +02:00
Gerd Hoffmann
3bfecee2cb ehci: fix frame timer invocation.
ehci registers ehci_frame_timer as both timer and bottom half, which
turned out to be a bad idea as it can be called as bottom half then
while it is running as timer, and it isn't prepared to handle recursive
calls.

Change the timer func to just schedule the bottom half to avoid this.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1449609
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170519120428.25981-1-kraxel@redhat.com
2017-05-29 14:19:16 +02:00
Gerd Hoffmann
3ae7eb88c4 ehci: fix overflow in frame timer code
In case the frame timer doesn't run for a while due to the host being
busy skipped_uframes can become big enough that UFRAME_TIMER_NS *
skipped_uframes overflows.  Which in turn throws off all subsequent
ehci frame timer calculations.

Reported-by: 李林 <8610_28@163.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20170515104543.32044-1-kraxel@redhat.com
2017-05-29 14:03:35 +02:00
Li Qiang
d710e1e7bd usb: ehci: fix memory leak in ehci
In usb_ehci_init function, it initializes 's->ipacket', but there
is no corresponding function to free this. As the ehci can be hotplug
and unplug, this will leak host memory leak. In order to make the
hierarchy clean, we should add a ehci pci finalize function, then call
the clean function in ehci device.

Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Message-id: 589a85b8.3c2b9d0a.b8e6.1434@mx.google.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2017-02-21 08:11:42 +01:00
Li Qiang
791f97758e usb: ehci: fix memory leak in ehci_init_transfer
In ehci_init_transfer function, if the 'cpage' is bigger than 4,
it doesn't free the 'p->sgl' once allocated previously thus leading
a memory leak issue. This patch avoid this.

Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Message-id: 5821c0f4.091c6b0a.e0c92.e811@mx.google.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-11-10 15:29:58 +00:00
Li Qiang
b16c129daf usb: ehci: fix memory leak in ehci_process_itd
While processing isochronous transfer descriptors(iTD), if the page
select(PG) field value is out of bands it will return. In this
situation the ehci's sg list is not freed thus leading to a memory
leak issue. This patch avoid this.

Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2016-10-08 11:25:29 +03:00
Evgeny Yakovlev
72aa364b1d ehci: faster frame index calculation for skipped frames
ehci_update_frindex takes time linearly proportional to a number
of uframes to calculate new frame index and raise FLR interrupts,
which is a problem for large amounts of uframes.

If we experience large delays between echi timer callbacks (i.e. because
other periodic handlers have taken a lot of time to complete) we
get a lot of skipped frames which then delay ehci timer callback more
and this leads to deadlocking the system when ehci schedules next
callback to be too soon.

Observable behaviour is qemu consuming 100% host CPU time while guest
is unresponsive. This misbehavior could happen for a while and QEMU does
not get out from this state automatically without the patch.

This change makes ehci_update_frindex execute in constant time.

Signed-off-by: Evgeny Yakovlev <eyakovlev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-id: 1469638520-32706-1-git-send-email-den@openvz.org
CC: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-08-02 13:35:24 +02:00
Gerd Hoffmann
a49923d283 Revert "ehci: make idt processing more robust"
This reverts commit 156a2e4dbf.

Breaks FreeBSD.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-04-19 08:20:56 +02:00
Gerd Hoffmann
1ae3f2f178 ehci: apply limit to iTD/sidt descriptors
Commit "156a2e4 ehci: make idt processing more robust" tries to avoid a
DoS by the guest (create a circular iTD queue and let qemu ehci
emulation run in circles forever).  Unfortunately this has two problems:
First it misses the case of siTDs, and second it reportedly breaks
FreeBSD.

So lets go for a different approach: just count the number of iTDs and
siTDs we have seen per frame and apply a limit.  That should really
catch all cases now.

Reported-by: 杜少博 <dushaobo@360.cn>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-04-19 08:18:27 +02:00
Peter Maydell
84a5a80148 * Log filtering from Alex and Peter
* Chardev fix from Marc-André
 * config.status tweak from David
 * Header file tweaks from Markus, myself and Veronia (Outreachy candidate)
 * get_ticks_per_sec() removal from Rutuja (Outreachy candidate)
 * Coverity fix from myself
 * PKE implementation from myself, based on rth's XSAVE support
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2
 
 iQEcBAABCAAGBQJW9ErPAAoJEL/70l94x66DJfEH/A/QkMpAhrgNdyVsahzsGrzE
 wx5gHFIc1nBYxyr62w4apUb5jPB7zaXu0LA7EAWDeAe0pyP8hZzLT9kJyOEDsuJu
 zwKN2QeLSNMtPbnbKN0I/YQ2za2xX1V5ruhSeOJoVslUI214hgnAURaGshhQNzuZ
 2CluDT9KgL5cQifAnKs5kJrwhIYShYNQB+1eDC/7wk28dd/EH+sPALIoF+rqrSmt
 Zu4Mdqd+9Ns+oKOjA6br9ULq/Hzg0aDfY82J+XLVVqfF3PXQe8rTDmuMf/7jTn+M
 Un7ZOcei9oZF2/9vfAfKQpDCcgD9HvOUSbgqV/ubmkPPmN/LNJzeKj0fBhrRN+Y=
 =K12D
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging

* Log filtering from Alex and Peter
* Chardev fix from Marc-André
* config.status tweak from David
* Header file tweaks from Markus, myself and Veronia (Outreachy candidate)
* get_ticks_per_sec() removal from Rutuja (Outreachy candidate)
* Coverity fix from myself
* PKE implementation from myself, based on rth's XSAVE support

# gpg: Signature made Thu 24 Mar 2016 20:15:11 GMT using RSA key ID 78C7AE83
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>"
# gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>"

* remotes/bonzini/tags/for-upstream: (28 commits)
  target-i386: implement PKE for TCG
  config.status: Pass extra parameters
  char: translate from QIOChannel error to errno
  exec: fix error handling in file_ram_alloc
  cputlb: modernise the debug support
  qemu-log: support simple pid substitution for logs
  target-arm: dfilter support for in_asm
  qemu-log: dfilter-ise exec, out_asm, op and opt_op
  qemu-log: new option -dfilter to limit output
  qemu-log: Improve the "exec" TB execution logging
  qemu-log: Avoid function call for disabled qemu_log_mask logging
  qemu-log: correct help text for -d cpu
  tcg: pass down TranslationBlock to tcg_code_gen
  util: move declarations out of qemu-common.h
  Replaced get_tick_per_sec() by NANOSECONDS_PER_SECOND
  hw: explicitly include qemu-common.h and cpu.h
  include/crypto: Include qapi-types.h or qemu/bswap.h instead of qemu-common.h
  isa: Move DMA_transfer_handler from qemu-common.h to hw/isa/isa.h
  Move ParallelIOArg from qemu-common.h to sysemu/char.h
  Move QEMU_ALIGN_*() from qemu-common.h to qemu/osdep.h
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Conflicts:
	scripts/clean-includes
2016-03-24 21:42:40 +00:00
Rutuja Shah
73bcb24d93 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>
2016-03-22 22:20:17 +01:00
Markus Armbruster
da34e65cb4 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>
2016-03-22 22:20:15 +01:00
Prasad J Pandit
dff0367cf6 usb: ehci: add capability mmio write function
USB Ehci emulation supports host controller capability registers.
But its mmio '.write' function was missing, which lead to a null
pointer dereference issue. Add a do nothing 'ehci_caps_write'
definition to avoid it; Do nothing because capability registers
are Read Only(RO).

Reported-by: Zuozhi Fzz <zuozhi.fzz@alibaba-inc.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 1454072434-16045-1-git-send-email-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-03-18 14:20:39 +01:00
Gerd Hoffmann
5a8660741a ehci: update irq on reset
After clearing the status register we also have to update the irq line
status.  Otherwise a irq which happends to be pending at reset time
causes a interrupt storm.  And the guest can't stop as the status
register doesn't indicate any pending interrupt.

Both NetBSD and FreeBSD hang on shutdown because of that.

Cc: qemu-stable@nongnu.org
Reported-by: Andrey Korolyov <andrey@xdel.ru>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1453203884-4125-1-git-send-email-kraxel@redhat.com
2016-02-02 14:11:01 +01:00
Prasad J Pandit
49d925ce50 usb: check page select value while processing iTD
While processing isochronous transfer descriptors(iTD), the page
select(PG) field value could lead to an OOB read access. Add
check to avoid it.

Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 1453233406-12165-1-git-send-email-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2016-02-02 14:11:01 +01:00
Peter Maydell
e532b2e008 usb: 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-20-git-send-email-peter.maydell@linaro.org
2016-01-29 15:07:23 +00:00
Gerd Hoffmann
156a2e4dbf ehci: make idt processing more robust
Make ehci_process_itd return an error in case we didn't do any actual
iso transfer because we've found no active transaction.  That'll avoid
ehci happily run in circles forever if the guest builds a loop out of
idts.

This is CVE-2015-8558.

Cc: qemu-stable@nongnu.org
Reported-by: Qinghao Tang <luodalongde@gmail.com>
Tested-by: P J P <ppandit@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2015-12-15 09:49:03 +01:00
Gerd Hoffmann
cbf82fa01e ehci: clear suspend bit on detach
When a device is detached, clear the suspend bit (PORTSC_SUSPEND)
in the port status register.

The specs are not *that* clear what is supposed to happen in case
a suspended device is unplugged.  But the enable bit (PORTSC_PED)
is cleared, and the specs mention setting suspend with enable being
unset is undefined behavior.  So clearing them both looks reasonable,
and it actually fixes the reported bug.

https://bugzilla.redhat.com/show_bug.cgi?id=1268879

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Message-id: 1445413462-18004-1-git-send-email-kraxel@redhat.com
2015-11-03 11:55:51 +01:00
Stefan Hajnoczi
13566fe3e5 timer: rename NSEC_PER_SEC due to Mac OS X header clash
Commit e0cf11f31c ("timer: Use a single
definition of NSEC_PER_SEC for the whole codebase") renamed
NANOSECONDS_PER_SECOND to NSEC_PER_SEC.

On Mac OS X there is a <dispatch/time.h> system header which also
defines NSEC_PER_SEC.  This causes compiler warnings.

Let's use the old name instead.  It's longer but it doesn't clash.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1436364609-7929-1-git-send-email-stefanha@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-20 17:01:00 +01:00
Alberto Garcia
e0cf11f31c timer: Use a single definition of NSEC_PER_SEC for the whole codebase
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: c6e55468856ba0b8f95913c4da111cc0ef266541.1434113783.git.berto@igalia.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-02 09:20:18 +01:00
Gonglei
4e289b1b62 ehci: fix segfault when hot-unplugging ehci controller
When hot-unplugging the usb controllers (ehci/uhci),
we have to clean all resouce of these devices,
involved registered reset handler. Otherwise, it
may cause NULL pointer access and/or segmentation fault
if we reboot the guest os after hot-unplugging.

Let's hook up reset via DeviceClass->reset() and drop
the qemu_register_reset() call. Then Qemu will register
and unregister the reset handler automatically.

Cc: qemu-stable <qemu-stable@nongnu.org>
Reported-by: Lidonglin <lidonglin@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2015-03-20 08:50:12 +01:00
Markus Armbruster
2e269f3d9d usb: Improve companion configuration error messages
The previous commit broke the additional messages explaining the error
messages.  Improve the error messages, so they don't need explaining
so much.  Helps QMP users as well, unlike additional explanations.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2015-03-17 14:11:43 +01:00
Markus Armbruster
f4bbaaf584 usb: Propagate errors through usb_register_companion()
This loses the messages explaining the error printed with
error_printf_unless_qmp().  The next commit will make up for the loss.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2015-03-17 14:11:42 +01:00