Re: [Qemu-devel] [RFC PATCH 1/2] e1000: Use Address_Available bit as HW latch

[Jason Wang]

On 11/22/2013 04:04 AM, Vlad Yasevich wrote:
> e1000 provides a E1000_RAH_AV bit on every complete write
> to the Receive Address Register.  We can use this bit
> 2 ways:
>  1) To trigger HMP notifications.  When the bit is set the
>     mac address is fully set and we can update the HMP.
>
>  2) We can turn off he bit on the write to low order bits of
>     the Receive Address Register, so that we would not try
>     to match received traffic to this address when it is
>     not completely set.
>
> Signed-off-by: Vlad Yasevich <address@hidden>
> ---
>  hw/net/e1000.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index ae63591..82978ea 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1106,10 +1106,19 @@ mac_writereg(E1000State *s, int index, uint32_t val)
>  
>      s->mac_reg[index] = val;
>  
> -    if (index == RA || index == RA + 1) {
> +    switch (index) {
> +    case RA:
> +        /* Mask off AV bit on the write of the low dword.  The write of
> +         * the high dword will set the bit.  This way a half-written
> +         * mac address will not be used to filter on rx.
> +         */
> +        s->mac_reg[RA+1] &= ~E1000_RAH_AV;

If a stupid driver write high dword first, it won't receive any packets.
> +        break;
> +    case (RA + 1):
>          macaddr[0] = cpu_to_le32(s->mac_reg[RA]);
>          macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]);
>          qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t *)macaddr);

Guest may invalid the mac address by clearing the AV bit through writing
to high dword. So this may notify a wrong mac address.

Generally, we could teset the AV bit before notification, and try to do
the this on both high and low dword. This obeys specs and
receive_filter() above.

If we don't want half-written status, driver should clear AV bit before
each writing of new mac address. But looks like linux and freebsd does
not do this.  But the window is really small and harmless.
> +        break;
>      }
>  }
>