Re: [PATCH 00/25 v7] SBSA UART emulation support in Xen

From: Bhupinder Thakur <bhupinder.thakur@linaro.org>
To: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	Andre Przywara <andre.przywara@arm.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 00/25 v7] SBSA UART emulation support in Xen
Date: Mon, 14 Aug 2017 13:22:48 +0530	[thread overview]
Message-ID: <CACtJ1JRAQ=iKQrcJ=azR=A0b1hba33Y0WyskXoD_nxKxa179MA@mail.gmail.com> (raw)
In-Reply-To: <CACtJ1JS-UDAJOqmsf+OKYgK_+py7iONa5yw_YPDZjmaHUjd1CQ@mail.gmail.com>

Hi Julien,

>> After that I tried to stress the emulation a bit with "find ." to get a lot
>> of output. And I noticed a lot of message similar to the one below on xen
>> console:
>>
>> d6v0 vpl011: Unexpected OUT ring buffer full
>>
>> Associated to that the character have been eaten resulting to non-sense log.
>>
>> A bit above the printk printing this message, there are a comment saying:
>>
>>     /*
>>      * It is expected that the ring is not full when this function is called
>>      * as the guest is expected to write to the data register only when the
>>      * TXFF flag is not set.
>>      * In case the guest does write even when the TXFF flag is set then the
>>      * data will be silently dropped.
>>      */
>>
>> I am quite surprised that Linux is not looking at the TXFF flags. So this
>> needs some investigation.
>
> I ran 'find' but could not reproduce the issue.
>
> I will try to reproduce this issue by reducing the OUT buffer size.

I could not reproduce the issue with the reduced buffer of 16 bytes
also. I think it may not be reproducible on the foundation model. I am
trying to bring up xen on an ARM machine here to reproduce the issue.

While looking at the pl011 driver in linux, I see one potential case
where the the driver may send more data even when the TX FIFO is full.
It seems the pl011 driver expects that the TX interrupt must be raised
only when at least half of TX FIFO queue is empty.

pl011_tx_chars() calls pl011_tx_char() in a loop for (fifosize/2)
number of times. Since these APIs are getting called in the interrupt
context, pl011_tx_char() does not check for TX FIFO full status
because it expects that fifosize/2 space is available.

In the emulation logic, we should set the TX bit in the status
register only if at least space for 16 bytes is available (since SBSA
fifosize is 32). Currently, we may be setting the TX bit even if there
is space for 1 byte. In that scenario, the driver may write more data
then emulation logic can queue up.

Regards,
Bhupinder

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