From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41844) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b04Gr-0002h0-6N for qemu-devel@nongnu.org; Tue, 10 May 2016 05:48:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b04Gm-0004xQ-3n for qemu-devel@nongnu.org; Tue, 10 May 2016 05:48:04 -0400 Received: from mail-db3on0134.outbound.protection.outlook.com ([157.55.234.134]:64006 helo=emea01-db3-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b04Gk-0004wy-UK for qemu-devel@nongnu.org; Tue, 10 May 2016 05:48:00 -0400 References: <1462384435-1034-1-git-send-email-rkagan@virtuozzo.com> From: "Denis V. Lunev" Message-ID: <57319878.6060507@openvz.org> Date: Tue, 10 May 2016 11:14:48 +0300 MIME-Version: 1.0 In-Reply-To: <1462384435-1034-1-git-send-email-rkagan@virtuozzo.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] usb:xhci: no DMA on HC reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Kagan , Gerd Hoffmann , qemu-devel@nongnu.org On 05/04/2016 08:53 PM, Roman Kagan wrote: > This patch is a rough fix to a memory corruption we are observing when > running VMs with xhci USB controller and OVMF firmware. > > Specifically, on the following call chain > > xhci_reset > xhci_disable_slot > xhci_disable_ep > xhci_set_ep_state > > QEMU overwrites guest memory using stale guest addresses. > > This doesn't happen when the guest (firmware) driver sets up xhci for > the first time as there are no slots configured yet. However when the > firmware hands over the control to the OS some slots and endpoints are > already set up with their context in the guest RAM. Now the OS' driver > resets the controller again and xhci_set_ep_state then reads and writes > that memory which is now owned by the OS. > > As a quick fix, skip calling xhci_set_ep_state in xhci_disable_ep if the > device context base address array pointer is zero (indicating we're in > the HC reset and no DMA is possible). > > Signed-off-by: Roman Kagan > Cc: Gerd Hoffmann > --- > hw/usb/hcd-xhci.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index bcde8a2..43ba615 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -1531,7 +1531,10 @@ static TRBCCode xhci_disable_ep(XHCIState *xhci, unsigned int slotid, > usb_packet_cleanup(&epctx->transfers[i].packet); > } > > - xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED); > + /* only touch guest RAM if we're not resetting the HC */ > + if (xhci->dcbaap_low || xhci->dcbaap_high) { > + xhci_set_ep_state(xhci, epctx, NULL, EP_DISABLED); > + } > > timer_free(epctx->kick_timer); > g_free(epctx); ping