qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/3] dp8393x: fix dp8393x_receive()


From: Hervé Poussineau
Subject: Re: [PATCH 2/3] dp8393x: fix dp8393x_receive()
Date: Wed, 6 Nov 2019 07:13:51 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0

Le 05/11/2019 à 22:53, Laurent Vivier a écrit :
Le 05/11/2019 à 22:06, Hervé Poussineau a écrit :
Le 02/11/2019 à 18:15, Laurent Vivier a écrit :
address_space_rw() access size must be multiplied by the width.

This fixes DHCP for Q800 guest.

Signed-off-by: Laurent Vivier <address@hidden>
---
   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 85d3f3788e..b8c4473f99 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -833,7 +833,7 @@ static ssize_t dp8393x_receive(NetClientState *nc,
const uint8_t * buf,
       } else {
           dp8393x_put(s, width, 0, 0); /* in_use */
           address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t)
* 6 * width,
-            MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data,
sizeof(uint16_t), 1);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1);
           s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
           s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
           s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) |
(((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);


This patch is problematic.
The code was initially created with "size".
It was changed in 409b52bfe199d8106dadf7c5ff3d88d2228e89b5 to fix
networking in NetBSD 5.1.

To test with NetBSD 5.1
- boot the installer (arccd-5.1.iso)
- choose (S)hell option
- "ifconfig sn0 10.0.2.15 netmask 255.255.255.0"
- "route add default 10.0.2.2"
- networking should work (I test with "ftp 212.27.63.3")

I've the firmware from
http://hpoussineau.free.fr/qemu/firmware/magnum-4000/setup.zip
Which file to use? NTPROM.RAW?

Without this patch, I get the FTP banner.
With this patch, connection can't be established.

In datasheet page 17, you can see the "Receive Descriptor Format", which
contains the in_use field.
It is clearly said that RXpkt.in_use is 16 bit wide, and that the bits
16-31 are not used in 32-bit mode.

So, I don't see why you need to clear 32 bits in 32-bit mode. Maybe you
need to clear only the other
16 bits ? Maybe it depends of endianness ?

Thank you for the details. I think the problem should likely come from
the endianness.

The offset must be adjusted according to the access mode (endianness and
size).

The following patch fixes the problem for me, and should not break other
targets:

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 85d3f3788e..3d991af163 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -831,9 +831,15 @@ static ssize_t dp8393x_receive(NetClientState *nc,
const uint8_t * buf,
          /* EOL detected */
          s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
      } else {
-        dp8393x_put(s, width, 0, 0); /* in_use */
-        address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 6
* width,
-            MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data,
sizeof(uint16_t), 1);
+        /* Clear in_use, but it is always 16bit wide */
+        int offset = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width;
+        if (s->big_endian && width == 2) {
+            /* we need to adjust the offset of the 16bit field */
+            offset += sizeof(uint16_t);
+        }
+        s->data[0] = 0;
+        address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED,
+                         (uint8_t *)s->data, sizeof(uint16_t), 1);
          s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
          s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
          s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) |
(((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);

What is your opinion?

This one works for NetBSD.
Tested-by: Hervé Poussineau <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]