From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49211 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OTi6E-0007HC-3G for qemu-devel@nongnu.org; Tue, 29 Jun 2010 17:12:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OTi6C-0006jL-Qw for qemu-devel@nongnu.org; Tue, 29 Jun 2010 17:12:09 -0400 Received: from hall.aurel32.net ([88.191.82.174]:33126) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OTi6C-0006j2-MV for qemu-devel@nongnu.org; Tue, 29 Jun 2010 17:12:08 -0400 Date: Tue, 29 Jun 2010 23:11:50 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH v2] lsi53c895a: fix Phase Mismatch Jump Message-ID: <20100629211150.GA3811@ohm.aurel32.net> References: <4C166149.2010909@siemens.com> <1276535514-19724-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1276535514-19724-1-git-send-email-pbonzini@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Mon, Jun 14, 2010 at 07:11:54PM +0200, Paolo Bonzini wrote: > lsi_bad_phase has a bug in the choice of pmjad1/pmjad2. This does > not matter with Linux guests because it uses just one routine for > both, but it breaks Windows 64-bit guests. This is the text > from the spec: > > "[The PMJCTL] bit controls which decision mechanism is used > when jumping on phase mismatch. When this bit is cleared the > LSI53C895A will use Phase Mismatch Jump Address 1 (PMJAD1) when > the WSR bit is cleared and Phase Mismatch Jump Address 2 (PMJAD2) > when the WSR bit is set. When this bit is set the LSI53C895A will > use jump address one (PMJAD1) on data out (data out, command, > message out) transfers and jump address two (PMJAD2) on data in > (data in, status, message in) transfers." > > Which means: > > CCNTL0.PMJCTL > 0 SCNTL2.WSR = 0 PMJAD1 > 0 SCNTL2.WSR = 1 PMJAD2 > 1 out PMJAD1 > 1 in PMJAD2 > > In qemu, what you get instead is: > > CCNTL0.PMJCTL > 0 out PMJAD1 > 0 in PMJAD2 <<<<< > 1 out PMJAD1 > 1 in PMJAD1 <<<<< > > Considering that qemu always has SCNTL2.WSR cleared, the two marked cases > (corresponding to phase mismatch on input) are always jumping to the > wrong PMJAD register. The patch implements the correct semantics. > > Signed-off-by: Paolo Bonzini > --- > > Looks correct. But why not assigning s->pmjad[12] directly? Would > > improve readability IMO. > > No particular reason, hence fine by me. Thanks, applied. > hw/lsi53c895a.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c > index f5a91ba..9a37fed 100644 > --- a/hw/lsi53c895a.c > +++ b/hw/lsi53c895a.c > @@ -490,10 +490,10 @@ static void lsi_bad_phase(LSIState *s, int out, int new_phase) > { > /* Trigger a phase mismatch. */ > if (s->ccntl0 & LSI_CCNTL0_ENPMJ) { > - if ((s->ccntl0 & LSI_CCNTL0_PMJCTL) || out) { > - s->dsp = s->pmjad1; > + if ((s->ccntl0 & LSI_CCNTL0_PMJCTL)) { > + s->dsp = out ? s->pmjad1 : s->pmjad2; > } else { > - s->dsp = s->pmjad2; > + s->dsp = (s->scntl2 & LSI_SCNTL2_WSR ? s->pmjad2 : s->pmjad1); > } > DPRINTF("Data phase mismatch jump to %08x\n", s->dsp); > } else { > -- > 1.7.0.1 > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net