All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Canokeys.org" <contact@canokeys.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Paul Durrant" <paul@xen.org>,
	"Akihiko Odaki" <akihiko.odaki@gmail.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Hongren (Zenithal) Zheng" <i@zenithal.me>,
	xen-devel@lists.xenproject.org,
	"Alex Williamson" <alex.williamson@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Arnout Engelen" <arnout@bzzt.net>
Subject: [PULL 11/17] hw/usb/hcd-ehci: fix writeback order
Date: Fri, 10 Jun 2022 11:20:37 +0200	[thread overview]
Message-ID: <20220610092043.1874654-12-kraxel@redhat.com> (raw)
In-Reply-To: <20220610092043.1874654-1-kraxel@redhat.com>

From: Arnout Engelen <arnout@bzzt.net>

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>
---
 hw/usb/hcd-ehci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 33a8a377bd95..d4da8dcb8d15 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2011,7 +2011,10 @@ static int ehci_state_writeback(EHCIQueue *q)
     ehci_trace_qtd(q, NLPTR_GET(p->qtdaddr), (EHCIqtd *) &q->qh.next_qtd);
     qtd = (uint32_t *) &q->qh.next_qtd;
     addr = NLPTR_GET(p->qtdaddr);
-    put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 2);
+    /* First write back the offset */
+    put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qtd + 3, 1);
+    /* Then write back the token, clearing the 'active' bit */
+    put_dwords(q->ehci, addr + 2 * sizeof(uint32_t), qtd + 2, 1);
     ehci_free_packet(p);
 
     /*
-- 
2.36.1



  parent reply	other threads:[~2022-06-10 10:41 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-10  9:20 [PULL 00/17] Kraxel 20220610 patches Gerd Hoffmann
2022-06-10  9:20 ` [PULL 01/17] ui/gtk-gl-area: implement GL context destruction Gerd Hoffmann
2022-06-10  9:20 ` [PULL 02/17] ui/gtk-gl-area: create the requested GL context version Gerd Hoffmann
2022-06-10  9:20 ` [PULL 03/17] ui/cocoa: Fix poweroff request code Gerd Hoffmann
2022-06-10  9:20 ` [PULL 04/17] hw/audio/cs4231a: Const'ify global tables Gerd Hoffmann
2022-06-10  9:20 ` [PULL 05/17] hw/usb: Add CanoKey Implementation Gerd Hoffmann
2022-06-10  9:20 ` [PULL 06/17] hw/usb/canokey: Add trace events Gerd Hoffmann
2022-06-10  9:20 ` [PULL 07/17] meson: Add CanoKey Gerd Hoffmann
2022-06-10  9:20 ` [PULL 08/17] docs: Add CanoKey documentation Gerd Hoffmann
2022-06-10  9:20 ` [PULL 09/17] docs/system/devices/usb: Add CanoKey to USB devices examples Gerd Hoffmann
2022-06-10  9:20 ` [PULL 10/17] MAINTAINERS: add myself as CanoKey maintainer Gerd Hoffmann
2022-06-10  9:20 ` Gerd Hoffmann [this message]
2022-06-10  9:20 ` [PULL 12/17] usbredir: avoid queuing hello packet on snapshot restore Gerd Hoffmann
2022-06-10  9:20 ` [PULL 13/17] virtio-gpu: update done only on the scanout associated with rect Gerd Hoffmann
2022-06-10  9:20 ` [PULL 14/17] ui: move 'pc-bios/keymaps' to 'ui/keymaps' Gerd Hoffmann
2022-06-10  9:20 ` [PULL 15/17] ui/console: Do not return a value with ui_info Gerd Hoffmann
2022-06-10  9:20 ` [PULL 16/17] ui: Deliver refresh rate via QemuUIInfo Gerd Hoffmann
2022-06-10  9:20 ` [PULL 17/17] virtio-gpu: Respect UI refresh rate for EDID Gerd Hoffmann
2022-06-10 20:16 ` [PULL 00/17] Kraxel 20220610 patches Richard Henderson
2022-06-11 16:34   ` Volker Rümelin
2022-06-12  1:10     ` Akihiko Odaki
2022-06-13  8:45     ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220610092043.1874654-12-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=akihiko.odaki@gmail.com \
    --cc=alex.williamson@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=arnout@bzzt.net \
    --cc=contact@canokeys.org \
    --cc=f4bug@amsat.org \
    --cc=i@zenithal.me \
    --cc=mst@redhat.com \
    --cc=paul@xen.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.