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

From: Julien Grall <julien.grall@arm.com>
To: Bhupinder Thakur <bhupinder.thakur@linaro.org>
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 14:54:39 +0100	[thread overview]
Message-ID: <55b4a4b8-8afe-ba86-c3af-8879c3453e55@arm.com> (raw)
In-Reply-To: <CACtJ1JRAQ=iKQrcJ=azR=A0b1hba33Y0WyskXoD_nxKxa179MA@mail.gmail.com>

On 14/08/17 08:52, Bhupinder Thakur wrote:
> Hi Julien,

Hi Bhupinder,

>
>
>>> 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.

I understand that it is the behavior expected by Linux. However did you 
check it was compliant with the spec?

If I looked at the PL011 (ARM DDI 0183G), the interrupt FIFO level is 
set via UARTIFLS. The reset value is half-way mark (i.e 16 byte).

However, looking at the SBSA, this register is inexistent and I can't 
find anything mentioning the half-way mark. So we need some 
clarification here. Let me ask it.

Meanwhile, trying the half-way mark would be good to see if it helps.

Cheers,

-- 
Julien Grall

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