search

PDP-10 / its

Incompatible Timesharing System
Other
834 stars 80 forks source link

Make CH10 pick up address from interface #2293

Closed bictorv closed 3 months ago

bictorv commented 3 months ago

just like CH11 does (see PR #2038). This makes it easier to change a system's address (e.g. for joining the Global Chaosnet).

(Version numbers should be bumped.)

larsbrinkhoff commented 3 months ago

Thank you. I will check this, but I'll be busy the coming weeks so it may be a while.

eswenson1 commented 3 months ago

I haven't actually built and tested this on a CH10 system yet. Will do that next.

eswenson1 commented 3 months ago

In the booted KL instance, I manually tried to build ITS. I see the following messages (among others, of course):

KL:INET 139 SYSTEM; IPMTU1+2 55003 3. 10-034 Amazing MIT-Specific crocks near IPMTU.! .. ==> INSERTED: INET 139 ==> INSERTED: TCP 276 ==> INSERTED: CHSDEF 15 ==> INSERTED: CHAOS 290 ==> INSERTED: NET 33 ==> INSERTED: ITSMSP 30 ==> INSERTED: TS3TTY 402 ==> INSERTED: ITSDEV 977 ==> INSERTED: EVSYMS 21 ==> INSERTED: CORE 82 ==> INSERTED: SYSJOB 119 HIGHEST USED = 171460 SPACE LEFT IN UPT = 13 STORAGE PER LOSER = 1000 KL:ITS 1651 SYSTEM; BUGBUG+5 174430 1. 305-081 Stray ] IEND 175000 1. 309-036 MAKE THE BUGTAB BIGGER LBUGTB-LBUGT2 = 2 IEND 175000 1. 309-038 FATAL ERROR, RUNNING THIS WOULD CAUSE T! OTALLY WEIRD BUGS Run time = ,.^N7 9134 Symbols including initial ones (76% used)

So, clearly, there is something wrong with either ITS or CONFIG. It doesn’t create a binary, and thus, we never (in the KL build) created a new ITS. We simply got the MX one that we got off the binary bootstrap tape.

I’ll try to track this down. Not sure if it is a bug in your ITS changes, or a bug in the CONFIG for KL yet.

eswenson1 commented 3 months ago

Turns out there was an extra ] -- where the double ]] is. Also, even if you fix that, you get an error that the BUGTAB needs to get increased in size. Fixing that allows a pdp10-kl build to build the ITS, which correctly gets CH10P defined as 1 and an ITSNMS table that matches the MCOND KL entry in CONFIG 202. However, that ITSNMS table needs to have a KL entry in it, and probably shouldn't have MC in it.

eswenson1 commented 3 months ago

With my fixes to ITS and CONFIG, I can boot a KL instance now that correctly gets MYCHDR set. I will, with your permission, update this branch/PR. Please let me know if ok.

bictorv commented 3 months ago

Turns out there was an extra ] -- where the double ]] is.

Aaargh. Sorry about that! Weird about the BUGTAB though? But please go ahead!

eswenson1 commented 3 months ago

The new conditional for CH10P must have added one or two BUGHLTs that KL didn’t have before.

eswenson1 commented 3 months ago

Ok. I believe this is ready to go now. I updated the version numbers of SYSTEM;CHAOS, SYSTEM;CONFIG, and SYSTEM;ITS since this PR changes all those files.

bictorv commented 3 months ago

Strictly speaking, I think neither the CONFIG nor the BUGTAB patches are related to the picking up the Chaos address from the CH10 device, but personally I think it's OK. @larsbrinkhoff would have to complain? :-)

larsbrinkhoff commented 3 months ago

Right, Eric's commit are really three separate things. CONFIG and BUGTBL can be made separate commits, and the last to fix the double bracket should be folded back into Björn's commit.

eswenson1 commented 3 months ago

While I agree that the change to the IRPNMS table for the KL machine is unrelated, I consider the BUGTBL change directly related to the CH10 chaos change. The new code added when CH10P is 1 causes the BUGTBL table to overflow. So I would think any CH10P build would fail without this bump. Strictly speaking the old size could be used for CH10P=0 builds and the new value for CH10P builds. But I didn’t think it was that critical. Do you?

