[v2,2/2] m68k: Fix interrupt stack frames for 68000

  The plain old 68000 does not push the frame type/vector on the
stack when an interrupt starts like the brand new 68010 does.

This means that currently everything in struct pt_regs is
a bit off because it expects the processor to push an extra
short before the kernel interrupt code adds the rest.

In entry.S for the 68000 we already need to manually put
the vector number on the stack to work out what interrupt
is being handled because the cpu doesn't push that to the
stack.

So we can jiggle this around a bit to fix the issue:
- For 68000 use the same struct pt_regs layout as coldfire
  where frame/vector is after pc and sp.
- In entry.S push the vector number first, the stack pointer
  now lines up with the sktadj field in pt_regs and when saving
  the remaining registers the offsets match the fields in the
  struct.
- Remove the vec argument from the DragonBall interrupt
  decoding logic as it's not pushed on the stack anymore
  and not used either way.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 arch/m68k/68000/entry.S             | 9 ++++-----
 arch/m68k/68000/ints.c              | 2 +-
 arch/m68k/include/asm/entry.h       | 3 +++
 arch/m68k/include/uapi/asm/ptrace.h | 2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)
  

Hi Daniel,

Thanks for your patch!

On Mon, Jan 8, 2024 at 10:32 AM Daniel Palmer <daniel@0x0f.com> wrote:
> The plain old 68000 does not push the frame type/vector on the
> stack when an interrupt starts like the brand new 68010 does.

;-)

> This means that currently everything in struct pt_regs is
> a bit off because it expects the processor to push an extra
> short before the kernel interrupt code adds the rest.
>
> In entry.S for the 68000 we already need to manually put
> the vector number on the stack to work out what interrupt
> is being handled because the cpu doesn't push that to the
> stack.
>
> So we can jiggle this around a bit to fix the issue:
> - For 68000 use the same struct pt_regs layout as coldfire
>   where frame/vector is after pc and sp.
> - In entry.S push the vector number first, the stack pointer
>   now lines up with the sktadj field in pt_regs and when saving
>   the remaining registers the offsets match the fields in the
>   struct.
> - Remove the vec argument from the DragonBall interrupt
>   decoding logic as it's not pushed on the stack anymore
>   and not used either way.
>
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>

> --- a/arch/m68k/include/uapi/asm/ptrace.h
> +++ b/arch/m68k/include/uapi/asm/ptrace.h
> @@ -39,7 +39,7 @@ struct pt_regs {
>    long     d0;
>    long     orig_d0;
>    long     stkadj;
> -#ifdef CONFIG_COLDFIRE
> +#if defined(CONFIG_COLDFIRE) || defined(CONFIG_M68000)
>    unsigned format :  4; /* frame format specifier */
>    unsigned vector : 12; /* vector offset */
>    unsigned short sr;

I think it would be better to use the classic m68k stack frame.
That would pave the way for building a single nommu kernel for
MC680[012346]0 that runs on e.g. any Amiga.
MC68000 and Coldfire are incompatible anyway.

Gr{oetje,eeting}s,

                        Geert
  

Hi Geert,

On Mon, 8 Jan 2024 at 18:56, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> I think it would be better to use the classic m68k stack frame.
> That would pave the way for building a single nommu kernel for
> MC680[012346]0 that runs on e.g. any Amiga.
> MC68000 and Coldfire are incompatible anyway.

Ok, I'll work that out. I have an A600 with an 020 board so I could
actually test it on real hardware. :)

Cheers,

Daniel
  

Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on gerg-m68knommu/for-next]
[also build test ERROR on geert-m68k/for-next geert-m68k/for-linus linus/master v6.7 next-20240108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Palmer/m68k-Use-macro-to-generate-68000-interrupt-entry-sleds/20240108-175040
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gerg/m68knommu.git for-next
patch link:    https://lore.kernel.org/r/20240108093221.1477020-3-daniel%400x0f.com
patch subject: [PATCH v2 2/2] m68k: Fix interrupt stack frames for 68000
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240109/202401091123.K60t1Foe-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240109/202401091123.K60t1Foe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401091123.K60t1Foe-lkp@intel.com/

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
   scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
>> error: arch/m68k/include/uapi/asm/ptrace.h: leak CONFIG_M68000 to user-space
   make[3]: *** [scripts/Makefile.headersinst:63: usr/include/asm/ptrace.h] Error 1
   make[3]: Target '__headers' not remade because of errors.
   make[2]: *** [Makefile:1288: headers] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:234: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:234: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.
  

Hi All,

Sorry for the spam..

On Mon, 8 Jan 2024 at 18:56, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> I think it would be better to use the classic m68k stack frame.
> That would pave the way for building a single nommu kernel for
> MC680[012346]0 that runs on e.g. any Amiga.
> MC68000 and Coldfire are incompatible anyway.

While looking at how to do this I realised that the addql #2,%sp in
RESTORE_ALL in entry.h will now break the stack frames for those fancy
68010+ users.
So that needs to be #ifdef'd to make it only compile for 68000. I saw
an error email from the next build stuff so I guess the change has
been queued somewhere? If so I should send a fix..
I'm not sure how to actually make that generic without patching the
code at runtime (remove the 68000 specific bit, reserve enough extra
space to rewrite the code..) but it's a macro so not so simple.

Anyhow, and more importantly, it seems like there is another issue in
68000/entry.S that breaks syscalls (especially vfork). After fixing
that I now have a working nommu 68000 system. I'll send a fix for that
too.

Cheers,

Daniel
  

Hi Daniel,

On Tue, Jan 9, 2024 at 3:10 PM Daniel Palmer <daniel@0x0f.com> wrote:
> On Mon, 8 Jan 2024 at 18:56, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > I think it would be better to use the classic m68k stack frame.
> > That would pave the way for building a single nommu kernel for
> > MC680[012346]0 that runs on e.g. any Amiga.
> > MC68000 and Coldfire are incompatible anyway.
>
> While looking at how to do this I realised that the addql #2,%sp in
> RESTORE_ALL in entry.h will now break the stack frames for those fancy
> 68010+ users.
> So that needs to be #ifdef'd to make it only compile for 68000. I saw
> an error email from the next build stuff so I guess the change has
> been queued somewhere? If so I should send a fix..

AFAIK it hasn't been applied yet.  These days the bots also test
patches from mailing lists...

> I'm not sure how to actually make that generic without patching the
> code at runtime (remove the 68000 specific bit, reserve enough extra
> space to rewrite the code..) but it's a macro so not so simple.

Or use different entry points depending on CPU type?

Gr{oetje,eeting}s,

                        Geert