[v2] xen/arm: initialize vpl011 flag register

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

Posted by Jeff Kubascik 4 years, 5 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>

Changes in v2:
- Moved uartfr initialization to later point in function after potential
return/failure points
---
 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..895f436cc4 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -668,6 +668,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
         goto out2;
     }
 
+    vpl011->uartfr = TXFE | RXFE;
+
     spin_lock_init(&vpl011->lock);
 
     register_mmio_handler(d, &vpl011_mmio_handler,
-- 
2.17.1


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

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

Posted by Julien Grall 4 years, 5 months ago

Hi,

On 25/11/2019 20:58, 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.
> 
> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>

You could have retained my acked-by here :).

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

We are in late stage for Xen 4.13 and from what you say this will only 
spam the console (though it is rate-limited). So I don't intend to 
request to be merged in Xen 4.13 (feel free to request it if you think 
it is worth it).

Instead, I will queue it for the next release in my branch for-next/4.14.

> 
> Changes in v2:
> - Moved uartfr initialization to later point in function after potential
> return/failure points
We don't commit the changelog. To help making the committers life 
boring, I would recommend to add --- before it. git am will stripped 
anything after it.

> ---

Similar to this one.

>   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..895f436cc4 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -668,6 +668,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
>           goto out2;
>       }
>   
> +    vpl011->uartfr = TXFE | RXFE;
> +
>       spin_lock_init(&vpl011->lock);
>   
>       register_mmio_handler(d, &vpl011_mmio_handler,
> 

-- 
Julien Grall

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

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

Posted by Jeff Kubascik 4 years, 5 months ago

On 11/25/2019 5:17 PM, Julien Grall wrote:
> Hi,
> 
> On 25/11/2019 20:58, 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.
>>
>> Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
> 
> You could have retained my acked-by here :).
> 
> Acked-by: Julien Grall <julien@xen.org>

Apologies, I'm still getting used to submitting patches by mailing list.

> We are in late stage for Xen 4.13 and from what you say this will only
> spam the console (though it is rate-limited). So I don't intend to
> request to be merged in Xen 4.13 (feel free to request it if you think
> it is worth it).
> 
> Instead, I will queue it for the next release in my branch for-next/4.14.

Sounds good!

>>
>> Changes in v2:
>> - Moved uartfr initialization to later point in function after potential
>> return/failure points
> We don't commit the changelog. To help making the committers life
> boring, I would recommend to add --- before it. git am will stripped
> anything after it.
> 
>> ---
> 
> Similar to this one.
> 
>>   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..895f436cc4 100644
>> --- a/xen/arch/arm/vpl011.c
>> +++ b/xen/arch/arm/vpl011.c
>> @@ -668,6 +668,8 @@ int domain_vpl011_init(struct domain *d, struct vpl011_init_info *info)
>>           goto out2;
>>       }
>>
>> +    vpl011->uartfr = TXFE | RXFE;
>> +
>>       spin_lock_init(&vpl011->lock);
>>
>>       register_mmio_handler(d, &vpl011_mmio_handler,
>>
> 
> --
> Julien Grall
> 

Good to know, I will do this next time.

Thanks,
Jeff Kubascik

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