bictorv commented 3 months ago

But you get the BUGTAB overflow also without the CH10-pickup-address fix, if you enable CH10P, so it is not related to "my" PR? There are no BUGxxx in the code affected by "my" fix?

larsbrinkhoff commented 3 months ago

In my view, it's not overly problematic if weakly related changes are grouped together in one pull request.

bictorv commented 3 months ago

I already claimed I think it's OK. So what needs to change, @larsbrinkhoff ?

larsbrinkhoff commented 3 months ago

One thing I do think is important, is the notion of atomic commits. I.e. a commit should only address one concern, and it should contain everything needed for that one concern. So e.g. Eric's commit can be split in three logically different concern. One of those addresses something in a commit earlier in this same pull request, so that earlier commit should be amended.

eswenson1 commented 3 months ago

But you get the BUGTAB overflow also without the CH10-pickup-address fix, if you enable CH10P, so it is not related to "my" PR? There are no BUGxxx in the code affected by "my" fix?

Well, I've been building KL builds for a while (it was the basis for my EXL machine) and this, clearly, defined CH10P=1, since this is the only way to get chaos on the KL10. And I didn't have the BUGTBL overflow before. So I was assuming that @bictorv's change was what pushed the BUGHLT use to overflow the table.

Note that the pdp10-kl build has had CH10P=1 there for a while, and I would have thought that if we were overflowing BUGTBL, we would have noticed (I guess it is possible that this has been failing for a while and we never noticed).

I noticed this first in this branch -- which is why I assumed it was the new code that was causing the overflow.

I suppose I could build a master branch pdp10-kl build to see if it, too, overflowed the BUGTBL. I'll do that and report on the results.

eswenson1 commented 3 months ago

Just started a build of pdp10-kl from master branch. It did NOT get an overflow of the BUGTBL when it assembled. So this seems to confirm that it was the changes in this branch that caused the overflow.

eswenson1 commented 3 months ago

I deleted my previous post because I only grabbed ITS and not also CHAOS. Retrying build on master branch with those two changed files (original versions from @bictorv).

eswenson1 commented 3 months ago

I dropped @bictorv's CHAOS and ITS on top of the master branch and tried a build. It failed in the way as my earlier experience:

...
    ==> INSERTED:  INET 139
    ==> INSERTED:  TCP 276
    ==> INSERTED:  CHSDEF 15
    ==> INSERTED:  CHAOS 291
    ==> INSERTED:  NET 33
    ==> INSERTED:  ITSMSP 30
    ==> INSERTED:  TS3TTY 402
    ==> INSERTED:  ITSDEV 977
    ==> INSERTED:  EVSYMS 21
    ==> INSERTED:  CORE 82
    ==> INSERTED:  SYSJOB 119
HIGHEST USED = 171460
SPACE LEFT IN UPT = 13
STORAGE PER LOSER = 1000
KL:ITS 1652 SYSTEM;
IEND            175000   1.   309-036   MAKE THE BUGTAB BIGGER
LBUGTB-LBUGT2 = 2
IEND            175000   1.   309-038   FATAL ERROR, RUNNING THIS WOULD CAUSE TOTALLY WEIRD BUGS
Run time = 0.00
9134 Symbols including initial ones (76% used)

:KILL

So the BUGTAB size increase is necessary for the combination of the new ITS and CHAOS.

Note that if you don't include the CHAOS change, the same ITS builds just fine (not that it is useful -- but it assembles).

Net of all this: the BUGTAB increase should be part of the same commit as the one that includes the changes to ITS and CHAOS, and it not a separate commit. The commit without this change doesn't build.

eswenson1 commented 3 months ago

@larsbrinkhoff Please let me know if you are now happy with this PR (@bictorv too).

I combined Victor's original commit with a commit with only the fix to ITS and the bumps of the version numbers. And I made the second commit be just the CONFIG change.