From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tswv0-0001fK-Q1 for qemu-devel@nongnu.org; Wed, 09 Jan 2013 09:46:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tswuw-0004qh-0a for qemu-devel@nongnu.org; Wed, 09 Jan 2013 09:46:14 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:60641) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tswuv-0004qT-Sj for qemu-devel@nongnu.org; Wed, 09 Jan 2013 09:46:09 -0500 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 9 Jan 2013 09:46:05 -0500 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 51C88C9006A for ; Wed, 9 Jan 2013 09:46:01 -0500 (EST) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r09Ek1vQ283836 for ; Wed, 9 Jan 2013 09:46:01 -0500 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r09Ek0rE021224 for ; Wed, 9 Jan 2013 12:46:00 -0200 From: Anthony Liguori In-Reply-To: <20130109105100.GA17137@redhat.com> References: <20130109105100.GA17137@redhat.com> Date: Wed, 09 Jan 2013 08:45:55 -0600 Message-ID: <877gnmbejg.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH] e1000: make ICS write-only List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , qemu-devel@nongnu.org, wpaul@windriver.com, yan@daynix.com Cc: Paolo Bonzini , Jason Wang , Stefan Hajnoczi "Michael S. Tsirkin" 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 > Reported-by: Yan Vugenfirer > Signed-off-by: Michael S. Tsirkin 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 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