Re: [Xen-devel] [PATCH 27/27 v12] arm/xen: vpl011: Correct the logic for asserting/de-asserting SBSA UART TX interrupt

[Dave Martin <Dave.Martin@xxxxxxx>]

On Fri, Oct 13, 2017 at 04:10:31PM +0530, Bhupinder Thakur wrote:
> This patch fixes the issue observed when pl011 patches were tested on
> the junos hardware by Andre/Julien. It was observed that when large
> output is generated such as on running 'find /', output was getting
> truncated intermittently due to OUT ring buffer getting full.
> 
> This issue was due to the fact that the SBSA UART driver expects that
> when a TX interrupt is asserted then the FIFO queue should be atleast
> half empty and that it can write N bytes in the FIFO, where N is half
> the FIFO queue size, without the bytes getting dropped due to FIFO
> getting full.
> 
> The SBSA UART emulation logic was asserting the TX interrupt as soon
> as any space became available in the FIFO and the SBSA UART driver
> tried to write more data (upto 16 bytes) in the FIFO expecting that
> there is enough space available leading to dropped bytes.
> 
> The SBSA spec [1] does not specify when the TX interrupt should be
> asserted or de-asserted. Due to lack of clarity on the expected
> behavior, it is assumed for now that TX interrupt should be asserted
> only when the FIFO goes half empty.
> 
> TBD: Once the SBSA spec is updated with the expected behavior, the
> implementation will be modified to align with the spec requirement.
> 
> [1] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0183f/DDI0183.pdf
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---

[...]

> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c

[...]

> @@ -355,28 +382,46 @@ static void vpl011_data_avail(struct domain *d)

[...]

> +        /*
> +         * Currently, the RXI bit is getting set even if there is a single
> +         * byte of data in the rx fifo. Ideally, the RXI bit should be set
> +         * only if the rx fifo level reaches the threshold.
> +         *
> +         * However, since currently RX timeout interrupt is not
> +         * implemented as there is not enough clarity in the SBSA spec,
> +         * the guest may keep waiting for an interrupt to read more
> +         * data. To ensure that guest reads all the data without
> +         * any delay, the RXI interrupt is raised if there is RX data
> +         * available without checking whether fifo level has reached
> +         * the threshold.
> +         *
> +         * TBD: Once there is more clarity in the SBSA spec on whether RX
> +         * timeout interrupt needs to be implemented, the RXI interrupt
> +         * will be raised only when rx fifo level reaches the threshold.
> +         */

This looks OK to me: it makes the issues clear to future maintainers
of this code.

[...]

Cheers
---Dave

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel