[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] lsi53c895a: fix Phase Mismatch Jump
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH] lsi53c895a: fix Phase Mismatch Jump |
Date: |
Mon, 14 Jun 2010 19:05:13 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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>
> ---
> 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