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 <pbonz...@redhat.com> > --- > hw/lsi53c895a.c | 12 +++++++++--- > 1 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c > index f5a91ba..00df2bd 100644 > --- a/hw/lsi53c895a.c > +++ b/hw/lsi53c895a.c > @@ -490,11 +490,14 @@ 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; > + int dest; > + if ((s->ccntl0 & LSI_CCNTL0_PMJCTL)) { > + dest = out ? 1 : 2; > } else { > - s->dsp = s->pmjad2; > + dest = (s->scntl2 & LSI_SCNTL2_WSR ? 2 : 1); > } > + > + s->dsp = (dest == 1) ? s->pmjad1 : s->pmjad2; > DPRINTF("Data phase mismatch jump to %08x\n", s->dsp); > } else { > DPRINTF("Phase mismatch interrupt\n");
Looks correct. But why not assigning s->pmjad[12] directly? Would improve readability IMO. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux