[Skiboot] [RFC PATCH 1/2] move the __this_cpu register to r16, reserve r13-r15

Nicholas Piggin npiggin at gmail.com
Tue Dec 3 01:16:21 AEDT 2019


Oliver O'Halloran's on December 2, 2019 9:44 pm:
> On Mon, Dec 2, 2019 at 6:46 PM Nicholas Piggin <npiggin at gmail.com> wrote:
>>
>> There have been several bugs between Linux and OPAL caused by both
>> using r13 for their primary per-CPU data address. This patch moves
>> OPAL to use r16 for this, and prevents the compiler from touching
>> r13-r15. This helps things to be a little more robust, and also makes
>> crashes in OPAL easier to debug.
> 
> I'm ok with using r16 instead of r13 in skiboot, but what's the
> significance of r14 and r15?

Ah yes I didn't actually explain that. It's just a couple of registers
that Linux *might* use as fixed registers kind of similar to r13. I've
got some patches which put the per-cpu data offset and several useful
flags into r14 for example.

>> Later, if we allow interrupts (other than non-maskable) to be taken when
>> running in skiboot, Linux's interrupt return handler does not restore
>> r13 if the interrupt was taken in PR=0 state, which would corrupt the
>> skiboot r13 register.
> 
> What's the goal here? Is it just to avoid hard-disabling IRQs when we
> make an OPAL call or do you have bigger plans? There's a lot of
> potential for that to turn into a total trainwreck, but you're welcome
> to try.

Not _entirely_ sure yet. You're right asynchronous interrupts might end
up being a bit over-complicated, although the watchdog "soft-nmi"
interrupt can be enabled easily. For synchronous interrupts we might
keep the traps patched in to skiboot and take program check interrupts,
although those tend not to return (unless we implement a warn_on kind
of facility).

Runtime virtual memory was the case I hit that prompted this patch, 
taking page faults in skiboot. I think possibly a better design there
is to always have the necessary mappings set up so we never take a page
fault though (although that may not be desirable with HPT, although we
may just choose not to support runtime VM with HPT).

So yeah there's a few possibilties but nothing concrete just yet. At
the moment the benefit is just to debugging and possible avoidance of
subtle bugs.

It's also not exactly tied to patch 2, but I'm thinking the
availability of that dt property would signal that OPAL does not
clobber those regs (I should update the device tree binding and
comment on the OPAL API to that end).

Thanks,
Nick


More information about the Skiboot mailing list