From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=45593 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OSD6w-0003QL-2C for qemu-devel@nongnu.org; Fri, 25 Jun 2010 13:54:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OS3rj-0007kL-0A for qemu-devel@nongnu.org; Fri, 25 Jun 2010 04:02:24 -0400 Received: from mail-ww0-f45.google.com ([74.125.82.45]:53624) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OS3ri-0007kB-SM for qemu-devel@nongnu.org; Fri, 25 Jun 2010 04:02:22 -0400 Received: by wwb17 with SMTP id 17so697438wwb.4 for ; Fri, 25 Jun 2010 01:02:21 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4C246289.1060908@redhat.com> Date: Fri, 25 Jun 2010 10:02:17 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <4C166149.2010909@siemens.com> <1276535514-19724-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1276535514-19724-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v2] lsi53c895a: fix Phase Mismatch Jump List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On 06/14/2010 07:11 PM, 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. > > 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 { PING Paolo