[PATCH resend v2] net: cadence_gem: clear RX control descriptor

Ramon Fried posted 1 patch 4 years ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200418085145.489726-1-rfried.dev@gmail.com
hw/net/cadence_gem.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH resend v2] net: cadence_gem: clear RX control descriptor
Posted by Ramon Fried 4 years ago
The RX ring descriptors control field is used for setting
SOF and EOF (start of frame and end of frame).
The SOF and EOF weren't cleared from the previous descriptors,
causing inconsistencies in ring buffer.
Fix that by clearing the control field of every descriptors we're
processing.

Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

---
v2:
  * change function name to rx_desc_clear_control as proposed by
      Jason Wang
  * Move the function call above the comment, as proposed by
    Philippe Mathieu-Daudé

 hw/net/cadence_gem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index b7b7985bf2..22a0b1b1f9 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -411,6 +411,11 @@ static inline void rx_desc_set_sof(uint32_t *desc)
     desc[1] |= DESC_1_RX_SOF;
 }
 
+static inline void rx_desc_clear_control(uint32_t *desc)
+{
+    desc[1]  = 0;
+}
+
 static inline void rx_desc_set_eof(uint32_t *desc)
 {
     desc[1] |= DESC_1_RX_EOF;
@@ -999,6 +1004,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
         bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
 
+        rx_desc_clear_control(s->rx_desc[q]);
+
         /* Update the descriptor.  */
         if (first_desc) {
             rx_desc_set_sof(s->rx_desc[q]);
-- 
2.26.0


Re: [PATCH resend v2] net: cadence_gem: clear RX control descriptor
Posted by Edgar E. Iglesias 4 years ago
On Sat, Apr 18, 2020 at 11:51:45AM +0300, Ramon Fried wrote:
> The RX ring descriptors control field is used for setting
> SOF and EOF (start of frame and end of frame).
> The SOF and EOF weren't cleared from the previous descriptors,
> causing inconsistencies in ring buffer.
> Fix that by clearing the control field of every descriptors we're
> processing.
> 
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> ---
> v2:
>   * change function name to rx_desc_clear_control as proposed by
>       Jason Wang
>   * Move the function call above the comment, as proposed by
>     Philippe Mathieu-Daudé
> 
>  hw/net/cadence_gem.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index b7b7985bf2..22a0b1b1f9 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -411,6 +411,11 @@ static inline void rx_desc_set_sof(uint32_t *desc)
>      desc[1] |= DESC_1_RX_SOF;
>  }
>  
> +static inline void rx_desc_clear_control(uint32_t *desc)
> +{
> +    desc[1]  = 0;
> +}
> +
>  static inline void rx_desc_set_eof(uint32_t *desc)
>  {
>      desc[1] |= DESC_1_RX_EOF;
> @@ -999,6 +1004,8 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>          rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
>          bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
>  
> +        rx_desc_clear_control(s->rx_desc[q]);
> +
>          /* Update the descriptor.  */
>          if (first_desc) {
>              rx_desc_set_sof(s->rx_desc[q]);
> -- 
> 2.26.0
> 

Re: [PATCH resend v2] net: cadence_gem: clear RX control descriptor
Posted by Peter Maydell 4 years ago
On Sat, 18 Apr 2020 at 09:52, Ramon Fried <rfried.dev@gmail.com> wrote:
>
> The RX ring descriptors control field is used for setting
> SOF and EOF (start of frame and end of frame).
> The SOF and EOF weren't cleared from the previous descriptors,
> causing inconsistencies in ring buffer.
> Fix that by clearing the control field of every descriptors we're
> processing.
>
> Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



Applied to target-arm.next for 5.1, thanks.

-- PMM