Re: [Qemu-devel] [PATCH] dp8393x: fix dp8393x_receive

[Jason Wang]

On 2017年01月03日 15:26, Laurent Vivier wrote:
> Le 03/01/2017 à 04:37, Jason Wang a écrit :
> > On 2017年01月02日 07:00, Laurent Vivier wrote:
> > > address_space_rw() access size must be multiplied by width.
> > > dp8393x_receive() must return the number of bytes read, not the length
> > > of the last memory access.
> > >
> > > Signed-off-by: Laurent Vivier <address@hidden>
> > > ---
> > >    hw/net/dp8393x.c | 5 +++--
> > >    1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> > > index 17f0338..3506bca 100644
> > > --- a/hw/net/dp8393x.c
> > > +++ b/hw/net/dp8393x.c
> > > @@ -766,10 +766,11 @@ static ssize_t dp8393x_receive(NetClientState
> > > *nc, const uint8_t * buf,
> > >            /* EOL detected */
> > >            s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
> > >        } else {
> > > +        size = sizeof(uint16_t) * width;
> > >            data[0 * width] = 0; /* in_use */
> > >            address_space_rw(&s->as,
> > >                ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) +
> > > sizeof(uint16_t) * 6 * width,
> > > -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data,
> > > sizeof(uint16_t), 1);
> > > +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
> > I think patch makes different only when width is 2. But if we just want
> > to tag in_use flag, sizeof(uint16_t) should be sufficient?
> In fact, I'm using this device to implement Quadra 800 macsonic
> emulation, and in this case all registers are byteswapped according the
> size of width (32bit in my case, see kernel sources,
> drivers/net/ethernet/natsemi/sonic.h and macsonic.c), so debugging the
> kernel I've seen used part of the register is never cleared.

Looking at git history this seems a revert of 
409b52bfe199d8106dadf7c5ff3d88d2228e89b5 ("net/dp8393x: correctly reset 
in_use field") which claims to fix an issue in NetBSD 5.1. According to 
the spec (is this correct 
http://www.ti.com/lit/ds/symlink/dp83936avul-33.pdf?) upper 16 bits were 
not used in 32bit mode, so I'm not sure this is correct.

Hope Hervé or Aurelien can review this patch.

Thanks

> Thanks,
> Laurent