qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] e1000: make ICS write-only


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] e1000: make ICS write-only
Date: Wed, 09 Jan 2013 08:45:55 -0600
User-agent: Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu)

"Michael S. Tsirkin" <address@hidden> writes:

> Since commit b1332393cdd7d023de8f1f8aa136ee7866a18968,
> qemu started updating ICS register when interrupt
> is sent, with the intent to match spec better
> (guests do not actually read this register).
> However, the function set_interrupt_cause where ICS
> is updated is often called internally by
> device emulation so reading it does not produce the last value
> written by driver.  Looking closer at the spec,
> it documents ICS as write-only, so there's no need
> to update it at all. I conclude that while harmless this line is useless
> code so removing it is a bit cleaner than keeping it in.
>
> Tested with windows and linux guests.
>
> Cc: Bill Paul <address@hidden>
> Reported-by: Yan Vugenfirer <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>

Yeah, I'm a little confused as to why we would need to set the ICS
register too.  Unless Bill had a reason that I'm missing:

Reviewed-by: Anthony Liguori <address@hidden>

Regards,

Anthony Liguori

> ---
>  hw/e1000.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 92fb00a..928d804 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -230,7 +230,6 @@ set_interrupt_cause(E1000State *s, int index, uint32_t 
> val)
>          val |= E1000_ICR_INT_ASSERTED;
>      }
>      s->mac_reg[ICR] = val;
> -    s->mac_reg[ICS] = val;
>      qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0);
>  }
>  
> -- 
> MST




reply via email to

[Prev in Thread] Current Thread [Next in Thread]