Re: [Qemu-devel] [PATCH for-1.7] Revert "e1000/rtl8139: update HMP NIC when every bit is written"

[Alex Williamson]

On Mon, 2013-11-18 at 21:47 +0200, Michael S. Tsirkin wrote:
> This reverts commit cd5be5829c1ce87aa6b3a7806524fac07ac9a757.
> Digging into hardware specs shows this does not
> actually make QEMU behave more like hardware.
> Let's stick to the tried heuristic for 1.7 and
> possibly revisit for 1.8.

If this is broken, then so are these:

23c37c37f0280761072c23bf67d3a4f3c0ff25aa
7c36507c2b8776266f50c5e2739bd18279953b93

None of these change the behavior of hardware, they only change when the
monitor gets told about mac address changes.  I'd suggest either add the
emulation described in each spec or revert all of them.  A partial
revert is just noise.  Thanks,

Alex

> 
> Reported-by: Vlad Yasevich <address@hidden>
> Cc: Amos Kong <address@hidden>
> Cc: Alex Williamson <address@hidden>
> ---
>  hw/net/e1000.c   | 2 +-
>  hw/net/rtl8139.c | 5 ++++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index ae63591..8387443 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1106,7 +1106,7 @@ mac_writereg(E1000State *s, int index, uint32_t val)
>  
>      s->mac_reg[index] = val;
>  
> -    if (index == RA || index == RA + 1) {
> +    if (index == 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);
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 7f2b4db..5329f44 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2741,7 +2741,10 @@ static void rtl8139_io_writeb(void *opaque, uint8_t 
> addr, uint32_t val)
>  
>      switch (addr)
>      {
> -        case MAC0 ... MAC0+5:
> +        case MAC0 ... MAC0+4:
> +            s->phys[addr - MAC0] = val;
> +            break;
> +        case MAC0+5:
>              s->phys[addr - MAC0] = val;
>              qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
>              break;