Send SIGILL on HEA illegal instruction exception

[mst_semihalf.com]

Summary

Currently Hypervisor Emulation Assistance interrupt is unhandled. Executing an undefined instruction in userland triggers kernel panic. Handle this the same way as Facility Unavailable Interrupt - send SIGILL signal to userspace.

[nwhitehorn]

I'm looking at this again. Does this actually work? HEA interrupts set HSRR0/HSSR1, which we don't save, so I think this will jump back to some random other memory location (whatever happened to be in SRR0).

[mst_semihalf.com]

In D14437#309033, @nwhitehorn wrote:

I'm looking at this again. Does this actually work? HEA interrupts set HSRR0/HSSR1, which we don't save, so I think this will jump back to some random other memory location (whatever happened to be in SRR0).

It seemed to work, I tested it by altering an opcode in a binary to an invalid one.

before patch:

root@:~/mst # ./arraycpy

fatal user trap:

exception       = 0xe40 (hypervisor emulation assistance)
srr0            = 0x810281d48 (0x80edb1d48)
srr1            = 0x900000000000f032
lr              = 0x10000a48 (0xeb30a48)
curthread       = 0x1af03560
       pid = 1129, comm = arraycpy

[ thread pid 1129 tid 100880 ]
Stopped at 0x810281d48
db>
db>

after patch:

root@:/mst # ./arraycpy
Illegal instruction (core dumped)
root@:/mst #

[nwhitehorn]

Sure, it will deliver the signal. But the value of MSR when control is returned to userland may be wrong (it may even have MSR[PR] unset and return in supervisor mode if the previous trap on this CPU was in the kernel!) and it will resume execution in the wrong place if the signal is caught and handled.

[mst_semihalf.com]

I have no access to HW currently so cannot check this, but it seems you're right. I did not think of the case where userspace catches the signal.

[nwhitehorn]

I think there is also a subtlety for uncaught signals if the previous trap was a kernel fault, since you then end up in the wrong branch of the kernel/user if in trap() and probably crash.

In any case, I think the simplest solution is to have a replacement for trapcode that does the same things as trapcode, but additionally moves hsrr0/hsrr1 into srr0/srr1. Until we try to support bhyve, which is a larger project, they are safe to clobber in this context. trapcode is currently using 7 of a possible 32 instructions, so adding a variant with four more (mfhsrr0 %r1; mtsrr0 %r1; mfhsrr1 %r1; mtsrr1 %r1) in the middle should be fine. That also avoids the need for hrfid and related things when returning. We would then just install the variant for traps like HEA that are known to set HSRR* instead of SRR* and keep the rest of the trap-handling code as-is.

[mst_semihalf.com]

In D14437#309219, @nwhitehorn wrote:

I think there is also a subtlety for uncaught signals if the previous trap was a kernel fault, since you then end up in the wrong branch of the kernel/user if in trap() and probably crash.

In any case, I think the simplest solution is to have a replacement for trapcode that does the same things as trapcode, but additionally moves hsrr0/hsrr1 into srr0/srr1. Until we try to support bhyve, which is a larger project, they are safe to clobber in this context. trapcode is currently using 7 of a possible 32 instructions, so adding a variant with four more (mfhsrr0 %r1; mtsrr0 %r1; mfhsrr1 %r1; mtsrr1 %r1) in the middle should be fine. That also avoids the need for hrfid and related things when returning. We would then just install the variant for traps like HEA that are known to set HSRR* instead of SRR* and keep the rest of the trap-handling code as-is.

This seems like a simple change but I have no way of testing it as I still don't have access to any ppc64 HW. Is it possible for you to commit this change yourself?