Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] xen/arm: vpl011 fixes continuation
@ 2023-04-05 11:17 Michal Orzel
  2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Michal Orzel @ 2023-04-05 11:17 UTC (permalink / raw
  To: xen-devel
  Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

Another portion of vpl011 fixes/hardening.

Michal Orzel (3):
  xen/arm: vpl011: Fix misleading comments
  xen/arm: vpl011: Handle correctly TXFE when backend in Xen
  xen/arm: vpl011: Do not try to handle TX FIFO status when backend in
    Xen

 xen/arch/arm/vpl011.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
  2023-04-05 11:17 [PATCH 0/3] xen/arm: vpl011 fixes continuation Michal Orzel
@ 2023-04-05 11:17 ` Michal Orzel
  2023-04-06 10:38   ` Ayan Kumar Halder
  2023-04-12  6:21   ` Henry Wang
  2023-04-05 11:17 ` [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen Michal Orzel
  2023-04-05 11:17 ` [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status " Michal Orzel
  2 siblings, 2 replies; 11+ messages in thread
From: Michal Orzel @ 2023-04-05 11:17 UTC (permalink / raw
  To: xen-devel
  Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
stating that the guest is expected to read the DR register only if the
TXFE bit of FR register is not set. This is obviously logically wrong and
it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/vpl011.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2fa80bc15ac4..0186d8a31834 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
     /*
      * It is expected that there will be data in the ring buffer when this
      * function is called since the guest is expected to read the data register
-     * only if the TXFE flag is not set.
-     * If the guest still does read when TXFE bit is set then 0 will be returned.
+     * only if the RXFE flag is not set.
+     * If the guest still does read when RXFE bit is set then 0 will be returned.
      */
     if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
     {
@@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
     /*
      * It is expected that there will be data in the ring buffer when this
      * function is called since the guest is expected to read the data register
-     * only if the TXFE flag is not set.
-     * If the guest still does read when TXFE bit is set then 0 will be returned.
+     * only if the RXFE flag is not set.
+     * If the guest still does read when RXFE bit is set then 0 will be returned.
      */
     if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
     {
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen
  2023-04-05 11:17 [PATCH 0/3] xen/arm: vpl011 fixes continuation Michal Orzel
  2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
@ 2023-04-05 11:17 ` Michal Orzel
  2023-04-07 21:42   ` Stefano Stabellini
  2023-04-12  7:17   ` Henry Wang
  2023-04-05 11:17 ` [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status " Michal Orzel
  2 siblings, 2 replies; 11+ messages in thread
From: Michal Orzel @ 2023-04-05 11:17 UTC (permalink / raw
  To: xen-devel
  Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

When backend is in Xen, the handling of data written to DR register is a
bit special because we want to tell guest that we are always ready for new
data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always
set). This conflicts with the current handling of TXFE bit, which we
always clear and never set on a write path (we happen to set it when we
receive char from serial input due to use of vpl011_data_avail() but this
might never be called). This can lead to issues if a guest driver makes
use of TXFE bit to check for TX transmission completion (such guest could
then wait endlessly). Fix it by keeping TXFE always set to match the
current emulation logic.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
We don't have to look far for an example of a PL011/SBSA driver relying on TXFE.
If a guest had a driver like we have in Xen, we would end up with no messages
being printed.
---
 xen/arch/arm/vpl011.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 0186d8a31834..ff06deeb645c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -112,8 +112,14 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
         }
     }
 
+    /*
+     * When backend is in Xen, we tell guest we are always ready for new data
+     * to be written. This is fulfilled by having:
+     * - TXI/TXFE -> always set,
+     * - TXFF/BUSY -> never set.
+     */
     vpl011->uartris |= TXI;
-    vpl011->uartfr &= ~TXFE;
+    vpl011->uartfr |= TXFE;
     vpl011_update_interrupt_status(d);
 
     VPL011_UNLOCK(d, flags);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status when backend in Xen
  2023-04-05 11:17 [PATCH 0/3] xen/arm: vpl011 fixes continuation Michal Orzel
  2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
  2023-04-05 11:17 ` [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen Michal Orzel
@ 2023-04-05 11:17 ` Michal Orzel
  2023-04-07 21:45   ` Stefano Stabellini
  2023-04-12  6:55   ` Henry Wang
  2 siblings, 2 replies; 11+ messages in thread
From: Michal Orzel @ 2023-04-05 11:17 UTC (permalink / raw
  To: xen-devel
  Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

From vpl011_rx_char_xen(), we call vpl011_data_avail() that handles both
RX and TX state. Because we are passing 0 as out_fifo_level and
SBSA_UART_FIFO_SIZE as out_size, we end up calling a function
vpl011_update_tx_fifo_status() which performs TXI bit handling
depending on the FIFO trigger level. This does not make sense when backend
is in Xen, as we maintain a single TX state where data can always be
written and as such there is no TX FIFO handling. Furthermore, this
function assumes that the backend is in domain by making use of struct
xencons_interface unconditionally. Fix it by calling this function only
when backend is in domain. Also add an assert for sanity.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/arm/vpl011.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index ff06deeb645c..7856b4b5f5a3 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -261,6 +261,9 @@ static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011,
     struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
     unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL;
 
+    /* No TX FIFO handling when backend is in Xen */
+    ASSERT(vpl011->backend_in_domain);
+
     BUILD_BUG_ON(sizeof(intf->out) < SBSA_UART_FIFO_SIZE);
 
     /*
@@ -547,7 +550,13 @@ static void vpl011_data_avail(struct domain *d,
          */
         vpl011->uartfr &= ~BUSY;
 
-        vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
+        /*
+         * When backend is in Xen, we are always ready for new data to be
+         * written (i.e. no TX FIFO handling), therefore we do not want
+         * to change the TX FIFO status in such case.
+         */
+        if ( vpl011->backend_in_domain )
+            vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
     }
 
     vpl011_update_interrupt_status(d);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
  2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
@ 2023-04-06 10:38   ` Ayan Kumar Halder
  2023-04-07 21:33     ` Stefano Stabellini
  2023-04-12  6:21   ` Henry Wang
  1 sibling, 1 reply; 11+ messages in thread
From: Ayan Kumar Halder @ 2023-04-06 10:38 UTC (permalink / raw
  To: Michal Orzel, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk


On 05/04/2023 12:17, Michal Orzel wrote:
> In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
> stating that the guest is expected to read the DR register only if the
> TXFE bit of FR register is not set. This is obviously logically wrong and
> it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
NIT:- I will prefer if the PL011 TRM is mentioned with the relevant section.
>
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
>   xen/arch/arm/vpl011.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 2fa80bc15ac4..0186d8a31834 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
>       /*
>        * It is expected that there will be data in the ring buffer when this
>        * function is called since the guest is expected to read the data register
> -     * only if the TXFE flag is not set.
> -     * If the guest still does read when TXFE bit is set then 0 will be returned.
> +     * only if the RXFE flag is not set.
> +     * If the guest still does read when RXFE bit is set then 0 will be returned.
>        */
>       if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
>       {
> @@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
>       /*
>        * It is expected that there will be data in the ring buffer when this
>        * function is called since the guest is expected to read the data register
> -     * only if the TXFE flag is not set.
> -     * If the guest still does read when TXFE bit is set then 0 will be returned.
> +     * only if the RXFE flag is not set.
> +     * If the guest still does read when RXFE bit is set then 0 will be returned.
>        */
>       if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
>       {


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
  2023-04-06 10:38   ` Ayan Kumar Halder
@ 2023-04-07 21:33     ` Stefano Stabellini
  0 siblings, 0 replies; 11+ messages in thread
From: Stefano Stabellini @ 2023-04-07 21:33 UTC (permalink / raw
  To: Ayan Kumar Halder
  Cc: Michal Orzel, xen-devel, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk

On Thu, 6 Apr 2023, Ayan Kumar Halder wrote:
> On 05/04/2023 12:17, Michal Orzel wrote:
> > In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
> > stating that the guest is expected to read the DR register only if the
> > TXFE bit of FR register is not set. This is obviously logically wrong and
> > it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
> NIT:- I will prefer if the PL011 TRM is mentioned with the relevant section.
> > 
> > Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> > ---
> >   xen/arch/arm/vpl011.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 2fa80bc15ac4..0186d8a31834 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
> >       /*
> >        * It is expected that there will be data in the ring buffer when this
> >        * function is called since the guest is expected to read the data
> > register
> > -     * only if the TXFE flag is not set.
> > -     * If the guest still does read when TXFE bit is set then 0 will be
> > returned.
> > +     * only if the RXFE flag is not set.
> > +     * If the guest still does read when RXFE bit is set then 0 will be
> > returned.
> >        */
> >       if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
> >       {
> > @@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
> >       /*
> >        * It is expected that there will be data in the ring buffer when this
> >        * function is called since the guest is expected to read the data
> > register
> > -     * only if the TXFE flag is not set.
> > -     * If the guest still does read when TXFE bit is set then 0 will be
> > returned.
> > +     * only if the RXFE flag is not set.
> > +     * If the guest still does read when RXFE bit is set then 0 will be
> > returned.
> >        */
> >       if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
> >       {
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen
  2023-04-05 11:17 ` [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen Michal Orzel
@ 2023-04-07 21:42   ` Stefano Stabellini
  2023-04-12  7:17   ` Henry Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Stefano Stabellini @ 2023-04-07 21:42 UTC (permalink / raw
  To: Michal Orzel
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

On Wed, 5 Apr 2023, Michal Orzel wrote:
> When backend is in Xen, the handling of data written to DR register is a
> bit special because we want to tell guest that we are always ready for new
> data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always
> set). This conflicts with the current handling of TXFE bit, which we
> always clear and never set on a write path (we happen to set it when we
> receive char from serial input due to use of vpl011_data_avail() but this
> might never be called). This can lead to issues if a guest driver makes
> use of TXFE bit to check for TX transmission completion (such guest could
> then wait endlessly). Fix it by keeping TXFE always set to match the
> current emulation logic.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
> We don't have to look far for an example of a PL011/SBSA driver relying on TXFE.
> If a guest had a driver like we have in Xen, we would end up with no messages
> being printed.
> ---
>  xen/arch/arm/vpl011.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 0186d8a31834..ff06deeb645c 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -112,8 +112,14 @@ static void vpl011_write_data_xen(struct domain *d, uint8_t data)
>          }
>      }
>  
> +    /*
> +     * When backend is in Xen, we tell guest we are always ready for new data
> +     * to be written. This is fulfilled by having:
> +     * - TXI/TXFE -> always set,
> +     * - TXFF/BUSY -> never set.
> +     */
>      vpl011->uartris |= TXI;
> -    vpl011->uartfr &= ~TXFE;
> +    vpl011->uartfr |= TXFE;
>      vpl011_update_interrupt_status(d);
>  
>      VPL011_UNLOCK(d, flags);
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status when backend in Xen
  2023-04-05 11:17 ` [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status " Michal Orzel
@ 2023-04-07 21:45   ` Stefano Stabellini
  2023-04-12  6:55   ` Henry Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Stefano Stabellini @ 2023-04-07 21:45 UTC (permalink / raw
  To: Michal Orzel
  Cc: xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

On Wed, 5 Apr 2023, Michal Orzel wrote:
> >From vpl011_rx_char_xen(), we call vpl011_data_avail() that handles both
> RX and TX state. Because we are passing 0 as out_fifo_level and
> SBSA_UART_FIFO_SIZE as out_size, we end up calling a function
> vpl011_update_tx_fifo_status() which performs TXI bit handling
> depending on the FIFO trigger level. This does not make sense when backend
> is in Xen, as we maintain a single TX state where data can always be
> written and as such there is no TX FIFO handling. Furthermore, this
> function assumes that the backend is in domain by making use of struct
> xencons_interface unconditionally. Fix it by calling this function only
> when backend is in domain. Also add an assert for sanity.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/vpl011.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index ff06deeb645c..7856b4b5f5a3 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -261,6 +261,9 @@ static void vpl011_update_tx_fifo_status(struct vpl011 *vpl011,
>      struct xencons_interface *intf = vpl011->backend.dom.ring_buf;
>      unsigned int fifo_threshold = sizeof(intf->out) - SBSA_UART_FIFO_LEVEL;
>  
> +    /* No TX FIFO handling when backend is in Xen */
> +    ASSERT(vpl011->backend_in_domain);
> +
>      BUILD_BUG_ON(sizeof(intf->out) < SBSA_UART_FIFO_SIZE);
>  
>      /*
> @@ -547,7 +550,13 @@ static void vpl011_data_avail(struct domain *d,
>           */
>          vpl011->uartfr &= ~BUSY;
>  
> -        vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
> +        /*
> +         * When backend is in Xen, we are always ready for new data to be
> +         * written (i.e. no TX FIFO handling), therefore we do not want
> +         * to change the TX FIFO status in such case.
> +         */
> +        if ( vpl011->backend_in_domain )
> +            vpl011_update_tx_fifo_status(vpl011, out_fifo_level);
>      }
>  
>      vpl011_update_interrupt_status(d);
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
  2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
  2023-04-06 10:38   ` Ayan Kumar Halder
@ 2023-04-12  6:21   ` Henry Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Henry Wang @ 2023-04-12  6:21 UTC (permalink / raw
  To: Michal Orzel, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

Hi Michal,

> -----Original Message-----
> Subject: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
> 
> In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
> stating that the guest is expected to read the DR register only if the
> TXFE bit of FR register is not set. This is obviously logically wrong and
> it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status when backend in Xen
  2023-04-05 11:17 ` [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status " Michal Orzel
  2023-04-07 21:45   ` Stefano Stabellini
@ 2023-04-12  6:55   ` Henry Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Henry Wang @ 2023-04-12  6:55 UTC (permalink / raw
  To: Michal Orzel, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

Hi Michal,

> -----Original Message-----
> Subject: [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status
> when backend in Xen
> 
> From vpl011_rx_char_xen(), we call vpl011_data_avail() that handles both
> RX and TX state. Because we are passing 0 as out_fifo_level and
> SBSA_UART_FIFO_SIZE as out_size, we end up calling a function
> vpl011_update_tx_fifo_status() which performs TXI bit handling
> depending on the FIFO trigger level. This does not make sense when backend
> is in Xen, as we maintain a single TX state where data can always be
> written and as such there is no TX FIFO handling. Furthermore, this
> function assumes that the backend is in domain by making use of struct
> xencons_interface unconditionally. Fix it by calling this function only
> when backend is in domain. Also add an assert for sanity.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

I've tested the patch by manually running the XTP on FVP_Base, and this
patch works fine. So:

Tested-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen
  2023-04-05 11:17 ` [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen Michal Orzel
  2023-04-07 21:42   ` Stefano Stabellini
@ 2023-04-12  7:17   ` Henry Wang
  1 sibling, 0 replies; 11+ messages in thread
From: Henry Wang @ 2023-04-12  7:17 UTC (permalink / raw
  To: Michal Orzel, xen-devel@lists.xenproject.org
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

Hi Michal,

> -----Original Message-----
> Subject: [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in
> Xen
> 
> When backend is in Xen, the handling of data written to DR register is a
> bit special because we want to tell guest that we are always ready for new
> data to be written (i.e. no real FIFO, TXFF/BUSY never set and TXI always
> set). This conflicts with the current handling of TXFE bit, which we
> always clear and never set on a write path (we happen to set it when we
> receive char from serial input due to use of vpl011_data_avail() but this
> might never be called). This can lead to issues if a guest driver makes
> use of TXFE bit to check for TX transmission completion (such guest could
> then wait endlessly). Fix it by keeping TXFE always set to match the
> current emulation logic.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Tested-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-12  7:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-05 11:17 [PATCH 0/3] xen/arm: vpl011 fixes continuation Michal Orzel
2023-04-05 11:17 ` [PATCH 1/3] xen/arm: vpl011: Fix misleading comments Michal Orzel
2023-04-06 10:38   ` Ayan Kumar Halder
2023-04-07 21:33     ` Stefano Stabellini
2023-04-12  6:21   ` Henry Wang
2023-04-05 11:17 ` [PATCH 2/3] xen/arm: vpl011: Handle correctly TXFE when backend in Xen Michal Orzel
2023-04-07 21:42   ` Stefano Stabellini
2023-04-12  7:17   ` Henry Wang
2023-04-05 11:17 ` [PATCH 3/3] xen/arm: vpl011: Do not try to handle TX FIFO status " Michal Orzel
2023-04-07 21:45   ` Stefano Stabellini
2023-04-12  6:55   ` Henry Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).