captainys / TOWNSEMU

FM Towns Emulator "Tsugaru"
BSD 3-Clause "New" or "Revised" License
247 stars 18 forks source link

High-fidelity CPU core ignores expand-down segment flag when "accessed" bit is set #122

Open jckarter opened 7 months ago

jckarter commented 7 months ago

If a data segment has both the "expand down" and "accessed" bits set, then the high-fidelity CPU core will treat it as a normal expand-up segment and fault for reads at offsets above rather than below the segment limit. (It appears the "default" core doesn't yet try to honor the expand-down flag on segments at all.) Looking through the source, it appears that this happens because the segment type (bits 40 through 44 of the GDT entry, including the "accessed" bit) is tested here without masking out the accessed bit:

https://github.com/captainys/TOWNSEMU/blob/d5c53f00dc68736fa4e400f7ec786ed12914d6b9/src/cpu/i486fidelity.h#L396

although In SegmentProperty::GetType(), it appears to try to mask out the "accessed" bit for types >= 8 (which would correspond to code and data segments):

https://github.com/captainys/TOWNSEMU/blob/d5c53f00dc68736fa4e400f7ec786ed12914d6b9/src/cpu/i486.h#L279

I'm using the 20240223 automated release on macOS 23D60, Apple silicon.

captainys commented 7 months ago

Thank you for finding an issue! But, isn't it the other way? I realized the condition in the GetType should be 16<=type since I am taking low 5 bits of the attribute byte, and the 0x10 bit distinguishes the system and application, and accessed bit is only for the application segment types. I rather feel that GetType was never returning Available 32-bit TSS, Busy 32-bit TSS, and 32-bit Trap Gate. (Looks like it never happens during Windows 3.1 and WIndows 95 boot process).

Also realized was line 453 of i486fidelity.h was only checking SEGTYPE_DATA_EXPAND_DOWN_RW and not checking SEGTYPE_DATA_EXPAND_DOWN_READONLY, if you were having an issue with read-only expand-down segment, probably the error in line 453 was the culprit.

I've made above changes and at least confirmed Windows 3.1 L12 Enhanced Mode/Standard Mode, and Windows 95 starts OK. I'm going to push changes soon.

captainys commented 7 months ago

Update: I had separate GetType() for InterruptDescriptor, that's why the error in the SegmentProperty::GetType was not doing anything. I've fixed it anyway. I've pushed the source. Thank you!

jckarter commented 7 months ago

Thank you for the quick fix!