On 01/09/2013 06:51 PM, Michael S. Tsirkin wrote: > 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 <wp...@windriver.com> > Reported-by: Yan Vugenfirer <y...@daynix.com> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > --- > 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); > } >
If my memory is correct, though ICS is marked as read only in the spec, we do can read it when I'm examining a real e1000 card.
