> On 17 Oct 2016, at 01:35 AM, Kevin Wolf <m...@kevin-wolf.de> wrote:
>
> The e1000e emulation zeroes out any used rx descriptor and then writes a
> completely newly constructed value there. By doing this, it doesn't only
> update the write-back area of the descriptors (as it's supposed to do),
> but it also clears the buffer address, which real hardware doesn't do.
>
> The spec explicitly mentions in chapter 7.1.8 that it is valid for a
> driver to reuse a descriptor and only update the status field while
> doing so, i.e. reusing the old buffer address:
>
> If software statically allocates buffers, and uses memory read to
> check for completed descriptors, it simply has to zero the status
> byte in the descriptor to make it ready for reuse by hardware.
>
> This patch fixes the behaviour to leave the buffer address in
> descriptors unchanged even after the descriptor has been used.
Hi Kevin,
Reviewed-by: Dmitry Fleytman <dmi...@daynix.com>
Thanks for catching this!
~Dmitry
>
> Signed-off-by: Kevin Wolf <m...@kevin-wolf.de>
> ---
> hw/net/e1000e_core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 9fa4116..a5ca97d 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -1280,11 +1280,10 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t
> *desc,
>
> struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
>
> - memset(d, 0, sizeof(*d));
> -
> assert(!rss_info->enabled);
>
> d->length = cpu_to_le16(length);
> + d->csum = 0;
>
> e1000e_build_rx_metadata(core, pkt, pkt != NULL,
> rss_info,
> @@ -1293,6 +1292,7 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t
> *desc,
> &d->special);
> d->errors = (uint8_t) (le32_to_cpu(status_flags) >> 24);
> d->status = (uint8_t) le32_to_cpu(status_flags);
> + d->special = 0;
> }
>
> static inline void
> @@ -1303,7 +1303,7 @@ e1000e_write_ext_rx_descr(E1000ECore *core, uint8_t
> *desc,
> {
> union e1000_rx_desc_extended *d = (union e1000_rx_desc_extended *) desc;
>
> - memset(d, 0, sizeof(*d));
> + memset(&d->wb, 0, sizeof(d->wb));
>
> d->wb.upper.length = cpu_to_le16(length);
>
> @@ -1327,7 +1327,7 @@ e1000e_write_ps_rx_descr(E1000ECore *core, uint8_t
> *desc,
> union e1000_rx_desc_packet_split *d =
> (union e1000_rx_desc_packet_split *) desc;
>
> - memset(d, 0, sizeof(*d));
> + memset(&d->wb, 0, sizeof(d->wb));
>
> d->wb.middle.length0 = cpu_to_le16((*written)[0]);
>
> --
> 2.1.4
>
