search

philpem / freebee

FreeBee - AT&T 3B1 / 7300 UNIX PC emulator
GNU General Public License v3.0
117 stars 9 forks source link

fix for /bin/cc, month off by one, and some readability clarifications #18

Closed agentbooth closed 4 years ago

agentbooth commented 4 years ago

Hey Phil. After many hours of investigation I finally got 'cc' working! There were some issues with 32-bit writes straddling two pages which does occur. The ACCESS_CHECK_WR for 32-bits has some issues so I just replaced with two 16-bit access checks. This seems to work fine and therefore checks the first page and then the second page in a 32-bit write straddling pages. This should allow both or either page to pagefault. I think proper pagefaulting wasn't occurring in this scenario which somehow must have been triggered by cc.

philpem commented 4 years ago

Wow. I completely overlooked that the 68K can do word-aligned 32-bit loads and stores. I must have somehow got it into my head that you could only store 32bit values on a 4-byte aligned address, but the real requirement is that the address be even. Nice spot!

I've added a couple of minor review comments, and you've highlighted a couple of things I need to look into regarding the ACCESS_CHECK macros. (my preference would be to refactor them into static inlines)

For my own reference -- when merged this would close #15 .

agentbooth commented 4 years ago

I think specifically what was happening was that in the ACCESS_CHECK_WR, if the 2nd word needed to page fault, which would have been detected by the address+3 check, the address to page fault wasn't updated (e.g. to address+3), and then the first page ("address") was pagefaulting. But this was already in present, so that's what triggered the kernel panic. I looked at BSD source for the virtual memory (https://minnie.tuhs.org/cgi-bin/utree.pl?file=4.2BSD/usr/src/sys/sys/vm_page.c) to see what exactly triggered the "panic (pagein)" and it was when a page was already present -- that shouldn't be the case when trying to bring in a new page.

philpem commented 4 years ago

It sounds like the issue is one of hardware emulation then. The 68k has a 16-bit data bus, so will do a 32bit write as two 16bit ones. I think the priority is probably to get this bug fixed, then look at refactoring later. I'll okay this for merge and open an issue for the refactor (assuming there isn't one already)