[v2] Fixes for DP8393X SONIC device emulation

[PATCH v2 13/13] dp8393x: Correctly advance RRP

Posted by Finn Thain 6 years, 1 month ago

The last entry in the RRA is at the address given by the REA register.
The address wrap-around logic is off-by-one entry. The last resource
never gets used and RRP can jump over the RWP. The guest driver fails
badly because the SONIC starts re-using old buffer addresses. Fix this.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index bd92fa28f6..92a30f9f69 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -340,7 +340,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
     s->regs[SONIC_RRP] += size;
 
     /* Handle wrap */
-    if (s->regs[SONIC_RRP] == s->regs[SONIC_REA]) {
+    if (s->regs[SONIC_RRP] == s->regs[SONIC_REA] + size) {
         s->regs[SONIC_RRP] = s->regs[SONIC_RSA];
     }
 
-- 
2.23.0

Re: [PATCH v2 13/13] dp8393x: Correctly advance RRP

Posted by Finn Thain 6 years, 1 month ago

Please disregard this patch. An off-by-one bug was found in one of my 
Linux sonic driver patches. When I fixed that bug, this patch (13/13) was 
shown to be incorrect.

The Linux sonic driver patches are being tested on actual SONIC hardware 
(Mac Centris 610). I will send v3 of this series after I've finished 
debugging the Linux sonic driver.

On Fri, 20 Dec 2019, Finn Thain wrote:

> The last entry in the RRA is at the address given by the REA register.
> The address wrap-around logic is off-by-one entry. The last resource
> never gets used and RRP can jump over the RWP. The guest driver fails
> badly because the SONIC starts re-using old buffer addresses. Fix this.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>  hw/net/dp8393x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index bd92fa28f6..92a30f9f69 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -340,7 +340,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
>      s->regs[SONIC_RRP] += size;
>  
>      /* Handle wrap */
> -    if (s->regs[SONIC_RRP] == s->regs[SONIC_REA]) {
> +    if (s->regs[SONIC_RRP] == s->regs[SONIC_REA] + size) {
>          s->regs[SONIC_RRP] = s->regs[SONIC_RSA];
>      }
>  
>