On Fri, May 08, 2020 at 04:30:40PM +0530, Sai Pavan Boddu wrote:
> Moving this buffers to CadenceGEMState, as their size will be increased
> more when JUMBO frames support is added.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
> ---
> hw/net/cadence_gem.c | 52
> ++++++++++++++++++++++++++------------------
> include/hw/net/cadence_gem.h | 2 ++
> 2 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 77a0588..5ccec1a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -314,6 +314,8 @@
>
> #define GEM_MODID_VALUE 0x00020118
>
> +#define MAX_FRAME_SIZE 2048
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
> {
> uint64_t ret = desc[0];
> @@ -928,17 +930,14 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
> */
> static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t
> size)
> {
> - CadenceGEMState *s;
> + CadenceGEMState *s = qemu_get_nic_opaque(nc);
> unsigned rxbufsize, bytes_to_copy;
> unsigned rxbuf_offset;
> - uint8_t rxbuf[2048];
> uint8_t *rxbuf_ptr;
> bool first_desc = true;
> int maf;
> int q = 0;
>
> - s = qemu_get_nic_opaque(nc);
> -
> /* Is this destination MAC address "for us" ? */
> maf = gem_mac_address_filter(s, buf);
> if (maf == GEM_RX_REJECT) {
> @@ -994,19 +993,19 @@ static ssize_t gem_receive(NetClientState *nc, const
> uint8_t *buf, size_t size)
> } else {
> unsigned crc_val;
>
> - if (size > sizeof(rxbuf) - sizeof(crc_val)) {
> - size = sizeof(rxbuf) - sizeof(crc_val);
> + if (size > MAX_FRAME_SIZE - sizeof(crc_val)) {
> + size = MAX_FRAME_SIZE - sizeof(crc_val);
> }
> bytes_to_copy = size;
> /* The application wants the FCS field, which QEMU does not provide.
> * We must try and calculate one.
> */
>
> - memcpy(rxbuf, buf, size);
> - memset(rxbuf + size, 0, sizeof(rxbuf) - size);
> - rxbuf_ptr = rxbuf;
> - crc_val = cpu_to_le32(crc32(0, rxbuf, MAX(size, 60)));
> - memcpy(rxbuf + size, &crc_val, sizeof(crc_val));
> + memcpy(s->rx_packet, buf, size);
> + memset(s->rx_packet + size, 0, MAX_FRAME_SIZE - size);
> + rxbuf_ptr = s->rx_packet;
> + crc_val = cpu_to_le32(crc32(0, s->rx_packet, MAX(size, 60)));
> + memcpy(s->rx_packet + size, &crc_val, sizeof(crc_val));
>
> bytes_to_copy += 4;
> size += 4;
> @@ -1152,7 +1151,6 @@ static void gem_transmit(CadenceGEMState *s)
> {
> uint32_t desc[DESC_MAX_NUM_WORDS];
> hwaddr packet_desc_addr;
> - uint8_t tx_packet[2048];
> uint8_t *p;
> unsigned total_bytes;
> int q = 0;
> @@ -1168,7 +1166,7 @@ static void gem_transmit(CadenceGEMState *s)
> * Packets scattered across multiple descriptors are gathered to this
> * one contiguous buffer first.
> */
> - p = tx_packet;
> + p = s->tx_packet;
> total_bytes = 0;
>
> for (q = s->num_priority_queues - 1; q >= 0; q--) {
> @@ -1198,12 +1196,12 @@ static void gem_transmit(CadenceGEMState *s)
> break;
> }
>
> - if (tx_desc_get_length(desc) > sizeof(tx_packet) -
> - (p - tx_packet)) {
> + if (tx_desc_get_length(desc) > MAX_FRAME_SIZE -
> + (p - s->tx_packet)) {
> DB_PRINT("TX descriptor @ 0x%" HWADDR_PRIx \
> " too large: size 0x%x space 0x%zx\n",
> packet_desc_addr, tx_desc_get_length(desc),
> - sizeof(tx_packet) - (p - tx_packet));
> + MAX_FRAME_SIZE - (p - s->tx_packet));
> break;
> }
>
> @@ -1248,24 +1246,24 @@ static void gem_transmit(CadenceGEMState *s)
>
> /* Is checksum offload enabled? */
> if (s->regs[GEM_DMACFG] & GEM_DMACFG_TXCSUM_OFFL) {
> - net_checksum_calculate(tx_packet, total_bytes);
> + net_checksum_calculate(s->tx_packet, total_bytes);
> }
>
> /* Update MAC statistics */
> - gem_transmit_updatestats(s, tx_packet, total_bytes);
> + gem_transmit_updatestats(s, s->tx_packet, total_bytes);
>
> /* Send the packet somewhere */
> if (s->phy_loop || (s->regs[GEM_NWCTRL] &
> GEM_NWCTRL_LOCALLOOP)) {
> - gem_receive(qemu_get_queue(s->nic), tx_packet,
> + gem_receive(qemu_get_queue(s->nic), s->tx_packet,
> total_bytes);
> } else {
> - qemu_send_packet(qemu_get_queue(s->nic), tx_packet,
> + qemu_send_packet(qemu_get_queue(s->nic), s->tx_packet,
> total_bytes);
> }
>
> /* Prepare for next packet */
> - p = tx_packet;
> + p = s->tx_packet;
> total_bytes = 0;
> }
>
> @@ -1612,6 +1610,17 @@ static void gem_realize(DeviceState *dev, Error **errp)
>
> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
> object_get_typename(OBJECT(dev)), dev->id, s);
> +
> + s->tx_packet = g_new0(uint8_t, MAX_FRAME_SIZE);
> + s->rx_packet = g_new0(uint8_t, MAX_FRAME_SIZE);
Hi Sai,
Since you're only using MAX_FRAME_SIZE these could be arrays
in CadenceGEMState.
With that change:
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
