Re: [PATCH v2] powerpc/traps: Enhance readability for trap types

On Thu, Apr 01, 2021 at 06:01:29PM +1000, Nicholas Piggin wrote:

Excerpts from Michael Ellerman's message of April 1, 2021 12:39 pm:

quoted

Segher Boessenkool [off-list ref] writes:

quoted

On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:

quoted

So perhaps:

  EXC_SYSTEM_RESET
  EXC_MACHINE_CHECK
  EXC_DATA_STORAGE
  EXC_DATA_SEGMENT
  EXC_INST_STORAGE
  EXC_INST_SEGMENT
  EXC_EXTERNAL_INTERRUPT
  EXC_ALIGNMENT
  EXC_PROGRAM_CHECK
  EXC_FP_UNAVAILABLE
  EXC_DECREMENTER
  EXC_HV_DECREMENTER
  EXC_SYSTEM_CALL
  EXC_HV_DATA_STORAGE
  EXC_PERF_MONITOR

These are interrupt (vectors), not exceptions.  It doesn't matter all
that much, but confusing things more isn't useful either!  There can be
multiple exceptions that all can trigger the same interrupt.

Yeah I know, but I think that ship has already sailed as far as the
naming we have in the kernel.

It has, but there are also several other ships also sailing in different 
directions. It could be worse though, at least they are not sideways in 
the Suez.

:-)

quoted

We have over 250 uses of "exc", and several files called "exception"
something.

Using "interrupt" can also be confusing because Linux uses that to mean
"external interrupt".

But I dunno, maybe INT or VEC is clearer? .. or TRAP :)

We actually already have defines that follow Segher's suggestion, it's 
just that they're hidden away in a KVM header.

#define BOOK3S_INTERRUPT_SYSTEM_RESET   0x100
#define BOOK3S_INTERRUPT_MACHINE_CHECK  0x200
#define BOOK3S_INTERRUPT_DATA_STORAGE   0x300
#define BOOK3S_INTERRUPT_DATA_SEGMENT   0x380
#define BOOK3S_INTERRUPT_INST_STORAGE   0x400
#define BOOK3S_INTERRUPT_INST_SEGMENT   0x480
#define BOOK3S_INTERRUPT_EXTERNAL       0x500
#define BOOK3S_INTERRUPT_EXTERNAL_HV    0x502
#define BOOK3S_INTERRUPT_ALIGNMENT      0x600

It would take just a small amount of work to move these to general 
powerpc header, add #ifdefs for Book E/S where the numbers differ,
and remove the BOOK3S_ prefix.

I don't mind INTERRUPT_ but INT_ would be okay too. VEC_ actually
doesn't match what Book E does (which is some weirdness to map some
of them to match Book S but not all, arguably we should clean that
up too and just use vector numbers consistently, but the INTERRUPT_
prefix would still be valid if we did that).

VEC also is pretty incorrect: there is code at those addresses, not
vectors pointing to code (as similar things on some other architectures
have).  Everyone understands what it means of course, except it is
confusing with a thing we *do* have on Power called VEC (the MSR bit) :-P

(And TRAP is just one cause of 700...)


Segher