bug in pic_prot.diff (was Re: Bootfile)

bug in pic_prot.diff (was Re: Bootfile)

[Bart Oldeman]

On Wed, 5 Feb 2003, Witold Filipczyk wrote:

> BTW, release 1.1.4.5 was the last where snake.com is working under xdos/xdosemu.
> This game is available for free download from PTSDOS site, it is small, then
> I put it in attachment.

after some binary searches I found out that Stas' pic_prot.diff is guilty
for doing this. Stas, can you figure out why?

> If text mode was fully emulated in xdosemu, it would be nice.

yes, this is what Eric Auer's patch is about (if you mean font changes in
text mode).

Bart

Re: bug in pic_prot.diff (was Re: Bootfile)

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]

Hello.

Bart Oldeman wrote:
>> BTW, release 1.1.4.5 was the last where snake.com is working under 
>> xdos/xdosemu.
>  after some binary searches I found out that Stas' pic_prot.diff is 
>  guilty for doing this.
There might be something wrong here.
The report states that this game
stopped working under "xdos/xdosemu",
and I am sure my patch have nothing
to do with an X code (that's why I
haven't even tried the game - I was
sure I couldn't break the X code).
Now I tried, and indeed it doesn't
work. However it misses the Eric A's
userfont patch, so I think it never
worked under xdosemu unless someone
applied a userfont patch manually.

> Stas, can you figure out why?
I can only figure out why it doesn't work
under console, but that's an entirely
different story. Anyway, it seems to
me that the watchdog patch was flawed:
if I set the $_pic_watchdog=(1) or 2
or 3, the game works perfectly (under
console of course), but if I set it to
10 and more - it locks up. That looks
like a problem.
The attached patch fixes it: now
setting $_pic_watchdog to any reasonable
value (within a range of 1-100 approx)
will work for that game.
However I have found my test-case
for that problem screwed up by an
XMS changes of 1.1.4.1. For now I just
reverted an XMS changes to test the
attached patch. I'll return to that
problem when I'll have more time.

Background: this game works inside a
keyboard interrupt handler (probably
many pop-up progs do so if they use
a hot-key), so essentially it needs a
reentrancy, which was restricted due
to the timer jerkiness discussed recently
around the Cosmo problem. That's why
we need (sometimes) a $_pic_watchdog.

[-- Attachment #2: pic_wc.diff --]
[-- Type: text/plain, Size: 752 bytes --]

--- src/base/dev/pic/pic.c	Wed Feb  5 07:13:57 2003
+++ src/base/dev/pic/pic.c	Thu Feb  6 08:53:07 2003
@@ -733,7 +733,6 @@
      if (test_bit(ilevel, &pic_irqall)) {
        pic_push(ilevel);
        set_bit(ilevel, &pic_irqs_active);
-       pic_wcount = 0;
      }
 
      if (!in_dpmi || in_dpmi_dos_int) {
@@ -838,7 +837,8 @@
         pic_wirr&=~pic_newirr;        /* clear watchdog timer */
 	pic_activate();
     }
-    pic_wcount = 0;
+    if (!pic_icount)
+      pic_wcount = 0;
 }
 
 /* DANG_BEGIN_FUNCTION pic_request
@@ -1031,9 +1031,8 @@
       if(++pic_wcount >= config.pic_watchdog) {
         error("PIC: force reschedule\n");
         pic_resched();
+	pic_wcount = 0;
       }
-    } else {
-      pic_wcount = 0;
     }
   }
 }      

Re: bug in pic_prot.diff (was Re: Bootfile)

[Bart Oldeman]

On Thu, 6 Feb 2003, Stas Sergeev wrote:

> There might be something wrong here. The report states that this game
> stopped working under "xdos/xdosemu", and I am sure my patch have
> nothing to do with an X code (that's why I haven't even tried the game -
> I was sure I couldn't break the X code).

yes, this is what confused me too (I thought it would be a vgaemu bug),
but it was really a keyboard related problem.

> Now I tried, and indeed it doesn't work. However it misses the Eric A's
> userfont patch, so I think it never worked under xdosemu unless someone
> applied a userfont patch manually.

maybe "working" just meant "playing with funny characters". As that's how
it worked in 1.1.4.5.

> I can only figure out why it doesn't work under console, but that's an
> entirely different story. Anyway, it seems to me that the watchdog patch
> was flawed: if I set the $_pic_watchdog=(1) or 2 or 3, the game works
> perfectly (under console of course), but if I set it to 10 and more - it
> locks up. That looks like a problem. The attached patch fixes it: now
> setting $_pic_watchdog to any reasonable value (within a range of 1-100
> approx) will work for that game.

Hmm why do you now need to enable the hack in the first place where it was
unnecessary before? That doesn't sound like a good thing to me.

> However I have found my test-case for that problem screwed up by an XMS
> changes of 1.1.4.1. For now I just reverted an XMS changes to test the
> attached patch. I'll return to that problem when I'll have more time.

i'll look at the XMS changes again to see if I missed something there.

Bart

Re: bug in pic_prot.diff (was Re: Bootfile)

[Stas Sergeev]

Hello.

Bart Oldeman wrote:
>> locks up. That looks like a problem. The attached patch fixes it: now
>> setting $_pic_watchdog to any reasonable value (within a range of 1-100
>> approx) will work for that game. 
>  Hmm why do you now need to enable the hack in the first place where it 
>  was unnecessary before? That doesn't sound like a good thing to me.
Yes, but this is not as straightforward.
There was a big discussion about a year
ago in a (unfortunately not archived
these days) dosemu-devel@ about that issue,
where the several possibilities were discussed:
Eric was standing for the time-based
measurements, which are, unfortunately,
not reliable with the current timer
precision. I was standing for the hlt-
based protection and Larry at one point
proposed to make that optional, which
is now done. As I was a proponent of a
hlt-based protection, I have made it by
default, and optionally we can enable a
time-based watchdog.
The problem is that the both approaches
are the hacks:(
It was tested and proved (at least I
hope so:) that the hlt-based approach
satisfies a majority of apps out there,
however some (counted by fingers) are
still in need of a time-based approach.
Before it was used a much weaker protection
(also a hlt-based, but flawed), and many
apps used to ocasionally crash. Now the
behaveour is at least predictable and for
the programs which can't get along with
the current scheme there is an optional
detour.
Eric wanted the time-based approach to
be enabled by default and Larry agreed
but noted that this can potentially harm
all the programs. I also think it can harm
sometimes (esp. with the current state
of an interrupt handling for DPMI!)
that's why I decided to disable it by
default.
The choice here is as follows: either
we want all the programs to work, but
potentially unreliably, or we want to
work reliably an absolute majority of
progs, but some will not work at all,
then the option.
So when the option exists, the
default value is not so important after
all. We can set it to some large value
by default if it is really necessary
for the any noticeable amount of progs,
but it really seems like it is not.
Alternatively we can always track a
timeouts and, in case $_pic_watchdog
is disabled and the timeout is frightening,
we throw an error message to hint the
user to try enabling the option if he
have a lock-up.

>> However I have found my test-case for that problem screwed up by an 
>> XMS changes of 1.1.4.1.
> i'll look at the XMS changes again to see if I missed something there.
Would be great. Let me know in case you
need a logs or a test-case.

Re: bug in pic_prot.diff (was Re: Bootfile)

[Bart Oldeman]

On Thu, 6 Feb 2003, Stas Sergeev wrote:

[...]
Ok.

> >> However I have found my test-case for that problem screwed up by an
> >> XMS changes of 1.1.4.1.
> > i'll look at the XMS changes again to see if I missed something there.
> Would be great. Let me know in case you
> need a logs or a test-case.

sure, send me (privately) a -D+x log for the working and non-working case;
I didn't spot anything obvious.

Bart