Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZEXALL on a Z80 machine - CRC errors #17

Open
hlide opened this issue May 1, 2023 · 19 comments
Open

ZEXALL on a Z80 machine - CRC errors #17

hlide opened this issue May 1, 2023 · 19 comments

Comments

@hlide
Copy link

hlide commented May 1, 2023

I used the zexall from: https://github.com/anotherlin/z80emu/blob/master/testfiles/zexall.z80!

To do so, I tweaked the firmware so it can run on my SHARP MZ-80 K this way:

  1. Mode 3 for RAM to speed up ZEXALL execution as it may be quite slow on a 2MHz machine,
  2. Virtual RAM from $0000 to $CFFF since ZEXALL starts from $0100,
  3. I/O memory from $D000 to $FFFF is kept but never used,
  4. IN A,(n) is tweaked to output a string to Serial USB terminal,
  5. OUT (n),A is tweaked to issue a HALT forever (interrupts are ignored),
  6. $0000 and $0005 are also tweaked to issue OUT and IN.

So when booting the machine, here is what I get:

image

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 2, 2023 via email

@hlide
Copy link
Author

hlide commented May 3, 2023

The CRC errors are totally reproductive and have the same values.

I also tried with a full 64KB RAM ($0000-$FFFF) but same result so far.

I have no reason to think mode 3 is a cause to those CRC errors. And having CPI and CPD to set the flags shows they both use temp8 = SUB_Bytes(register_a, Read_byte(REGISTER_HL));. And what differs is Writeback_Reg16(REG_HL, (REGISTER_HL -/+ 1)); which doesn't even assign the flags.

So I checked SUB_Bytes code against your source in GitHub and no difference.

The only thing I noticed is that you are not setting cp_opcode = 1; for CPD/CPI when calling temp8 = SUB_Bytes(register_a, Read_byte(REGISTER_HL)); so I'm adding it to check.

Nope, result is always the same.

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 3, 2023 via email

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 3, 2023 via email

@hlide
Copy link
Author

hlide commented May 4, 2023

So I tried:
image

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 4, 2023 via email

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 8, 2023 via email

@hlide
Copy link
Author

hlide commented May 23, 2023

Sorry, I was on holiday and a very bad news occurred so I was not inclined to come back here until a certain moment.

Might you add the change you did in the source for executing that ZEXALL test so everybody can check it? I just tweaked the original firmware to be running a virtual 64-bit RAM space and load ZEXALL code in RAM from a PROGMEM array. So I don't understand those weird behaviors.

Might you also explain how you compile the firmware (Single USB Serial? 600 Mhz? Faster mode? etc.)?

@hlide
Copy link
Author

hlide commented May 25, 2023

So getting back to your last source and just adding ZEXALL and make it running in mode 3 with full 64KB RAM and loading ZEXALL in RAM in setup, I'm getting that result:

bit n,<b,c,d,e,h,l,(hl),a>.... ERROR **** crc expected:5e020e98 found:b560ef65
cpd........................ ERROR **** crc expected:134b622d found:0241a81f

Everything else is OK.

Using another laptop, Arduino 2.1.0 et Teensy package 1.58.1

Note that you just need a Teensy 4.1, no need for MCLZ8 board and a Z80 machine.

The source is: https://gist.github.com/hlide/4a0d504a4c886cba6a87427f81f2a71c

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 25, 2023 via email

@hlide
Copy link
Author

hlide commented May 25, 2023

Please do a DIFF between my MCLZ8_ZEXALL.ino and yours MCLZ8.ino in Projects/MCLZ8/Code/Standard_Z80. You will see no change in the Z80 instruction emulation side except for IN A,(n) and OUT A,(n) which should have no effect because they are not directly used by ZEXALL code but but when calling CALL 5 (BDOS) which is out of the scope of checking the instruction.

That's 99% your code. The changes are very minimal: full mode 3 use, full internal RAM use, no real BIU operations, and so on. But Z80 instructions are exactly yours without changes. I also changed the wait_CLK_edge routines to emulate Z80 CLK pulses using ARM cycles so I can run it on a standalone Teensy 4.1 (no need for a MCLZ8 PCB and a Z80 motherboard).

I don't have "NOMATCH their_register_a" but "OK" for the other instructions. That is an issue with your code by just changing BIU routine which is not enough in my opinion since I have no idea of your other changes compared to the original MCLZ8.ino.

I have no idea what I have this result but you should really use my code running on a teensy 4.1 and not just changing BIU code. That way, we will know if the issue is due to anything else the source (compiler issue?).

Since I have no access to your modified code running ZEXALL, I cannot compare and so I cannot accept your results because I cannot reproduce them. You have mine, just run it on a standalone Teensy 4.1 with an USB serial connected to your PC/Mac and tell me what you get. By the way, you should also specify what are your Arduino version, the Teensy package version and the compiling options (Speed option, CPU freq., Serial mode). Mine was: Faster, 600MHz, [Single] Serial.

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 26, 2023 via email

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 26, 2023 via email

