[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] lsi53c895a: fix Phase Mismatch Jump
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v2] lsi53c895a: fix Phase Mismatch Jump |
Date: |
Tue, 29 Jun 2010 23:11:50 +0200 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
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 <address@hidden>
> ---
> > 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
address@hidden http://www.aurel32.net