From patchwork Tue Oct 24 17:09:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andre Przywara X-Patchwork-Id: 116976 Delivered-To: patch@linaro.org Received: by 10.140.22.164 with SMTP id 33csp6035173qgn; Tue, 24 Oct 2017 10:07:05 -0700 (PDT) X-Google-Smtp-Source: ABhQp+RsNk5zR2VvsJt5UX035pWr3aJlBd8mWA8DBNIpI9AdgR5FgYappnx3mjwf2qhYp7KTtzan X-Received: by 10.36.58.3 with SMTP id m3mr13994666itm.81.1508864825497; Tue, 24 Oct 2017 10:07:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1508864825; cv=none; d=google.com; s=arc-20160816; b=N2btLKcOAnuUcLfHYRX4pbFyItl0jTB7c25dA89xUF2jSuCeeYF0NwxcxfLtYVSRHB MH/B85EX2ssPW5tJ8gxN6H3s8zfHHS9jJFmhip+ULiUFfJ+pITdR2YTuc5jVaQTm1cXx 5811Dol5U6y4zEVLLRakc6URKbbQR110Rpd5PMowZNidC+EkeEe9V26AM7GlX+hU0y4s xnyoZXBJo+nuQQYAQmbJeKLWC7s73Va06qYzih+SGF8oVpsCwvsXp9gorBYItvfxuIWc H6c11F8YE9EYc12CEcwtjXnRQjncbSqsrt7XDHNgxaqIQZRXqedxNm6touSZoBiNsJpL g5AA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version :list-subscribe:list-help:list-post:list-unsubscribe:list-id :precedence:subject:cc:references:in-reply-to:message-id:date:to :from:dkim-signature:arc-authentication-results; bh=c1tJI2eOXrDCJ1ZwmI3Zr81ShPgq6xGsCV+IXDIrtNo=; b=pP0zxHA+9gnjoJ2HdmljqAl8hkAe8vAzQHSaMACtuDA7YCQtZUnRckrtTEBMzhtvDV w8Yc+HfoV2d0tiDrZqc4vMYbu/lV+qdSqve+OzD6MEDyM+310wOkzLr8Bn89YCa9nq+4 El2uwjF6aNx+DS66zKxawXuJqatKKQXl4VGHiqwMVR2hw3CC+e+qz9DW1iO+6Z9o9SlJ QiZ67QoQvAD9TVR2rU9vd3iojoDIIzmTsbuFRTdiYIM98vUDJCq7GHgm5cLdkjedDfy8 L1GBKt0U4AQ5b8/xAyyLEnrpWqy3d+I2tZ1/ayHT8AmCVFRnuTaMR/Q5ogzoMGw6r4ij oKsw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=iJDIbNZi; spf=neutral (google.com: 192.237.175.120 is neither permitted nor denied by best guess record for domain of xen-devel-bounces@lists.xen.org) smtp.mailfrom=xen-devel-bounces@lists.xen.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id e91si475948iod.318.2017.10.24.10.07.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Oct 2017 10:07:05 -0700 (PDT) Received-SPF: neutral (google.com: 192.237.175.120 is neither permitted nor denied by best guess record for domain of xen-devel-bounces@lists.xen.org) client-ip=192.237.175.120; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=iJDIbNZi; spf=neutral (google.com: 192.237.175.120 is neither permitted nor denied by best guess record for domain of xen-devel-bounces@lists.xen.org) smtp.mailfrom=xen-devel-bounces@lists.xen.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e72dS-0001hR-Jr; Tue, 24 Oct 2017 17:05:02 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e72dR-0001hJ-VU for xen-devel@lists.xenproject.org; Tue, 24 Oct 2017 17:05:02 +0000 Received: from [193.109.254.147] by server-1.bemta-6.messagelabs.com id 2E/4A-31121-DB27FE95; Tue, 24 Oct 2017 17:05:01 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrOIsWRWlGSWpSXmKPExsXiVRvkqLun6H2 kwfm9Ghbft0xmcmD0OPzhCksAYxRrZl5SfkUCa0bD6kaWguteFbs+BzcwvjfvYuTiEBKYwSjR PXEyK4jDIjCPWeLx/HdsII6EQD+rxM22Y4xdjJxATpbE5r972SHsNIkX72ayQdiVEt9+PmQGs YUENCRuTJ7BCGE3MUksX+kGYrMJGEjM6prMDDJURKCTUWLxmz1MIAlmASWJ/WevgTUIC3hJ/H /eCDaURUBVYvLCKWDLeAXsJJ4t+wy1WF7i3IPbYMs4Bewltk58xgKxzE7i9v+7LBMYBRcwMqx iVC9OLSpLLdI11UsqykzPKMlNzMzRNTQw08tNLS5OTE/NSUwq1kvOz93ECAw4BiDYwTj9sv8h RkkOJiVR3oD095FCfEn5KZUZicUZ8UWlOanFhxhlODiUJHjtCoFygkWp6akVaZk5wNCHSUtw8 CiJ8O4HSfMWFyTmFmemQ6ROMVpy7Ntz6w8TR8fNu0Dy2czXDcxCLHn5ealS4rwxIA0CIA0ZpX lw42DxeYlRVkqYlxHoQCGegtSi3MwSVPlXjOIcjErCvN4gU3gy80rgtr4COogJ6CBZ+zcgB5U kIqSkGhh3l23ZyjblpE63qOoyJ80jr5Z6WStKKmsdf8WoP8X6irvDqp6fXOmasnqVEa8XvyjZ Z6i2/kjf5iPnq56tPbbpx3YOh8lZ/nbFk0y+rVE+v0574Zl1fzhkLly/+t23p0putouk3bPfO apml4qnnt+youpB9t+ZGarXqz3ynybt7mbW7mNuTjZVYinOSDTUYi4qTgQASeP+msoCAAA= X-Env-Sender: andre.przywara@linaro.org X-Msg-Ref: server-7.tower-27.messagelabs.com!1508864700!108620422!1 X-Originating-IP: [74.125.82.65] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.45; banners=-,-,- X-VirusChecked: Checked Received: (qmail 5625 invoked from network); 24 Oct 2017 17:05:00 -0000 Received: from mail-wm0-f65.google.com (HELO mail-wm0-f65.google.com) (74.125.82.65) by server-7.tower-27.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 24 Oct 2017 17:05:00 -0000 Received: by mail-wm0-f65.google.com with SMTP id p75so17036124wmg.3 for ; Tue, 24 Oct 2017 10:05:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=mnoslIbACxmHn2OMdnbrbD/TUNeQxiatqb9cnQkYEfk=; b=iJDIbNZiqAFhwUAfIWJq2ERz/BjS3bSLgSxtPPPNq6H0RaPPMcu2IdAdYWaQlzdTjA 2WeGIf4gLOV3REgdMFtZ/jI0Xe8hs0z6O2OZFX+yWyXDxwg1rKX+MtCukIYcW0KCqKZ+ SWWJYAXhOL5kC7bxgfGWnAGCUfQWCp5AOXhlU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=mnoslIbACxmHn2OMdnbrbD/TUNeQxiatqb9cnQkYEfk=; b=sgwUiayUvjt2db74k71tCO9PUkv9/kFrI+zDLsQPl3TF4hJ23PLBtTgr8QmNVP9hhW 90jA71+nu2jjybeMcNx6TtoCASEqPVPtNYbqIn9gqa51XatEyQx5086e1LtQ3kcIGEZs yOyn9b6alX8tgUectV5JChLlx6pQGLB1WlpDAjIdFgXBBsNbobhvXDy/Toi0ysNIMU+e 4YFVf3W/fweM0cTvmO3AFtgFaRdT5901+cTAQ+tP9Tx0WnmpnUS7XfpS8acBFpPmL3sb zbzBq0dvBifeHIi/PdbidHOmDZzqXGQkGZi5BcdbSRtR7yA5ohRPMtV7INdbTshVieED sTSg== X-Gm-Message-State: AMCzsaXCTiMB9wnOe47Fecu5RVViDHo9KRjyfjnOUsy7enDfuBJuQuZh HCiaAp/8WKXDIv3mw6yzGts+kg== X-Received: by 10.28.98.212 with SMTP id w203mr9828432wmb.88.1508864700085; Tue, 24 Oct 2017 10:05:00 -0700 (PDT) Received: from e104803-lin.lan (mail.andrep.de. [217.160.17.100]) by smtp.gmail.com with ESMTPSA id j27sm773304wrd.42.2017.10.24.10.04.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 24 Oct 2017 10:04:59 -0700 (PDT) From: Andre Przywara To: Julien Grall , Stefano Stabellini , Bhupinder Thakur Date: Tue, 24 Oct 2017 18:09:22 +0100 Message-Id: <20171024170922.17207-3-andre.przywara@linaro.org> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20171024170922.17207-1-andre.przywara@linaro.org> References: <20171024170922.17207-1-andre.przywara@linaro.org> Cc: xen-devel@lists.xenproject.org Subject: [Xen-devel] [PATCH v3 2/2] arm/xen: vpl011: Fix SBSA UART interrupt assertion X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" From: Bhupinder Thakur With the current SBSA UART emulation, streaming larger amounts of data (as caused by "find /", for instance) can lead to character loses. This is due to the OUT ring buffer getting full, because we change the TX interrupt bit only when the FIFO is actually full, and not already when it's half-way filled, as the Linux driver expects. The SBSA spec does not explicitly state this, but we assume that an SBSA compliant UART uses the PL011 default "interrupt FIFO level select register" value of "1/2 way". The Linux driver certainly makes this assumption, so it expect to be able to write a number of characters after the TX interrupt line has been asserted. On a similar issue we have the same wrong behaviour on the receive side. However changing the RX interrupt to trigger on reaching half of the FIFO level will lead to lag, because the guest would not be notified of incoming characters until the FIFO is half way filled. This leads to inacceptible lags when typing on a terminal. Real hardware solves this issue by using the "receive timeout interrupt" (RTI), which is triggered when character reception stops for 32 baud cycles. As we cannot and do not want to emulate any timing here, we slightly abuse the timeout interrupt to notify the guest of new characters: when a new character comes in, the RTI is asserted, when the FIFO is cleared, the interrupt gets cleared. So this patch changes the emulated interrupt trigger behaviour to come as close to real hardware as possible: the RX and TX interrupt trigger when the FIFO gets half full / half empty, and the RTI interrupt signals new incoming characters. [Andre: reword commit message, introduce receive timeout interrupt, add comments] Signed-off-by: Bhupinder Thakur Reviewed-by: Andre Przywara Signed-off-by: Andre Przywara Acked-by: Stefano Stabellini --- xen/arch/arm/vpl011.c | 131 ++++++++++++++++++++++++++++++------------- xen/include/asm-arm/vpl011.h | 2 + 2 files changed, 94 insertions(+), 39 deletions(-) diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index 0b0743679f..6d02406acf 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -18,6 +18,9 @@ #define XEN_WANT_FLEX_CONSOLE_RING 1 +/* We assume the PL011 default of "1/2 way" for the FIFO trigger level. */ +#define SBSA_UART_FIFO_LEVEL (SBSA_UART_FIFO_SIZE / 2) + #include #include #include @@ -93,24 +96,37 @@ static uint8_t vpl011_read_data(struct domain *d) */ if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 ) { + unsigned int fifo_level; + data = intf->in[xencons_mask(in_cons, sizeof(intf->in))]; in_cons += 1; smp_mb(); intf->in_cons = in_cons; + + fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); + + /* If the FIFO is now empty, we clear the receive timeout interrupt. */ + if ( fifo_level == 0 ) + { + vpl011->uartfr |= RXFE; + vpl011->uartris &= ~RTI; + } + + /* If the FIFO is more than half empty, we clear the RX interrupt. */ + if ( fifo_level < sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) + vpl011->uartris &= ~RXI; + + vpl011_update_interrupt_status(d); } else gprintk(XENLOG_ERR, "vpl011: Unexpected IN ring buffer empty\n"); - if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) == 0 ) - { - vpl011->uartfr |= RXFE; - vpl011->uartris &= ~RXI; - } - + /* + * We have consumed a character or the FIFO was empty, so clear the + * "FIFO full" bit. + */ vpl011->uartfr &= ~RXFF; - vpl011_update_interrupt_status(d); - VPL011_UNLOCK(d, flags); /* @@ -122,6 +138,24 @@ static uint8_t vpl011_read_data(struct domain *d) return data; } +static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011, + unsigned int fifo_level) +{ + struct xencons_interface *intf = vpl011->ring_buf; + unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL; + + BUILD_BUG_ON(sizeof (intf->out) < SBSA_UART_FIFO_SIZE); + + /* + * Set the TXI bit only when there is space for fifo_size/2 bytes which + * is the trigger level for asserting/de-assterting the TX interrupt. + */ + if ( fifo_level <= fifo_threshold ) + vpl011->uartris |= TXI; + else + vpl011->uartris &= ~TXI; +} + static void vpl011_write_data(struct domain *d, uint8_t data) { unsigned long flags; @@ -146,33 +180,37 @@ static void vpl011_write_data(struct domain *d, uint8_t data) if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) != sizeof (intf->out) ) { + unsigned int fifo_level; + intf->out[xencons_mask(out_prod, sizeof(intf->out))] = data; out_prod += 1; smp_wmb(); intf->out_prod = out_prod; - } - else - gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); - if ( xencons_queued(out_prod, out_cons, sizeof(intf->out)) == - sizeof (intf->out) ) - { - vpl011->uartfr |= TXFF; - vpl011->uartris &= ~TXI; + fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); - /* - * This bit is set only when FIFO becomes full. This ensures that - * the SBSA UART driver can write the early console data as fast as - * possible, without waiting for the BUSY bit to get cleared before - * writing each byte. - */ - vpl011->uartfr |= BUSY; + if ( fifo_level == sizeof (intf->out) ) + { + vpl011->uartfr |= TXFF; + + /* + * This bit is set only when FIFO becomes full. This ensures that + * the SBSA UART driver can write the early console data as fast as + * possible, without waiting for the BUSY bit to get cleared before + * writing each byte. + */ + vpl011->uartfr |= BUSY; + } + + vpl011_update_tx_fifo_status(vpl011, fifo_level); + + vpl011_update_interrupt_status(d); } + else + gprintk(XENLOG_ERR, "vpl011: Unexpected OUT ring buffer full\n"); vpl011->uartfr &= ~TXFE; - vpl011_update_interrupt_status(d); - VPL011_UNLOCK(d, flags); /* @@ -344,7 +382,7 @@ static void vpl011_data_avail(struct domain *d) struct vpl011 *vpl011 = &d->arch.vpl011; struct xencons_interface *intf = vpl011->ring_buf; XENCONS_RING_IDX in_cons, in_prod, out_cons, out_prod; - XENCONS_RING_IDX in_ring_qsize, out_ring_qsize; + XENCONS_RING_IDX in_fifo_level, out_fifo_level; VPL011_LOCK(d, flags); @@ -355,28 +393,41 @@ static void vpl011_data_avail(struct domain *d) smp_rmb(); - in_ring_qsize = xencons_queued(in_prod, + in_fifo_level = xencons_queued(in_prod, in_cons, sizeof(intf->in)); - out_ring_qsize = xencons_queued(out_prod, + out_fifo_level = xencons_queued(out_prod, out_cons, sizeof(intf->out)); - /* Update the uart rx state if the buffer is not empty. */ - if ( in_ring_qsize != 0 ) - { + /**** Update the UART RX state ****/ + + /* Clear the FIFO_EMPTY bit if the FIFO holds at least one character. */ + if ( in_fifo_level > 0 ) vpl011->uartfr &= ~RXFE; - if ( in_ring_qsize == sizeof(intf->in) ) - vpl011->uartfr |= RXFF; + + /* Set the FIFO_FULL bit if the Xen buffer is full. */ + if ( in_fifo_level == sizeof(intf->in) ) + vpl011->uartfr |= RXFF; + + /* Assert the RX interrupt if the FIFO is more than half way filled. */ + if ( in_fifo_level >= sizeof(intf->in) - SBSA_UART_FIFO_LEVEL ) vpl011->uartris |= RXI; - } - /* Update the uart tx state if the buffer is not full. */ - if ( out_ring_qsize != sizeof(intf->out) ) + /* + * If the input queue is not empty, we assert the receive timeout interrupt. + * As we don't emulate any timing here, so we ignore the actual timeout + * of 32 baud cycles. + */ + if ( in_fifo_level > 0 ) + vpl011->uartris |= RTI; + + /**** Update the UART TX state ****/ + + if ( out_fifo_level != sizeof(intf->out) ) { vpl011->uartfr &= ~TXFF; - vpl011->uartris |= TXI; /* * Clear the BUSY bit as soon as space becomes available @@ -385,12 +436,14 @@ static void vpl011_data_avail(struct domain *d) */ vpl011->uartfr &= ~BUSY; - if ( out_ring_qsize == 0 ) - vpl011->uartfr |= TXFE; + vpl011_update_tx_fifo_status(vpl011, out_fifo_level); } vpl011_update_interrupt_status(d); + if ( out_fifo_level == 0 ) + vpl011->uartfr |= TXFE; + VPL011_UNLOCK(d, flags); } diff --git a/xen/include/asm-arm/vpl011.h b/xen/include/asm-arm/vpl011.h index 1b583dac3c..db95ff822f 100644 --- a/xen/include/asm-arm/vpl011.h +++ b/xen/include/asm-arm/vpl011.h @@ -28,6 +28,8 @@ #define VPL011_LOCK(d,flags) spin_lock_irqsave(&(d)->arch.vpl011.lock, flags) #define VPL011_UNLOCK(d,flags) spin_unlock_irqrestore(&(d)->arch.vpl011.lock, flags) +#define SBSA_UART_FIFO_SIZE 32 + struct vpl011 { void *ring_buf; struct page_info *ring_page;