[v1] xen/arm: initialize vpl011 flag register

[Xen-devel] [PATCH] xen/arm: initialize vpl011 flag register

Posted by Jeff Kubascik 4 years, 4 months ago

The tx/rx fifo flags were not set when the vpl011 is initialized. This
is a problem for certain guests that are operating in polled mode, as a
guest will generally check the rx fifo empty flag to determine if there
is data before doing a read. The result is a continuous spam of the
message "vpl011: Unexpected IN ring buffer empty" before the first valid
character is received. This initializes the flag status register to the
default specified in the PL011 technical reference manual.

Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
---
 xen/arch/arm/vpl011.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 7bc5eeb207..31b7d56d7d 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -626,6 +626,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
     if ( vpl011->backend.dom.ring_buf )
         return -EINVAL;
 
+    vpl011->uartfr = TXFE | RXFE;
+
     /*
      * info is NULL when the backend is in Xen.
      * info is != NULL when the backend is in a domain.
-- 
2.17.1


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

Re: [Xen-devel] [PATCH] xen/arm: initialize vpl011 flag register

Posted by Julien Grall 4 years, 4 months ago

Hi,

On 25/11/2019 18:35, Jeff Kubascik wrote:
> The tx/rx fifo flags were not set when the vpl011 is initialized. This
> is a problem for certain guests that are operating in polled mode, as a
> guest will generally check the rx fifo empty flag to determine if there
> is data before doing a read. The result is a continuous spam of the
> message "vpl011: Unexpected IN ring buffer empty" before the first valid
> character is received. This initializes the flag status register to the
> default specified in the PL011 technical reference manual.

Note that the vpl011 is not meant to emulate a full PL011. Instead it
emulates the SBSA UART which is a subset of the PL011. They have some 
differences and I would be cautious to try to drive it as a PL011.

> 
> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
> ---
>   xen/arch/arm/vpl011.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 7bc5eeb207..31b7d56d7d 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -626,6 +626,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
>       if ( vpl011->backend.dom.ring_buf )
>           return -EINVAL;
>   
> +    vpl011->uartfr = TXFE | RXFE;

I know that it does not make much difference, but I would prefer if 
uartfr is initialized once we know nothing else can fail.

With or without this suggestion:

Acked-by: Julien Gral <julien@xen.org>

Cheers,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH] xen/arm: initialize vpl011 flag register

Posted by Jeff Kubascik 4 years, 4 months ago

Hello,

On 11/25/2019 2:20 PM, Julien Grall wrote:
> Hi,
> 
> On 25/11/2019 18:35, Jeff Kubascik wrote:
>> The tx/rx fifo flags were not set when the vpl011 is initialized. This
>> is a problem for certain guests that are operating in polled mode, as a
>> guest will generally check the rx fifo empty flag to determine if there
>> is data before doing a read. The result is a continuous spam of the
>> message "vpl011: Unexpected IN ring buffer empty" before the first valid
>> character is received. This initializes the flag status register to the
>> default specified in the PL011 technical reference manual.
> 
> Note that the vpl011 is not meant to emulate a full PL011. Instead it
> emulates the SBSA UART which is a subset of the PL011. They have some
> differences and I would be cautious to try to drive it as a PL011.

I was not aware of this, but it makes sense. I took a quick peek at the SBSA
design doc and the fifo flags are defined.

>>
>> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
>> ---
>>   xen/arch/arm/vpl011.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
>> index 7bc5eeb207..31b7d56d7d 100644
>> --- a/xen/arch/arm/vpl011.c
>> +++ b/xen/arch/arm/vpl011.c
>> @@ -626,6 +626,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
>>       if ( vpl011->backend.dom.ring_buf )
>>           return -EINVAL;
>>
>> +    vpl011->uartfr = TXFE | RXFE;
> 
> I know that it does not make much difference, but I would prefer if
> uartfr is initialized once we know nothing else can fail.

Easy enough change, I'll send out an updated patch.

> With or without this suggestion:
> 
> Acked-by: Julien Gral <julien@xen.org>
> 
> Cheers,
> 
> --
> Julien Grall
> 

Thanks!
Jeff Kubascik

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