Re: CVS commit: src/sys/arch/m68k/m68k

[Tetsuya Isaki <isaki%pastel-flower.jp@localhost>]

At Tue, 1 Apr 2025 02:26:35 +1100,
Nathanial Sloss wrote:
> Consider the following (including my gas patch for binutils):
> 
> movq.....
> nop
> fnop
> 
> Thats as it's assembled but at run time it could posslibly
> run like this
> 
> movq....
> nop
> (switch task)
> fnop
> 
> So the nops are inserted before rts statements in the switcher
> just in case 
> should the next instruction be an f-line instruction.

Your understanding should be wrong, if you are talking about
timer interrupt and/or process context switch.
All nops you've inserted are far from the userland.
(And the instruction to return to the userland is rte, not rts.)

So these all nops are never executed just before any of f-line
instructions.  More precisely, the only first 3 are never
executed just before any of f-line instructions.
The remaining 5 are never executed on 68LC040 due to fputype==0.
Furthermore, 2 of these 5 are completely omitted from mac68k
kernel binary due to #if M68060.

> I'ts possible I've added too many (9 words in total) but I'm
> being over 
> cautious--- its better to be safe than sorry!

Correction: nops you've inserted is 8, not 9.

> With these changes (along with the modified assembler and
> recompiled userland) 
> I've been running a PowerBook 520c (with an XC68LC040 buggy
> processor) for a 
> few months now, flawlessly with the fpu emulation.
> 
> What I was after was a uniform approach to all m68k
> (buggy lc040's included).

I'm not against supporting XC processors, but I don't think
it should come at any cost.  Because it's well known as
experimental and (heavily) buggy.
Of course, just my personal opinion.

But that is not the main point.  What I ask here is 
inconsistency between your explanation and your code.
Adding such unverified code (especially in a critical part)
will soon become difficult to maintain.

> If you have an alternate proposal I'm very interested.

Heh, No such phase yet
because you haven't answered all questions yet.

Nevertheless, if I were to suggest something at this point,
(But who will evaluate? :-)

o Fix comment.  Even if your patch really makes your LC040
  stable, it looks unrelated to the described workaround.
  Old wrong(suspicious) comments have often got us in trouble.
  So we shouldn't do that for our future selves.

o Replace #if 1 to #ifdef __mac68k__ or,
  #ifdef XC68LC040_HACK and define it in mac68k.
  This is critical part in the kernel, and XC68LC040 is not
  widely used.  The impact should be reduced as possible.

--- sys/arch/m68k/m68k/switch_subr.s.orig
+++ sys/arch/m68k/m68k/switch_subr.s
@@ -77,7 +77,7 @@
  *
  *  Refer to: NetBSD PR #13078.
  */
-#if 1
+#if defined(__mac68k__)
 #define LC_NOP 	nop
 #else
 #define LC_NOP

o Remove these five nops that are never executed.

--- sys/arch/m68k/m68k/switch_subr.s.orig
+++ sys/arch/m68k/m68k/switch_subr.s
@@ -304,7 +304,6 @@
 	frestore (%sp)
 	fnop
 	addql	#4,%sp
-	LC_NOP
 	rts
 #endif
 
@@ -326,7 +325,6 @@
 	fmovem	%fp0-%fp7,FPF_REGS(%a0)	| save FP general registers
 	fmovem	%fpcr/%fpsr/%fpi,FPF_FPCR(%a0) | save FP control registers
 .Lm68881sdone:
-	LC_NOP
 	rts
 #endif
 #if defined(M68060)
@@ -338,7 +336,6 @@
 	fmovem	%fpsr,FPF_FPSR(%a0)
 	fmovem	%fpi,FPF_FPI(%a0)
 .Lm68060sdone:
-	LC_NOP
         rts
 #endif
 
@@ -356,7 +353,6 @@
 	fmovem	FPF_REGS(%a0),%fp0-%fp7	| restore FP general registers
 .Lm68881rdone:
 	frestore (%a0)			| restore state
-	LC_NOP
 	rts
 #endif
 #if defined(M68060)
@@ -369,7 +365,6 @@
 	fmovem	FPF_REGS(%a0),%fp0-%fp7 | restore FP general registers
 .Lm68060fprdone:
 	frestore (%a0)			| restore state
-	LC_NOP
 	rts
 #endif
 #endif
---
Tetsuya Isaki <isaki%pastel-flower.jp@localhost / isaki%NetBSD.org@localhost>