An abort happens in ohci_frame_boundary() when ohci->done is 0 [1].
``` c
static void ohci_frame_boundary(void *opaque)
{
// ...
if (ohci->done_count == 0 && !(ohci->intr_status & OHCI_INTR_WD)) {
if (!ohci->done)
abort(); <----------------------------------------- [1]
```
This was reported in https://bugs.launchpad.net/qemu/+bug/1911216/,
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg03613.html, and
https://gitlab.com/qemu-project/qemu/-/issues/545. I can still reproduce it with
the latest QEMU.
This happends due to crafted ED with putting ISO_TD at physical address 0.
Suppose ed->head & OHCI_DPTR_MASK is 0 [2], and we memset 0 to the phyiscal
memory from 0 to sizeof(ohci_iso_td). Then, starting_frame [3] and frame_count
[4] are both 0. As we can control the value of ohci->frame_number (0 to 0x1f,
suppose 1), we then control the value of relative_frame_number to be 1 [6]. The
control flow goes to [7] where ohci->done is 0. Have returned from
ohci_service_iso_td(), ohci_frame_boundary() will abort() [1].
``` c
static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
{
// ...
addr = ed->head & OHCI_DPTR_MASK; // <--------------------- [2]
if (ohci_read_iso_td(ohci, addr, &iso_td)) { // <-------- [3]
// ...
starting_frame = OHCI_BM(iso_td.flags, TD_SF); // <-------- [4]
frame_count = OHCI_BM(iso_td.flags, TD_FC); // <-------- [5]
relative_frame_number = USUB(ohci->frame_number, starting_frame);
// <-------- [6]
if (relative_frame_number < 0) {
return 1;
} else if (relative_frame_number > frame_count) {
// ...
ohci->done = addr; // <-------- [7]
// ...
}
```
As only (afaik) a guest root user can manipulate ED, TD and the physical memory,
this assertion failure is not a security bug.
The idea to fix this issue is to drop ohci_service_iso_td() if ed->head &
OHCI_DPTR_MASK is 0, which is similar to the drop operation for
ohci_service_ed_list() when head is 0. Probably, a similar issue is in
ohci_service_td(). I drop ohci_service_td() if ed->head & OHCI_DPTR_MASK is 0.
Fixes: 7bfe577702 ("OHCI USB isochronous transfers support (Arnon Gilboa)")
Reported-by: Gaoning Pan <pgn@zju.edu.cn>
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Qiang Liu <cyruscyliu@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/545
Buglink: https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg03613.html
Buglink: https://bugs.launchpad.net/qemu/+bug/1911216
Signed-off-by: Qiang Liu <cyruscyliu@gmail.com>
Message-Id: <20220826051557.119570-1-cyruscyliu@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The compiler isn't clever enough to figure 'width' is a constant,
so help it by using a definitions instead.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20220819153931.3147384-10-peter.maydell@linaro.org
Since isochronous transfers cannot be handled async (the function
returns error in that case) we don't need to remember the packet.
Avoid using the usb_packet field in OHCIState (as that can be a
waiting async packet on another endpoint) and allocate and use a local
USBPacket for the iso transfer instead. After this we don't have to
care if we're called from a completion callback or not so we can drop
that parameter as well.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-Id: <bf523d40f8088a84383cb00ffd2e6e82fa47790d.1643117600.git.balaton@eik.bme.hu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
These two do the same and only used once so no need to have two
functions, simplify by merging them.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <5fc8ba0bbf55703014d22dd06ab2f9eabaf370bf.1643117600.git.balaton@eik.bme.hu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This is always done before calling this function so remove duplicated
code and do it within the function at one place.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-Id: <ce766722506bfd7145cccbec750692ff57072280.1643117600.git.balaton@eik.bme.hu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Let devices specify transaction attributes when calling
dma_memory_rw().
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-5-philmd@redhat.com>
An assorted set of spelling fixes in various places.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210309111510.79495-1-mjt@msgid.tls.msk.ru>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
There is no "version 2" of the "Lesser" General Public License.
It is either "GPL version 2.0" or "Lesser GPL version 2.1".
This patch replaces all occurrences of "Lesser GPL version 2" with
"Lesser GPL version 2.1" in comment section.
Signed-off-by: Chetan Pant <chetan4windows@gmail.com>
Message-Id: <20201023122332.19369-1-chetan4windows@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
While servicing OHCI transfer descriptors(TD), ohci_service_iso_td
retires a TD if it has passed its time frame. It does not check if
the TD was already processed once and holds an error code in TD_CC.
It may happen if the TD list has a loop. Add check to avoid an
infinite loop condition.
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Message-id: 20200915182259.68522-3-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
We need to be able to use OHCISysBusState outside hcd-ohci.c, so move it
to its include file.
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Message-id: 20200217204812.9857-2-linux@roeck-us.net
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In my "build everything" tree, changing hw/qdev-properties.h triggers
a recompile of some 2700 out of 6600 objects (not counting tests and
objects that don't depend on qemu/osdep.h).
Many places including hw/qdev-properties.h (directly or via hw/qdev.h)
actually need only hw/qdev-core.h. Include hw/qdev-core.h there
instead.
hw/qdev.h is actually pointless: all it does is include hw/qdev-core.h
and hw/qdev-properties.h, which in turn includes hw/qdev-core.h.
Replace the remaining uses of hw/qdev.h by hw/qdev-properties.h.
While there, delete a few superfluous inclusions of hw/qdev-core.h.
Touching hw/qdev-properties.h now recompiles some 1200 objects.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <20190812052359.30071-22-armbru@redhat.com>
In my "build everything" tree, changing hw/hw.h triggers a recompile
of some 2600 out of 6600 objects (not counting tests and objects that
don't depend on qemu/osdep.h).
The previous commits have left only the declaration of hw_error() in
hw/hw.h. This permits dropping most of its inclusions. Touching it
now recompiles less than 200 objects.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20190812052359.30071-19-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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>
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>
Some machines (like the pxa2xx-based ARM machines) only have a sysbus
OHCI controller, but no PCI. With the new Kconfig-style build system,
it will soon be possible to create QEMU binaries that only contain
such PCI-less machines. However, the two OHCI controllers, for sysbus
and for PCI, are currently both located in one file, so the PCI code
is still required for linking here. Move the OHCI-PCI device code
into a separate file, so that it is possible to use the sysbus OHCI
device also without the PCI dependency.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-3-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The ohci_die() function always assumes to be running with a PCI OHCI
controller and calls the PCI-specific functions pci_set_word(). However,
this function might also get called for the sysbus OHCI devices, so it
likely fails in that case. To fix this issue, change the code now, so that
there are two implementations now, one for sysbus and one for PCI, and
use the right function via a function pointer in the OHCIState structure.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-2-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
A call to ohci_find_device() can return NULL if it doesn't find a
device matching 'addr' so for the two callers, 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-6-git-send-email-liam.merwick@oracle.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This can avoid setting OCHIState.num_ports to a negative num.
Signed-off-by: Li Qiang <liq3ea@gmail.com>
Message-id: 1540263618-18344-1-git-send-email-liq3ea@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
USB frame rate is slightly lower than 1kHz: ie. ~950Hz.
Thus usb-audio device is not able to perform a simple audio playback
without underruns on audio backend.
eg. "-device pci-ohci,id=ohci -device usb-audio,bus=ohci.0" vs PulseAudio
backend. more than 50 underruns are observed per second.
Update ohci_sof_time computation, using QEMU_CLOCK_VIRTUAL in
ohci_usb_start(), and increment by usb_frame_time in ohci_sof()
makes USB frame rate close to 1kHz.
This way, no audio underrun are observed during audio playback.
Signed-off-by: Miguel GAIO <mgaio35@gmail.com>
Message-Id: <20180927151936.3647-1-mgaio35@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This is mandated by the ohci specification. It tells at 6.4.4 on page 104
that for transfer descriptors that are retired with an error the done
queue interrupt counter is cleared as if the interrupt delay field of the
descriptions were zero.
Before this change, error conditions were handled similarly to the
successful condition which is especially troublesome for control transfers.
Some drivers (e.g., the AmigaOS-one) as well as the example code in the
spec, set the setup stage with an interrupt delay of seven (which means no
interrupt). This is fine under normal conditions, because one usually
doesn't want to be notified about the completion of this stage. However, if
an error occurs in this stage, these drivers will not get notified with the
current implementation. The fix addresses this by following the spec more
closely. Also, otherwise, the ability to set interrupt delay to seven would
be useless.
Note that Linux drivers that I looked at don't seem to be affected as they
set six as the interrupt delay presumably for the reason that they won't
get notified otherwise.
Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Message-id: 20180729191928.11254-1-mail@sebastianbauer.info
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Add INTERFACE_CONVENTIONAL_PCI_DEVICE to all direct subtypes of
TYPE_PCI_DEVICE, except:
1) The ones that already have INTERFACE_PCIE_DEVICE set:
* base-xhci
* e1000e
* nvme
* pvscsi
* vfio-pci
* virtio-pci
* vmxnet3
2) base-pci-bridge
Not all PCI bridges are Conventional PCI devices, so
INTERFACE_CONVENTIONAL_PCI_DEVICE is added only to the subtypes
that are actually Conventional PCI:
* dec-21154-p2p-bridge
* i82801b11-bridge
* pbm-bridge
* pci-bridge
The direct subtypes of base-pci-bridge not touched by this patch
are:
* xilinx-pcie-root: Already marked as PCIe-only.
* pcie-pci-bridge: Already marked as PCIe-only.
* pcie-port: all non-abstract subtypes of pcie-port are already
marked as PCIe-only devices.
3) megasas-base
Not all megasas devices are Conventional PCI devices, so the
interface names are added to the subclasses registered by
megasas_register_types(), according to information in the
megasas_devices[] array.
"megasas-gen2" already implements INTERFACE_PCIE_DEVICE, so add
INTERFACE_CONVENTIONAL_PCI_DEVICE only to "megasas".
Acked-by: Alberto Garcia <berto@igalia.com>
Acked-by: John Snow <jsnow@redhat.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Some PPC SoCs have an EHCI with OHCI companion USB controller. To
emulate this allow the sysbus version of OHCI to be used as a companion.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Code that checks dstate is unaware of SystemTap and LTTng UST dstate, so
the following trace event will not fire when solely enabled by SystemTap
or LTTng UST:
if (trace_event_get_state(TRACE_MY_EVENT)) {
str = g_strdup_printf("Expensive string to generate ...",
...);
trace_my_event(str);
g_free(str);
}
Add trace_event_get_state_backends() to fetch backend dstate. Those
backends that use QEMU dstate fetch it as part of
generate_h_backend_dstate().
Update existing trace_event_get_state() callers to use
trace_event_get_state_backends() instead.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20170731140718.22010-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The strict td link limit added by commit "95ed569 usb: ohci: limit the
number of link eds" causes problems with macos guests. Lets raise the
limit.
Reported-by: Programmingkid <programmingkidx@gmail.com>
Reported-by: Howard Spoelstra <hsp.cat7@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: John Arbuckle <programmingkidx@gmail.com>
Message-id: 1488876018-31576-1-git-send-email-kraxel@redhat.com
It should return 1 if an error occurs when reading td.
This will avoid an infinite loop issue in ohci_service_ed_list.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 1487760990-115925-1-git-send-email-liqiang6-s@360.cn
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The guest may builds an infinite loop with link eds. This patch
limit the number of linked ed to avoid this.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Message-id: 5899a02e.45ca240a.6c373.93c1@mx.google.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
It should return 1 if an error occurs when reading iso td.
This will avoid an infinite loop issue in ohci_service_ed_list.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Message-id: 5899ac3e.1033240a.944d5.9a2d@mx.google.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The default DMA offset is set to 3. When the property is not set by
the consumer, the default causes DMA access to be shifted by 3
bytes. In PXA, this results in incorrect DMA access, leading to error
notification in the USB controller driver. A better default would be
0, so that there is no offset, when the consumer does not specify one.
Signed-off-by: Vijay Kumar B. <vijaykumar@zilogic.com>
Reviewed-by: Deepak S. <deepak@zilogic.com>
Message-id: 1475060958-7760-1-git-send-email-vijaykumar@zilogic.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
muldiv64(a, 1, b) is like "a / b".
This patch is the result of coccinelle script
scripts/coccinelle/remove_muldiv64.cocci.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
muldiv64() is "uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)"
Some time it is used as muldiv64(uint32_t a, uint64_t b, uint32_t c)"
This patch is the result of coccinelle script
scripts/coccinelle/swap_muldiv64.cocci to reorder arguments.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
QEMU currently crashes when an OHCI controller is instantiated with
too many ports, e.g. "-device pci-ohci,num-ports=100,masterbus=1".
Thus add a proper check in usb_ohci_init() to make sure that we
do not use more than OHCI_MAX_PORTS = 15 ports here.
Ticket: https://bugs.launchpad.net/qemu/+bug/1581308
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1463995387-11710-1-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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: Rutuja Shah <rutu.shah.26@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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>
Allocate timer once, at init time, instead of allocating/freeing
it all the time when starting/stopping the bus. Simplifies the
code, also fixes bugs (memory leak) due to missing checks whenever
the time is already allocated or not.
Cc: Prasad J Pandit <pjp@fedoraproject.org>
Reported-by: Zuozhi Fzz <zuozhi.fzz@alibaba-inc.com>
Signed-off-by: Gerd Hoffmann <kraxel@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: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-20-git-send-email-peter.maydell@linaro.org
On overcommitted CPU, kernel can be so slow that an interrupt can
be triggered by the device whereas the driver is not ready to receive
it. This drives us into an infinite loop.
On suspend, if a SOF interrupt is raised between the stop of the
device processing and the change of the device internal state to
OHCI_USB_SUSPEND (QEMU stops SOF timer on this state change), this
interrupt is never acknowledged.
This patch clears pending SOF interrupt on OHCI_USB_SUSPEND setting.
Some details:
- ohci_irq(): the OHCI interrupt handler, acknowledges the SOF IRQ
only if the state of the driver (rh_state) is OHCI_STATE_RUNNING.
So if this interrupt happens and the driver is not in this state,
the function is called again and again, moving the system to a
CPU starvation.
- ohci_rh_suspend(): the function stop the operation and acknowledge
pending interrupts (but doesn't disable it). Later in the function,
the device is moved to OHCI_SUSPEND_STATE, and the driver to
OHCI_RH_SUSPENDED. If between the moment when the interrupt is
acknowledged and the moment when the device is suspended a new
interrupt is raised, it will be never acknowledged because the
driver is now not in OHCI_RH_RUNNING state.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1452109525-32150-3-git-send-email-lvivier@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
On overcommitted CPU, kernel can be so slow that an interrupt can
be triggered by the device whereas the driver is not ready to receive
it. This drives us into an infinite loop.
This does not happen on real hardware because real hardware never send
interrupt immediately after the controller has been moved to OPERATION state.
This patch tries to delay the first SOF interrupt to let driver exits from
the critical section (which is not protected against interrupts...)
Some details:
- ohci_irq(): the OHCI interrupt handler, acknowledges the SOF IRQ
only if the state of the driver (rh_state) is OHCI_STATE_RUNNING.
So if this interrupt happens and the driver is not in this state,
the function is called again and again, moving the system to a
CPU starvation.
- ohci_rh_resume(): the driver re-enables operation with OHCI_USB_OPER.
In QEMU this start the SOF timer and QEMU starts to send IRQs. As
the driver is not in OHCI_STATE_RUNNING and not protected against IRQ,
the ohci_irq() can be called and the driver never moved to
OHCI_STATE_RUNNING.
Suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1452109525-32150-2-git-send-email-lvivier@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Specification says that: "This bit is set by HCD to initiate a software reset of HC."
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 1450567431-31795-4-git-send-email-hpoussin@reactos.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Specification says that, when entering this state, "the contents of the registers
(except Root Hub registers) are preserved by the HC. [...] The Root Hub is being reset,
which causes the Root Hub's downstream ports to be reset and possibly powered off."
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-id: 1450567431-31795-3-git-send-email-hpoussin@reactos.org
Signed-off-by: Gerd Hoffmann <kraxel@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: Juan Quintela <quintela@redhat.com>
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.
Ohci does't support hotplugging/hotunplugging yet, but
existing resource cleanup leak logic likes ehci/uhci.
Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
