Re: [Qemu-devel] [PATCH 02/17] imx_fec: Do not calculate FEC

[Andrey Smirnov]

On Fri, Oct 6, 2017 at 6:48 AM, Peter Maydell <address@hidden> wrote:
> On 18 September 2017 at 20:50, Andrey Smirnov <address@hidden> wrote:
>> Save some computation time and avoid calculating CRC's frame
>>
>> Cc: Peter Maydell <address@hidden>
>> Cc: Jason Wang <address@hidden>
>> Cc: address@hidden
>> Cc: address@hidden
>> Cc: address@hidden
>> Signed-off-by: Andrey Smirnov <address@hidden>
>> ---
>>  hw/net/imx_fec.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
>> index 88b4b049d7..75822344fc 100644
>> --- a/hw/net/imx_fec.c
>> +++ b/hw/net/imx_fec.c
>> @@ -1032,9 +1032,7 @@ static ssize_t imx_enet_receive(NetClientState *nc, 
>> const uint8_t *buf,
>>      IMXENETBufDesc bd;
>>      uint32_t flags = 0;
>>      uint32_t addr;
>> -    uint32_t crc;
>>      uint32_t buf_addr;
>> -    uint8_t *crc_ptr;
>>      unsigned int buf_len;
>>      size_t size = len;
>>
>> @@ -1048,8 +1046,6 @@ static ssize_t imx_enet_receive(NetClientState *nc, 
>> const uint8_t *buf,
>>
>>      /* 4 bytes for the CRC.  */
>>      size += 4;
>> -    crc = cpu_to_be32(crc32(~0, buf, size));
>> -    crc_ptr = (uint8_t *) &crc;
>>
>>      /* Huge frames are truncted.  */
>>      if (size > ENET_MAX_FRAME_SIZE) {
>> @@ -1090,9 +1086,10 @@ static ssize_t imx_enet_receive(NetClientState *nc, 
>> const uint8_t *buf,
>>          dma_memory_write(&address_space_memory, buf_addr, buf, buf_len);
>>          buf += buf_len;
>>          if (size < 4) {
>> +            const uint8_t zeros[4] = { 0 };
>> +
>>              dma_memory_write(&address_space_memory, buf_addr + buf_len,
>> -                             crc_ptr, 4 - size);
>> -            crc_ptr += 4 - size;
>> +                             zeros, 4 - size);
>>          }
>>          bd.flags &= ~ENET_BD_E;
>>          if (size == 0) {
>
> This looks a bit odd. Doesn't the hardware calculate the CRC here?
>

It does, it just seemed to me that since the hardware also has a "CRC
error" bit in its status register, there would be few if any users of
the actual calculated CRC value. Given how eTSEC emulation layer gets
away without calculating CRC I thought that it might be possible to
have this optimization here as well.

I can drop this patch if this seems risky.

Thanks,
Andrey Smirnov