[Qemu-devel] Re: [PATCH] e1000: clear EOP for multi-buffer descriptors

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2011-02-07 at 14:30 +0200, Michael S. Tsirkin wrote:
> The e1000 spec says: 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 is not a hardware requirement (moving the
> hardware tail pointer is), but is necessary for performing an in–memory
> scan.
> 
> Thus the guest does not have to clear the status byte.  In case it
> doesn't we need to clear EOP for all descriptors
> except the last.  While I don't know of any such guests,
> it's probably a good idea to stick to the spec.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reported-by: Juan Quintela <quintela@redhat.com>
> 
> ---
>  hw/e1000.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 3427ff3..7853c12 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -694,6 +694,9 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
>                  desc.length = cpu_to_le16(desc_size + fcs_len(s));
>                  desc.status |= E1000_RXD_STAT_EOP | E1000_RXD_STAT_IXSM;
>              } else {
> +                /* Guest zeroing out status is not a hardware requirement.
> +                   Clear EOP in case guest didn't do it. */
> +                desc.status &= ~E1000_RXD_STAT_EOP;
>                  desc.length = cpu_to_le16(desc_size);
>              }
>          } else { // as per intel docs; skip descriptors with null buf addr

Acked-by: Alex Williamson <alex.williamson@redhat.com>