[Skiboot] [PATCH 3/8] cpu: add debug check in cpu_relax

Nicholas Piggin npiggin at gmail.com
Sat Nov 27 21:25:56 AEDT 2021


Excerpts from Cédric Le Goater's message of November 27, 2021 7:40 pm:
> On 11/27/21 08:39, Nicholas Piggin wrote:
>> Excerpts from Cédric Le Goater's message of November 26, 2021 8:39 pm:
>>> Hello,
>>>
>>> On 10/3/21 03:22, Nicholas Piggin wrote:
>>>> If cpu_relax() is called when not at medium SMT priority, it will lose
>>>> the prior priority and return at medium. Add a debug check to catch
>>>> this, which would have flagged the previous bug.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>>>> ---
>>>>    core/cpu.c          | 6 ++++++
>>>>    include/processor.h | 1 +
>>>>    2 files changed, 7 insertions(+)
>>>>
>>>> diff --git a/core/cpu.c b/core/cpu.c
>>>> index 5c10fc6e8..0f2da1524 100644
>>>> --- a/core/cpu.c
>>>> +++ b/core/cpu.c
>>>> @@ -80,6 +80,12 @@ unsigned long __attrconst cpu_emergency_stack_top(unsigned int pir)
>>>>    
>>>>    void __nomcount cpu_relax(void)
>>>>    {
>>>> +	if ((mfspr(SPR_PPR32) >> 18) != 0x4) {
>>>
>>>
>>> Why not use  SPR_PPR ? SPR_PPR32 is for the embedded world according
>>> to ISA.
>> 
>> Hmm I don't see that mentioned in ISA 3.1. I used PPR32 because of the
>> programming note in 3.1 Program Priority Registers which says "The
>> ability to access the low-order half of the PPR (and thus the use of
>> mfppr and mtppr) might be phased out in a future version of the
>> architecture".
> 
> I was looking at v2.07 because the QEMU powernv8 machine started
> complaining. We might want to model the PPR reg and the 'or' insn
> for it. No big deal either, it's just a warning.

Hmm. We can change skiboot to PPR if it's causing issues with existing 
code. We should update qemu, and some point after that we might 
eventually change Linux over to use PPR32. Linux of course is the bigger 
problem than skiboot if the ISA ever wants to deprecate PPR.

Thanks,
Nick



More information about the Skiboot mailing list