| Message ID | 20210703141947.352295-7-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dp8393x: Housekeeping | expand |
On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote: > Instead of accessing N registers via a single address_space API > call using a temporary buffer (stored in the device state) and > updating each register, move the address_space call in the > register put/get. The load/store and word size checks are moved > to put/get too. This simplifies a bit, making the code easier > to read. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/net/dp8393x.c | 157 ++++++++++++++++++----------------------------- > 1 file changed, 60 insertions(+), 97 deletions(-) > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index bbe241ef9db..db9cfd786f5 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -162,7 +162,6 @@ struct dp8393xState { > > /* Temporaries */ > uint8_t tx_buffer[0x10000]; > - uint16_t data[12]; > int loopback_packet; > > /* Memory access */ > @@ -219,34 +218,48 @@ static uint32_t dp8393x_wt(dp8393xState *s) > return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; > } > > -static uint16_t dp8393x_get(dp8393xState *s, int width, int offset) > +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) > { > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > uint16_t val; > > - if (s->big_endian) { > - val = be16_to_cpu(s->data[offset * width + width - 1]); > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > + addr += 2 * ofs16; > + if (s->big_endian) { > + val = address_space_ldl_be(&s->as, addr, attrs, NULL); > + } else { > + val = address_space_ldl_le(&s->as, addr, attrs, NULL); > + } > } else { > - val = le16_to_cpu(s->data[offset * width]); > + addr += 1 * ofs16; > + if (s->big_endian) { > + val = address_space_lduw_be(&s->as, addr, attrs, NULL); > + } else { > + val = address_space_lduw_le(&s->as, addr, attrs, NULL); > + } > } > + > return val; > } > > -static void dp8393x_put(dp8393xState *s, int width, int offset, > - uint16_t val) > +static void dp8393x_put(dp8393xState *s, > + hwaddr addr, unsigned ofs16, uint16_t val) > { > - if (s->big_endian) { > - if (width == 2) { > - s->data[offset * 2] = 0; > - s->data[offset * 2 + 1] = cpu_to_be16(val); > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > + > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > + addr += 2 * ofs16; > + if (s->big_endian) { > + address_space_stl_be(&s->as, addr, val, attrs, NULL); > } else { > - s->data[offset] = cpu_to_be16(val); > + address_space_stl_le(&s->as, addr, val, attrs, NULL); > } > } else { > - if (width == 2) { > - s->data[offset * 2] = cpu_to_le16(val); > - s->data[offset * 2 + 1] = 0; > + addr += 1 * ofs16; > + if (s->big_endian) { > + address_space_stw_be(&s->as, addr, val, attrs, NULL); > } else { > - s->data[offset] = cpu_to_le16(val); > + address_space_stw_le(&s->as, addr, val, attrs, NULL); > } > } > } > @@ -277,12 +290,10 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_read(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - index = dp8393x_get(s, width, 0) & 0xf; > - s->cam[index][0] = dp8393x_get(s, width, 1); > - s->cam[index][1] = dp8393x_get(s, width, 2); > - s->cam[index][2] = dp8393x_get(s, width, 3); > + index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf; > + s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1); > + s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2); > + s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3); > trace_dp8393x_load_cam(index, > s->cam[index][0] >> 8, s->cam[index][0] & 0xff, > s->cam[index][1] >> 8, s->cam[index][1] & 0xff, > @@ -293,9 +304,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_read(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > + s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0); > trace_dp8393x_load_cam_done(s->regs[SONIC_CE]); > > /* Done */ > @@ -311,14 +320,12 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_read(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > /* Update SONIC registers */ > - s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > - s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1); > - s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2); > - s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3); > + s->regs[SONIC_CRBA0] = dp8393x_get(s, dp8393x_rrp(s), 0); > + s->regs[SONIC_CRBA1] = dp8393x_get(s, dp8393x_rrp(s), 1); > + s->regs[SONIC_RBWC0] = dp8393x_get(s, dp8393x_rrp(s), 2); > + s->regs[SONIC_RBWC1] = dp8393x_get(s, dp8393x_rrp(s), 3); > trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1], > s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]); > > @@ -414,28 +421,22 @@ static void dp8393x_do_receiver_disable(dp8393xState *s) > static void dp8393x_do_transmit_packets(dp8393xState *s) > { > NetClientState *nc = qemu_get_queue(s->nic); > - int width, size; > int tx_len, len; > uint16_t i; > > - width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > - > while (1) { > /* Read memory */ > - size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > trace_dp8393x_transmit_packet(dp8393x_ttda(s)); > - address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, s->data, size); > tx_len = 0; > > /* Update registers */ > - s->regs[SONIC_TCR] = dp8393x_get(s, width, 0) & 0xf000; > - s->regs[SONIC_TPS] = dp8393x_get(s, width, 1); > - s->regs[SONIC_TFC] = dp8393x_get(s, width, 2); > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 3); > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 4); > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 5); > + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; > + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); > + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); > > /* Handle programmable interrupt */ > if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { > @@ -457,15 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > - size = sizeof(uint16_t) * 3 * width; > - address_space_read(&s->as, > - dp8393x_ttda(s) > - + sizeof(uint16_t) * width * (4 + 3 * i), > - MEMTXATTRS_UNSPECIFIED, s->data, > - size); > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); > } > } > > @@ -498,22 +493,12 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > s->regs[SONIC_TCR] |= SONIC_TCR_PTX; > > /* Write status */ > - dp8393x_put(s, width, 0, > - s->regs[SONIC_TCR] & 0x0fff); /* status */ > - size = sizeof(uint16_t) * width; > - address_space_write(&s->as, dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > + dp8393x_put(s, dp8393x_ttda(s), 0, s->regs[SONIC_TCR] & 0x0fff); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > - size = sizeof(uint16_t) * width; > - address_space_read(&s->as, > - dp8393x_ttda(s) > - + sizeof(uint16_t) * width > - * (4 + 3 * s->regs[SONIC_TFC]), > - MEMTXATTRS_UNSPECIFIED, s->data, > - size); > - s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_CTDA] = dp8393x_get(s, dp8393x_ttda(s), > + 4 + 3 * s->regs[SONIC_TFC]); > if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) { > /* EOL detected */ > break; > @@ -762,7 +747,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393xState *s = qemu_get_nic_opaque(nc); > int packet_type; > uint32_t available, address; > - int width, rx_len, padded_len; > + int rx_len, padded_len; > uint32_t checksum; > int size; > > @@ -775,10 +760,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > > rx_len = pkt_size + sizeof(checksum); > if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > - width = 2; > padded_len = ((rx_len - 1) | 3) + 1; > } else { > - width = 1; > padded_len = ((rx_len - 1) | 1) + 1; > } > > @@ -799,11 +782,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Check for EOL */ > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* Are we still in resource exhaustion? */ > - size = sizeof(uint16_t) * 1 * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - s->data, size); > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* Still EOL ; stop reception */ > return -1; > @@ -811,11 +790,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Link has been updated by host */ > > /* Clear in_use */ > - size = sizeof(uint16_t) * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > - dp8393x_put(s, width, 0, 0); > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > @@ -869,32 +844,20 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > > /* Write status to memory */ > trace_dp8393x_receive_write_status(dp8393x_crda(s)); > - dp8393x_put(s, width, 0, s->regs[SONIC_RCR]); /* status */ > - dp8393x_put(s, width, 1, rx_len); /* byte count */ > - dp8393x_put(s, width, 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ > - dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > - dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > - size = sizeof(uint16_t) * 5 * width; > - address_space_write(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, > - s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 0, s->regs[SONIC_RCR]); /* status */ > + dp8393x_put(s, dp8393x_crda(s), 1, rx_len); /* byte count */ > + dp8393x_put(s, dp8393x_crda(s), 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ > + dp8393x_put(s, dp8393x_crda(s), 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > + dp8393x_put(s, dp8393x_crda(s), 4, s->regs[SONIC_RSC]); /* seq_no */ > > /* Check link field */ > - size = sizeof(uint16_t) * width; > - address_space_read(&s->as, > - dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* EOL detected */ > s->regs[SONIC_ISR] |= SONIC_ISR_RDE; > } else { > /* Clear in_use */ > - size = sizeof(uint16_t) * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > - dp8393x_put(s, width, 0, 0); > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; This seems like a nice tidy-up. I'll need a bit of time to give some more R-b and T-b tags since MacOS is running on a very diverged branch, so I'll need to pick through and update the changes to run across all my test images accordingly. With this patch is it now possible to remove s->data completely? ATB, Mark.
On 7/3/21 5:00 PM, Mark Cave-Ayland wrote: > On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote: > >> Instead of accessing N registers via a single address_space API >> call using a temporary buffer (stored in the device state) and >> updating each register, move the address_space call in the >> register put/get. The load/store and word size checks are moved >> to put/get too. This simplifies a bit, making the code easier >> to read. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/net/dp8393x.c | 157 ++++++++++++++++++----------------------------- >> 1 file changed, 60 insertions(+), 97 deletions(-) >> >> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c >> index bbe241ef9db..db9cfd786f5 100644 >> --- a/hw/net/dp8393x.c >> +++ b/hw/net/dp8393x.c >> @@ -162,7 +162,6 @@ struct dp8393xState { >> /* Temporaries */ >> uint8_t tx_buffer[0x10000]; >> - uint16_t data[12]; >> int loopback_packet; >> /* Memory access */ > This seems like a nice tidy-up. I'll need a bit of time to give some > more R-b and T-b tags since MacOS is running on a very diverged branch, > so I'll need to pick through and update the changes to run across all my > test images accordingly. No hurry. > With this patch is it now possible to remove s->data completely? First line remove it ;) (It was not part of the migration stream).
On Sat, 3 Jul 2021, Philippe Mathieu-Daudé wrote: > Instead of accessing N registers via a single address_space API > call using a temporary buffer (stored in the device state) and > updating each register, move the address_space call in the > register put/get. The load/store and word size checks are moved > to put/get too. This simplifies a bit, making the code easier > to read. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> I tried this series with a Linux/m68k guest but network activity hanged the emulator. The cause of the problem is somewhere in this patch. BTW, I've become suspicious of the word "rewrite". In this case I think it describes a commit that is attempting to do too much and needs to be split up to make it easier to review (and debug).
On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote: > Instead of accessing N registers via a single address_space API > call using a temporary buffer (stored in the device state) and > updating each register, move the address_space call in the > register put/get. The load/store and word size checks are moved > to put/get too. This simplifies a bit, making the code easier > to read. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/net/dp8393x.c | 157 ++++++++++++++++++----------------------------- > 1 file changed, 60 insertions(+), 97 deletions(-) > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index bbe241ef9db..db9cfd786f5 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -162,7 +162,6 @@ struct dp8393xState { > > /* Temporaries */ > uint8_t tx_buffer[0x10000]; > - uint16_t data[12]; > int loopback_packet; > > /* Memory access */ > @@ -219,34 +218,48 @@ static uint32_t dp8393x_wt(dp8393xState *s) > return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; > } > > -static uint16_t dp8393x_get(dp8393xState *s, int width, int offset) > +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) > { > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > uint16_t val; > > - if (s->big_endian) { > - val = be16_to_cpu(s->data[offset * width + width - 1]); > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > + addr += 2 * ofs16; > + if (s->big_endian) { > + val = address_space_ldl_be(&s->as, addr, attrs, NULL); > + } else { > + val = address_space_ldl_le(&s->as, addr, attrs, NULL); > + } > } else { > - val = le16_to_cpu(s->data[offset * width]); > + addr += 1 * ofs16; > + if (s->big_endian) { > + val = address_space_lduw_be(&s->as, addr, attrs, NULL); > + } else { > + val = address_space_lduw_le(&s->as, addr, attrs, NULL); > + } > } > + > return val; > } > > -static void dp8393x_put(dp8393xState *s, int width, int offset, > - uint16_t val) > +static void dp8393x_put(dp8393xState *s, > + hwaddr addr, unsigned ofs16, uint16_t val) > { > - if (s->big_endian) { > - if (width == 2) { > - s->data[offset * 2] = 0; > - s->data[offset * 2 + 1] = cpu_to_be16(val); > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > + > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > + addr += 2 * ofs16; > + if (s->big_endian) { > + address_space_stl_be(&s->as, addr, val, attrs, NULL); > } else { > - s->data[offset] = cpu_to_be16(val); > + address_space_stl_le(&s->as, addr, val, attrs, NULL); > } > } else { > - if (width == 2) { > - s->data[offset * 2] = cpu_to_le16(val); > - s->data[offset * 2 + 1] = 0; > + addr += 1 * ofs16; > + if (s->big_endian) { > + address_space_stw_be(&s->as, addr, val, attrs, NULL); > } else { > - s->data[offset] = cpu_to_le16(val); > + address_space_stw_le(&s->as, addr, val, attrs, NULL); > } > } > } > @@ -277,12 +290,10 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > /* Fill current entry */ > - address_space_read(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - index = dp8393x_get(s, width, 0) & 0xf; > - s->cam[index][0] = dp8393x_get(s, width, 1); > - s->cam[index][1] = dp8393x_get(s, width, 2); > - s->cam[index][2] = dp8393x_get(s, width, 3); > + index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf; > + s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1); > + s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2); > + s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3); > trace_dp8393x_load_cam(index, > s->cam[index][0] >> 8, s->cam[index][0] & 0xff, > s->cam[index][1] >> 8, s->cam[index][1] & 0xff, > @@ -293,9 +304,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) > } > > /* Read CAM enable */ > - address_space_read(&s->as, dp8393x_cdp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > + s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0); > trace_dp8393x_load_cam_done(s->regs[SONIC_CE]); > > /* Done */ > @@ -311,14 +320,12 @@ static void dp8393x_do_read_rra(dp8393xState *s) > /* Read memory */ > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > size = sizeof(uint16_t) * 4 * width; > - address_space_read(&s->as, dp8393x_rrp(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > /* Update SONIC registers */ > - s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > - s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1); > - s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2); > - s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3); > + s->regs[SONIC_CRBA0] = dp8393x_get(s, dp8393x_rrp(s), 0); > + s->regs[SONIC_CRBA1] = dp8393x_get(s, dp8393x_rrp(s), 1); > + s->regs[SONIC_RBWC0] = dp8393x_get(s, dp8393x_rrp(s), 2); > + s->regs[SONIC_RBWC1] = dp8393x_get(s, dp8393x_rrp(s), 3); > trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1], > s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]); > > @@ -414,28 +421,22 @@ static void dp8393x_do_receiver_disable(dp8393xState *s) > static void dp8393x_do_transmit_packets(dp8393xState *s) > { > NetClientState *nc = qemu_get_queue(s->nic); > - int width, size; > int tx_len, len; > uint16_t i; > > - width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > - > while (1) { > /* Read memory */ > - size = sizeof(uint16_t) * 6 * width; > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > trace_dp8393x_transmit_packet(dp8393x_ttda(s)); > - address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, > - MEMTXATTRS_UNSPECIFIED, s->data, size); > tx_len = 0; > > /* Update registers */ > - s->regs[SONIC_TCR] = dp8393x_get(s, width, 0) & 0xf000; > - s->regs[SONIC_TPS] = dp8393x_get(s, width, 1); > - s->regs[SONIC_TFC] = dp8393x_get(s, width, 2); > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 3); > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 4); > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 5); > + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; > + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); > + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); > > /* Handle programmable interrupt */ > if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { > @@ -457,15 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > - size = sizeof(uint16_t) * 3 * width; > - address_space_read(&s->as, > - dp8393x_ttda(s) > - + sizeof(uint16_t) * width * (4 + 3 * i), > - MEMTXATTRS_UNSPECIFIED, s->data, > - size); > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); > } > } > > @@ -498,22 +493,12 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > s->regs[SONIC_TCR] |= SONIC_TCR_PTX; > > /* Write status */ > - dp8393x_put(s, width, 0, > - s->regs[SONIC_TCR] & 0x0fff); /* status */ > - size = sizeof(uint16_t) * width; > - address_space_write(&s->as, dp8393x_ttda(s), > - MEMTXATTRS_UNSPECIFIED, s->data, size); > + dp8393x_put(s, dp8393x_ttda(s), 0, s->regs[SONIC_TCR] & 0x0fff); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > /* Read footer of packet */ > - size = sizeof(uint16_t) * width; > - address_space_read(&s->as, > - dp8393x_ttda(s) > - + sizeof(uint16_t) * width > - * (4 + 3 * s->regs[SONIC_TFC]), > - MEMTXATTRS_UNSPECIFIED, s->data, > - size); > - s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_CTDA] = dp8393x_get(s, dp8393x_ttda(s), > + 4 + 3 * s->regs[SONIC_TFC]); > if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) { > /* EOL detected */ > break; > @@ -762,7 +747,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > dp8393xState *s = qemu_get_nic_opaque(nc); > int packet_type; > uint32_t available, address; > - int width, rx_len, padded_len; > + int rx_len, padded_len; > uint32_t checksum; > int size; > > @@ -775,10 +760,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > > rx_len = pkt_size + sizeof(checksum); > if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > - width = 2; > padded_len = ((rx_len - 1) | 3) + 1; > } else { > - width = 1; > padded_len = ((rx_len - 1) | 1) + 1; > } > > @@ -799,11 +782,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Check for EOL */ > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* Are we still in resource exhaustion? */ > - size = sizeof(uint16_t) * 1 * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > - address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - s->data, size); > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* Still EOL ; stop reception */ > return -1; > @@ -811,11 +790,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > /* Link has been updated by host */ > > /* Clear in_use */ > - size = sizeof(uint16_t) * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > - dp8393x_put(s, width, 0, 0); > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - (uint8_t *)s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > @@ -869,32 +844,20 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, > > /* Write status to memory */ > trace_dp8393x_receive_write_status(dp8393x_crda(s)); > - dp8393x_put(s, width, 0, s->regs[SONIC_RCR]); /* status */ > - dp8393x_put(s, width, 1, rx_len); /* byte count */ > - dp8393x_put(s, width, 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ > - dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > - dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > - size = sizeof(uint16_t) * 5 * width; > - address_space_write(&s->as, dp8393x_crda(s), > - MEMTXATTRS_UNSPECIFIED, > - s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 0, s->regs[SONIC_RCR]); /* status */ > + dp8393x_put(s, dp8393x_crda(s), 1, rx_len); /* byte count */ > + dp8393x_put(s, dp8393x_crda(s), 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ > + dp8393x_put(s, dp8393x_crda(s), 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > + dp8393x_put(s, dp8393x_crda(s), 4, s->regs[SONIC_RSC]); /* seq_no */ > > /* Check link field */ > - size = sizeof(uint16_t) * width; > - address_space_read(&s->as, > - dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > - MEMTXATTRS_UNSPECIFIED, s->data, size); > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > /* EOL detected */ > s->regs[SONIC_ISR] |= SONIC_ISR_RDE; > } else { > /* Clear in_use */ > - size = sizeof(uint16_t) * width; > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > - dp8393x_put(s, width, 0, 0); > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > - s->data, size); > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; This patch breaks networking in its current form, but I was able to make it work by applying the following diff: diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index db9cfd786f..b08843172b 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -218,20 +218,20 @@ static uint32_t dp8393x_wt(dp8393xState *s) return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; } -static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, int offset) { const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; uint16_t val; if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { - addr += 2 * ofs16; + addr += offset << 2; if (s->big_endian) { val = address_space_ldl_be(&s->as, addr, attrs, NULL); } else { val = address_space_ldl_le(&s->as, addr, attrs, NULL); } } else { - addr += 1 * ofs16; + addr += offset << 1; if (s->big_endian) { val = address_space_lduw_be(&s->as, addr, attrs, NULL); } else { @@ -243,19 +243,19 @@ static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) } static void dp8393x_put(dp8393xState *s, - hwaddr addr, unsigned ofs16, uint16_t val) + hwaddr addr, int offset, uint16_t val) { const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { - addr += 2 * ofs16; + addr += offset << 2; if (s->big_endian) { address_space_stl_be(&s->as, addr, val, attrs, NULL); } else { address_space_stl_le(&s->as, addr, val, attrs, NULL); } } else { - addr += 1 * ofs16; + addr += offset << 1; if (s->big_endian) { address_space_stw_be(&s->as, addr, val, attrs, NULL); } else { @@ -431,12 +431,12 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) tx_len = 0; /* Update registers */ - s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; - s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); - s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); - s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); - s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); - s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 1) & 0xf000; + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 2); + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 3); + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 4); + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 5); + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 6); /* Handle programmable interrupt */ if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { @@ -458,9 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) i++; if (i != s->regs[SONIC_TFC]) { /* Read next fragment details */ - s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); - s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); - s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 4 + 3 * i); + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 5 + 3 * i); + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 6 + 3 * i); } } The main change is that the offset argument for dp8393x_get() and dp8393x_put() is actually an entry number and not a count of 16 bit words. Other than that there were a couple of offset calculations that needed adjustment to get things working again. Taking git master and applying your outstanding PR + this series without patch 3 + this diff gave me working networking on Linux/m68k, MacOS 8.0 and NetBSD/arc. Unfortunately I don't have a test mips64el image available to see if this combination works for Linux. Phil, do you have a suitable test kernel and rootfs image available to allow this to be tested? ATB, Mark.
On Sun, 4 Jul 2021, Mark Cave-Ayland wrote: > On 03/07/2021 15:19, Philippe Mathieu-Daudé wrote: > > > Instead of accessing N registers via a single address_space API > > call using a temporary buffer (stored in the device state) and > > updating each register, move the address_space call in the > > register put/get. The load/store and word size checks are moved > > to put/get too. This simplifies a bit, making the code easier > > to read. > > > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > --- > > hw/net/dp8393x.c | 157 ++++++++++++++++++----------------------------- > > 1 file changed, 60 insertions(+), 97 deletions(-) > > > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > > index bbe241ef9db..db9cfd786f5 100644 > > --- a/hw/net/dp8393x.c > > +++ b/hw/net/dp8393x.c > > @@ -162,7 +162,6 @@ struct dp8393xState { > > /* Temporaries */ > > uint8_t tx_buffer[0x10000]; > > - uint16_t data[12]; > > int loopback_packet; > > /* Memory access */ > > @@ -219,34 +218,48 @@ static uint32_t dp8393x_wt(dp8393xState *s) > > return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; > > } > > -static uint16_t dp8393x_get(dp8393xState *s, int width, int offset) > > +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) > > { > > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > uint16_t val; > > - if (s->big_endian) { > > - val = be16_to_cpu(s->data[offset * width + width - 1]); > > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > > + addr += 2 * ofs16; > > + if (s->big_endian) { > > + val = address_space_ldl_be(&s->as, addr, attrs, NULL); > > + } else { > > + val = address_space_ldl_le(&s->as, addr, attrs, NULL); > > + } > > } else { > > - val = le16_to_cpu(s->data[offset * width]); > > + addr += 1 * ofs16; > > + if (s->big_endian) { > > + val = address_space_lduw_be(&s->as, addr, attrs, NULL); > > + } else { > > + val = address_space_lduw_le(&s->as, addr, attrs, NULL); > > + } > > } > > + > > return val; > > } > > -static void dp8393x_put(dp8393xState *s, int width, int offset, > > - uint16_t val) > > +static void dp8393x_put(dp8393xState *s, > > + hwaddr addr, unsigned ofs16, uint16_t val) > > { > > - if (s->big_endian) { > > - if (width == 2) { > > - s->data[offset * 2] = 0; > > - s->data[offset * 2 + 1] = cpu_to_be16(val); > > + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > + > > + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > > + addr += 2 * ofs16; > > + if (s->big_endian) { > > + address_space_stl_be(&s->as, addr, val, attrs, NULL); > > } else { > > - s->data[offset] = cpu_to_be16(val); > > + address_space_stl_le(&s->as, addr, val, attrs, NULL); > > } > > } else { > > - if (width == 2) { > > - s->data[offset * 2] = cpu_to_le16(val); > > - s->data[offset * 2 + 1] = 0; > > + addr += 1 * ofs16; > > + if (s->big_endian) { > > + address_space_stw_be(&s->as, addr, val, attrs, NULL); > > } else { > > - s->data[offset] = cpu_to_le16(val); > > + address_space_stw_le(&s->as, addr, val, attrs, NULL); > > } > > } > > } > > @@ -277,12 +290,10 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > while (s->regs[SONIC_CDC] & 0x1f) { > > /* Fill current entry */ > > - address_space_read(&s->as, dp8393x_cdp(s), > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > - index = dp8393x_get(s, width, 0) & 0xf; > > - s->cam[index][0] = dp8393x_get(s, width, 1); > > - s->cam[index][1] = dp8393x_get(s, width, 2); > > - s->cam[index][2] = dp8393x_get(s, width, 3); > > + index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf; > > + s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1); > > + s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2); > > + s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3); > > trace_dp8393x_load_cam(index, > > s->cam[index][0] >> 8, s->cam[index][0] & > > 0xff, > > s->cam[index][1] >> 8, s->cam[index][1] & > > 0xff, > > @@ -293,9 +304,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) > > } > > /* Read CAM enable */ > > - address_space_read(&s->as, dp8393x_cdp(s), > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > - s->regs[SONIC_CE] = dp8393x_get(s, width, 0); > > + s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0); > > trace_dp8393x_load_cam_done(s->regs[SONIC_CE]); > > /* Done */ > > @@ -311,14 +320,12 @@ static void dp8393x_do_read_rra(dp8393xState *s) > > /* Read memory */ > > width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > > size = sizeof(uint16_t) * 4 * width; > > - address_space_read(&s->as, dp8393x_rrp(s), > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > /* Update SONIC registers */ > > - s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); > > - s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1); > > - s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2); > > - s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3); > > + s->regs[SONIC_CRBA0] = dp8393x_get(s, dp8393x_rrp(s), 0); > > + s->regs[SONIC_CRBA1] = dp8393x_get(s, dp8393x_rrp(s), 1); > > + s->regs[SONIC_RBWC0] = dp8393x_get(s, dp8393x_rrp(s), 2); > > + s->regs[SONIC_RBWC1] = dp8393x_get(s, dp8393x_rrp(s), 3); > > trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], > > s->regs[SONIC_CRBA1], > > s->regs[SONIC_RBWC0], > > s->regs[SONIC_RBWC1]); > > @@ -414,28 +421,22 @@ static void dp8393x_do_receiver_disable(dp8393xState > > *s) > > static void dp8393x_do_transmit_packets(dp8393xState *s) > > { > > NetClientState *nc = qemu_get_queue(s->nic); > > - int width, size; > > int tx_len, len; > > uint16_t i; > > - width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; > > - > > while (1) { > > /* Read memory */ > > - size = sizeof(uint16_t) * 6 * width; > > s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; > > trace_dp8393x_transmit_packet(dp8393x_ttda(s)); > > - address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * > > width, > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > tx_len = 0; > > /* Update registers */ > > - s->regs[SONIC_TCR] = dp8393x_get(s, width, 0) & 0xf000; > > - s->regs[SONIC_TPS] = dp8393x_get(s, width, 1); > > - s->regs[SONIC_TFC] = dp8393x_get(s, width, 2); > > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 3); > > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 4); > > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 5); > > + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; > > + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); > > + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); > > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); > > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); > > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); > > /* Handle programmable interrupt */ > > if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { > > @@ -457,15 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState > > *s) > > i++; > > if (i != s->regs[SONIC_TFC]) { > > /* Read next fragment details */ > > - size = sizeof(uint16_t) * 3 * width; > > - address_space_read(&s->as, > > - dp8393x_ttda(s) > > - + sizeof(uint16_t) * width * (4 + 3 * > > i), > > - MEMTXATTRS_UNSPECIFIED, s->data, > > - size); > > - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); > > - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); > > - s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); > > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); > > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); > > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); > > } > > } > > @@ -498,22 +493,12 @@ static void dp8393x_do_transmit_packets(dp8393xState > > *s) > > s->regs[SONIC_TCR] |= SONIC_TCR_PTX; > > /* Write status */ > > - dp8393x_put(s, width, 0, > > - s->regs[SONIC_TCR] & 0x0fff); /* status */ > > - size = sizeof(uint16_t) * width; > > - address_space_write(&s->as, dp8393x_ttda(s), > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > + dp8393x_put(s, dp8393x_ttda(s), 0, s->regs[SONIC_TCR] & 0x0fff); > > if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { > > /* Read footer of packet */ > > - size = sizeof(uint16_t) * width; > > - address_space_read(&s->as, > > - dp8393x_ttda(s) > > - + sizeof(uint16_t) * width > > - * (4 + 3 * s->regs[SONIC_TFC]), > > - MEMTXATTRS_UNSPECIFIED, s->data, > > - size); > > - s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0); > > + s->regs[SONIC_CTDA] = dp8393x_get(s, dp8393x_ttda(s), > > + 4 + 3 * s->regs[SONIC_TFC]); > > if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) { > > /* EOL detected */ > > break; > > @@ -762,7 +747,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const > > uint8_t * buf, > > dp8393xState *s = qemu_get_nic_opaque(nc); > > int packet_type; > > uint32_t available, address; > > - int width, rx_len, padded_len; > > + int rx_len, padded_len; > > uint32_t checksum; > > int size; > > @@ -775,10 +760,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, > > const uint8_t * buf, > > rx_len = pkt_size + sizeof(checksum); > > if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > > - width = 2; > > padded_len = ((rx_len - 1) | 3) + 1; > > } else { > > - width = 1; > > padded_len = ((rx_len - 1) | 1) + 1; > > } > > @@ -799,11 +782,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, > > const uint8_t * buf, > > /* Check for EOL */ > > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > > /* Are we still in resource exhaustion? */ > > - size = sizeof(uint16_t) * 1 * width; > > - address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; > > - address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, > > - s->data, size); > > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > > /* Still EOL ; stop reception */ > > return -1; > > @@ -811,11 +790,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, > > const uint8_t * buf, > > /* Link has been updated by host */ > > /* Clear in_use */ > > - size = sizeof(uint16_t) * width; > > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > > - dp8393x_put(s, width, 0, 0); > > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > > - (uint8_t *)s->data, size); > > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > > @@ -869,32 +844,20 @@ static ssize_t dp8393x_receive(NetClientState *nc, > > const uint8_t * buf, > > /* Write status to memory */ > > trace_dp8393x_receive_write_status(dp8393x_crda(s)); > > - dp8393x_put(s, width, 0, s->regs[SONIC_RCR]); /* status */ > > - dp8393x_put(s, width, 1, rx_len); /* byte count */ > > - dp8393x_put(s, width, 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ > > - dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ > > - dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ > > - size = sizeof(uint16_t) * 5 * width; > > - address_space_write(&s->as, dp8393x_crda(s), > > - MEMTXATTRS_UNSPECIFIED, > > - s->data, size); > > + dp8393x_put(s, dp8393x_crda(s), 0, s->regs[SONIC_RCR]); /* status */ > > + dp8393x_put(s, dp8393x_crda(s), 1, rx_len); /* byte count */ > > + dp8393x_put(s, dp8393x_crda(s), 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 > > */ > > + dp8393x_put(s, dp8393x_crda(s), 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 > > */ > > + dp8393x_put(s, dp8393x_crda(s), 4, s->regs[SONIC_RSC]); /* seq_no */ > > /* Check link field */ > > - size = sizeof(uint16_t) * width; > > - address_space_read(&s->as, > > - dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, > > - MEMTXATTRS_UNSPECIFIED, s->data, size); > > - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); > > + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); > > if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { > > /* EOL detected */ > > s->regs[SONIC_ISR] |= SONIC_ISR_RDE; > > } else { > > /* Clear in_use */ > > - size = sizeof(uint16_t) * width; > > - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; > > - dp8393x_put(s, width, 0, 0); > > - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, > > - s->data, size); > > + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); > > /* Move to next descriptor */ > > s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; > > This patch breaks networking in its current form, but I was able to make it > work by applying the following diff: > > > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c > index db9cfd786f..b08843172b 100644 > --- a/hw/net/dp8393x.c > +++ b/hw/net/dp8393x.c > @@ -218,20 +218,20 @@ static uint32_t dp8393x_wt(dp8393xState *s) > return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; > } > > -static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) > +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, int offset) > { > const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > uint16_t val; > > if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > - addr += 2 * ofs16; > + addr += offset << 2; > if (s->big_endian) { > val = address_space_ldl_be(&s->as, addr, attrs, NULL); > } else { > val = address_space_ldl_le(&s->as, addr, attrs, NULL); > } > } else { > - addr += 1 * ofs16; > + addr += offset << 1; > if (s->big_endian) { > val = address_space_lduw_be(&s->as, addr, attrs, NULL); > } else { > @@ -243,19 +243,19 @@ static uint16_t dp8393x_get(dp8393xState *s, hwaddr > addr, unsigned ofs16) > } > > static void dp8393x_put(dp8393xState *s, > - hwaddr addr, unsigned ofs16, uint16_t val) > + hwaddr addr, int offset, uint16_t val) > { > const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > > if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { > - addr += 2 * ofs16; > + addr += offset << 2; > if (s->big_endian) { > address_space_stl_be(&s->as, addr, val, attrs, NULL); > } else { > address_space_stl_le(&s->as, addr, val, attrs, NULL); > } > } else { > - addr += 1 * ofs16; > + addr += offset << 1; > if (s->big_endian) { > address_space_stw_be(&s->as, addr, val, attrs, NULL); > } else { > @@ -431,12 +431,12 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > tx_len = 0; > > /* Update registers */ > - s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; > - s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); > - s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); > - s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); > - s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); > - s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); > + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 1) & 0xf000; > + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 2); > + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 3); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 4); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 5); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 6); > > /* Handle programmable interrupt */ > if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { > @@ -458,9 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) > i++; > if (i != s->regs[SONIC_TFC]) { > /* Read next fragment details */ > - s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); > - s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); > - s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); > + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 4 + 3 * > i); > + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 5 + 3 * > i); > + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 6 + 3 * > i); > } > } > > > The main change is that the offset argument for dp8393x_get() and > dp8393x_put() is actually an entry number and not a count of 16 bit words. > Other than that there were a couple of offset calculations that needed > adjustment to get things working again. > > Taking git master and applying your outstanding PR + this series without patch > 3 + this diff gave me working networking on Linux/m68k, MacOS 8.0 and > NetBSD/arc. > Nice job getting MacOS 8.0 to run! That functionality could be very useful for working on the Linux bootloaders (Penguin and EMILE) as they don't work under MESS/MAME or Mini VMac etc. > Unfortunately I don't have a test mips64el image available to see if this > combination works for Linux. Phil, do you have a suitable test kernel and > rootfs image available to allow this to be tested? > You can build and boot a mipsel vmlinux by following the steps I described previously. In the kernel messages you'll see the jazzsonic driver attempt to probe the device. When it succeeds, you'll see the MAC address reported. You can also observe the regression I reported with regards to patch 2/6, "dp8393x: don't force 32-bit register access". > > ATB, > > Mark. >
On 05/07/2021 02:36, Finn Thain wrote: >> Unfortunately I don't have a test mips64el image available to see if this >> combination works for Linux. Phil, do you have a suitable test kernel and >> rootfs image available to allow this to be tested? >> > > You can build and boot a mipsel vmlinux by following the steps I described > previously. In the kernel messages you'll see the jazzsonic driver attempt > to probe the device. When it succeeds, you'll see the MAC address > reported. You can also observe the regression I reported with regards to > patch 2/6, "dp8393x: don't force 32-bit register access". Those instructions are useful, but since I am not a MIPS developer I don't have an existing toolchain/kernel tree and rootfs available to test this. If you can provide me with a link to your vmlinux and rootfs with busybox or similar in it, I can take a look to see what is happening here. Otherwise it's almost impossible for me to understand and debug the problem you are seeing on your setup. ATB, Mark.
On Mon, 5 Jul 2021, Mark Cave-Ayland wrote: > On 05/07/2021 02:36, Finn Thain wrote: > > > > Unfortunately I don't have a test mips64el image available to see if > > > this combination works for Linux. Phil, do you have a suitable test > > > kernel and rootfs image available to allow this to be tested? > > > > > > > You can build and boot a mipsel vmlinux by following the steps I > > described previously. In the kernel messages you'll see the jazzsonic > > driver attempt to probe the device. When it succeeds, you'll see the > > MAC address reported. You can also observe the regression I reported > > with regards to patch 2/6, "dp8393x: don't force 32-bit register > > access". > > Those instructions are useful, but since I am not a MIPS developer I > don't have an existing toolchain/kernel tree and rootfs available to > test this. > You don't need a rootfs to see the jazzsonic driver messages. But if you still want one, you could try the mipsel builds from these distros (not the 64-bit ones): https://ftp.jaist.ac.jp/pub/Linux/Gentoo/experimental/mips/stages/ https://landley.net/aboriginal/downloads/binaries/ > If you can provide me with a link to your vmlinux and rootfs with > busybox or similar in it, I can take a look to see what is happening > here. Otherwise it's almost impossible for me to understand and debug > the problem you are seeing on your setup. > Uploading kernels is a hassle (for me) as it brings a trust question and requires a file hosting service. I really should use PGP and organise a web of trust but that's very difficult given my rural location.
On 07/07/2021 02:30, Finn Thain wrote: > On Mon, 5 Jul 2021, Mark Cave-Ayland wrote: > >> On 05/07/2021 02:36, Finn Thain wrote: >> >>>> Unfortunately I don't have a test mips64el image available to see if >>>> this combination works for Linux. Phil, do you have a suitable test >>>> kernel and rootfs image available to allow this to be tested? >>>> >>> >>> You can build and boot a mipsel vmlinux by following the steps I >>> described previously. In the kernel messages you'll see the jazzsonic >>> driver attempt to probe the device. When it succeeds, you'll see the >>> MAC address reported. You can also observe the regression I reported >>> with regards to patch 2/6, "dp8393x: don't force 32-bit register >>> access". >> >> Those instructions are useful, but since I am not a MIPS developer I >> don't have an existing toolchain/kernel tree and rootfs available to >> test this. >> > > You don't need a rootfs to see the jazzsonic driver messages. But if you > still want one, you could try the mipsel builds from these distros (not > the 64-bit ones): > > https://ftp.jaist.ac.jp/pub/Linux/Gentoo/experimental/mips/stages/ > https://landley.net/aboriginal/downloads/binaries/ That's true, but then this wouldn't enable testing of Phil's proposed CRC changes. Having a simple shell with ping and wget/curl is a real help here. >> If you can provide me with a link to your vmlinux and rootfs with >> busybox or similar in it, I can take a look to see what is happening >> here. Otherwise it's almost impossible for me to understand and debug >> the problem you are seeing on your setup. >> > > Uploading kernels is a hassle (for me) as it brings a trust question and > requires a file hosting service. I really should use PGP and organise a > web of trust but that's very difficult given my rural location. Given that these are only running in a VM I'm not too worried about trust. I also have a VPS with scp access that I could temporarily grant you access via an SSH public key if that helps? ATB, Mark.
On Wed, 7 Jul 2021, Mark Cave-Ayland wrote: > > You don't need a rootfs to see the jazzsonic driver messages. But if > > you still want one, you could try the mipsel builds from these distros > > (not the 64-bit ones): > > > > https://ftp.jaist.ac.jp/pub/Linux/Gentoo/experimental/mips/stages/ When I tried following my own advice I ran into ABI compatibility problems. It looks like my kernel build doesn't like those binaries, but maybe it's a limitation of the Magnum CPU... ... Run /bin/sh as init process request_module: kmod_concurrent_max (0) close to 0 (max_modprobes: 50), for module binfmt-464c, throttling... request_module: modprobe binfmt-464c cannot be processed, kmod busy with 50 threads for more than 5 seconds now Kernel panic - not syncing: Requested init /bin/sh failed (error -8). > > https://landley.net/aboriginal/downloads/binaries/ The binaries from Aboriginal Linux work okay (that is, rootfs.cpio.gz found in system-image-mipsel.tar.gz). I got a shell prompt using init=/bin/sh but there's no networking support in this minimal busybox build unfortunately. Those binaries have the same ABI as the ones in my busybox build: $ file busybox busybox: ELF 32-bit LSB executable, MIPS, MIPS-I version 1 (SYSV), statically linked, stripped Whereas, Debian/mipsel binaries (like the Gentoo ones) look like this: $ file busybox busybox: ELF 32-bit LSB pie executable, MIPS, MIPS32 rel2 version 1 (SYSV), dynamically linked, interpreter /lib/ld.so.1, for GNU/Linux 3.2.0, BuildID[sha1]=febe1809f2ad8dacb067dfd74505b19c6c69ba65, stripped Eventually I found this page, https://wiki.debian.org/MIPSPort which explains that the Debian/mipsel port switched ABI between Debian 8 and Debian 9. Unfortunately, the Debian 7 and 8 installer ISO images have no initrd so they are no use. I got them from this archive: https://cdimage.debian.org/cdimage/archive/ Anyway, the Debian 8 binaries look like this, and they work too: # file bin/dash bin/dash: ELF 32-bit LSB shared object, MIPS, MIPS-II version 1 (SYSV), dynamically linked, interpreter /lib/ld.so.1, for GNU/Linux 2.6.32, BuildID[sha1]=44f7c1d61d9941db2b1de5dd9629c99e06c30ea8, stripped > > That's true, but then this wouldn't enable testing of Phil's proposed > CRC changes. Having a simple shell with ping and wget/curl is a real > help here. > To generate network traffic you can get the kernel to configure the NIC over DHCP but that does require a different kernel config (see below). > > > If you can provide me with a link to your vmlinux and rootfs with > > > busybox or similar in it, I can take a look to see what is happening > > > here. Otherwise it's almost impossible for me to understand and > > > debug the problem you are seeing on your setup. > > > > > > > Uploading kernels is a hassle (for me) as it brings a trust question > > and requires a file hosting service. I really should use PGP and > > organise a web of trust but that's very difficult given my rural > > location. > > Given that these are only running in a VM I'm not too worried about > trust. Well, untrusted images are okay as long as we are talking about debugging QEMU issues that are not exploitable... With a bit of searching I was able to find a Debian/mipsel 8 rootfs at https://github.com/jubinson/debian-rootfs I can't vouch for it though. It appears to be a page of links to tar files in someone's dropbox. $ sha256sum mipsel-rootfs-20170318T103423Z.tar.gz e6ed1871b29317c85170a07621966a013951ced1c5fb8d679b7519996b803fe8 mipsel-rootfs-20170318T103423Z.tar.gz > I also have a VPS with scp access that I could temporarily grant you > access via an SSH public key if that helps? > Thanks for the offer. But that wouldn't help anyone else reading this. In anycase, I wanted to see whether a real distro could be used. So my plan was to cross-compile a Linux kernel and debootstrap a Debian/mipsel rootfs disk image. The first stage of a debootstrap installation runs on the host... sudo -s truncate -s 1G jessie-mipsel.img mke2fs jessie-mipsel.img mount -o loop jessie-mipsel.img /mnt wget -q -O- https://ftp-master.debian.org/keys/release-8.asc | gpg --import --no-default-keyring --keyring ./debian-release-8.gpg debootstrap --keyring=./debian-release-8.gpg --foreign --arch=mipsel jessie /mnt http://archive.debian.org/debian/ umount /mnt exit git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git cd linux git checkout linux-5.10.y make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- clean jazz_defconfig scripts/config -d IPV6 -d WIRELESS -d WLAN -d DEBUG_KERNEL -d EXPERT -d CC_OPTIMIZE_FOR_PERFORMANCE -e CC_OPTIMIZE_FOR_SIZE -e ISO9660_FS -m EXT3_FS -e NFS_FS -e IP_PNP -e ROOT_NFS -e NFS_V2 -e IP_PNP_DHCP -e CMDLINE_BOOL -e MIPS_CMDLINE_BUILTIN_EXTEND --set-str CMDLINE "console=ttyS0 root=/dev/nfs rw" make ARCH=mips CROSS_COMPILE=mipsel-linux-gnu- olddefconfig vmlinux mkisofs -o vmlinux.iso -J -iso-level 3 vmlinux qemu-system-mips64el -M magnum -L . -drive if=scsi,unit=0,format=raw,file=jessie-mipsel.img -drive if=scsi,unit=2,readonly=y,media=cdrom,format=raw,file=vmlinux.iso -drive if=scsi,unit=4,readonly=y,media=cdrom,format=raw,file=NetBSD-9.2-arc.iso -global ds1225y.filename=nvram -serial mon:stdio -serial null -net nic,model=dp83932,addr=00:00:00:aa:bb:cc -net bridge Actions: Start Windows NT -> Run a program Run setup scsi(0)cdrom(4)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)vmlinux root=/dev/sda init=/bin/sh This boots to a shell prompt. Well, so far, so good. Now the rest of the debootstrap installation can be completed by the guest... mount -t devtmpfs none /dev mount -t proc none /proc mount -t sysfs none /sys /debootstrap/debootstrap --second-stage Unfortunately, this produced a badly corrupted root filesystem and the installation failed. Seems to be a bug in either Linux/mipsel or QEMU. (Has anyone tried a NetBSD installation?) I was able to avoid all that block IO entirely by using NFS. That way, the installation completed successfully... mount -o loop jessie-mipsel.img /export/jessie-mipsel echo "/export/jessie-mipsel 192.168.66.0/24(sync,rw,insecure,no_root_squash,no_subtree_check)" >> /etc/exports /etc/init.d/nfs start qemu-system-mips64el -M magnum -L . -drive if=scsi,unit=2,readonly=y,media=cdrom,format=raw,file=vmlinux.iso -drive if=scsi,unit=4,readonly=y,media=cdrom,format=raw,file=NetBSD-9.2-arc.iso -global ds1225y.filename=nvram -serial mon:stdio -serial null -net nic,model=dp83932,addr=00:00:00:aa:bb:cc -net bridge Actions: Start Windows NT -> Run a program Run setup scsi(0)cdrom(4)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)vmlinux ip=dhcp nfsroot=192.168.66.1:/export/jessie-mipsel init=/bin/sh NetBSD/arc Bootstrap, Revision 1.1 (Wed May 12 13:15:55 UTC 2021) devopen: scsi(0)cdrom(2)fdisk(0) type disk file vmlinux 5706728+198952 [840960+1017335]=0x767d14 Linux version 5.10.47 (fthain@nippy) (mipsel-linux-gnu-gcc (btc) 6.4.0, GNU ld (btc) 2.28) #2 Fri Jul 9 16:28:12 AEST 2021 ARCH: Microsoft-Jazz PROMLIB: ARC firmware Version 1 Revision 2 CPU0 revision is: 00000400 (R4000PC) FPU revision is: 00000500 Primary instruction cache 8kB, VIPT, direct mapped, linesize 16 bytes. Primary data cache 8kB, direct mapped, VIPT, cache aliases, linesize 16 bytes Zone ranges: DMA [mem 0x0000000000000000-0x0000000000ffffff] Normal [mem 0x0000000001000000-0x0000000007ffffff] Movable zone start for each node Early memory node ranges node 0: [mem 0x0000000000000000-0x0000000007ffffff] Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff] Built 1 zonelists, mobility grouping on. Total pages: 32512 Kernel command line: console=ttyS0 root=/dev/nfs rw scsi(0)cdrom(2)fdisk(0)vmlinux ip=dhcp nfsroot=192.168.66.1:/export/jessie-mipsel init=/bin/sh Dentry cache hash table entries: 16384 (order: 4, 65536 bytes, linear) Inode-cache hash table entries: 8192 (order: 3, 32768 bytes, linear) mem auto-init: stack:off, heap alloc:off, heap free:off Memory: 123668K/131072K available (4249K kernel code, 234K rwdata, 928K rodata, 212K init, 135K bss, 7404K reserved, 0K cma-reserved) NR_IRQS: 256 random: get_random_bytes called from start_kernel+0x300/0x4b0 with crng_init=0 Console: colour dummy device 80x25 sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 21474836475000000ns Calibrating delay loop... 1404.10 BogoMIPS (lpj=7020544) pid_max: default: 32768 minimum: 301 Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear) Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear) devtmpfs: initialized clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns futex hash table entries: 256 (order: -1, 3072 bytes, linear) NET: Registered protocol family 16 VDMA: R4030 DMA pagetables initialized. SCSI subsystem initialized NET: Registered protocol family 2 IP idents hash table entries: 2048 (order: 2, 16384 bytes, linear) tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 4096 bytes, linear) TCP established hash table entries: 1024 (order: 0, 4096 bytes, linear) TCP bind hash table entries: 1024 (order: 0, 4096 bytes, linear) TCP: Hash tables configured (established 1024 bind 1024) UDP hash table entries: 256 (order: 0, 4096 bytes, linear) UDP-Lite hash table entries: 256 (order: 0, 4096 bytes, linear) NET: Registered protocol family 1 RPC: Registered named UNIX socket transport module. RPC: Registered udp transport module. RPC: Registered tcp transport module. RPC: Registered tcp NFSv4.1 backchannel transport module. workingset: timestamp_bits=30 max_order=15 bucket_order=0 Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254) io scheduler mq-deadline registered io scheduler kyber registered Console: switching to colour frame buffer device 100x37 Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled printk: console [ttyS0] disabled serial8250.0: ttyS0 at MMIO 0xe0006000 (irq = 32, base_baud = 115200) is a 16550A printk: console [ttyS0] enabled serial8250.0: ttyS1 at MMIO 0xe0007000 (irq = 33, base_baud = 115200) is a 16550A jazz_esp jazz_esp.0: esp0: regs[(ptrval):0] irq[29] jazz_esp jazz_esp.0: esp0: is a FAS100A, 40 MHz (ccf=0), SCSI ID 7 random: fast init done scsi host0: esp PDC20230-C/20630 VLB ATA controller detected. scsi host1: pata_legacy ata1: PATA max PIO2 cmd 0x1f0 ctl 0x3f6 irq 14 scsi 0:0:2:0: CD-ROM QEMU QEMU CD-ROM 2.5+ PQ: 0 ANSI: 5 scsi target0:0:2: Beginning Domain Validation VDMA: Channel 0: Memory error! VDMA: Channel 0: Memory error! VDMA: Channel 0: Memory error! scsi target0:0:2: Domain Validation skipping write tests scsi target0:0:2: Ending Domain Validation VDMA: Channel 0: Memory error! scsi 0:0:4:0: CD-ROM QEMU QEMU CD-ROM 2.5+ PQ: 0 ANSI: 5 scsi target0:0:4: Beginning Domain Validation VDMA: Channel 0: Memory error! VDMA: Channel 0: Memory error! VDMA: Channel 0: Memory error! scsi target0:0:4: Domain Validation skipping write tests scsi target0:0:4: Ending Domain Validation VDMA: Channel 0: Memory error! scsi host1: pata_legacy ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15 scsi host1: pata_legacy ata3: PATA max PIO4 cmd 0x1e8 ctl 0x3ee irq 11 scsi host1: pata_legacy ata4: PATA max PIO4 cmd 0x168 ctl 0x36e irq 10 scsi host1: pata_legacy ata5: PATA max PIO4 cmd 0x1e0 ctl 0x3e6 irq 8 scsi host1: pata_legacy ata6: PATA max PIO4 cmd 0x160 ctl 0x366 irq 12 SONIC ethernet @e0001000, MAC 52:54:00:12:34:56, IRQ 28 serio: i8042 KBD port at 0xe0005000,0xe0005001 irq 30 serio: i8042 AUX port at 0xe0005000,0xe0005001 irq 31 input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0 Sending DHCP requests ., OK IP-Config: Got DHCP answer from 192.168.66.1, my address is 192.168.66.112 IP-Config: Complete: device=eth0, hwaddr=52:54:00:12:34:56, ipaddr=192.168.66.112, mask=255.255.255.0, gw=192.168.66.1 host=192.168.66.112, domain=local, nis-domain=(none) bootserver=0.0.0.0, rootserver=192.168.66.1, rootpath= nameserver0=139.130.4.4 input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2 VFS: Mounted root (nfs filesystem) on device 0:12. Freeing prom memory: 376k freed Freeing prom memory: 60k freed Freeing prom memory: 4k freed Freeing unused kernel memory: 212K This architecture does not have kernel memory protection. Run /bin/sh as init process process '/bin/dash' started with executable stack /bin/sh: 0: can't access tty; job control turned off # mount -t devtmpfs none /dev # mount -t proc none /proc # mount -t sysfs none /sys # /debootstrap/debootstrap --second-stage ... # ping -c3 192.168.66.1 PING 192.168.66.1 (192.168.66.1) 56(84) bytes of data. 64 bytes from 192.168.66.1: icmp_seq=1 ttl=64 time=10.0 ms 64 bytes from 192.168.66.1: icmp_seq=2 ttl=64 time=0.000 ms 64 bytes from 192.168.66.1: icmp_seq=3 ttl=64 time=0.000 ms --- 192.168.66.1 ping statistics --- 3 packets transmitted, 3 received, 0% packet loss, time 2030ms rtt min/avg/max/mdev = 0.000/3.333/10.000/4.714 ms # After the debootstrap command completes, the last steps are manual configuration, as per the usual debootstrap procedure: https://wiki.debian.org/Debootstrap https://gist.github.com/varqox/42e213b6b2dde2b636ef Note that this only _barely_ works. A slightly larger vmlinux breaks the bootloader, and a slightly longer boot command breaks ARC.
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index bbe241ef9db..db9cfd786f5 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -162,7 +162,6 @@ struct dp8393xState { /* Temporaries */ uint8_t tx_buffer[0x10000]; - uint16_t data[12]; int loopback_packet; /* Memory access */ @@ -219,34 +218,48 @@ static uint32_t dp8393x_wt(dp8393xState *s) return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; } -static uint16_t dp8393x_get(dp8393xState *s, int width, int offset) +static uint16_t dp8393x_get(dp8393xState *s, hwaddr addr, unsigned ofs16) { + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; uint16_t val; - if (s->big_endian) { - val = be16_to_cpu(s->data[offset * width + width - 1]); + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { + addr += 2 * ofs16; + if (s->big_endian) { + val = address_space_ldl_be(&s->as, addr, attrs, NULL); + } else { + val = address_space_ldl_le(&s->as, addr, attrs, NULL); + } } else { - val = le16_to_cpu(s->data[offset * width]); + addr += 1 * ofs16; + if (s->big_endian) { + val = address_space_lduw_be(&s->as, addr, attrs, NULL); + } else { + val = address_space_lduw_le(&s->as, addr, attrs, NULL); + } } + return val; } -static void dp8393x_put(dp8393xState *s, int width, int offset, - uint16_t val) +static void dp8393x_put(dp8393xState *s, + hwaddr addr, unsigned ofs16, uint16_t val) { - if (s->big_endian) { - if (width == 2) { - s->data[offset * 2] = 0; - s->data[offset * 2 + 1] = cpu_to_be16(val); + const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; + + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { + addr += 2 * ofs16; + if (s->big_endian) { + address_space_stl_be(&s->as, addr, val, attrs, NULL); } else { - s->data[offset] = cpu_to_be16(val); + address_space_stl_le(&s->as, addr, val, attrs, NULL); } } else { - if (width == 2) { - s->data[offset * 2] = cpu_to_le16(val); - s->data[offset * 2 + 1] = 0; + addr += 1 * ofs16; + if (s->big_endian) { + address_space_stw_be(&s->as, addr, val, attrs, NULL); } else { - s->data[offset] = cpu_to_le16(val); + address_space_stw_le(&s->as, addr, val, attrs, NULL); } } } @@ -277,12 +290,10 @@ static void dp8393x_do_load_cam(dp8393xState *s) while (s->regs[SONIC_CDC] & 0x1f) { /* Fill current entry */ - address_space_read(&s->as, dp8393x_cdp(s), - MEMTXATTRS_UNSPECIFIED, s->data, size); - index = dp8393x_get(s, width, 0) & 0xf; - s->cam[index][0] = dp8393x_get(s, width, 1); - s->cam[index][1] = dp8393x_get(s, width, 2); - s->cam[index][2] = dp8393x_get(s, width, 3); + index = dp8393x_get(s, dp8393x_cdp(s), 0) & 0xf; + s->cam[index][0] = dp8393x_get(s, dp8393x_cdp(s), 1); + s->cam[index][1] = dp8393x_get(s, dp8393x_cdp(s), 2); + s->cam[index][2] = dp8393x_get(s, dp8393x_cdp(s), 3); trace_dp8393x_load_cam(index, s->cam[index][0] >> 8, s->cam[index][0] & 0xff, s->cam[index][1] >> 8, s->cam[index][1] & 0xff, @@ -293,9 +304,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) } /* Read CAM enable */ - address_space_read(&s->as, dp8393x_cdp(s), - MEMTXATTRS_UNSPECIFIED, s->data, size); - s->regs[SONIC_CE] = dp8393x_get(s, width, 0); + s->regs[SONIC_CE] = dp8393x_get(s, dp8393x_cdp(s), 0); trace_dp8393x_load_cam_done(s->regs[SONIC_CE]); /* Done */ @@ -311,14 +320,12 @@ static void dp8393x_do_read_rra(dp8393xState *s) /* Read memory */ width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; size = sizeof(uint16_t) * 4 * width; - address_space_read(&s->as, dp8393x_rrp(s), - MEMTXATTRS_UNSPECIFIED, s->data, size); /* Update SONIC registers */ - s->regs[SONIC_CRBA0] = dp8393x_get(s, width, 0); - s->regs[SONIC_CRBA1] = dp8393x_get(s, width, 1); - s->regs[SONIC_RBWC0] = dp8393x_get(s, width, 2); - s->regs[SONIC_RBWC1] = dp8393x_get(s, width, 3); + s->regs[SONIC_CRBA0] = dp8393x_get(s, dp8393x_rrp(s), 0); + s->regs[SONIC_CRBA1] = dp8393x_get(s, dp8393x_rrp(s), 1); + s->regs[SONIC_RBWC0] = dp8393x_get(s, dp8393x_rrp(s), 2); + s->regs[SONIC_RBWC1] = dp8393x_get(s, dp8393x_rrp(s), 3); trace_dp8393x_read_rra_regs(s->regs[SONIC_CRBA0], s->regs[SONIC_CRBA1], s->regs[SONIC_RBWC0], s->regs[SONIC_RBWC1]); @@ -414,28 +421,22 @@ static void dp8393x_do_receiver_disable(dp8393xState *s) static void dp8393x_do_transmit_packets(dp8393xState *s) { NetClientState *nc = qemu_get_queue(s->nic); - int width, size; int tx_len, len; uint16_t i; - width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; - while (1) { /* Read memory */ - size = sizeof(uint16_t) * 6 * width; s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; trace_dp8393x_transmit_packet(dp8393x_ttda(s)); - address_space_read(&s->as, dp8393x_ttda(s) + sizeof(uint16_t) * width, - MEMTXATTRS_UNSPECIFIED, s->data, size); tx_len = 0; /* Update registers */ - s->regs[SONIC_TCR] = dp8393x_get(s, width, 0) & 0xf000; - s->regs[SONIC_TPS] = dp8393x_get(s, width, 1); - s->regs[SONIC_TFC] = dp8393x_get(s, width, 2); - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 3); - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 4); - s->regs[SONIC_TFS] = dp8393x_get(s, width, 5); + s->regs[SONIC_TCR] = dp8393x_get(s, dp8393x_ttda(s), 0) & 0xf000; + s->regs[SONIC_TPS] = dp8393x_get(s, dp8393x_ttda(s), 1); + s->regs[SONIC_TFC] = dp8393x_get(s, dp8393x_ttda(s), 2); + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 3); + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 4); + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 5); /* Handle programmable interrupt */ if (s->regs[SONIC_TCR] & SONIC_TCR_PINT) { @@ -457,15 +458,9 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) i++; if (i != s->regs[SONIC_TFC]) { /* Read next fragment details */ - size = sizeof(uint16_t) * 3 * width; - address_space_read(&s->as, - dp8393x_ttda(s) - + sizeof(uint16_t) * width * (4 + 3 * i), - MEMTXATTRS_UNSPECIFIED, s->data, - size); - s->regs[SONIC_TSA0] = dp8393x_get(s, width, 0); - s->regs[SONIC_TSA1] = dp8393x_get(s, width, 1); - s->regs[SONIC_TFS] = dp8393x_get(s, width, 2); + s->regs[SONIC_TSA0] = dp8393x_get(s, dp8393x_ttda(s), 0); + s->regs[SONIC_TSA1] = dp8393x_get(s, dp8393x_ttda(s), 1); + s->regs[SONIC_TFS] = dp8393x_get(s, dp8393x_ttda(s), 2); } } @@ -498,22 +493,12 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) s->regs[SONIC_TCR] |= SONIC_TCR_PTX; /* Write status */ - dp8393x_put(s, width, 0, - s->regs[SONIC_TCR] & 0x0fff); /* status */ - size = sizeof(uint16_t) * width; - address_space_write(&s->as, dp8393x_ttda(s), - MEMTXATTRS_UNSPECIFIED, s->data, size); + dp8393x_put(s, dp8393x_ttda(s), 0, s->regs[SONIC_TCR] & 0x0fff); if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { /* Read footer of packet */ - size = sizeof(uint16_t) * width; - address_space_read(&s->as, - dp8393x_ttda(s) - + sizeof(uint16_t) * width - * (4 + 3 * s->regs[SONIC_TFC]), - MEMTXATTRS_UNSPECIFIED, s->data, - size); - s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0); + s->regs[SONIC_CTDA] = dp8393x_get(s, dp8393x_ttda(s), + 4 + 3 * s->regs[SONIC_TFC]); if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) { /* EOL detected */ break; @@ -762,7 +747,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, dp8393xState *s = qemu_get_nic_opaque(nc); int packet_type; uint32_t available, address; - int width, rx_len, padded_len; + int rx_len, padded_len; uint32_t checksum; int size; @@ -775,10 +760,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, rx_len = pkt_size + sizeof(checksum); if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { - width = 2; padded_len = ((rx_len - 1) | 3) + 1; } else { - width = 1; padded_len = ((rx_len - 1) | 1) + 1; } @@ -799,11 +782,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Check for EOL */ if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* Are we still in resource exhaustion? */ - size = sizeof(uint16_t) * 1 * width; - address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; - address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, - s->data, size); - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* Still EOL ; stop reception */ return -1; @@ -811,11 +790,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Link has been updated by host */ /* Clear in_use */ - size = sizeof(uint16_t) * width; - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; - dp8393x_put(s, width, 0, 0); - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, - (uint8_t *)s->data, size); + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); /* Move to next descriptor */ s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; @@ -869,32 +844,20 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Write status to memory */ trace_dp8393x_receive_write_status(dp8393x_crda(s)); - dp8393x_put(s, width, 0, s->regs[SONIC_RCR]); /* status */ - dp8393x_put(s, width, 1, rx_len); /* byte count */ - dp8393x_put(s, width, 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ - dp8393x_put(s, width, 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ - dp8393x_put(s, width, 4, s->regs[SONIC_RSC]); /* seq_no */ - size = sizeof(uint16_t) * 5 * width; - address_space_write(&s->as, dp8393x_crda(s), - MEMTXATTRS_UNSPECIFIED, - s->data, size); + dp8393x_put(s, dp8393x_crda(s), 0, s->regs[SONIC_RCR]); /* status */ + dp8393x_put(s, dp8393x_crda(s), 1, rx_len); /* byte count */ + dp8393x_put(s, dp8393x_crda(s), 2, s->regs[SONIC_TRBA0]); /* pkt_ptr0 */ + dp8393x_put(s, dp8393x_crda(s), 3, s->regs[SONIC_TRBA1]); /* pkt_ptr1 */ + dp8393x_put(s, dp8393x_crda(s), 4, s->regs[SONIC_RSC]); /* seq_no */ /* Check link field */ - size = sizeof(uint16_t) * width; - address_space_read(&s->as, - dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, - MEMTXATTRS_UNSPECIFIED, s->data, size); - s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); + s->regs[SONIC_LLFA] = dp8393x_get(s, dp8393x_crda(s), 5); if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* EOL detected */ s->regs[SONIC_ISR] |= SONIC_ISR_RDE; } else { /* Clear in_use */ - size = sizeof(uint16_t) * width; - address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; - dp8393x_put(s, width, 0, 0); - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, - s->data, size); + dp8393x_put(s, dp8393x_crda(s), 6, 0x0000); /* Move to next descriptor */ s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
Instead of accessing N registers via a single address_space API call using a temporary buffer (stored in the device state) and updating each register, move the address_space call in the register put/get. The load/store and word size checks are moved to put/get too. This simplifies a bit, making the code easier to read. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/net/dp8393x.c | 157 ++++++++++++++++++----------------------------- 1 file changed, 60 insertions(+), 97 deletions(-)