@hlide
Copy link
Author

hlide commented May 26, 2023

Wait! Are you telling me you're not using the same ZEXALL I used in the array?

Do you realize you're still trying to convince me with "facts" I cannot check myself because I cannot reproduce all you're doing? (how do you launch your ZEXALL? what is that ZEXALL you use? what is your code checking ZEXALL on a true Teensy? Inside a Z80 machine? etc.)

I was asking for you to check my code with your own development tools (that is, compile the source, upload it on a standalone Teensy 4.1 and run it and check the results on a USB terminal). That would be the first step to lead me in which direction I must go for the next step.

While I certainly understand BIT and CPD emulation code should NOT be the culprits and that it might come from the execution of instructions while checking or afterward, I see no reason for $DB being an issue because ZEXALL never use IN/OUT. BDOS call is an "OS operation" and the result of A should not enter in the CRC of the next instruction checking. It is quite irrational to think so and let me give you the evidence:

bdos push af
push bc
push de
push hl
call 5
pop hl
pop de
pop bc
pop af
ret

As you can see, bdos routine restores all the registers after the call to in al,(0) : ret at 0x0005.

ZEXALL doesn't use I/O operations and fully runs in RAM (so internal RAM is the right place) for its code, data and stack so the simplified BIU should not be an issue per se (if so, why the other instructions end OK?). Having mode = 3; gives us the insurance that there is no bus access which can introduce hazards in reading registers.

Right now, the results are irrational and MCLZ8 has no debug facility regarding Z80 emulation to help me to pinpoint the issue.
I'll try to reduce the set of instructions to check to the failing one to see it is still an issue.

@hlide
Copy link
Author

hlide commented May 26, 2023

Another thing I don't understand is:

start: ld hl,(6)
ld sp,hl
ld de,msg1
ld c,9
call bdos

the sequence in a,(0) : ret is at 0x0005 and is 3 bytes (0x0005-0x0007) so I would expect ld hl,(6) to load 0xC900 in hl then in sp (still in RAM). BDOS is at 0x0005 so unless the CP/M has a RST n at this place, it looks quite hazardous to fetch the top stack here.

EDIT: I already replaced with LD HL,$8000 as seen in another version of ZEXALL.

@hlide
Copy link
Author

hlide commented May 26, 2023

Another ZEXALL I edited so it only checks the "failed" instructions (BIT and CPD):

MCLZ8 - ZEXALL
Z80 instruction exerciser
bit n,<b,c,d,e,h,l,(hl),a>.... ERROR **** crc expected:5e020e98 found:b560ef65
cpd........................ ERROR **** crc expected:134b622d found:0241a81f
Tests complete

And if I reverse the order of them:
MCLZ8 - ZEXALL
Z80 instruction exerciser
cpd........................ ERROR **** crc expected:134b622d found:0241a81f
bit n,<b,c,d,e,h,l,(hl),a>.... ERROR **** crc expected:5e020e98 found:b560ef65
Tests complete

So the order of instruction checking doesn't count.

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented May 26, 2023 via email

@hlide
Copy link
Author

hlide commented Jul 18, 2023

Right now, I'm writing my own firmware with a cross emulation (probably what you call a lock-step?) using your emulator.

Basically, I first step an instruction with your emulator then I step an instruction with mine. I compare all the registers, including PC, IR, IFF1, IFF2, and so on. I also record any bus access done by those instructions (byte read/written and the same with your emulator) so I can compare them. That way, I can detect any divergence in registers and in memory whenever stepping an instruction. If no difference, it means same PC, same instruction, same IR and same contents for the registers. One note, I don't track F3 and F5 differences on F because I cannot trust them on both emulators on the actual stage.

So when I reach ZEXALL test bit n,<b,c,d,e,h,l,(hl),a>.... (the first one which is in ERROR when trying your emulator), I got that:

image

Register F appears to disaggree here (probably on F3 or/and F5 values) so I decided to disable the difference detection to see the final result of that ZEXALL test on bit n,<b,c,d,e,h,l,(hl),a>.... on my emulator. And I got bit n,<b,c,d,e,h,l,(hl),a>.... OK .

So it looks bit n,<b,c,d,e,h,l,(hl),a>.... is not correct on the way to compute F in your firmware (I mean the one you published here).

This is the code I use in my frimware to compute F on BIT (case 1:):
image
Note that this->wz contains the effective address of (HL), (IX+d) or (IY+d) when F is computed with.

Yours is using register_pc for BIT n,(HL) which is extremely dubitative.

@MicroCoreLabs
Copy link
Owner

MicroCoreLabs commented Sep 2, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants