* Re: [PATCH v2 24/37] KVM: PPC: Book3S HV P9: inline kvmhv_load_hv_regs_and_go into __kvmhv_vcpu_entry_p9
From: Fabiano Rosas @ 2021-03-02 13:48 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210225134652.2127648-25-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Now the initial C implementation is done, inline more HV code to make
> rearranging things easier.
>
> And rename __kvmhv_vcpu_entry_p9 to drop the leading underscores as it's
> now C, and is now a more complete vcpu entry.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_64.h | 2 +-
> arch/powerpc/kvm/book3s_hv.c | 181 +----------------------
> arch/powerpc/kvm/book3s_hv_interrupt.c | 168 ++++++++++++++++++++-
> 3 files changed, 169 insertions(+), 182 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index c214bcffb441..eaf3a562bf1e 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -153,7 +153,7 @@ static inline bool kvmhv_vcpu_is_radix(struct kvm_vcpu *vcpu)
> return radix;
> }
>
> -int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu);
> +int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpcr);
>
> #define KVM_DEFAULT_HPT_ORDER 24 /* 16MB HPT by default */
> #endif
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 28a2761515e3..f99503acdda5 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3442,183 +3442,6 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
> trace_kvmppc_run_core(vc, 1);
> }
>
> -static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u64 lpcr)
> -{
> - struct kvmppc_vcore *vc = vcpu->arch.vcore;
> - struct kvm_nested_guest *nested = vcpu->arch.nested;
> - u32 lpid;
> -
> - lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
> -
> - mtspr(SPRN_LPID, lpid);
> - mtspr(SPRN_LPCR, lpcr);
> - mtspr(SPRN_PID, vcpu->arch.pid);
> - isync();
> -
> - /* TLBIEL must have LPIDR set, so set guest LPID before flushing. */
> - kvmppc_check_need_tlb_flush(kvm, vc->pcpu, nested);
> -}
> -
> -static void switch_mmu_to_host_radix(struct kvm *kvm, u32 pid)
> -{
> - mtspr(SPRN_PID, pid);
> - mtspr(SPRN_LPID, kvm->arch.host_lpid);
> - mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
> - isync();
> -}
> -
> -/*
> - * Load up hypervisor-mode registers on P9.
> - */
> -static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> - unsigned long lpcr)
> -{
> - struct kvm *kvm = vcpu->kvm;
> - struct kvmppc_vcore *vc = vcpu->arch.vcore;
> - s64 hdec;
> - u64 tb, purr, spurr;
> - int trap;
> - unsigned long host_hfscr = mfspr(SPRN_HFSCR);
> - unsigned long host_ciabr = mfspr(SPRN_CIABR);
> - unsigned long host_dawr0 = mfspr(SPRN_DAWR0);
> - unsigned long host_dawrx0 = mfspr(SPRN_DAWRX0);
> - unsigned long host_psscr = mfspr(SPRN_PSSCR);
> - unsigned long host_pidr = mfspr(SPRN_PID);
> - unsigned long host_dawr1 = 0;
> - unsigned long host_dawrx1 = 0;
> -
> - if (cpu_has_feature(CPU_FTR_DAWR1)) {
> - host_dawr1 = mfspr(SPRN_DAWR1);
> - host_dawrx1 = mfspr(SPRN_DAWRX1);
> - }
> -
> - tb = mftb();
> - hdec = time_limit - tb;
> - if (hdec < 0)
> - return BOOK3S_INTERRUPT_HV_DECREMENTER;
> -
> - if (vc->tb_offset) {
> - u64 new_tb = tb + vc->tb_offset;
> - mtspr(SPRN_TBU40, new_tb);
> - tb = mftb();
> - if ((tb & 0xffffff) < (new_tb & 0xffffff))
> - mtspr(SPRN_TBU40, new_tb + 0x1000000);
> - vc->tb_offset_applied = vc->tb_offset;
> - }
> -
> - if (vc->pcr)
> - mtspr(SPRN_PCR, vc->pcr | PCR_MASK);
> - mtspr(SPRN_DPDES, vc->dpdes);
> - mtspr(SPRN_VTB, vc->vtb);
> -
> - local_paca->kvm_hstate.host_purr = mfspr(SPRN_PURR);
> - local_paca->kvm_hstate.host_spurr = mfspr(SPRN_SPURR);
> - mtspr(SPRN_PURR, vcpu->arch.purr);
> - mtspr(SPRN_SPURR, vcpu->arch.spurr);
> -
> - if (dawr_enabled()) {
> - mtspr(SPRN_DAWR0, vcpu->arch.dawr0);
> - mtspr(SPRN_DAWRX0, vcpu->arch.dawrx0);
> - if (cpu_has_feature(CPU_FTR_DAWR1)) {
> - mtspr(SPRN_DAWR1, vcpu->arch.dawr1);
> - mtspr(SPRN_DAWRX1, vcpu->arch.dawrx1);
> - }
> - }
> - mtspr(SPRN_CIABR, vcpu->arch.ciabr);
> - mtspr(SPRN_IC, vcpu->arch.ic);
> -
> - mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
> - (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> -
> - mtspr(SPRN_HFSCR, vcpu->arch.hfscr);
> -
> - mtspr(SPRN_SPRG0, vcpu->arch.shregs.sprg0);
> - mtspr(SPRN_SPRG1, vcpu->arch.shregs.sprg1);
> - mtspr(SPRN_SPRG2, vcpu->arch.shregs.sprg2);
> - mtspr(SPRN_SPRG3, vcpu->arch.shregs.sprg3);
> -
> - mtspr(SPRN_AMOR, ~0UL);
> -
> - switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
> -
> - /*
> - * P9 suppresses the HDEC exception when LPCR[HDICE] = 0,
> - * so set guest LPCR (with HDICE) before writing HDEC.
> - */
> - mtspr(SPRN_HDEC, hdec);
> -
> - mtspr(SPRN_SRR0, vcpu->arch.shregs.srr0);
> - mtspr(SPRN_SRR1, vcpu->arch.shregs.srr1);
> -
> - trap = __kvmhv_vcpu_entry_p9(vcpu);
> -
> - /* Advance host PURR/SPURR by the amount used by guest */
> - purr = mfspr(SPRN_PURR);
> - spurr = mfspr(SPRN_SPURR);
> - mtspr(SPRN_PURR, local_paca->kvm_hstate.host_purr +
> - purr - vcpu->arch.purr);
> - mtspr(SPRN_SPURR, local_paca->kvm_hstate.host_spurr +
> - spurr - vcpu->arch.spurr);
> - vcpu->arch.purr = purr;
> - vcpu->arch.spurr = spurr;
> -
> - vcpu->arch.ic = mfspr(SPRN_IC);
> - vcpu->arch.pid = mfspr(SPRN_PID);
> - vcpu->arch.psscr = mfspr(SPRN_PSSCR) & PSSCR_GUEST_VIS;
> -
> - vcpu->arch.shregs.sprg0 = mfspr(SPRN_SPRG0);
> - vcpu->arch.shregs.sprg1 = mfspr(SPRN_SPRG1);
> - vcpu->arch.shregs.sprg2 = mfspr(SPRN_SPRG2);
> - vcpu->arch.shregs.sprg3 = mfspr(SPRN_SPRG3);
> -
> - /* Preserve PSSCR[FAKE_SUSPEND] until we've called kvmppc_save_tm_hv */
> - mtspr(SPRN_PSSCR, host_psscr |
> - (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> - mtspr(SPRN_HFSCR, host_hfscr);
> - mtspr(SPRN_CIABR, host_ciabr);
> - mtspr(SPRN_DAWR0, host_dawr0);
> - mtspr(SPRN_DAWRX0, host_dawrx0);
> - if (cpu_has_feature(CPU_FTR_DAWR1)) {
> - mtspr(SPRN_DAWR1, host_dawr1);
> - mtspr(SPRN_DAWRX1, host_dawrx1);
> - }
> -
> - /*
> - * Since this is radix, do a eieio; tlbsync; ptesync sequence in
> - * case we interrupted the guest between a tlbie and a ptesync.
> - */
> - asm volatile("eieio; tlbsync; ptesync");
> -
> - /*
> - * cp_abort is required if the processor supports local copy-paste
> - * to clear the copy buffer that was under control of the guest.
> - */
> - if (cpu_has_feature(CPU_FTR_ARCH_31))
> - asm volatile(PPC_CP_ABORT);
> -
> - vc->dpdes = mfspr(SPRN_DPDES);
> - vc->vtb = mfspr(SPRN_VTB);
> - mtspr(SPRN_DPDES, 0);
> - if (vc->pcr)
> - mtspr(SPRN_PCR, PCR_MASK);
> -
> - if (vc->tb_offset_applied) {
> - u64 new_tb = mftb() - vc->tb_offset_applied;
> - mtspr(SPRN_TBU40, new_tb);
> - tb = mftb();
> - if ((tb & 0xffffff) < (new_tb & 0xffffff))
> - mtspr(SPRN_TBU40, new_tb + 0x1000000);
> - vc->tb_offset_applied = 0;
> - }
> -
> - /* HDEC must be at least as large as DEC, so decrementer_max fits */
> - mtspr(SPRN_HDEC, decrementer_max);
> -
> - switch_mmu_to_host_radix(kvm, host_pidr);
> -
> - return trap;
> -}
> -
> /*
> * Virtual-mode guest entry for POWER9 and later when the host and
> * guest are both using the radix MMU. The LPIDR has already been set.
> @@ -3710,7 +3533,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> * We need to save and restore the guest visible part of the
> * psscr (i.e. using SPRN_PSSCR_PR) since the hypervisor
> * doesn't do this for us. Note only required if pseries since
> - * this is done in kvmhv_load_hv_regs_and_go() below otherwise.
> + * this is done in kvmhv_vcpu_entry_p9() below otherwise.
> */
> unsigned long host_psscr;
> /* call our hypervisor to load up HV regs and go */
> @@ -3748,7 +3571,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>
> } else {
> kvmppc_xive_push_vcpu(vcpu);
> - trap = kvmhv_load_hv_regs_and_go(vcpu, time_limit, lpcr);
> + trap = kvmhv_vcpu_entry_p9(vcpu, time_limit, lpcr);
> /* H_CEDE has to be handled now, not later */
> /* XICS hcalls must be handled before xive is pulled */
> if (trap == BOOK3S_INTERRUPT_SYSCALL &&
> diff --git a/arch/powerpc/kvm/book3s_hv_interrupt.c b/arch/powerpc/kvm/book3s_hv_interrupt.c
> index 5a7b036c447f..dea3eca3648a 100644
> --- a/arch/powerpc/kvm/book3s_hv_interrupt.c
> +++ b/arch/powerpc/kvm/book3s_hv_interrupt.c
> @@ -55,6 +55,31 @@ static void __accumulate_time(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator
> #define accumulate_time(vcpu, next) do {} while (0)
> #endif
>
> +static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u64 lpcr)
> +{
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + struct kvm_nested_guest *nested = vcpu->arch.nested;
> + u32 lpid;
> +
> + lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
> +
> + mtspr(SPRN_LPID, lpid);
> + mtspr(SPRN_LPCR, lpcr);
> + mtspr(SPRN_PID, vcpu->arch.pid);
> + isync();
> +
> + /* TLBIEL must have LPIDR set, so set guest LPID before flushing. */
> + kvmppc_check_need_tlb_flush(kvm, vc->pcpu, nested);
> +}
> +
> +static void switch_mmu_to_host_radix(struct kvm *kvm, u32 pid)
> +{
> + mtspr(SPRN_PID, pid);
> + mtspr(SPRN_LPID, kvm->arch.host_lpid);
> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
> + isync();
> +}
> +
> static inline void mfslb(unsigned int idx, u64 *slbee, u64 *slbev)
> {
> asm volatile("slbmfev %0,%1" : "=r" (*slbev) : "r" (idx));
> @@ -94,11 +119,86 @@ static void radix_clear_slb(void)
> }
> }
>
> -int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu)
> +int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpcr)
> {
> + struct kvm *kvm = vcpu->kvm;
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + s64 hdec;
> + u64 tb, purr, spurr;
> u64 *exsave;
> unsigned long msr = mfmsr();
> int trap;
> + unsigned long host_hfscr = mfspr(SPRN_HFSCR);
> + unsigned long host_ciabr = mfspr(SPRN_CIABR);
> + unsigned long host_dawr0 = mfspr(SPRN_DAWR0);
> + unsigned long host_dawrx0 = mfspr(SPRN_DAWRX0);
> + unsigned long host_psscr = mfspr(SPRN_PSSCR);
> + unsigned long host_pidr = mfspr(SPRN_PID);
> + unsigned long host_dawr1 = 0;
> + unsigned long host_dawrx1 = 0;
> +
> + if (cpu_has_feature(CPU_FTR_DAWR1)) {
> + host_dawr1 = mfspr(SPRN_DAWR1);
> + host_dawrx1 = mfspr(SPRN_DAWRX1);
> + }
> +
> + tb = mftb();
> + hdec = time_limit - tb;
> + if (hdec < 0)
> + return BOOK3S_INTERRUPT_HV_DECREMENTER;
> +
> + if (vc->tb_offset) {
> + u64 new_tb = tb + vc->tb_offset;
> + mtspr(SPRN_TBU40, new_tb);
> + tb = mftb();
> + if ((tb & 0xffffff) < (new_tb & 0xffffff))
> + mtspr(SPRN_TBU40, new_tb + 0x1000000);
> + vc->tb_offset_applied = vc->tb_offset;
> + }
> +
> + if (vc->pcr)
> + mtspr(SPRN_PCR, vc->pcr | PCR_MASK);
> + mtspr(SPRN_DPDES, vc->dpdes);
> + mtspr(SPRN_VTB, vc->vtb);
> +
> + local_paca->kvm_hstate.host_purr = mfspr(SPRN_PURR);
> + local_paca->kvm_hstate.host_spurr = mfspr(SPRN_SPURR);
> + mtspr(SPRN_PURR, vcpu->arch.purr);
> + mtspr(SPRN_SPURR, vcpu->arch.spurr);
> +
> + if (dawr_enabled()) {
> + mtspr(SPRN_DAWR0, vcpu->arch.dawr0);
> + mtspr(SPRN_DAWRX0, vcpu->arch.dawrx0);
> + if (cpu_has_feature(CPU_FTR_DAWR1)) {
> + mtspr(SPRN_DAWR1, vcpu->arch.dawr1);
> + mtspr(SPRN_DAWRX1, vcpu->arch.dawrx1);
> + }
> + }
> + mtspr(SPRN_CIABR, vcpu->arch.ciabr);
> + mtspr(SPRN_IC, vcpu->arch.ic);
> +
> + mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
> + (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> +
> + mtspr(SPRN_HFSCR, vcpu->arch.hfscr);
> +
> + mtspr(SPRN_SPRG0, vcpu->arch.shregs.sprg0);
> + mtspr(SPRN_SPRG1, vcpu->arch.shregs.sprg1);
> + mtspr(SPRN_SPRG2, vcpu->arch.shregs.sprg2);
> + mtspr(SPRN_SPRG3, vcpu->arch.shregs.sprg3);
> +
> + mtspr(SPRN_AMOR, ~0UL);
> +
> + switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
> +
> + /*
> + * P9 suppresses the HDEC exception when LPCR[HDICE] = 0,
> + * so set guest LPCR (with HDICE) before writing HDEC.
> + */
> + mtspr(SPRN_HDEC, hdec);
> +
> + mtspr(SPRN_SRR0, vcpu->arch.shregs.srr0);
> + mtspr(SPRN_SRR1, vcpu->arch.shregs.srr1);
>
> start_timing(vcpu, &vcpu->arch.rm_entry);
>
> @@ -216,6 +316,70 @@ int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu)
>
> end_timing(vcpu);
>
> + /* Advance host PURR/SPURR by the amount used by guest */
> + purr = mfspr(SPRN_PURR);
> + spurr = mfspr(SPRN_SPURR);
> + mtspr(SPRN_PURR, local_paca->kvm_hstate.host_purr +
> + purr - vcpu->arch.purr);
> + mtspr(SPRN_SPURR, local_paca->kvm_hstate.host_spurr +
> + spurr - vcpu->arch.spurr);
> + vcpu->arch.purr = purr;
> + vcpu->arch.spurr = spurr;
> +
> + vcpu->arch.ic = mfspr(SPRN_IC);
> + vcpu->arch.pid = mfspr(SPRN_PID);
> + vcpu->arch.psscr = mfspr(SPRN_PSSCR) & PSSCR_GUEST_VIS;
> +
> + vcpu->arch.shregs.sprg0 = mfspr(SPRN_SPRG0);
> + vcpu->arch.shregs.sprg1 = mfspr(SPRN_SPRG1);
> + vcpu->arch.shregs.sprg2 = mfspr(SPRN_SPRG2);
> + vcpu->arch.shregs.sprg3 = mfspr(SPRN_SPRG3);
> +
> + /* Preserve PSSCR[FAKE_SUSPEND] until we've called kvmppc_save_tm_hv */
> + mtspr(SPRN_PSSCR, host_psscr |
> + (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> + mtspr(SPRN_HFSCR, host_hfscr);
> + mtspr(SPRN_CIABR, host_ciabr);
> + mtspr(SPRN_DAWR0, host_dawr0);
> + mtspr(SPRN_DAWRX0, host_dawrx0);
> + if (cpu_has_feature(CPU_FTR_DAWR1)) {
> + mtspr(SPRN_DAWR1, host_dawr1);
> + mtspr(SPRN_DAWRX1, host_dawrx1);
> + }
> +
> + /*
> + * Since this is radix, do a eieio; tlbsync; ptesync sequence in
> + * case we interrupted the guest between a tlbie and a ptesync.
> + */
> + asm volatile("eieio; tlbsync; ptesync");
> +
> + /*
> + * cp_abort is required if the processor supports local copy-paste
> + * to clear the copy buffer that was under control of the guest.
> + */
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + asm volatile(PPC_CP_ABORT);
> +
> + vc->dpdes = mfspr(SPRN_DPDES);
> + vc->vtb = mfspr(SPRN_VTB);
> + mtspr(SPRN_DPDES, 0);
> + if (vc->pcr)
> + mtspr(SPRN_PCR, PCR_MASK);
> +
> + if (vc->tb_offset_applied) {
> + u64 new_tb = mftb() - vc->tb_offset_applied;
> + mtspr(SPRN_TBU40, new_tb);
> + tb = mftb();
> + if ((tb & 0xffffff) < (new_tb & 0xffffff))
> + mtspr(SPRN_TBU40, new_tb + 0x1000000);
> + vc->tb_offset_applied = 0;
> + }
> +
> + /* HDEC must be at least as large as DEC, so decrementer_max fits */
> + mtspr(SPRN_HDEC, decrementer_max);
> +
> + switch_mmu_to_host_radix(kvm, host_pidr);
> +
> return trap;
> }
> -EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9);
> +EXPORT_SYMBOL_GPL(kvmhv_vcpu_entry_p9);
^ permalink raw reply
* Re: [PATCH next v3 12/15] printk: introduce a kmsg_dump iterator
From: John Ogness @ 2021-03-02 13:20 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-hyperv, Sergey Senozhatsky, Douglas Anderson,
Paul Mackerras, Miquel Raynal, K. Y. Srinivasan, Thomas Meyer,
Vignesh Raghavendra, Wei Liu, Madhavan Srinivasan,
Stephen Hemminger, Anton Vorontsov, Joel Stanley, Jason Wessel,
Anton Ivanov, Wei Li, Haiyang Zhang, Ravi Bangoria, Kees Cook,
Alistair Popple, Jeff Dike, Colin Cross, linux-um,
Daniel Thompson, Steven Rostedt, Davidlohr Bueso, Nicholas Piggin,
Oleg Nesterov, Thomas Gleixner, Andy Shevchenko, Jordan Niethe,
Michael Kelley, Christophe Leroy, Tony Luck, Pavel Tatashin,
linux-kernel, Sergey Senozhatsky, Richard Weinberger,
kgdb-bugreport, linux-mtd, linuxppc-dev, Mike Rapoport
In-Reply-To: <YD0tbVV+hZOFvWyB@alley>
On 2021-03-01, Petr Mladek <pmladek@suse.com> wrote:
>> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
>> index 532f22637783..5a64b24a91c2 100644
>> --- a/arch/powerpc/kernel/nvram_64.c
>> +++ b/arch/powerpc/kernel/nvram_64.c
>> @@ -72,8 +72,7 @@ static const char *nvram_os_partitions[] = {
>> NULL
>> };
>>
>> -static void oops_to_nvram(struct kmsg_dumper *dumper,
>> - enum kmsg_dump_reason reason);
>> +static void oops_to_nvram(enum kmsg_dump_reason reason);
>>
>> static struct kmsg_dumper nvram_kmsg_dumper = {
>> .dump = oops_to_nvram
>> @@ -642,11 +641,11 @@ void __init nvram_init_oops_partition(int rtas_partition_exists)
>> * that we think will compress sufficiently to fit in the lnx,oops-log
>> * partition. If that's too much, go back and capture uncompressed text.
>> */
>> -static void oops_to_nvram(struct kmsg_dumper *dumper,
>> - enum kmsg_dump_reason reason)
>> +static void oops_to_nvram(enum kmsg_dump_reason reason)
>> {
>> struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
>> static unsigned int oops_count = 0;
>> + static struct kmsg_dump_iter iter;
>> static bool panicking = false;
>> static DEFINE_SPINLOCK(lock);
>> unsigned long flags;
>> @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
>> return;
>>
>> if (big_oops_buf) {
>> - kmsg_dump_get_buffer(dumper, false,
>> + kmsg_dump_rewind(&iter);
>
> It would be nice to get rid of the kmsg_dump_rewind(&iter) calls
> in all callers.
>
> A solution might be to create the following in include/linux/kmsg_dump.h
>
> #define KMSG_DUMP_ITER_INIT(iter) { \
> .cur_seq = 0, \
> .next_seq = U64_MAX, \
> }
>
> #define DEFINE_KMSG_DUMP_ITER(iter) \
> struct kmsg_dump_iter iter = KMSG_DUMP_ITER_INIT(iter)
For this caller (arch/powerpc/kernel/nvram_64.c) and for
(kernel/debug/kdb/kdb_main.c), kmsg_dump_rewind() is called twice within
the dumper. So rewind will still be used there.
> Then we could do the following at the beginning of both
> kmsg_dump_get_buffer() and kmsg_dump_get_line():
>
> u64 clear_seq = latched_seq_read_nolock(&clear_seq);
>
> if (iter->cur_seq < clear_seq)
> cur_seq = clear_seq;
I suppose we need to add this part anyway, if we want to enforce that
records before @clear_seq are not to be available for dumpers.
> I am not completely sure about next_seq:
>
> + kmsg_dump_get_buffer() will set it for the next call anyway.
> It reads the blocks of messages from the newest.
>
> + kmsg_dump_get_line() wants to read the entire buffer anyway.
> But there is a small risk of an infinite loop when new messages
> are printed when dumping each line.
>
> It might be better to avoid the infinite loop. We could do the following:
>
> static void check_and_set_iter(struct kmsg_dump_iter)
> {
> if (iter->cur_seq == 0 && iter->next_seq == U64_MAX) {
> kmsg_dump_rewind(iter);
> }
>
> and call this at the beginning of both kmsg_dump_get_buffer()
> and kmsg_dump_get_line()
>
> What do you think?
On a technical level, it does not make any difference. It is pure
cosmetic.
Personally, I prefer the rewind directly before the kmsg_dump_get calls
because it puts the initializer directly next to the user.
As an example to illustrate my view, I prefer:
for (i = 0; i < n; i++)
...;
instead of:
int i = 0;
...
for (; i < n; i++)
...;
Also, I do not really like the special use of 0/U64_MAX to identify
special actions of the kmsg_dump_get functions.
> Note that I do not resist on it. But it might make the API easier to
> use from my POV.
Since you do not resist, I will keep the API the same for v4. But I will
add the @clear_seq check to the kmsg_dump_get functions.
John Ogness
^ permalink raw reply
* [PATCH] ASoC: fsl_xcvr: Use devm_platform_ioremap_resource_byname() to simplify code
From: Tang Bin @ 2021-03-02 12:50 UTC (permalink / raw)
To: broonie, timur, nicoleotsuka, Xiubo.Lee, perex, tiwai
Cc: alsa-devel, linuxppc-dev, linux-kernel, Tang Bin
In this function, devm_platform_ioremap_resource_byname() should be
suitable to simplify code.
Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
sound/soc/fsl/fsl_xcvr.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 6dd0a5fcd455..5e8284db857b 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -1131,7 +1131,7 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct fsl_xcvr *xcvr;
- struct resource *ram_res, *regs_res, *rx_res, *tx_res;
+ struct resource *rx_res, *tx_res;
void __iomem *regs;
int ret, irq;
@@ -1166,13 +1166,11 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
return PTR_ERR(xcvr->pll_ipg_clk);
}
- ram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ram");
- xcvr->ram_addr = devm_ioremap_resource(dev, ram_res);
+ xcvr->ram_addr = devm_platform_ioremap_resource_byname(pdev, "ram");
if (IS_ERR(xcvr->ram_addr))
return PTR_ERR(xcvr->ram_addr);
- regs_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
- regs = devm_ioremap_resource(dev, regs_res);
+ regs = devm_platform_ioremap_resource_byname(pdev, "regs");
if (IS_ERR(regs))
return PTR_ERR(regs);
--
2.18.2
^ permalink raw reply related
* Re: [PATCH] perf bench numa: Fix the condition checks for max number of numa nodes
From: Arnaldo Carvalho de Melo @ 2021-03-02 12:33 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: ravi.bangoria, Athira Rajeev, peterz, linux-kernel,
linux-perf-users, maddy, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <20210226085827.GF2028034@linux.vnet.ibm.com>
Em Fri, Feb 26, 2021 at 02:28:27PM +0530, Srikar Dronamraju escreveu:
> * Athira Rajeev <atrajeev@linux.vnet.ibm.com> [2021-02-25 11:50:02]:
>
> > In systems having higher node numbers available like node
> > 255, perf numa bench will fail with SIGABORT.
> >
> > <<>>
> > perf: bench/numa.c:1416: init: Assertion `!(g->p.nr_nodes > 64 || g->p.nr_nodes < 0)' failed.
> > Aborted (core dumped)
> > <<>>
> >
>
> Looks good to me.
>
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Thanks, applied.
- Arnaldo
^ permalink raw reply
* [PATCH AUTOSEL 4.4 3/8] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:59 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115935.63777-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index e593e7f856ed..7a80e1cff6e2 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2008,7 +2008,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2026,9 +2036,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 04/10] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:59 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115921.63636-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 1f1ac446ace9..f2d8f35c181f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2010,7 +2010,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2028,9 +2038,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 05/13] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:58 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115903.63458-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 56f16c803590..2669847434b8 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2055,7 +2055,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2073,9 +2083,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 04/13] powerpc: improve handling of unrecoverable system reset
From: Sasha Levin @ 2021-03-02 11:58 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210302115903.63458-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 11cb0a25f71818ca7ab4856548ecfd83c169aa4d ]
If an unrecoverable system reset hits in process context, the system
does not have to panic. Similar to machine check, call nmi_exit()
before die().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210130130852.2952424-26-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 0f1a888c04a8..05c1aabad01c 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -360,8 +360,11 @@ out:
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (!(regs->msr & MSR_RI)) {
+ /* For the reason explained in die_mce, nmi_exit before die */
+ nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
+ }
if (!nested)
nmi_exit();
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 08/21] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:58 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115835.63269-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 70de13822828..091bdeaf02a3 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2046,7 +2046,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2064,9 +2074,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 07/21] powerpc: improve handling of unrecoverable system reset
From: Sasha Levin @ 2021-03-02 11:58 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210302115835.63269-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 11cb0a25f71818ca7ab4856548ecfd83c169aa4d ]
If an unrecoverable system reset hits in process context, the system
does not have to panic. Similar to machine check, call nmi_exit()
before die().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210130130852.2952424-26-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 1b2d84cb373b..2379c4bf3979 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -433,8 +433,11 @@ out:
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (!(regs->msr & MSR_RI)) {
+ /* For the reason explained in die_mce, nmi_exit before die */
+ nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
+ }
if (!nested)
nmi_exit();
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 06/21] powerpc/pci: Add ppc_md.discover_phbs()
From: Sasha Levin @ 2021-03-02 11:58 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Oliver O'Halloran,
kernel test robot
In-Reply-To: <20210302115835.63269-1-sashal@kernel.org>
From: Oliver O'Halloran <oohall@gmail.com>
[ Upstream commit 5537fcb319d016ce387f818dd774179bc03217f5 ]
On many powerpc platforms the discovery and initalisation of
pci_controllers (PHBs) happens inside of setup_arch(). This is very early
in boot (pre-initcalls) and means that we're initialising the PHB long
before many basic kernel services (slab allocator, debugfs, a real ioremap)
are available.
On PowerNV this causes an additional problem since we map the PHB registers
with ioremap(). As of commit d538aadc2718 ("powerpc/ioremap: warn on early
use of ioremap()") a warning is printed because we're using the "incorrect"
API to setup and MMIO mapping in searly boot. The kernel does provide
early_ioremap(), but that is not intended to create long-lived MMIO
mappings and a seperate warning is printed by generic code if
early_ioremap() mappings are "leaked."
This is all fixable with dumb hacks like using early_ioremap() to setup
the initial mapping then replacing it with a real ioremap later on in
boot, but it does raise the question: Why the hell are we setting up the
PHB's this early in boot?
The old and wise claim it's due to "hysterical rasins." Aside from amused
grapes there doesn't appear to be any real reason to maintain the current
behaviour. Already most of the newer embedded platforms perform PHB
discovery in an arch_initcall and between the end of setup_arch() and the
start of initcalls none of the generic kernel code does anything PCI
related. On powerpc scanning PHBs occurs in a subsys_initcall so it should
be possible to move the PHB discovery to a core, postcore or arch initcall.
This patch adds the ppc_md.discover_phbs hook and a core_initcall stub that
calls it. The core_initcalls are the earliest to be called so this will
any possibly issues with dependency between initcalls. This isn't just an
academic issue either since on pseries and PowerNV EEH init occurs in an
arch_initcall and depends on the pci_controllers being available, similarly
the creation of pci_dns occurs at core_initcall_sync (i.e. between core and
postcore initcalls). These problems need to be addressed seperately.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[mpe: Make discover_phbs() static]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20201103043523.916109-1-oohall@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/machdep.h | 3 +++
arch/powerpc/kernel/pci-common.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index a47de82fb8e2..bda87cbf106d 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -71,6 +71,9 @@ struct machdep_calls {
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);
+ /* finds all the pci_controllers present at boot */
+ void (*discover_phbs)(void);
+
/* To setup PHBs when using automatic OF platform driver for PCI */
int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 88e4f69a09e5..74628aca2bf1 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1671,3 +1671,13 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
+
+
+static int __init discover_phbs(void)
+{
+ if (ppc_md.discover_phbs)
+ ppc_md.discover_phbs();
+
+ return 0;
+}
+core_initcall(discover_phbs);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 14/33] powerpc/64: Fix stack trace not displaying final frame
From: Sasha Levin @ 2021-03-02 11:57 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210302115749.62653-1-sashal@kernel.org>
From: Michael Ellerman <mpe@ellerman.id.au>
[ Upstream commit e3de1e291fa58a1ab0f471a4b458eff2514e4b5f ]
In commit bf13718bc57a ("powerpc: show registers when unwinding
interrupt frames") we changed our stack dumping logic to show the full
registers whenever we find an interrupt frame on the stack.
However we didn't notice that on 64-bit this doesn't show the final
frame, ie. the interrupt that brought us in from userspace, whereas on
32-bit it does.
That is due to confusion about the size of that last frame. The code
in show_stack() calls validate_sp(), passing it STACK_INT_FRAME_SIZE
to check the sp is at least that far below the top of the stack.
However on 64-bit that size is too large for the final frame, because
it includes the red zone, but we don't allocate a red zone for the
first frame.
So add a new define that encodes the correct size for 32-bit and
64-bit, and use it in show_stack().
This results in the full trace being shown on 64-bit, eg:
sysrq: Trigger a crash
Kernel panic - not syncing: sysrq triggered crash
CPU: 0 PID: 83 Comm: sh Not tainted 5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty #649
Call Trace:
[c00000000a1c3ac0] [c000000000897b70] dump_stack+0xc4/0x114 (unreliable)
[c00000000a1c3b00] [c00000000014334c] panic+0x178/0x41c
[c00000000a1c3ba0] [c00000000094e600] sysrq_handle_crash+0x40/0x50
[c00000000a1c3c00] [c00000000094ef98] __handle_sysrq+0xd8/0x210
[c00000000a1c3ca0] [c00000000094f820] write_sysrq_trigger+0x100/0x188
[c00000000a1c3ce0] [c0000000005559dc] proc_reg_write+0x10c/0x1b0
[c00000000a1c3d10] [c000000000479950] vfs_write+0xf0/0x360
[c00000000a1c3d60] [c000000000479d9c] ksys_write+0x7c/0x140
[c00000000a1c3db0] [c00000000002bf5c] system_call_exception+0x19c/0x2c0
[c00000000a1c3e10] [c00000000000d35c] system_call_common+0xec/0x278
--- interrupt: c00 at 0x7fff9fbab428
NIP: 00007fff9fbab428 LR: 000000001000b724 CTR: 0000000000000000
REGS: c00000000a1c3e80 TRAP: 0c00 Not tainted (5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty)
MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 22002884 XER: 00000000
IRQMASK: 0
GPR00: 0000000000000004 00007fffc3cb8960 00007fff9fc59900 0000000000000001
GPR04: 000000002a4b32d0 0000000000000002 0000000000000063 0000000000000063
GPR08: 000000002a4b32d0 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00007fff9fcca9a0 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000100b8fd0
GPR20: 000000002a4b3485 00000000100b8f90 0000000000000000 0000000000000000
GPR24: 000000002a4b0440 00000000100e77b8 0000000000000020 000000002a4b32d0
GPR28: 0000000000000001 0000000000000002 000000002a4b32d0 0000000000000001
NIP [00007fff9fbab428] 0x7fff9fbab428
LR [000000001000b724] 0x1000b724
--- interrupt: c00
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210209141627.2898485-1-mpe@ellerman.id.au
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/ptrace.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/process.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index c41220f4aad9..5a424f867c82 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -62,6 +62,9 @@ struct pt_regs
};
#endif
+
+#define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
+
#ifdef __powerpc64__
/*
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 5c0a1e17219b..af399675248e 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -285,7 +285,7 @@ int main(void)
/* Interrupt register frame */
DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
- DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_OVERHEAD + sizeof(struct pt_regs));
+ DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_WITH_PT_REGS);
STACK_PT_REGS_OFFSET(GPR0, gpr[0]);
STACK_PT_REGS_OFFSET(GPR1, gpr[1]);
STACK_PT_REGS_OFFSET(GPR2, gpr[2]);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index bd0c258a1d5d..c94bba9142e7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2081,7 +2081,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
* See if this is an exception frame.
* We look for the "regshere" marker in the current frame.
*/
- if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
+ if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS)
&& stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
struct pt_regs *regs = (struct pt_regs *)
(sp + STACK_FRAME_OVERHEAD);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 12/33] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:57 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115749.62653-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 02fc75ddcbb3..6f013e418834 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2077,7 +2077,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2095,9 +2105,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 11/33] powerpc: improve handling of unrecoverable system reset
From: Sasha Levin @ 2021-03-02 11:57 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210302115749.62653-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 11cb0a25f71818ca7ab4856548ecfd83c169aa4d ]
If an unrecoverable system reset hits in process context, the system
does not have to panic. Similar to machine check, call nmi_exit()
before die().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210130130852.2952424-26-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 206032c9b545..ecfa460f66d1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -513,8 +513,11 @@ out:
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (!(regs->msr & MSR_RI)) {
+ /* For the reason explained in die_mce, nmi_exit before die */
+ nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
+ }
if (saved_hsrrs) {
mtspr(SPRN_HSRR0, hsrr0);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 08/33] powerpc/pci: Add ppc_md.discover_phbs()
From: Sasha Levin @ 2021-03-02 11:57 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Oliver O'Halloran,
kernel test robot
In-Reply-To: <20210302115749.62653-1-sashal@kernel.org>
From: Oliver O'Halloran <oohall@gmail.com>
[ Upstream commit 5537fcb319d016ce387f818dd774179bc03217f5 ]
On many powerpc platforms the discovery and initalisation of
pci_controllers (PHBs) happens inside of setup_arch(). This is very early
in boot (pre-initcalls) and means that we're initialising the PHB long
before many basic kernel services (slab allocator, debugfs, a real ioremap)
are available.
On PowerNV this causes an additional problem since we map the PHB registers
with ioremap(). As of commit d538aadc2718 ("powerpc/ioremap: warn on early
use of ioremap()") a warning is printed because we're using the "incorrect"
API to setup and MMIO mapping in searly boot. The kernel does provide
early_ioremap(), but that is not intended to create long-lived MMIO
mappings and a seperate warning is printed by generic code if
early_ioremap() mappings are "leaked."
This is all fixable with dumb hacks like using early_ioremap() to setup
the initial mapping then replacing it with a real ioremap later on in
boot, but it does raise the question: Why the hell are we setting up the
PHB's this early in boot?
The old and wise claim it's due to "hysterical rasins." Aside from amused
grapes there doesn't appear to be any real reason to maintain the current
behaviour. Already most of the newer embedded platforms perform PHB
discovery in an arch_initcall and between the end of setup_arch() and the
start of initcalls none of the generic kernel code does anything PCI
related. On powerpc scanning PHBs occurs in a subsys_initcall so it should
be possible to move the PHB discovery to a core, postcore or arch initcall.
This patch adds the ppc_md.discover_phbs hook and a core_initcall stub that
calls it. The core_initcalls are the earliest to be called so this will
any possibly issues with dependency between initcalls. This isn't just an
academic issue either since on pseries and PowerNV EEH init occurs in an
arch_initcall and depends on the pci_controllers being available, similarly
the creation of pci_dns occurs at core_initcall_sync (i.e. between core and
postcore initcalls). These problems need to be addressed seperately.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[mpe: Make discover_phbs() static]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20201103043523.916109-1-oohall@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/machdep.h | 3 +++
arch/powerpc/kernel/pci-common.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 7bcb64444a39..f71c361dc356 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -59,6 +59,9 @@ struct machdep_calls {
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);
+ /* finds all the pci_controllers present at boot */
+ void (*discover_phbs)(void);
+
/* To setup PHBs when using automatic OF platform driver for PCI */
int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 1c448cf25506..a2c258a8d736 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1669,3 +1669,13 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
+
+
+static int __init discover_phbs(void)
+{
+ if (ppc_md.discover_phbs)
+ ppc_md.discover_phbs();
+
+ return 0;
+}
+core_initcall(discover_phbs);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 20/47] powerpc/64: Fix stack trace not displaying final frame
From: Sasha Levin @ 2021-03-02 11:56 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210302115646.62291-1-sashal@kernel.org>
From: Michael Ellerman <mpe@ellerman.id.au>
[ Upstream commit e3de1e291fa58a1ab0f471a4b458eff2514e4b5f ]
In commit bf13718bc57a ("powerpc: show registers when unwinding
interrupt frames") we changed our stack dumping logic to show the full
registers whenever we find an interrupt frame on the stack.
However we didn't notice that on 64-bit this doesn't show the final
frame, ie. the interrupt that brought us in from userspace, whereas on
32-bit it does.
That is due to confusion about the size of that last frame. The code
in show_stack() calls validate_sp(), passing it STACK_INT_FRAME_SIZE
to check the sp is at least that far below the top of the stack.
However on 64-bit that size is too large for the final frame, because
it includes the red zone, but we don't allocate a red zone for the
first frame.
So add a new define that encodes the correct size for 32-bit and
64-bit, and use it in show_stack().
This results in the full trace being shown on 64-bit, eg:
sysrq: Trigger a crash
Kernel panic - not syncing: sysrq triggered crash
CPU: 0 PID: 83 Comm: sh Not tainted 5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty #649
Call Trace:
[c00000000a1c3ac0] [c000000000897b70] dump_stack+0xc4/0x114 (unreliable)
[c00000000a1c3b00] [c00000000014334c] panic+0x178/0x41c
[c00000000a1c3ba0] [c00000000094e600] sysrq_handle_crash+0x40/0x50
[c00000000a1c3c00] [c00000000094ef98] __handle_sysrq+0xd8/0x210
[c00000000a1c3ca0] [c00000000094f820] write_sysrq_trigger+0x100/0x188
[c00000000a1c3ce0] [c0000000005559dc] proc_reg_write+0x10c/0x1b0
[c00000000a1c3d10] [c000000000479950] vfs_write+0xf0/0x360
[c00000000a1c3d60] [c000000000479d9c] ksys_write+0x7c/0x140
[c00000000a1c3db0] [c00000000002bf5c] system_call_exception+0x19c/0x2c0
[c00000000a1c3e10] [c00000000000d35c] system_call_common+0xec/0x278
--- interrupt: c00 at 0x7fff9fbab428
NIP: 00007fff9fbab428 LR: 000000001000b724 CTR: 0000000000000000
REGS: c00000000a1c3e80 TRAP: 0c00 Not tainted (5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty)
MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 22002884 XER: 00000000
IRQMASK: 0
GPR00: 0000000000000004 00007fffc3cb8960 00007fff9fc59900 0000000000000001
GPR04: 000000002a4b32d0 0000000000000002 0000000000000063 0000000000000063
GPR08: 000000002a4b32d0 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00007fff9fcca9a0 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000100b8fd0
GPR20: 000000002a4b3485 00000000100b8f90 0000000000000000 0000000000000000
GPR24: 000000002a4b0440 00000000100e77b8 0000000000000020 000000002a4b32d0
GPR28: 0000000000000001 0000000000000002 000000002a4b32d0 0000000000000001
NIP [00007fff9fbab428] 0x7fff9fbab428
LR [000000001000b724] 0x1000b724
--- interrupt: c00
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210209141627.2898485-1-mpe@ellerman.id.au
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/ptrace.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/process.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e2c778c176a3..7bb064ad04d8 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -62,6 +62,9 @@ struct pt_regs
};
#endif
+
+#define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
+
#ifdef __powerpc64__
/*
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index c2722ff36e98..5c125255571c 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -307,7 +307,7 @@ int main(void)
/* Interrupt register frame */
DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
- DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_OVERHEAD + sizeof(struct pt_regs));
+ DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_WITH_PT_REGS);
STACK_PT_REGS_OFFSET(GPR0, gpr[0]);
STACK_PT_REGS_OFFSET(GPR1, gpr[1]);
STACK_PT_REGS_OFFSET(GPR2, gpr[2]);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d421a2c7f822..1a1d2657fe8d 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2170,7 +2170,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
* See if this is an exception frame.
* We look for the "regshere" marker in the current frame.
*/
- if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
+ if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS)
&& stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
struct pt_regs *regs = (struct pt_regs *)
(sp + STACK_FRAME_OVERHEAD);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 17/47] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:56 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115646.62291-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 43599e671d38..d84ab867b986 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2112,7 +2112,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2130,9 +2140,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 16/47] powerpc: improve handling of unrecoverable system reset
From: Sasha Levin @ 2021-03-02 11:56 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210302115646.62291-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 11cb0a25f71818ca7ab4856548ecfd83c169aa4d ]
If an unrecoverable system reset hits in process context, the system
does not have to panic. Similar to machine check, call nmi_exit()
before die().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210130130852.2952424-26-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 5006dcbe1d9f..77dffea3d537 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -509,8 +509,11 @@ out:
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (!(regs->msr & MSR_RI)) {
+ /* For the reason explained in die_mce, nmi_exit before die */
+ nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
+ }
if (saved_hsrrs) {
mtspr(SPRN_HSRR0, hsrr0);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 13/47] powerpc/pci: Add ppc_md.discover_phbs()
From: Sasha Levin @ 2021-03-02 11:56 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Oliver O'Halloran,
kernel test robot
In-Reply-To: <20210302115646.62291-1-sashal@kernel.org>
From: Oliver O'Halloran <oohall@gmail.com>
[ Upstream commit 5537fcb319d016ce387f818dd774179bc03217f5 ]
On many powerpc platforms the discovery and initalisation of
pci_controllers (PHBs) happens inside of setup_arch(). This is very early
in boot (pre-initcalls) and means that we're initialising the PHB long
before many basic kernel services (slab allocator, debugfs, a real ioremap)
are available.
On PowerNV this causes an additional problem since we map the PHB registers
with ioremap(). As of commit d538aadc2718 ("powerpc/ioremap: warn on early
use of ioremap()") a warning is printed because we're using the "incorrect"
API to setup and MMIO mapping in searly boot. The kernel does provide
early_ioremap(), but that is not intended to create long-lived MMIO
mappings and a seperate warning is printed by generic code if
early_ioremap() mappings are "leaked."
This is all fixable with dumb hacks like using early_ioremap() to setup
the initial mapping then replacing it with a real ioremap later on in
boot, but it does raise the question: Why the hell are we setting up the
PHB's this early in boot?
The old and wise claim it's due to "hysterical rasins." Aside from amused
grapes there doesn't appear to be any real reason to maintain the current
behaviour. Already most of the newer embedded platforms perform PHB
discovery in an arch_initcall and between the end of setup_arch() and the
start of initcalls none of the generic kernel code does anything PCI
related. On powerpc scanning PHBs occurs in a subsys_initcall so it should
be possible to move the PHB discovery to a core, postcore or arch initcall.
This patch adds the ppc_md.discover_phbs hook and a core_initcall stub that
calls it. The core_initcalls are the earliest to be called so this will
any possibly issues with dependency between initcalls. This isn't just an
academic issue either since on pseries and PowerNV EEH init occurs in an
arch_initcall and depends on the pci_controllers being available, similarly
the creation of pci_dns occurs at core_initcall_sync (i.e. between core and
postcore initcalls). These problems need to be addressed seperately.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[mpe: Make discover_phbs() static]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20201103043523.916109-1-oohall@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/machdep.h | 3 +++
arch/powerpc/kernel/pci-common.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index 475687f24f4a..d319160d790c 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -59,6 +59,9 @@ struct machdep_calls {
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);
+ /* finds all the pci_controllers present at boot */
+ void (*discover_phbs)(void);
+
/* To setup PHBs when using automatic OF platform driver for PCI */
int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index be108616a721..7920559a1ca8 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1625,3 +1625,13 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
+
+
+static int __init discover_phbs(void)
+{
+ if (ppc_md.discover_phbs)
+ ppc_md.discover_phbs();
+
+ return 0;
+}
+core_initcall(discover_phbs);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.11 24/52] powerpc/64: Fix stack trace not displaying final frame
From: Sasha Levin @ 2021-03-02 11:55 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev
In-Reply-To: <20210302115534.61800-1-sashal@kernel.org>
From: Michael Ellerman <mpe@ellerman.id.au>
[ Upstream commit e3de1e291fa58a1ab0f471a4b458eff2514e4b5f ]
In commit bf13718bc57a ("powerpc: show registers when unwinding
interrupt frames") we changed our stack dumping logic to show the full
registers whenever we find an interrupt frame on the stack.
However we didn't notice that on 64-bit this doesn't show the final
frame, ie. the interrupt that brought us in from userspace, whereas on
32-bit it does.
That is due to confusion about the size of that last frame. The code
in show_stack() calls validate_sp(), passing it STACK_INT_FRAME_SIZE
to check the sp is at least that far below the top of the stack.
However on 64-bit that size is too large for the final frame, because
it includes the red zone, but we don't allocate a red zone for the
first frame.
So add a new define that encodes the correct size for 32-bit and
64-bit, and use it in show_stack().
This results in the full trace being shown on 64-bit, eg:
sysrq: Trigger a crash
Kernel panic - not syncing: sysrq triggered crash
CPU: 0 PID: 83 Comm: sh Not tainted 5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty #649
Call Trace:
[c00000000a1c3ac0] [c000000000897b70] dump_stack+0xc4/0x114 (unreliable)
[c00000000a1c3b00] [c00000000014334c] panic+0x178/0x41c
[c00000000a1c3ba0] [c00000000094e600] sysrq_handle_crash+0x40/0x50
[c00000000a1c3c00] [c00000000094ef98] __handle_sysrq+0xd8/0x210
[c00000000a1c3ca0] [c00000000094f820] write_sysrq_trigger+0x100/0x188
[c00000000a1c3ce0] [c0000000005559dc] proc_reg_write+0x10c/0x1b0
[c00000000a1c3d10] [c000000000479950] vfs_write+0xf0/0x360
[c00000000a1c3d60] [c000000000479d9c] ksys_write+0x7c/0x140
[c00000000a1c3db0] [c00000000002bf5c] system_call_exception+0x19c/0x2c0
[c00000000a1c3e10] [c00000000000d35c] system_call_common+0xec/0x278
--- interrupt: c00 at 0x7fff9fbab428
NIP: 00007fff9fbab428 LR: 000000001000b724 CTR: 0000000000000000
REGS: c00000000a1c3e80 TRAP: 0c00 Not tainted (5.11.0-rc2-gcc-8.2.0-00188-g571abcb96b10-dirty)
MSR: 900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 22002884 XER: 00000000
IRQMASK: 0
GPR00: 0000000000000004 00007fffc3cb8960 00007fff9fc59900 0000000000000001
GPR04: 000000002a4b32d0 0000000000000002 0000000000000063 0000000000000063
GPR08: 000000002a4b32d0 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00007fff9fcca9a0 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000100b8fd0
GPR20: 000000002a4b3485 00000000100b8f90 0000000000000000 0000000000000000
GPR24: 000000002a4b0440 00000000100e77b8 0000000000000020 000000002a4b32d0
GPR28: 0000000000000001 0000000000000002 000000002a4b32d0 0000000000000001
NIP [00007fff9fbab428] 0x7fff9fbab428
LR [000000001000b724] 0x1000b724
--- interrupt: c00
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210209141627.2898485-1-mpe@ellerman.id.au
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/ptrace.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/process.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 58f9dc060a7b..8236c5e749e4 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -70,6 +70,9 @@ struct pt_regs
};
#endif
+
+#define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
+
#ifdef __powerpc64__
/*
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index b12d7c049bfe..989006b5ad0f 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -309,7 +309,7 @@ int main(void)
/* Interrupt register frame */
DEFINE(INT_FRAME_SIZE, STACK_INT_FRAME_SIZE);
- DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_OVERHEAD + sizeof(struct pt_regs));
+ DEFINE(SWITCH_FRAME_SIZE, STACK_FRAME_WITH_PT_REGS);
STACK_PT_REGS_OFFSET(GPR0, gpr[0]);
STACK_PT_REGS_OFFSET(GPR1, gpr[1]);
STACK_PT_REGS_OFFSET(GPR2, gpr[2]);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index a66f435dabbf..b65a73e4d642 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2176,7 +2176,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack,
* See if this is an exception frame.
* We look for the "regshere" marker in the current frame.
*/
- if (validate_sp(sp, tsk, STACK_INT_FRAME_SIZE)
+ if (validate_sp(sp, tsk, STACK_FRAME_WITH_PT_REGS)
&& stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
struct pt_regs *regs = (struct pt_regs *)
(sp + STACK_FRAME_OVERHEAD);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.11 20/52] powerpc/perf: Record counter overflow always if SAMPLE_IP is unset
From: Sasha Levin @ 2021-03-02 11:55 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, Athira Rajeev, linuxppc-dev
In-Reply-To: <20210302115534.61800-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit d137845c973147a22622cc76c7b0bc16f6206323 ]
While sampling for marked events, currently we record the sample only
if the SIAR valid bit of Sampled Instruction Event Register (SIER) is
set. SIAR_VALID bit is used for fetching the instruction address from
Sampled Instruction Address Register(SIAR). But there are some
usecases, where the user is interested only in the PMU stats at each
counter overflow and the exact IP of the overflow event is not
required. Dropping SIAR invalid samples will fail to record some of
the counter overflows in such cases.
Example of such usecase is dumping the PMU stats (event counts) after
some regular amount of instructions/events from the userspace (ex: via
ptrace). Here counter overflow is indicated to userspace via signal
handler, and captured by monitoring and enabling I/O signaling on the
event file descriptor. In these cases, we expect to get
sample/overflow indication after each specified sample_period.
Perf event attribute will not have PERF_SAMPLE_IP set in the
sample_type if exact IP of the overflow event is not requested. So
while profiling if SAMPLE_IP is not set, just record the counter
overflow irrespective of SIAR_VALID check.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[mpe: Reflow comment and if formatting]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1612516492-1428-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 28206b1fe172..8b529daf40ea 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2149,7 +2149,17 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
left += period;
if (left <= 0)
left = period;
- record = siar_valid(regs);
+
+ /*
+ * If address is not requested in the sample via
+ * PERF_SAMPLE_IP, just record that sample irrespective
+ * of SIAR valid check.
+ */
+ if (event->attr.sample_type & PERF_SAMPLE_IP)
+ record = siar_valid(regs);
+ else
+ record = 1;
+
event->hw.last_period = event->hw.sample_period;
}
if (left < 0x80000000LL)
@@ -2167,9 +2177,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
* MMCR2. Check attr.exclude_kernel and address to drop the sample in
* these cases.
*/
- if (event->attr.exclude_kernel && record)
- if (is_kernel_addr(mfspr(SPRN_SIAR)))
- record = 0;
+ if (event->attr.exclude_kernel &&
+ (event->attr.sample_type & PERF_SAMPLE_IP) &&
+ is_kernel_addr(mfspr(SPRN_SIAR)))
+ record = 0;
/*
* Finally record data if requested.
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.11 19/52] powerpc: improve handling of unrecoverable system reset
From: Sasha Levin @ 2021-03-02 11:55 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Sasha Levin, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210302115534.61800-1-sashal@kernel.org>
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 11cb0a25f71818ca7ab4856548ecfd83c169aa4d ]
If an unrecoverable system reset hits in process context, the system
does not have to panic. Similar to machine check, call nmi_exit()
before die().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210130130852.2952424-26-npiggin@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/traps.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 3ec7b443fe6b..4be05517f2db 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -503,8 +503,11 @@ out:
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (!(regs->msr & MSR_RI)) {
+ /* For the reason explained in die_mce, nmi_exit before die */
+ nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
+ }
if (saved_hsrrs) {
mtspr(SPRN_HSRR0, hsrr0);
--
2.30.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.11 16/52] powerpc/pci: Add ppc_md.discover_phbs()
From: Sasha Levin @ 2021-03-02 11:54 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Oliver O'Halloran,
kernel test robot
In-Reply-To: <20210302115534.61800-1-sashal@kernel.org>
From: Oliver O'Halloran <oohall@gmail.com>
[ Upstream commit 5537fcb319d016ce387f818dd774179bc03217f5 ]
On many powerpc platforms the discovery and initalisation of
pci_controllers (PHBs) happens inside of setup_arch(). This is very early
in boot (pre-initcalls) and means that we're initialising the PHB long
before many basic kernel services (slab allocator, debugfs, a real ioremap)
are available.
On PowerNV this causes an additional problem since we map the PHB registers
with ioremap(). As of commit d538aadc2718 ("powerpc/ioremap: warn on early
use of ioremap()") a warning is printed because we're using the "incorrect"
API to setup and MMIO mapping in searly boot. The kernel does provide
early_ioremap(), but that is not intended to create long-lived MMIO
mappings and a seperate warning is printed by generic code if
early_ioremap() mappings are "leaked."
This is all fixable with dumb hacks like using early_ioremap() to setup
the initial mapping then replacing it with a real ioremap later on in
boot, but it does raise the question: Why the hell are we setting up the
PHB's this early in boot?
The old and wise claim it's due to "hysterical rasins." Aside from amused
grapes there doesn't appear to be any real reason to maintain the current
behaviour. Already most of the newer embedded platforms perform PHB
discovery in an arch_initcall and between the end of setup_arch() and the
start of initcalls none of the generic kernel code does anything PCI
related. On powerpc scanning PHBs occurs in a subsys_initcall so it should
be possible to move the PHB discovery to a core, postcore or arch initcall.
This patch adds the ppc_md.discover_phbs hook and a core_initcall stub that
calls it. The core_initcalls are the earliest to be called so this will
any possibly issues with dependency between initcalls. This isn't just an
academic issue either since on pseries and PowerNV EEH init occurs in an
arch_initcall and depends on the pci_controllers being available, similarly
the creation of pci_dns occurs at core_initcall_sync (i.e. between core and
postcore initcalls). These problems need to be addressed seperately.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[mpe: Make discover_phbs() static]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20201103043523.916109-1-oohall@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/machdep.h | 3 +++
arch/powerpc/kernel/pci-common.c | 10 ++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
index cf6ebbc16cb4..764f2732a821 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -59,6 +59,9 @@ struct machdep_calls {
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);
+ /* finds all the pci_controllers present at boot */
+ void (*discover_phbs)(void);
+
/* To setup PHBs when using automatic OF platform driver for PCI */
int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 2b555997b295..001e90cd8948 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1699,3 +1699,13 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
+
+
+static int __init discover_phbs(void)
+{
+ if (ppc_md.discover_phbs)
+ ppc_md.discover_phbs();
+
+ return 0;
+}
+core_initcall(discover_phbs);
--
2.30.1
^ permalink raw reply related
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Michael Ellerman @ 2021-03-02 11:40 UTC (permalink / raw)
To: Christophe Leroy, Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <08a96c5d-4ae7-03b4-208f-956226dee6bb@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 02/03/2021 à 10:53, Marco Elver a écrit :
>> On Tue, 2 Mar 2021 at 10:27, Christophe Leroy
>> <christophe.leroy@csgroup.eu> wrote:
>>> Le 02/03/2021 à 10:21, Alexander Potapenko a écrit :
>>>>> [ 14.998426] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c
>>>>> [ 14.998426]
>>>>> [ 15.007061] Invalid read at 0x(ptrval):
>>>>> [ 15.010906] finish_task_switch.isra.0+0x54/0x23c
>>>>> [ 15.015633] kunit_try_run_case+0x5c/0xd0
>>>>> [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30
>>>>> [ 15.025099] kthread+0x15c/0x174
>>>>> [ 15.028359] ret_from_kernel_thread+0x14/0x1c
>>>>> [ 15.032747]
>>>>> [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B
>>>>> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674
>>>>> [ 15.045811] ==================================================================
>>>>> [ 15.053324] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636
>>>>> [ 15.053324] Expected report_matches(&expect) to be true, but is false
>>>>> [ 15.068359] not ok 21 - test_invalid_access
>>>>
>>>> The test expects the function name to be test_invalid_access, i. e.
>>>> the first line should be "BUG: KFENCE: invalid read in
>>>> test_invalid_access".
>>>> The error reporting function unwinds the stack, skips a couple of
>>>> "uninteresting" frames
>>>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43)
>>>> and uses the first "interesting" one frame to print the report header
>>>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226).
>>>>
>>>> It's strange that test_invalid_access is missing altogether from the
>>>> stack trace - is that expected?
>>>> Can you try printing the whole stacktrace without skipping any frames
>>>> to see if that function is there?
>>>>
>>>
>>> Booting with 'no_hash_pointers" I get the following. Does it helps ?
>>>
>>> [ 16.837198] ==================================================================
>>> [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c
>>> [ 16.848521]
>>> [ 16.857158] Invalid read at 0xdf98800a:
>>> [ 16.861004] finish_task_switch.isra.0+0x54/0x23c
>>> [ 16.865731] kunit_try_run_case+0x5c/0xd0
>>> [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30
>>> [ 16.875199] kthread+0x15c/0x174
>>> [ 16.878460] ret_from_kernel_thread+0x14/0x1c
>>> [ 16.882847]
>>> [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B
>>> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674
>>> [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38
>>> [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: G B
>>> (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty)
>>> [ 16.911386] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 22000004 XER: 00000000
>>> [ 16.918153] DAR: df98800a DSISR: 20000000
>>> [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 00000008 c084b32b c016eb38
>>> [ 16.918153] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288
>>> [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108
>>> [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0
>>> [ 16.947292] Call Trace:
>>> [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable)
>>
>> The "(unreliable)" might be a clue that it's related to ppc32 stack
>> unwinding. Any ppc expert know what this is about?
>>
>>> [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0
>>> [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30
>>> [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174
>>> [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
>>> [ 16.981896] Instruction dump:
>>> [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 39000000 995f0024 907f0028 90ff001c
>>> [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f
>>> [ 17.000711] ==================================================================
>>> [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636
>>> [ 17.008223] Expected report_matches(&expect) to be true, but is false
>>> [ 17.023243] not ok 21 - test_invalid_access
>>
>> On a fault in test_invalid_access, KFENCE prints the stack trace based
>> on the information in pt_regs. So we do not think there's anything we
>> can do to improve stack printing pe-se.
>
> stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does.
>
> IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests,
> there is some function call being done before the fault, for instance
> test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the
> call in the stack. However this is fragile.
>
> This works for function calls because in order to call a subfunction, a function has to set up a
> stack frame in order to same the value in the Link Register, which contains the address of the
> function's parent and that will be clobbered by the sub-function call.
>
> However, it cannot be done by exceptions, because exceptions can happen in a function that has no
> stack frame (because that function has no need to call a subfunction and doesn't need to same
> anything on the stack). If the exception handler was writting the caller's address in the stack
> frame, it would in fact write it in the parent's frame, leading to a mess.
>
> But in fact the information is in pt_regs, it is in regs->nip so KFENCE should be able to use that
> instead of the stack.
>
>>
>> What's confusing is that it's only this test, and none of the others.
>> Given that, it might be code-gen related, which results in some subtle
>> issue with stack unwinding. There are a few things to try, if you feel
>> like it:
>>
>> -- Change the unwinder, if it's possible for ppc32.
>
> I don't think it is possible.
I think this actually is the solution.
It seems the good architectures have all added support for
arch_stack_walk(), and we have not.
Looking at some of the implementations of arch_stack_walk() it seems
it's expected that the first entry emitted includes the PC (or NIP on
ppc).
For us stack_trace_save() calls save_stack_trace() which only emits
entries from the stack, which doesn't necessarily include the function
NIP is pointing to.
So I think it's probably on us to update to that new API. Or at least
update our save_stack_trace() to fabricate an entry using the NIP, as it
seems that's what callers expect.
cheers
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Marco Elver @ 2021-03-02 11:39 UTC (permalink / raw)
To: Christophe Leroy
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <08a96c5d-4ae7-03b4-208f-956226dee6bb@csgroup.eu>
On Tue, 2 Mar 2021 at 12:21, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
[...]
> >> Booting with 'no_hash_pointers" I get the following. Does it helps ?
> >>
> >> [ 16.837198] ==================================================================
> >> [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c
> >> [ 16.848521]
> >> [ 16.857158] Invalid read at 0xdf98800a:
> >> [ 16.861004] finish_task_switch.isra.0+0x54/0x23c
> >> [ 16.865731] kunit_try_run_case+0x5c/0xd0
> >> [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30
> >> [ 16.875199] kthread+0x15c/0x174
> >> [ 16.878460] ret_from_kernel_thread+0x14/0x1c
> >> [ 16.882847]
> >> [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B
> >> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674
> >> [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38
> >> [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: G B
> >> (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty)
> >> [ 16.911386] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 22000004 XER: 00000000
> >> [ 16.918153] DAR: df98800a DSISR: 20000000
> >> [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 00000008 c084b32b c016eb38
> >> [ 16.918153] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288
> >> [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108
> >> [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0
> >> [ 16.947292] Call Trace:
> >> [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable)
> >
> > The "(unreliable)" might be a clue that it's related to ppc32 stack
> > unwinding. Any ppc expert know what this is about?
> >
> >> [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0
> >> [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30
> >> [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174
> >> [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
> >> [ 16.981896] Instruction dump:
> >> [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 39000000 995f0024 907f0028 90ff001c
> >> [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f
> >> [ 17.000711] ==================================================================
> >> [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636
> >> [ 17.008223] Expected report_matches(&expect) to be true, but is false
> >> [ 17.023243] not ok 21 - test_invalid_access
> >
> > On a fault in test_invalid_access, KFENCE prints the stack trace based
> > on the information in pt_regs. So we do not think there's anything we
> > can do to improve stack printing pe-se.
>
> stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does.
We use stack_trace_save_regs() + stack_trace_print().
> IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests,
> there is some function call being done before the fault, for instance
> test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the
> call in the stack. However this is fragile.
Interesting, this might explain it.
> This works for function calls because in order to call a subfunction, a function has to set up a
> stack frame in order to same the value in the Link Register, which contains the address of the
> function's parent and that will be clobbered by the sub-function call.
>
> However, it cannot be done by exceptions, because exceptions can happen in a function that has no
> stack frame (because that function has no need to call a subfunction and doesn't need to same
> anything on the stack). If the exception handler was writting the caller's address in the stack
> frame, it would in fact write it in the parent's frame, leading to a mess.
>
> But in fact the information is in pt_regs, it is in regs->nip so KFENCE should be able to use that
> instead of the stack.
Perhaps stack_trace_save_regs() needs fixing for ppc32? Although that
seems to use arch_stack_walk().
> > What's confusing is that it's only this test, and none of the others.
> > Given that, it might be code-gen related, which results in some subtle
> > issue with stack unwinding. There are a few things to try, if you feel
> > like it:
> >
> > -- Change the unwinder, if it's possible for ppc32.
>
> I don't think it is possible.
>
> >
> > -- Add code to test_invalid_access(), to get the compiler to emit
> > different code. E.g. add a bunch (unnecessary) function calls, or add
> > barriers, etc.
>
> The following does the trick
>
> diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c
> index 4acf4251ee04..22550676cd1f 100644
> --- a/mm/kfence/kfence_test.c
> +++ b/mm/kfence/kfence_test.c
> @@ -631,8 +631,11 @@ static void test_invalid_access(struct kunit *test)
> .addr = &__kfence_pool[10],
> .is_write = false,
> };
> + char *buf;
>
> + buf = test_alloc(test, 4, GFP_KERNEL, ALLOCATE_RIGHT);
> READ_ONCE(__kfence_pool[10]);
> + test_free(buf);
> KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> }
>
>
> But as I said above, this is fragile. If for some reason one day test_alloc() gets inlined, it may
> not work anymore.
Yeah, obviously that's hack, but interesting nevertheless.
Based on what you say above, however, it seems that
stack_trace_save_regs()/arch_stack_walk() don't exactly do what they
should? Can they be fixed for ppc32?
Thanks,
-- Marco
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox