Hi,
> Similarly to the system call change in the previous patch, the mtmsrd to
I don't actually know what patch this was - I assume it's from a series
that has since been applied?
> enable RI can be combined with the mtmsrd to enable EE for interrupts
> which enable the latter, which tends to be the important synchronous
> interrupts (i.e., page faults).
>
> Do this by enabling EE and RI together at the beginning of the entry
> wrapper if PACA_IRQ_HARD_DIS is clear, and just enabling RI if it is set
> (which means something wanted EE=0).
> diff --git a/arch/powerpc/include/asm/interrupt.h
> b/arch/powerpc/include/asm/interrupt.h
> index 6b800d3e2681..e3228a911b35 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -148,9 +148,21 @@ static inline void interrupt_enter_prepare(struct
> pt_regs *regs, struct interrup
> #endif
>
> #ifdef CONFIG_PPC64
> - if (irq_soft_mask_set_return(IRQS_ALL_DISABLED) == IRQS_ENABLED)
> + bool trace_enable = false;
> +
> + if (IS_ENABLED(CONFIG_TRACE_IRQFLAGS)) {
> + if (irq_soft_mask_set_return(IRQS_DISABLED) == IRQS_ENABLED)
In the previous code we had IRQS_ALL_DISABLED, now we just have
IRQS_DISABLED. Is that intentional?
> + trace_enable = true;
> + } else {
> + irq_soft_mask_set(IRQS_DISABLED);
> + }
> + /* If the interrupt was taken with HARD_DIS set, don't enable MSR[EE] */
> + if (local_paca->irq_happened & PACA_IRQ_HARD_DIS)
> + __hard_RI_enable();
> + else
> + __hard_irq_enable();
> + if (trace_enable)
> trace_hardirqs_off();
> - local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>
> if (user_mode(regs)) {
> CT_WARN_ON(ct_state() != CONTEXT_USER);
> @@ -901,11 +892,6 @@ INT_DEFINE_BEGIN(system_reset)
> IVEC=0x100
> IAREA=PACA_EXNMI
> IVIRT=0 /* no virt entry point */
> - /*
> - * MSR_RI is not enabled, because PACA_EXNMI and nmi stack is
> - * being used, so a nested NMI exception would corrupt it.
> - */
> - ISET_RI=0
> ISTACK=0
> IKVM_REAL=1
> INT_DEFINE_END(system_reset)
> @@ -986,8 +972,6 @@ EXC_COMMON_BEGIN(system_reset_common)
Right before this change, there's a comment that reads:
/*
* Increment paca->in_nmi then enable MSR_RI. SLB or MCE will be able
* to recover, but nested NMI will notice in_nmi and not recover
...
You've taken out the bit that enables MSR_RI, which means the comment is
no longer accurate.
Beyond that, I'm still trying to follow all the various changes in code
flows. It seems to make at least vague sense: we move the setting of
MSR_RI from the early asm to interrupt*enter_prepare. I'm just
struggling to convince myself that when we hit the underlying handler
that the RI states all line up.
Kind regards,
Daniel
