LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 19/41] KVM: PPC: Book3S HV P9: Stop handling hcalls in real-mode in the P9 path
From: Nicholas Piggin @ 2021-03-17 22:41 UTC (permalink / raw)
  To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87o8fh21iq.fsf@linux.ibm.com>

Excerpts from Fabiano Rosas's message of March 18, 2021 2:22 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> In the interest of minimising the amount of code that is run in
>> "real-mode", don't handle hcalls in real mode in the P9 path.
>>
>> POWER8 and earlier are much more expensive to exit from HV real mode
>> and switch to host mode, because on those processors HV interrupts get
>> to the hypervisor with the MMU off, and the other threads in the core
>> need to be pulled out of the guest, and SLBs all need to be saved,
>> ERATs invalidated, and host SLB reloaded before the MMU is re-enabled
>> in host mode. Hash guests also require a lot of hcalls to run. The
>> XICS interrupt controller requires hcalls to run.
>>
>> By contrast, POWER9 has independent thread switching, and in radix mode
>> the hypervisor is already in a host virtual memory mode when the HV
>> interrupt is taken. Radix + xive guests don't need hcalls to handle
>> interrupts or manage translations.
>>
>> So it's much less important to handle hcalls in real mode in P9.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
> 
> <snip>
> 
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 497f216ad724..1f2ba8955c6a 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -1147,7 +1147,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>>   * This has to be done early, not in kvmppc_pseries_do_hcall(), so
>>   * that the cede logic in kvmppc_run_single_vcpu() works properly.
>>   */
>> -static void kvmppc_nested_cede(struct kvm_vcpu *vcpu)
>> +static void kvmppc_cede(struct kvm_vcpu *vcpu)
> 
> The comment above needs to be updated I think.
> 
>>  {
>>  	vcpu->arch.shregs.msr |= MSR_EE;
>>  	vcpu->arch.ceded = 1;
>> @@ -1403,9 +1403,15 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
>>  		/* hcall - punt to userspace */
>>  		int i;
>>
>> -		/* hypercall with MSR_PR has already been handled in rmode,
>> -		 * and never reaches here.
>> -		 */
>> +		if (unlikely(vcpu->arch.shregs.msr & MSR_PR)) {
>> +			/*
>> +			 * Guest userspace executed sc 1, reflect it back as a
>> +			 * privileged program check interrupt.
>> +			 */
>> +			kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
>> +			r = RESUME_GUEST;
>> +			break;
>> +		}
>>
>>  		run->papr_hcall.nr = kvmppc_get_gpr(vcpu, 3);
>>  		for (i = 0; i < 9; ++i)
>> @@ -3740,15 +3746,36 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>>  		/* H_CEDE has to be handled now, not later */
>>  		if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
>>  		    kvmppc_get_gpr(vcpu, 3) == H_CEDE) {
>> -			kvmppc_nested_cede(vcpu);
>> +			kvmppc_cede(vcpu);
>>  			kvmppc_set_gpr(vcpu, 3, 0);
>>  			trap = 0;
>>  		}
>>  	} else {
>>  		kvmppc_xive_push_vcpu(vcpu);
>>  		trap = kvmhv_load_hv_regs_and_go(vcpu, time_limit, lpcr);
>> -		kvmppc_xive_pull_vcpu(vcpu);
>> +		/* H_CEDE has to be handled now, not later */
>> +		/* XICS hcalls must be handled before xive is pulled */
>> +		if (trap == BOOK3S_INTERRUPT_SYSCALL &&
>> +		    !(vcpu->arch.shregs.msr & MSR_PR)) {
>> +			unsigned long req = kvmppc_get_gpr(vcpu, 3);
>>
>> +			if (req == H_CEDE) {
>> +				kvmppc_cede(vcpu);
>> +				kvmppc_xive_cede_vcpu(vcpu); /* may un-cede */
>> +				kvmppc_set_gpr(vcpu, 3, 0);
>> +				trap = 0;
>> +			}
>> +			if (req == H_EOI || req == H_CPPR ||
>> +			    req == H_IPI || req == H_IPOLL ||
>> +			    req == H_XIRR || req == H_XIRR_X) {
>> +				unsigned long ret;
>> +
>> +				ret = kvmppc_xive_xics_hcall(vcpu, req);
>> +				kvmppc_set_gpr(vcpu, 3, ret);
>> +				trap = 0;
>> +			}
>> +		}
> 
> I tried running L2 with xive=off and this code slows down the boot
> considerably. I think we're missing a !vcpu->arch.nested in the
> conditional.

You might be right, the real mode handlers never run if nested is set
so none of these should run I think.

> 
> This may also be missing these checks from kvmppc_pseries_do_hcall:
> 
> 		if (kvmppc_xics_enabled(vcpu)) {
> 			if (xics_on_xive()) {
> 				ret = H_NOT_AVAILABLE;
> 				return RESUME_GUEST;
> 			}
> 			ret = kvmppc_xics_hcall(vcpu, req);
>                         (...)

Well this is the formerly real-mode part of the hcall, whereas 
pseries_do_hcall is the virt-mode handler so it expects the real mode 
has already run.

Hmm, probably it shouldn't be setting trap = 0 if it did not handle the
hcall. I don't know if that's the problem you have or if it's the nested
test but probably should test for this anyway.

> For H_CEDE there might be a similar situation since we're shadowing the
> code above that runs after H_ENTER_NESTED by setting trap to 0 here.

Yes.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Dave Hansen @ 2021-03-17 18:48 UTC (permalink / raw)
  To: Kefeng Wang, linux-kernel, Andrew Morton
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, linux-mm, Guo Ren, sparclinux, linux-riscv,
	Jonas Bonn, linux-s390, Yoshinori Sato, linux-hexagon,
	Huacai Chen, Russell King, linux-csky, Ingo Molnar,
	linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
	linux-parisc, linux-mips, Palmer Dabbelt, linux-alpha,
	linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>

On 3/16/21 6:52 PM, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

It's not a big deal but you might want to say something like:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # x86 bits

Just to make it clear that I didn't look at the alpha bits at all. :)

^ permalink raw reply

* Re: [PATCH 12/14] swiotlb: move global variables into a new io_tlb_mem structure
From: Konrad Rzeszutek Wilk @ 2021-03-17 18:18 UTC (permalink / raw)
  To: Christoph Hellwig, jgross
  Cc: xen-devel, iommu, Dongli Zhang, Claire Chang, linuxppc-dev
In-Reply-To: <20210317175742.GA29280@lst.de>

On Wed, Mar 17, 2021 at 06:57:42PM +0100, Christoph Hellwig wrote:
> On Wed, Mar 17, 2021 at 01:51:56PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Wed, Mar 17, 2021 at 02:53:27PM +0100, Christoph Hellwig wrote:
> > > On Wed, Mar 17, 2021 at 01:42:07PM +0000, Konrad Rzeszutek Wilk wrote:
> > > > > -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
> > > > > -	io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > > > > -	if (!io_tlb_alloc_size)
> > > > > -		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> > > > > -		      __func__, alloc_size, PAGE_SIZE);
> > > > 
> > > > Shouldn't this be converted to:
> > > > 	mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > > > 	if (...)
> > > > 
> > > > Seems that it got lost in the search and replace?
> > > 
> > > Yes, I messed that up during the rebase.  That being said it magically
> > > gets fixed in the next patch..
> > 
> > Yes. However if someone does a bisection they are going to be mighty unhappy
> > with you.
> 
> Sure, I was planning on fixing it anyway.  Just waiting for feedback
> on the rest of the patches before doing a respin.

I put the patches up-to this one on :

 git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb devel/for-linus-5.13

Would you be OK rebasing on top of that and sending the patches?

Juergen, would you be OK testing that branch on your Xen setup?

^ permalink raw reply

* Re: [PATCH 12/14] swiotlb: move global variables into a new io_tlb_mem structure
From: Christoph Hellwig @ 2021-03-17 17:57 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Dongli Zhang, iommu, xen-devel, Claire Chang, linuxppc-dev,
	Christoph Hellwig
In-Reply-To: <YFJBvFjtZUiBQj4k@Konrads-MacBook-Pro.local>

On Wed, Mar 17, 2021 at 01:51:56PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 17, 2021 at 02:53:27PM +0100, Christoph Hellwig wrote:
> > On Wed, Mar 17, 2021 at 01:42:07PM +0000, Konrad Rzeszutek Wilk wrote:
> > > > -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
> > > > -	io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > > > -	if (!io_tlb_alloc_size)
> > > > -		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> > > > -		      __func__, alloc_size, PAGE_SIZE);
> > > 
> > > Shouldn't this be converted to:
> > > 	mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > > 	if (...)
> > > 
> > > Seems that it got lost in the search and replace?
> > 
> > Yes, I messed that up during the rebase.  That being said it magically
> > gets fixed in the next patch..
> 
> Yes. However if someone does a bisection they are going to be mighty unhappy
> with you.

Sure, I was planning on fixing it anyway.  Just waiting for feedback
on the rest of the patches before doing a respin.

^ permalink raw reply

* Re: [PATCH] powerpc: kernel: Trivial typo fix in the file kgdb.c
From: Randy Dunlap @ 2021-03-17 17:56 UTC (permalink / raw)
  To: Bhaskar Chowdhury, mpe, benh, paulus, jniethe5, alistair,
	linuxppc-dev, linux-kernel
In-Reply-To: <20210317090413.120891-1-unixbhaskar@gmail.com>

On 3/17/21 2:04 AM, Bhaskar Chowdhury wrote:
> 
> s/procesing/processing/
> 
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>

> ---
>  arch/powerpc/kernel/kgdb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 409080208a6c..7dd2ad3603ad 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -376,7 +376,7 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
>  }
> 
>  /*
> - * This function does PowerPC specific procesing for interfacing to gdb.
> + * This function does PowerPC specific processing for interfacing to gdb.
>   */
>  int kgdb_arch_handle_exception(int vector, int signo, int err_code,
>  			       char *remcom_in_buffer, char *remcom_out_buffer,
> --


-- 
~Randy


^ permalink raw reply

* Re: [PATCH 12/14] swiotlb: move global variables into a new io_tlb_mem structure
From: Konrad Rzeszutek Wilk @ 2021-03-17 17:51 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: xen-devel, iommu, Dongli Zhang, Claire Chang, linuxppc-dev
In-Reply-To: <20210317135327.GA10797@lst.de>

On Wed, Mar 17, 2021 at 02:53:27PM +0100, Christoph Hellwig wrote:
> On Wed, Mar 17, 2021 at 01:42:07PM +0000, Konrad Rzeszutek Wilk wrote:
> > > -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
> > > -	io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > > -	if (!io_tlb_alloc_size)
> > > -		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> > > -		      __func__, alloc_size, PAGE_SIZE);
> > 
> > Shouldn't this be converted to:
> > 	mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > 	if (...)
> > 
> > Seems that it got lost in the search and replace?
> 
> Yes, I messed that up during the rebase.  That being said it magically
> gets fixed in the next patch..

Yes. However if someone does a bisection they are going to be mighty unhappy
with you.

^ permalink raw reply

* Re: [PATCH v3 19/41] KVM: PPC: Book3S HV P9: Stop handling hcalls in real-mode in the P9 path
From: Fabiano Rosas @ 2021-03-17 16:22 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210305150638.2675513-20-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> In the interest of minimising the amount of code that is run in
> "real-mode", don't handle hcalls in real mode in the P9 path.
>
> POWER8 and earlier are much more expensive to exit from HV real mode
> and switch to host mode, because on those processors HV interrupts get
> to the hypervisor with the MMU off, and the other threads in the core
> need to be pulled out of the guest, and SLBs all need to be saved,
> ERATs invalidated, and host SLB reloaded before the MMU is re-enabled
> in host mode. Hash guests also require a lot of hcalls to run. The
> XICS interrupt controller requires hcalls to run.
>
> By contrast, POWER9 has independent thread switching, and in radix mode
> the hypervisor is already in a host virtual memory mode when the HV
> interrupt is taken. Radix + xive guests don't need hcalls to handle
> interrupts or manage translations.
>
> So it's much less important to handle hcalls in real mode in P9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

<snip>

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 497f216ad724..1f2ba8955c6a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1147,7 +1147,7 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>   * This has to be done early, not in kvmppc_pseries_do_hcall(), so
>   * that the cede logic in kvmppc_run_single_vcpu() works properly.
>   */
> -static void kvmppc_nested_cede(struct kvm_vcpu *vcpu)
> +static void kvmppc_cede(struct kvm_vcpu *vcpu)

The comment above needs to be updated I think.

>  {
>  	vcpu->arch.shregs.msr |= MSR_EE;
>  	vcpu->arch.ceded = 1;
> @@ -1403,9 +1403,15 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
>  		/* hcall - punt to userspace */
>  		int i;
>
> -		/* hypercall with MSR_PR has already been handled in rmode,
> -		 * and never reaches here.
> -		 */
> +		if (unlikely(vcpu->arch.shregs.msr & MSR_PR)) {
> +			/*
> +			 * Guest userspace executed sc 1, reflect it back as a
> +			 * privileged program check interrupt.
> +			 */
> +			kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
> +			r = RESUME_GUEST;
> +			break;
> +		}
>
>  		run->papr_hcall.nr = kvmppc_get_gpr(vcpu, 3);
>  		for (i = 0; i < 9; ++i)
> @@ -3740,15 +3746,36 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>  		/* H_CEDE has to be handled now, not later */
>  		if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
>  		    kvmppc_get_gpr(vcpu, 3) == H_CEDE) {
> -			kvmppc_nested_cede(vcpu);
> +			kvmppc_cede(vcpu);
>  			kvmppc_set_gpr(vcpu, 3, 0);
>  			trap = 0;
>  		}
>  	} else {
>  		kvmppc_xive_push_vcpu(vcpu);
>  		trap = kvmhv_load_hv_regs_and_go(vcpu, time_limit, lpcr);
> -		kvmppc_xive_pull_vcpu(vcpu);
> +		/* H_CEDE has to be handled now, not later */
> +		/* XICS hcalls must be handled before xive is pulled */
> +		if (trap == BOOK3S_INTERRUPT_SYSCALL &&
> +		    !(vcpu->arch.shregs.msr & MSR_PR)) {
> +			unsigned long req = kvmppc_get_gpr(vcpu, 3);
>
> +			if (req == H_CEDE) {
> +				kvmppc_cede(vcpu);
> +				kvmppc_xive_cede_vcpu(vcpu); /* may un-cede */
> +				kvmppc_set_gpr(vcpu, 3, 0);
> +				trap = 0;
> +			}
> +			if (req == H_EOI || req == H_CPPR ||
> +			    req == H_IPI || req == H_IPOLL ||
> +			    req == H_XIRR || req == H_XIRR_X) {
> +				unsigned long ret;
> +
> +				ret = kvmppc_xive_xics_hcall(vcpu, req);
> +				kvmppc_set_gpr(vcpu, 3, ret);
> +				trap = 0;
> +			}
> +		}

I tried running L2 with xive=off and this code slows down the boot
considerably. I think we're missing a !vcpu->arch.nested in the
conditional.

This may also be missing these checks from kvmppc_pseries_do_hcall:

		if (kvmppc_xics_enabled(vcpu)) {
			if (xics_on_xive()) {
				ret = H_NOT_AVAILABLE;
				return RESUME_GUEST;
			}
			ret = kvmppc_xics_hcall(vcpu, req);
                        (...)

For H_CEDE there might be a similar situation since we're shadowing the
code above that runs after H_ENTER_NESTED by setting trap to 0 here.

> +		kvmppc_xive_pull_vcpu(vcpu);
>  	}
>
>  	vcpu->arch.slb_max = 0;
> @@ -4408,8 +4435,11 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>  		else
>  			r = kvmppc_run_vcpu(vcpu);
>
> -		if (run->exit_reason == KVM_EXIT_PAPR_HCALL &&
> -		    !(vcpu->arch.shregs.msr & MSR_PR)) {
> +		if (run->exit_reason == KVM_EXIT_PAPR_HCALL) {
> +			if (WARN_ON_ONCE(vcpu->arch.shregs.msr & MSR_PR)) {
> +				r = RESUME_GUEST;
> +				continue;
> +			}
>  			trace_kvm_hcall_enter(vcpu);
>  			r = kvmppc_pseries_do_hcall(vcpu);
>  			trace_kvm_hcall_exit(vcpu, r);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index c11597f815e4..2d0d14ed1d92 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1397,9 +1397,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>  	mr	r4,r9
>  	bge	fast_guest_return
>  2:
> +	/* If we came in through the P9 short path, no real mode hcalls */
> +	lwz	r0, STACK_SLOT_SHORT_PATH(r1)
> +	cmpwi	r0, 0
> +	bne	no_try_real
>  	/* See if this is an hcall we can handle in real mode */
>  	cmpwi	r12,BOOK3S_INTERRUPT_SYSCALL
>  	beq	hcall_try_real_mode
> +no_try_real:
>
>  	/* Hypervisor doorbell - exit only if host IPI flag set */
>  	cmpwi	r12, BOOK3S_INTERRUPT_H_DOORBELL
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 52cdb9e2660a..1e4871bbcad4 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -158,6 +158,40 @@ void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
>  }
>  EXPORT_SYMBOL_GPL(kvmppc_xive_pull_vcpu);
>
> +void kvmppc_xive_cede_vcpu(struct kvm_vcpu *vcpu)
> +{
> +	void __iomem *esc_vaddr = (void __iomem *)vcpu->arch.xive_esc_vaddr;
> +
> +	if (!esc_vaddr)
> +		return;
> +
> +	/* we are using XIVE with single escalation */
> +
> +	if (vcpu->arch.xive_esc_on) {
> +		/*
> +		 * If we still have a pending escalation, abort the cede,
> +		 * and we must set PQ to 10 rather than 00 so that we don't
> +		 * potentially end up with two entries for the escalation
> +		 * interrupt in the XIVE interrupt queue.  In that case
> +		 * we also don't want to set xive_esc_on to 1 here in
> +		 * case we race with xive_esc_irq().
> +		 */
> +		vcpu->arch.ceded = 0;
> +		/*
> +		 * The escalation interrupts are special as we don't EOI them.
> +		 * There is no need to use the load-after-store ordering offset
> +		 * to set PQ to 10 as we won't use StoreEOI.
> +		 */
> +		__raw_readq(esc_vaddr + XIVE_ESB_SET_PQ_10);
> +	} else {
> +		vcpu->arch.xive_esc_on = true;
> +		mb();
> +		__raw_readq(esc_vaddr + XIVE_ESB_SET_PQ_00);
> +	}
> +	mb();
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_xive_cede_vcpu);
> +
>  /*
>   * This is a simple trigger for a generic XIVE IRQ. This must
>   * only be called for interrupts that support a trigger page
> @@ -2106,6 +2140,32 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
>  	return 0;
>  }
>
> +int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
> +{
> +	struct kvmppc_vcore *vc = vcpu->arch.vcore;
> +
> +	switch (req) {
> +	case H_XIRR:
> +		return xive_vm_h_xirr(vcpu);
> +	case H_CPPR:
> +		return xive_vm_h_cppr(vcpu, kvmppc_get_gpr(vcpu, 4));
> +	case H_EOI:
> +		return xive_vm_h_eoi(vcpu, kvmppc_get_gpr(vcpu, 4));
> +	case H_IPI:
> +		return xive_vm_h_ipi(vcpu, kvmppc_get_gpr(vcpu, 4),
> +					  kvmppc_get_gpr(vcpu, 5));
> +	case H_IPOLL:
> +		return xive_vm_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
> +	case H_XIRR_X:
> +		xive_vm_h_xirr(vcpu);
> +		kvmppc_set_gpr(vcpu, 5, get_tb() + vc->tb_offset);
> +		return H_SUCCESS;
> +	}
> +
> +	return H_UNSUPPORTED;
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_xive_xics_hcall);
> +
>  int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
>  {
>  	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;

^ permalink raw reply

* Re: Advice needed on SMP regression after cpu_core_mask change
From: Daniel Henrique Barboza @ 2021-03-17 16:05 UTC (permalink / raw)
  To: Cédric Le Goater, linuxppc-dev
  Cc: Greg Kurz, aneesh.kumar, Srikar Dronamraju, David Gibson
In-Reply-To: <4569097d-ce89-5a13-33a9-2a4ca10be7bd@kaod.org>



On 3/17/21 12:30 PM, Cédric Le Goater wrote:
> On 3/17/21 2:00 PM, Daniel Henrique Barboza wrote:
>> Hello,
>>
>> Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
>> a regression in both upstream and RHEL downstream kernels [1]. The assumption made
>> in the commit:
>>
>> "Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
>> equal on Power"
>>
>> Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
>> node SMP topologies such as:
>>
>> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
>>
>> lscpu will give the following output in this case:
>>
>> # lscpu
>> Architecture:        ppc64le
>> Byte Order:          Little Endian
>> CPU(s):              8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core:  2
>> Core(s) per socket:  4
>> Socket(s):           1
>> NUMA node(s):        1
>> Model:               2.2 (pvr 004e 1202)
>> Model name:          POWER9 (architected), altivec supported
>> Hypervisor vendor:   KVM
>> Virtualization type: para
>> L1d cache:           32K
>> L1i cache:           32K
>> NUMA node0 CPU(s):   0-7
>>
>>
>> This is happening because the macro cpu_cpu_mask(cpu) expands to
>> cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
>> node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
>> correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.
>>
>> If I add a second NUMA node then I can get the intended smp topology:
>>
>> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
>> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
>> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>>
>> # lscpu
>> Architecture:        ppc64le
>> Byte Order:          Little Endian
>> CPU(s):              8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core:  2
>> Core(s) per socket:  2
>> Socket(s):           2
>> NUMA node(s):        2
>> Model:               2.2 (pvr 004e 1202)
>> Model name:          POWER9 (architected), altivec supported
>> Hypervisor vendor:   KVM
>> Virtualization type: para
>> L1d cache:           32K
>> L1i cache:           32K
>> NUMA node0 CPU(s):   0-3
>> NUMA node1 CPU(s):   4-7
>>
>>
>> However, if I try a single socket with multiple NUMA nodes topology, which is the case
>> of Power10, e.g.:
>>
>>
>> -smp 8,maxcpus=8,cores=4,threads=2,sockets=1
>> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
>> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
>>
>>
>> This is the result:
>>
>> # lscpu
>> Architecture:        ppc64le
>> Byte Order:          Little Endian
>> CPU(s):              8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core:  2
>> Core(s) per socket:  2
>> Socket(s):           2
>> NUMA node(s):        2
>> Model:               2.2 (pvr 004e 1202)
>> Model name:          POWER9 (architected), altivec supported
>> Hypervisor vendor:   KVM
>> Virtualization type: para
>> L1d cache:           32K
>> L1i cache:           32K
>> NUMA node0 CPU(s):   0-3
>> NUMA node1 CPU(s):   4-7
>>
>>
>> This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.
> 
> Yes. I don't think we can do better on PAPR and the above examples
> seem to confirm that the "sockets" definition is simply ignored.
>   
>> Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
>> after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
>> cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
>> its shortcomings that caused its removal, was giving a precise SMP topology. And it was
>> using physical_package_id/'ibm,chip-id' for that.
> 
> ibm,chip-id is a no-no on pSeries. I guess this is inherent to PAPR which
> is hiding a lot of the underlying HW and topology. May be we are trying
> to reconcile two orthogonal views of machine virtualization ...
> 
>> Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
>> that cares about cores per socket information. The kernel is now ignoring that, starting
>> on 4bce545903fa, and now QEMU is unable to provide this info to the guest.
>>
>> If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
>> not declare it, we need another way of letting the guest know how much cores per socket
>> we want.
> The RTAS call "ibm,get-system-parameter" with token "Processor Module
> Information" returns that kind of information :
> 
>    2 byte binary number (N) of module types followed by N module specifiers of the form:
>    2 byte binary number (M) of sockets of this module type
>    2 byte binary number (L) of chips per this module type
>    2 byte binary number (K) of cores per chip in this module type.
> 
> See the values in these sysfs files :
> 
>    cat /sys/devices/hv_24x7/interface/{sockets,chipspersocket,coresperchip}
> 
> But I am afraid these are host level information and not guest/LPAR.


I believe there might be some sort of reasoning behind not having this on
PAPR, but I'll say in advance that the virtual machine should act as the
real hardware, as close as possible. This is the kind of hcall that could
be used in this situation.


> 
> I didn't find any LPAR level properties or hcalls in the PAPR document.
> They need to be specified.
> 
> or
> 
> We can add extra properties like ibm,chip-id but making sure it's only
> used under the KVM hypervisor. My understanding is that's something we
> are trying to avoid.

We can change PAPR to add ibm,chip-id. Problem is that ibm,chip-id today, with
the current kernel codebase, does not fix the issue because the code is
ignoring it hehehe


If we're going to change PAPR -  and I believe we should, there's a clear
lack of proper support for SMP topologies - we're better make sure that whatever
attribute/hcall we add there fixes it in a robust way for the long term.


Thanks,


DHB


> 
> C.
> 

^ permalink raw reply

* Re: Advice needed on SMP regression after cpu_core_mask change
From: Cédric Le Goater @ 2021-03-17 15:30 UTC (permalink / raw)
  To: Daniel Henrique Barboza, linuxppc-dev
  Cc: Greg Kurz, aneesh.kumar, Srikar Dronamraju, David Gibson
In-Reply-To: <daa5d05f-dbd0-05ad-7395-5d5a3d364fc6@gmail.com>

On 3/17/21 2:00 PM, Daniel Henrique Barboza wrote:
> Hello,
> 
> Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
> a regression in both upstream and RHEL downstream kernels [1]. The assumption made
> in the commit:
> 
> "Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
> equal on Power"
> 
> Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
> node SMP topologies such as:
> 
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> 
> lscpu will give the following output in this case:
> 
> # lscpu
> Architecture:        ppc64le
> Byte Order:          Little Endian
> CPU(s):              8
> On-line CPU(s) list: 0-7
> Thread(s) per core:  2
> Core(s) per socket:  4
> Socket(s):           1
> NUMA node(s):        1
> Model:               2.2 (pvr 004e 1202)
> Model name:          POWER9 (architected), altivec supported
> Hypervisor vendor:   KVM
> Virtualization type: para
> L1d cache:           32K
> L1i cache:           32K
> NUMA node0 CPU(s):   0-7
> 
> 
> This is happening because the macro cpu_cpu_mask(cpu) expands to
> cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
> node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
> correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.
> 
> If I add a second NUMA node then I can get the intended smp topology:
> 
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
> 
> # lscpu
> Architecture:        ppc64le
> Byte Order:          Little Endian
> CPU(s):              8
> On-line CPU(s) list: 0-7
> Thread(s) per core:  2
> Core(s) per socket:  2
> Socket(s):           2
> NUMA node(s):        2
> Model:               2.2 (pvr 004e 1202)
> Model name:          POWER9 (architected), altivec supported
> Hypervisor vendor:   KVM
> Virtualization type: para
> L1d cache:           32K
> L1i cache:           32K
> NUMA node0 CPU(s):   0-3
> NUMA node1 CPU(s):   4-7
> 
> 
> However, if I try a single socket with multiple NUMA nodes topology, which is the case
> of Power10, e.g.:
> 
> 
> -smp 8,maxcpus=8,cores=4,threads=2,sockets=1
> -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
> 
> 
> This is the result:
> 
> # lscpu
> Architecture:        ppc64le
> Byte Order:          Little Endian
> CPU(s):              8
> On-line CPU(s) list: 0-7
> Thread(s) per core:  2
> Core(s) per socket:  2
> Socket(s):           2
> NUMA node(s):        2
> Model:               2.2 (pvr 004e 1202)
> Model name:          POWER9 (architected), altivec supported
> Hypervisor vendor:   KVM
> Virtualization type: para
> L1d cache:           32K
> L1i cache:           32K
> NUMA node0 CPU(s):   0-3
> NUMA node1 CPU(s):   4-7
> 
> 
> This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.

Yes. I don't think we can do better on PAPR and the above examples 
seem to confirm that the "sockets" definition is simply ignored.
 
> Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
> after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
> cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
> its shortcomings that caused its removal, was giving a precise SMP topology. And it was
> using physical_package_id/'ibm,chip-id' for that.

ibm,chip-id is a no-no on pSeries. I guess this is inherent to PAPR which
is hiding a lot of the underlying HW and topology. May be we are trying 
to reconcile two orthogonal views of machine virtualization ...

> Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
> that cares about cores per socket information. The kernel is now ignoring that, starting
> on 4bce545903fa, and now QEMU is unable to provide this info to the guest.
> 
> If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
> not declare it, we need another way of letting the guest know how much cores per socket
> we want.
The RTAS call "ibm,get-system-parameter" with token "Processor Module 
Information" returns that kind of information :

  2 byte binary number (N) of module types followed by N module specifiers of the form:
  2 byte binary number (M) of sockets of this module type
  2 byte binary number (L) of chips per this module type
  2 byte binary number (K) of cores per chip in this module type.

See the values in these sysfs files :

  cat /sys/devices/hv_24x7/interface/{sockets,chipspersocket,coresperchip} 

But I am afraid these are host level information and not guest/LPAR.

I didn't find any LPAR level properties or hcalls in the PAPR document. 
They need to be specified.

or

We can add extra properties like ibm,chip-id but making sure it's only 
used under the KVM hypervisor. My understanding is that's something we 
are trying to avoid. 

C.

^ permalink raw reply

* Re: [PATCH v3 34/41] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9
From: Aneesh Kumar K.V @ 2021-03-17 15:11 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210305150638.2675513-35-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> Radix guest support will be removed from the P7/8 path, so disallow
> dependent threads mode on P9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/kvm_host.h |  1 -
>  arch/powerpc/kvm/book3s_hv.c        | 27 +++++----------------------
>  2 files changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 05fb00d37609..dd017dfa4e65 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -304,7 +304,6 @@ struct kvm_arch {
>  	u8 fwnmi_enabled;
>  	u8 secure_guest;
>  	u8 svm_enabled;
> -	bool threads_indep;
>  	bool nested_enable;
>  	bool dawr1_enabled;
>  	pgd_t *pgtable;
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index cb428e2f7140..928ed8180d9d 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -103,13 +103,9 @@ static int target_smt_mode;
>  module_param(target_smt_mode, int, 0644);
>  MODULE_PARM_DESC(target_smt_mode, "Target threads per core (0 = max)");
>  
> -static bool indep_threads_mode = true;
> -module_param(indep_threads_mode, bool, S_IRUGO | S_IWUSR);
> -MODULE_PARM_DESC(indep_threads_mode, "Independent-threads mode (only on POWER9)");
> -
>  static bool one_vm_per_core;
>  module_param(one_vm_per_core, bool, S_IRUGO | S_IWUSR);
> -MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires indep_threads_mode=N)");
> +MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires POWER8 or older)");

Isn't this also a security feature, where there was an ask to make sure
threads/vCPU from other VM won't run on this core? In that context isn't
this applicable also for P9?


>  
>  #ifdef CONFIG_KVM_XICS
>  static const struct kernel_param_ops module_param_ops = {
> @@ -2227,7 +2223,7 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>   */
>  static int threads_per_vcore(struct kvm *kvm)
>  {
> -	if (kvm->arch.threads_indep)
> +	if (cpu_has_feature(CPU_FTR_ARCH_300))
>  		return 1;
>  	return threads_per_subcore;
>  }
> @@ -4319,7 +4315,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>  	vcpu->arch.state = KVMPPC_VCPU_BUSY_IN_HOST;
>  
>  	do {
> -		if (kvm->arch.threads_indep && kvm_is_radix(kvm))
> +		if (kvm_is_radix(kvm))
>  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
>  						  vcpu->arch.vcore->lpcr);
>  		else
> @@ -4934,21 +4930,8 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
>  	/*
>  	 * Track that we now have a HV mode VM active. This blocks secondary
>  	 * CPU threads from coming online.
> -	 * On POWER9, we only need to do this if the "indep_threads_mode"
> -	 * module parameter has been set to N.
>  	 */
> -	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> -		if (!indep_threads_mode && !cpu_has_feature(CPU_FTR_HVMODE)) {
> -			pr_warn("KVM: Ignoring indep_threads_mode=N in nested hypervisor\n");
> -			kvm->arch.threads_indep = true;
> -		} else if (!indep_threads_mode && cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) {
> -			pr_warn("KVM: Ignoring indep_threads_mode=N on pre-DD2.2 POWER9\n");
> -			kvm->arch.threads_indep = true;
> -		} else {
> -			kvm->arch.threads_indep = indep_threads_mode;
> -		}
> -	}
> -	if (!kvm->arch.threads_indep)
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300))
>  		kvm_hv_vm_activated();
>  
>  	/*
> @@ -4989,7 +4972,7 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm)
>  {
>  	debugfs_remove_recursive(kvm->arch.debugfs_dir);
>  
> -	if (!kvm->arch.threads_indep)
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300))
>  		kvm_hv_vm_deactivated();
>  
>  	kvmppc_free_vcores(kvm);
> -- 
> 2.23.0

^ permalink raw reply

* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Anatoly Pugachev @ 2021-03-17 14:03 UTC (permalink / raw)
  To: Kefeng Wang
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, linux-mips, linux-mm, Guo Ren, Sparc kernel list,
	linux-riscv, Jonas Bonn, linux-s390, Yoshinori Sato,
	linux-hexagon, Huacai Chen, Russell King, linux-csky, Ingo Molnar,
	linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
	linux-parisc, Linux Kernel list, Palmer Dabbelt, linux-alpha,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>

On Wed, Mar 17, 2021 at 4:51 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> v2:
> - Cleanup 'str' line suggested by Christophe and ACK

applied patch (5.12.0-rc3-00020-g1df27313f50a-dirty) over linus.git
and tested boot on a sparc64 virtual machine (ldom) - boots.

^ permalink raw reply

* Re: [PATCH 12/14] swiotlb: move global variables into a new io_tlb_mem structure
From: Christoph Hellwig @ 2021-03-17 13:53 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Dongli Zhang, iommu, xen-devel, Claire Chang, linuxppc-dev,
	Christoph Hellwig
In-Reply-To: <20210317134204.GA315788@konrad-char-us-oracle-com.allregionaliads.osdevelopmeniad.oraclevcn.com>

On Wed, Mar 17, 2021 at 01:42:07PM +0000, Konrad Rzeszutek Wilk wrote:
> > -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
> > -	io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> > -	if (!io_tlb_alloc_size)
> > -		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> > -		      __func__, alloc_size, PAGE_SIZE);
> 
> Shouldn't this be converted to:
> 	mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> 	if (...)
> 
> Seems that it got lost in the search and replace?

Yes, I messed that up during the rebase.  That being said it magically
gets fixed in the next patch..

^ permalink raw reply

* Re: [PATCH 12/14] swiotlb: move global variables into a new io_tlb_mem structure
From: Konrad Rzeszutek Wilk @ 2021-03-17 13:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: xen-devel, iommu, Dongli Zhang, Claire Chang, linuxppc-dev
In-Reply-To: <20210301074436.919889-13-hch@lst.de>

..snip..
>  int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
>  {
..snip..
>  	/*
>  	 * Allocate and initialize the free list array.  This array is used
>  	 * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
> -	 * between io_tlb_start and io_tlb_end.
> +	 * between mem->start and mem->end.
>  	 */
> -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(int));
> -	io_tlb_list = memblock_alloc(alloc_size, PAGE_SIZE);
> -	if (!io_tlb_list)
> +	alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(int));
> +	mem->list = memblock_alloc(alloc_size, PAGE_SIZE);
> +	if (!mem->list)
>  		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
>  		      __func__, alloc_size, PAGE_SIZE);
>  
> -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t));
> -	io_tlb_orig_addr = memblock_alloc(alloc_size, PAGE_SIZE);
> -	if (!io_tlb_orig_addr)
> +	alloc_size = PAGE_ALIGN(mem->nslabs * sizeof(phys_addr_t));
> +	mem->orig_addr = memblock_alloc(alloc_size, PAGE_SIZE);
> +	if (!mem->orig_addr)
>  		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
>  		      __func__, alloc_size, PAGE_SIZE);
>  
> -	alloc_size = PAGE_ALIGN(io_tlb_nslabs * sizeof(size_t));
> -	io_tlb_alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
> -	if (!io_tlb_alloc_size)
> -		panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
> -		      __func__, alloc_size, PAGE_SIZE);

Shouldn't this be converted to:
	mem->alloc_size = memblock_alloc(alloc_size, PAGE_SIZE);
	if (...)

Seems that it got lost in the search and replace?
> -
> -	for (i = 0; i < io_tlb_nslabs; i++) {
> -		io_tlb_list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
> -		io_tlb_orig_addr[i] = INVALID_PHYS_ADDR;
> -		io_tlb_alloc_size[i] = 0;
> +	for (i = 0; i < mem->nslabs; i++) {
> +		mem->list[i] = IO_TLB_SEGSIZE - io_tlb_offset(i);
> +		mem->orig_addr[i] = INVALID_PHYS_ADDR;
> +		mem->alloc_size[i] = 0;
>  	}
> -	io_tlb_index = 0;
>  	no_iotlb_memory = false;
>  
>  	if (verbose)
>  		swiotlb_print_info();
>  
> -	swiotlb_set_max_segment(io_tlb_nslabs << IO_TLB_SHIFT);
> +	swiotlb_set_max_segment(mem->nslabs << IO_TLB_SHIFT);
>  	return 0;
>  }
>  

..snip..

^ permalink raw reply

* [PATCH 1/2] ASoC: fsl-asoc-card: Add support for WM8958 codec
From: Shengjiu Wang @ 2021-03-17 13:05 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
	perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel, robh+dt,
	devicetree

WM8958 codec is used on some i.MX based platform.
So add it support in this generic driver.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/Kconfig         |  2 ++
 sound/soc/fsl/fsl-asoc-card.c | 17 +++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index c71c6024320b..0917d65d6921 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -310,6 +310,8 @@ config SND_SOC_FSL_ASOC_CARD
 	select SND_SOC_FSL_ESAI
 	select SND_SOC_FSL_SAI
 	select SND_SOC_FSL_SSI
+	select SND_SOC_WM8994
+	select MFD_WM8994
 	help
 	 ALSA SoC Audio support with ASRC feature for Freescale SoCs that have
 	 ESAI/SAI/SSI and connect with external CODECs such as WM8962, CS42888,
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index f62f81ceab0d..c62bfd1c3ac7 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -25,6 +25,7 @@
 #include "../codecs/sgtl5000.h"
 #include "../codecs/wm8962.h"
 #include "../codecs/wm8960.h"
+#include "../codecs/wm8994.h"
 
 #define CS427x_SYSCLK_MCLK 0
 
@@ -37,12 +38,14 @@
 /**
  * struct codec_priv - CODEC private data
  * @mclk_freq: Clock rate of MCLK
+ * @free_freq: Clock rate of MCLK for hw_free()
  * @mclk_id: MCLK (or main clock) id for set_sysclk()
  * @fll_id: FLL (or secordary clock) id for set_sysclk()
  * @pll_id: PLL id for set_pll()
  */
 struct codec_priv {
 	unsigned long mclk_freq;
+	unsigned long free_freq;
 	u32 mclk_id;
 	u32 fll_id;
 	u32 pll_id;
@@ -235,10 +238,10 @@ static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream)
 	priv->streams &= ~BIT(substream->stream);
 
 	if (!priv->streams && codec_priv->pll_id && codec_priv->fll_id) {
-		/* Force freq to be 0 to avoid error message in codec */
+		/* Force freq to be free_freq to avoid error message in codec */
 		ret = snd_soc_dai_set_sysclk(asoc_rtd_to_codec(rtd, 0),
 					     codec_priv->mclk_id,
-					     0,
+					     codec_priv->free_freq,
 					     SND_SOC_CLOCK_IN);
 		if (ret) {
 			dev_err(dev, "failed to switch away from FLL: %d\n", ret);
@@ -665,6 +668,15 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
 		priv->card.dapm_routes = audio_map_rx;
 		priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_rx);
+	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8958")) {
+		codec_dai_name = "wm8994-aif1";
+		priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
+		priv->codec_priv.mclk_id = WM8994_FLL_SRC_MCLK1;
+		priv->codec_priv.fll_id = WM8994_SYSCLK_FLL1;
+		priv->codec_priv.pll_id = WM8994_FLL1;
+		priv->codec_priv.free_freq = priv->codec_priv.mclk_freq;
+		priv->card.dapm_routes = NULL;
+		priv->card.num_dapm_routes = 0;
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
@@ -882,6 +894,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = {
 	{ .compatible = "fsl,imx-audio-mqs", },
 	{ .compatible = "fsl,imx-audio-wm8524", },
 	{ .compatible = "fsl,imx-audio-si476x", },
+	{ .compatible = "fsl,imx-audio-wm8958", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, fsl_asoc_card_dt_ids);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 2/2] ASoC: bindings: fsl-asoc-card: add compatible string for WM8958 codec
From: Shengjiu Wang @ 2021-03-17 13:05 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
	perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel, robh+dt,
	devicetree
In-Reply-To: <1615986303-27959-1-git-send-email-shengjiu.wang@nxp.com>

The WM8958 codec is used on i.MX7D val board.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index 90d9e9d81624..23d83fa7609f 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -42,6 +42,8 @@ The compatible list for this generic sound card currently:
 
  "fsl,imx-audio-si476x"
 
+ "fsl,imx-audio-wm8958"
+
 Required properties:
 
   - compatible		: Contains one of entries in the compatible list.
-- 
2.27.0


^ permalink raw reply related

* Advice needed on SMP regression after cpu_core_mask change
From: Daniel Henrique Barboza @ 2021-03-17 13:00 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: aneesh.kumar, Srikar Dronamraju, Cedric Le Goater

Hello,

Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
a regression in both upstream and RHEL downstream kernels [1]. The assumption made
in the commit:

"Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
equal on Power"

Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
node SMP topologies such as:

-smp 8,maxcpus=8,cores=2,threads=2,sockets=2

lscpu will give the following output in this case:

# lscpu
Architecture:        ppc64le
Byte Order:          Little Endian
CPU(s):              8
On-line CPU(s) list: 0-7
Thread(s) per core:  2
Core(s) per socket:  4
Socket(s):           1
NUMA node(s):        1
Model:               2.2 (pvr 004e 1202)
Model name:          POWER9 (architected), altivec supported
Hypervisor vendor:   KVM
Virtualization type: para
L1d cache:           32K
L1i cache:           32K
NUMA node0 CPU(s):   0-7


This is happening because the macro cpu_cpu_mask(cpu) expands to
cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.

If I add a second NUMA node then I can get the intended smp topology:

-smp 8,maxcpus=8,cores=2,threads=2,sockets=2
-numa node,memdev=mem0,cpus=0-3,nodeid=0 \
-numa node,memdev=mem1,cpus=4-7,nodeid=1 \

# lscpu
Architecture:        ppc64le
Byte Order:          Little Endian
CPU(s):              8
On-line CPU(s) list: 0-7
Thread(s) per core:  2
Core(s) per socket:  2
Socket(s):           2
NUMA node(s):        2
Model:               2.2 (pvr 004e 1202)
Model name:          POWER9 (architected), altivec supported
Hypervisor vendor:   KVM
Virtualization type: para
L1d cache:           32K
L1i cache:           32K
NUMA node0 CPU(s):   0-3
NUMA node1 CPU(s):   4-7


However, if I try a single socket with multiple NUMA nodes topology, which is the case
of Power10, e.g.:


-smp 8,maxcpus=8,cores=4,threads=2,sockets=1
-numa node,memdev=mem0,cpus=0-3,nodeid=0 \
-numa node,memdev=mem1,cpus=4-7,nodeid=1 \


This is the result:

# lscpu
Architecture:        ppc64le
Byte Order:          Little Endian
CPU(s):              8
On-line CPU(s) list: 0-7
Thread(s) per core:  2
Core(s) per socket:  2
Socket(s):           2
NUMA node(s):        2
Model:               2.2 (pvr 004e 1202)
Model name:          POWER9 (architected), altivec supported
Hypervisor vendor:   KVM
Virtualization type: para
L1d cache:           32K
L1i cache:           32K
NUMA node0 CPU(s):   0-3
NUMA node1 CPU(s):   4-7


This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.


Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
its shortcomings that caused its removal, was giving a precise SMP topology. And it was
using physical_package_id/'ibm,chip-id' for that.

Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
that cares about cores per socket information. The kernel is now ignoring that, starting
on 4bce545903fa, and now QEMU is unable to provide this info to the guest.

If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
not declare it, we need another way of letting the guest know how much cores per socket
we want.



[1] https://bugzilla.redhat.com/1934421



Thanks,


DHB

^ permalink raw reply

* Re: [PATCH v2] rpadlpar: fix potential drc_name corruption in store functions
From: Michael Ellerman @ 2021-03-17 12:23 UTC (permalink / raw)
  To: Tyrel Datwyler, bhelgaas; +Cc: linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <20210315214821.452959-1-tyreld@linux.ibm.com>

On Mon, 15 Mar 2021 15:48:21 -0600, Tyrel Datwyler wrote:
> Both add_slot_store() and remove_slot_store() try to fix up the drc_name
> copied from the store buffer by placing a NULL terminator at nbyte + 1
> or in place of a '\n' if present. However, the static buffer that we
> copy the drc_name data into is not zeored and can contain anything past
> the n-th byte. This is problematic if a '\n' byte appears in that buffer
> after nbytes and the string copied into the store buffer was not NULL
> terminated to start with as the strchr() search for a '\n' byte will mark
> this incorrectly as the end of the drc_name string resulting in a drc_name
> string that contains garbage data after the n-th byte. The following
> debugging shows an example of the drmgr utility writing "PHB 4543" to
> the add_slot sysfs attribute, but add_slot_store logging a corrupted
> string value.
> 
> [...]

Applied to powerpc/fixes.

[1/1] rpadlpar: fix potential drc_name corruption in store functions
      https://git.kernel.org/powerpc/c/cc7a0bb058b85ea03db87169c60c7cfdd5d34678

cheers

^ permalink raw reply

* Re: [PATCH 4/4] tools/perf: Support pipeline stage cycles for powerpc
From: Jiri Olsa @ 2021-03-17 12:16 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: Ravi Bangoria, Madhavan Srinivasan, Peter Zijlstra, Kajol Jain,
	linux-kernel, acme, linux-perf-users, jolsa, linuxppc-dev,
	kan.liang
In-Reply-To: <CA827A39-FA2A-4B0C-BF8F-9DB428CD58B8@linux.vnet.ibm.com>

On Wed, Mar 17, 2021 at 05:01:27PM +0530, Athira Rajeev wrote:
> <html><head></head><body dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="ApplePlainTextBody"><div class="ApplePlainTextBody"><br><br><blockquote type="cite">On 16-Mar-2021, at 4:48 AM, Jiri Olsa &lt;jolsa@redhat.com&gt; wrote:<br><br>On Mon, Mar 15, 2021 at 01:22:09PM +0530, Athira Rajeev wrote:<br><br>SNIP<br><br><blockquote type="cite">+<br>+static char *setup_dynamic_sort_keys(char *str)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>unsigned int j;<br>+<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>if (sort__mode == SORT_MODE__MEMORY)<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>for (j = 0; j &lt; ARRAY_SIZE(dynamic_sort_keys_mem); j++)<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>if (arch_support_dynamic_key(dynamic_sort_keys_mem[j])) {<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>str = suffix_if_not_in(dynamic_sort_keys_mem[j], str);<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>if (str == NULL)<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>return str;<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>}<br>+<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>return str;<br>+}<br>+<br>static int __setup_sorting(struct evlist *evlist)<br>{<br><span class="Apple-tab-span" style="white-space:pre">	</span>char *str;<br>@@ -3050,6 +3085,12 @@ static int __setup_sorting(struct evlist *evlist)<br><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>}<br><span class="Apple-tab-span" style="white-space:pre">	</span>}<br><br>+<span class="Apple-tab-span" style="white-space:pre">	</span>str = setup_dynamic_sort_keys(str);<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>if (str == NULL) {<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>pr_err("Not enough memory to setup dynamic sort keys");<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>return -ENOMEM;<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>}<br></blockquote><br>hum, so this is basicaly overloading the default_mem_sort_order for<br>architecture, right?<br><br>then I think it'd be easier just overload default_mem_sort_order directly<br><br>I was thinking more about adding extra (arch specific) loop to<br>sort_dimension__add or somehow add arch's specific stuff to<br>memory_sort_dimensions<br></blockquote><br>Hi Jiri,<br><br>Above patch was to append additional sort keys to sort order based on<br>sort mode and architecture support. I had initially thought of defining two<br>orders ( say default_mem_sort_order plus mem_sort_order_pstage ). But if<br>new sort keys gets added for mem mode in future, we will need to keep<br>updating both orders. So preferred the approach of "appending" supported sort<br>keys to default order.<br><br>Following your thought on using "sort_dimension__add", I tried below approach<br>which is easier. The new sort dimension "p_stage_cyc" is presently only supported<br>on powerpc. For unsupported platforms, we don't want to display it<br>in the perf report output columns. Hence added check in sort_dimension__add()<br>and skip the sort key incase its not applicable for particular arch.<br><br>Please help to check if below approach looks fine.<br><br><br>diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/powerpc/util/event.c<br>index b80fbee83b6e..7205767d75eb 100644<br>--- a/tools/perf/arch/powerpc/util/event.c<br>+++ b/tools/perf/arch/powerpc/util/event.c<br>@@ -44,3 +44,10 @@ const char *arch_perf_header_entry__add(const char *se_header)<br> <span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>return "Dispatch Cyc";<br> <span class="Apple-tab-span" style="white-space:pre">	</span>return se_header;<br> }<br>+<br>+int arch_support_sort_key(const char *sort_key)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>if (!strcmp(sort_key, "p_stage_cyc"))<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>return 1;<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>return 0;<br>+}<br>diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h<br>index 65f89e80916f..612a92aaaefb 100644<br>--- a/tools/perf/util/event.h<br>+++ b/tools/perf/util/event.h<br>@@ -429,5 +429,6 @@ char *get_page_size_name(u64 size, char *str);<br> void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 *array, u64 type);<br> void arch_perf_synthesize_sample_weight(const struct perf_sample *data, __u64 *array, u64 type);<br> const char *arch_perf_header_entry__add(const char *se_header);<br>+int arch_support_sort_key(const char *sort_key);<br><br> #endif /* __PERF_RECORD_H */<br>diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c<br>index cbb3899e7eca..d8b0b0b43a81 100644<br>--- a/tools/perf/util/sort.c<br>+++ b/tools/perf/util/sort.c<br>@@ -47,6 +47,7 @@ regex_t<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>ignore_callees_regex;<br> int<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>have_ignore_callees = 0;<br> enum sort_mode<span class="Apple-tab-span" style="white-space:pre">	</span>sort__mode = SORT_MODE__NORMAL;<br> const char<span class="Apple-tab-span" style="white-space:pre">	</span>*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};<br>+const char<span class="Apple-tab-span" style="white-space:pre">	</span>*arch_specific_sort_keys[] = {"p_stage_cyc"};<br><br> /*<br> &nbsp;* Replaces all occurrences of a char used with the:<br>@@ -1837,6 +1838,11 @@ struct sort_dimension {<br> <span class="Apple-tab-span" style="white-space:pre">	</span>int<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>taken;<br> };<br><br>+int __weak arch_support_sort_key(const char *sort_key __maybe_unused)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>return 0;<br>+}<br>+<br> const char * __weak arch_perf_header_entry__add(const char *se_header)<br> {<br> <span class="Apple-tab-span" style="white-space:pre">	</span>return se_header;<br>@@ -2773,6 +2779,18 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,<br> {<br> <span class="Apple-tab-span" style="white-space:pre">	</span>unsigned int i, j;<br><br>+<span class="Apple-tab-span" style="white-space:pre">	</span>/* Check to see if there are any arch specific<br>+<span class="Apple-tab-span" style="white-space:pre">	</span> * sort dimensions not applicable for the current<br>+<span class="Apple-tab-span" style="white-space:pre">	</span> * architecture. If so, Skip that sort key since<br>+<span class="Apple-tab-span" style="white-space:pre">	</span> * we don't want to display it in the output fields.<br>+<span class="Apple-tab-span" style="white-space:pre">	</span> */<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>for (j = 0; j &lt; ARRAY_SIZE(arch_specific_sort_keys); j++) {<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>if (!strcmp(arch_specific_sort_keys[j], tok) &amp;&amp;<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>!arch_support_sort_key(tok)) {<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>return 0;<br>+<span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>}<br>+<span class="Apple-tab-span" style="white-space:pre">	</span>}<br>+<br> <span class="Apple-tab-span" style="white-space:pre">	</span>for (i = 0; i &lt; ARRAY_SIZE(common_sort_dimensions); i++) {<br> <span class="Apple-tab-span" style="white-space:pre">	</span><span class="Apple-tab-span" style="white-space:pre">	</span>struct sort_dimension *sd = &amp;common_sort_dimensions[i];<br><br>— <br>2.26.2<br><br>Thanks<br>Athira<br><br><blockquote type="cite"><br>jirka<br><br></blockquote><br></div></body></html>
> 

apart from the html content, looks ok ;-)

jirka


^ permalink raw reply

* Re: [PATCH v9 2/8] powerpc/lib/code-patching: Set up Strict RWX patching earlier
From: Michael Ellerman @ 2021-03-17 12:04 UTC (permalink / raw)
  To: Jordan Niethe, Christophe Leroy
  Cc: Christophe Leroy, ajd, Nicholas Piggin, naveen.n.rao,
	linuxppc-dev, Daniel Axtens
In-Reply-To: <CACzsE9rbgjNtLOjCHL9+LN6_Xoo6mJ_D5pewuMg7ktqA_OnR0w@mail.gmail.com>

Jordan Niethe <jniethe5@gmail.com> writes:
> On Tue, Mar 16, 2021 at 5:32 PM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> Le 16/03/2021 à 04:17, Jordan Niethe a écrit :
>> > setup_text_poke_area() is a late init call so it runs before
>> > mark_rodata_ro() and after the init calls. This lets all the init code
>> > patching simply write to their locations. In the future, kprobes is
>> > going to allocate its instruction pages RO which means they will need
>> > setup_text__poke_area() to have been already called for their code
>> > patching. However, init_kprobes() (which allocates and patches some
>> > instruction pages) is an early init call so it happens before
>> > setup_text__poke_area().
>> >
>> > start_kernel() calls poking_init() before any of the init calls. On
>> > powerpc, poking_init() is currently a nop. setup_text_poke_area() relies
>> > on kernel virtual memory, cpu hotplug and per_cpu_areas being setup.
>> > setup_per_cpu_areas(), boot_cpu_hotplug_init() and mm_init() are called
>> > before poking_init().
>> >
>> > Turn setup_text_poke_area() into poking_init().
>> >
>> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>> > ---
>> > v9: New to series
>> > ---
>> >   arch/powerpc/lib/code-patching.c | 12 ++++--------
>> >   1 file changed, 4 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>> > index 2333625b5e31..b28afa1133db 100644
>> > --- a/arch/powerpc/lib/code-patching.c
>> > +++ b/arch/powerpc/lib/code-patching.c
>> > @@ -65,14 +65,11 @@ static int text_area_cpu_down(unsigned int cpu)
>> >   }
>> >
>> >   /*
>> > - * Run as a late init call. This allows all the boot time patching to be done
>> > - * simply by patching the code, and then we're called here prior to
>> > - * mark_rodata_ro(), which happens after all init calls are run. Although
>> > - * BUG_ON() is rude, in this case it should only happen if ENOMEM, and we judge
>> > - * it as being preferable to a kernel that will crash later when someone tries
>> > - * to use patch_instruction().
>> > + * Although BUG_ON() is rude, in this case it should only happen if ENOMEM, and
>> > + * we judge it as being preferable to a kernel that will crash later when
>> > + * someone tries to use patch_instruction().
>>
>> Please use WARN_ON(), see why at https://www.kernel.org/doc/html/latest/process/deprecated.html

> Ok I can include a change to WARN_ON() as a separate patch.

I'm not convinced we should change this to a WARN_ON.

Being able to patch the kernel text is not optional.

Patching jump labels has no ability to return an error, and the code
that uses them has no concept of the jump label not taking the correct
polarity.

Silently failing the patch is like randomly flipping an if condition
somewhere in the kernel and hoping that everything will continue
working.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: arch/powerpc/kernel/setup_64.c - cleanup warnings
From: Michael Ellerman @ 2021-03-17 11:57 UTC (permalink / raw)
  To: Daniel Axtens, heying (H), benh, paulus, npiggin, akpm,
	aneesh.kumar, rppt, ardb, clg, christophe.leroy
  Cc: johnny.chenyi, linuxppc-dev, linux-kernel
In-Reply-To: <87tupab4a1.fsf@dja-thinkpad.axtens.net>

Daniel Axtens <dja@axtens.net> writes:
> "heying (H)" <heying24@huawei.com> writes:
>
>> Thank you for your reply.
>>
>> 在 2021/3/17 11:04, Daniel Axtens 写道:
>>> Hi He Ying,
>>>
>>> Thank you for this patch.
>>>
>>> I'm not sure what the precise rules for Fixes are, but I wonder if this
>>> should have:
>>>
>>> Fixes: 9a32a7e78bd0 ("powerpc/64s: flush L1D after user accesses")
>>> Fixes: f79643787e0a ("powerpc/64s: flush L1D on kernel entry")
>>
>> Is that necessary for warning cleanups? I thought 'Fixes' tags are 
>> needed only for
>>
>> bugfix patches. Can someone tell me whether I am right?
>
> Yeah, I'm not sure either. Hopefully mpe will let us know.

It's not necessary to add a Fixes tag for a patch like this, but you can
add one if you think it's important that the fix gets backported.

I don't think the cleanups in this case are that important, so I
wouldn't bother with a Fixes tag.

cheers

^ permalink raw reply

* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: David Hildenbrand @ 2021-03-17 11:31 UTC (permalink / raw)
  To: Kefeng Wang, linux-kernel, Andrew Morton
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, linux-mm, Guo Ren, sparclinux, linux-riscv,
	Jonas Bonn, linux-s390, Yoshinori Sato, linux-hexagon,
	Huacai Chen, Russell King, linux-csky, Ingo Molnar,
	linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
	linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
	linux-parisc, linux-mips, Palmer Dabbelt, linux-alpha,
	linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>

On 17.03.21 02:52, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> v2:
> - Cleanup 'str' line suggested by Christophe and ACK
> 
>   arch/alpha/mm/init.c             |  1 -
>   arch/arc/mm/init.c               |  1 -
>   arch/arm/mm/init.c               |  2 --
>   arch/arm64/mm/init.c             |  2 --
>   arch/csky/mm/init.c              |  1 -
>   arch/h8300/mm/init.c             |  2 --
>   arch/hexagon/mm/init.c           |  1 -
>   arch/ia64/mm/init.c              |  1 -
>   arch/m68k/mm/init.c              |  1 -
>   arch/microblaze/mm/init.c        |  1 -
>   arch/mips/loongson64/numa.c      |  1 -
>   arch/mips/mm/init.c              |  1 -
>   arch/mips/sgi-ip27/ip27-memory.c |  1 -
>   arch/nds32/mm/init.c             |  1 -
>   arch/nios2/mm/init.c             |  1 -
>   arch/openrisc/mm/init.c          |  2 --
>   arch/parisc/mm/init.c            |  2 --
>   arch/powerpc/mm/mem.c            |  1 -
>   arch/riscv/mm/init.c             |  1 -
>   arch/s390/mm/init.c              |  2 --
>   arch/sh/mm/init.c                |  1 -
>   arch/sparc/mm/init_32.c          |  2 --
>   arch/sparc/mm/init_64.c          |  1 -
>   arch/um/kernel/mem.c             |  1 -
>   arch/x86/mm/init_32.c            |  2 --
>   arch/x86/mm/init_64.c            |  2 --
>   arch/xtensa/mm/init.c            |  1 -
>   include/linux/mm.h               |  2 +-
>   init/main.c                      |  1 +
>   mm/page_alloc.c                  | 10 +++++-----
>   30 files changed, 7 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/alpha/mm/init.c b/arch/alpha/mm/init.c
> index 3c42b3147fd6..a97650a618f1 100644
> --- a/arch/alpha/mm/init.c
> +++ b/arch/alpha/mm/init.c
> @@ -282,5 +282,4 @@ mem_init(void)
>   	set_max_mapnr(max_low_pfn);
>   	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
>   	memblock_free_all();
> -	mem_init_print_info(NULL);
>   }
> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index ce07e697916c..33832e36bdb7 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -194,7 +194,6 @@ void __init mem_init(void)
>   {
>   	memblock_free_all();
>   	highmem_init();
> -	mem_init_print_info(NULL);
>   }
>   
>   #ifdef CONFIG_HIGHMEM
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 828a2561b229..7022b7b5c400 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -316,8 +316,6 @@ void __init mem_init(void)
>   
>   	free_highpages();
>   
> -	mem_init_print_info(NULL);
> -
>   	/*
>   	 * Check boundaries twice: Some fundamental inconsistencies can
>   	 * be detected at build time already.
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 3685e12aba9b..e8f29a0bb2f1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -491,8 +491,6 @@ void __init mem_init(void)
>   	/* this will put all unused low memory onto the freelists */
>   	memblock_free_all();
>   
> -	mem_init_print_info(NULL);
> -
>   	/*
>   	 * Check boundaries twice: Some fundamental inconsistencies can be
>   	 * detected at build time already.
> diff --git a/arch/csky/mm/init.c b/arch/csky/mm/init.c
> index 894050a8ce09..bf2004aa811a 100644
> --- a/arch/csky/mm/init.c
> +++ b/arch/csky/mm/init.c
> @@ -107,7 +107,6 @@ void __init mem_init(void)
>   			free_highmem_page(page);
>   	}
>   #endif
> -	mem_init_print_info(NULL);
>   }
>   
>   void free_initmem(void)
> diff --git a/arch/h8300/mm/init.c b/arch/h8300/mm/init.c
> index 1f3b345d68b9..f7bf4693e3b2 100644
> --- a/arch/h8300/mm/init.c
> +++ b/arch/h8300/mm/init.c
> @@ -98,6 +98,4 @@ void __init mem_init(void)
>   
>   	/* this will put all low memory onto the freelists */
>   	memblock_free_all();
> -
> -	mem_init_print_info(NULL);
>   }
> diff --git a/arch/hexagon/mm/init.c b/arch/hexagon/mm/init.c
> index f2e6c868e477..f01e91e10d95 100644
> --- a/arch/hexagon/mm/init.c
> +++ b/arch/hexagon/mm/init.c
> @@ -55,7 +55,6 @@ void __init mem_init(void)
>   {
>   	/*  No idea where this is actually declared.  Seems to evade LXR.  */
>   	memblock_free_all();
> -	mem_init_print_info(NULL);
>   
>   	/*
>   	 *  To-Do:  someone somewhere should wipe out the bootmem map
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 16d0d7d22657..83280e2df807 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -659,7 +659,6 @@ mem_init (void)
>   	set_max_mapnr(max_low_pfn);
>   	high_memory = __va(max_low_pfn * PAGE_SIZE);
>   	memblock_free_all();
> -	mem_init_print_info(NULL);
>   
>   	/*
>   	 * For fsyscall entrpoints with no light-weight handler, use the ordinary
> diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
> index 14c1e541451c..1759ab875d47 100644
> --- a/arch/m68k/mm/init.c
> +++ b/arch/m68k/mm/init.c
> @@ -153,5 +153,4 @@ void __init mem_init(void)
>   	/* this will put all memory onto the freelists */
>   	memblock_free_all();
>   	init_pointer_tables();
> -	mem_init_print_info(NULL);
>   }
> diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c
> index 05cf1fb3f5ff..ab55c70380a5 100644
> --- a/arch/microblaze/mm/init.c
> +++ b/arch/microblaze/mm/init.c
> @@ -131,7 +131,6 @@ void __init mem_init(void)
>   	highmem_setup();
>   #endif
>   
> -	mem_init_print_info(NULL);
>   	mem_init_done = 1;
>   }
>   
> diff --git a/arch/mips/loongson64/numa.c b/arch/mips/loongson64/numa.c
> index 8315c871c435..fa9b4a487a47 100644
> --- a/arch/mips/loongson64/numa.c
> +++ b/arch/mips/loongson64/numa.c
> @@ -178,7 +178,6 @@ void __init mem_init(void)
>   	high_memory = (void *) __va(get_num_physpages() << PAGE_SHIFT);
>   	memblock_free_all();
>   	setup_zero_pages();	/* This comes from node 0 */
> -	mem_init_print_info(NULL);
>   }
>   
>   /* All PCI device belongs to logical Node-0 */
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 5cb73bf74a8b..c36358758969 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -467,7 +467,6 @@ void __init mem_init(void)
>   	memblock_free_all();
>   	setup_zero_pages();	/* Setup zeroed pages.  */
>   	mem_init_free_highmem();
> -	mem_init_print_info(NULL);
>   
>   #ifdef CONFIG_64BIT
>   	if ((unsigned long) &_text > (unsigned long) CKSEG0)
> diff --git a/arch/mips/sgi-ip27/ip27-memory.c b/arch/mips/sgi-ip27/ip27-memory.c
> index 87bb6945ec25..6173684b5aaa 100644
> --- a/arch/mips/sgi-ip27/ip27-memory.c
> +++ b/arch/mips/sgi-ip27/ip27-memory.c
> @@ -420,5 +420,4 @@ void __init mem_init(void)
>   	high_memory = (void *) __va(get_num_physpages() << PAGE_SHIFT);
>   	memblock_free_all();
>   	setup_zero_pages();	/* This comes from node 0 */
> -	mem_init_print_info(NULL);
>   }
> diff --git a/arch/nds32/mm/init.c b/arch/nds32/mm/init.c
> index fa86f7b2f416..f63f839738c4 100644
> --- a/arch/nds32/mm/init.c
> +++ b/arch/nds32/mm/init.c
> @@ -191,7 +191,6 @@ void __init mem_init(void)
>   
>   	/* this will put all low memory onto the freelists */
>   	memblock_free_all();
> -	mem_init_print_info(NULL);
>   
>   	pr_info("virtual kernel memory layout:\n"
>   		"    fixmap  : 0x%08lx - 0x%08lx   (%4ld kB)\n"
> diff --git a/arch/nios2/mm/init.c b/arch/nios2/mm/init.c
> index 61862dbb0e32..613fcaa5988a 100644
> --- a/arch/nios2/mm/init.c
> +++ b/arch/nios2/mm/init.c
> @@ -71,7 +71,6 @@ void __init mem_init(void)
>   
>   	/* this will put all memory onto the freelists */
>   	memblock_free_all();
> -	mem_init_print_info(NULL);
>   }
>   
>   void __init mmu_init(void)
> diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> index bf9b2310fc93..d5641198b90c 100644
> --- a/arch/openrisc/mm/init.c
> +++ b/arch/openrisc/mm/init.c
> @@ -211,8 +211,6 @@ void __init mem_init(void)
>   	/* this will put all low memory onto the freelists */
>   	memblock_free_all();
>   
> -	mem_init_print_info(NULL);
> -
>   	printk("mem_init_done ...........................................\n");
>   	mem_init_done = 1;
>   	return;
> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
> index 9ca4e4ff6895..591a4e939415 100644
> --- a/arch/parisc/mm/init.c
> +++ b/arch/parisc/mm/init.c
> @@ -573,8 +573,6 @@ void __init mem_init(void)
>   #endif
>   		parisc_vmalloc_start = SET_MAP_OFFSET(MAP_START);
>   
> -	mem_init_print_info(NULL);
> -
>   #if 0
>   	/*
>   	 * Do not expose the virtual kernel memory layout to userspace.
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 4e8ce6d85232..7e11c4cb08b8 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -312,7 +312,6 @@ void __init mem_init(void)
>   		(mfspr(SPRN_TLB1CFG) & TLBnCFG_N_ENTRY) - 1;
>   #endif
>   
> -	mem_init_print_info(NULL);
>   #ifdef CONFIG_PPC32
>   	pr_info("Kernel virtual memory layout:\n");
>   #ifdef CONFIG_KASAN
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 7f5036fbee8c..3c5ee3b7d811 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -102,7 +102,6 @@ void __init mem_init(void)
>   	high_memory = (void *)(__va(PFN_PHYS(max_low_pfn)));
>   	memblock_free_all();
>   
> -	mem_init_print_info(NULL);
>   	print_vm_layout();
>   }
>   
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 0e76b2127dc6..8ac710de1ab1 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -209,8 +209,6 @@ void __init mem_init(void)
>   	setup_zero_pages();	/* Setup zeroed pages. */
>   
>   	cmma_init_nodat();
> -
> -	mem_init_print_info(NULL);
>   }
>   
>   void free_initmem(void)
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 0db6919af8d3..168d7d4dd735 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -359,7 +359,6 @@ void __init mem_init(void)
>   
>   	vsyscall_init();
>   
> -	mem_init_print_info(NULL);
>   	pr_info("virtual kernel memory layout:\n"
>   		"    fixmap  : 0x%08lx - 0x%08lx   (%4ld kB)\n"
>   		"    vmalloc : 0x%08lx - 0x%08lx   (%4ld MB)\n"
> diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c
> index 6139c5700ccc..1e9f577f084d 100644
> --- a/arch/sparc/mm/init_32.c
> +++ b/arch/sparc/mm/init_32.c
> @@ -292,8 +292,6 @@ void __init mem_init(void)
>   
>   		map_high_region(start_pfn, end_pfn);
>   	}
> -
> -	mem_init_print_info(NULL);
>   }
>   
>   void sparc_flush_page_to_ram(struct page *page)
> diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
> index 182bb7bdaa0a..e454f179cf5d 100644
> --- a/arch/sparc/mm/init_64.c
> +++ b/arch/sparc/mm/init_64.c
> @@ -2520,7 +2520,6 @@ void __init mem_init(void)
>   	}
>   	mark_page_reserved(mem_map_zero);
>   
> -	mem_init_print_info(NULL);
>   
>   	if (tlb_type == cheetah || tlb_type == cheetah_plus)
>   		cheetah_ecache_flush_init();
> diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c
> index 9242dc91d751..9019ff5905b1 100644
> --- a/arch/um/kernel/mem.c
> +++ b/arch/um/kernel/mem.c
> @@ -54,7 +54,6 @@ void __init mem_init(void)
>   	memblock_free_all();
>   	max_low_pfn = totalram_pages();
>   	max_pfn = max_low_pfn;
> -	mem_init_print_info(NULL);
>   	kmalloc_ok = 1;
>   }
>   
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index da31c2635ee4..21ffb03f6c72 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -755,8 +755,6 @@ void __init mem_init(void)
>   	after_bootmem = 1;
>   	x86_init.hyper.init_after_bootmem();
>   
> -	mem_init_print_info(NULL);
> -
>   	/*
>   	 * Check boundaries twice: Some fundamental inconsistencies can
>   	 * be detected at build time already.
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 5430c81eefc9..aa8387aab9c1 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1350,8 +1350,6 @@ void __init mem_init(void)
>   		kclist_add(&kcore_vsyscall, (void *)VSYSCALL_ADDR, PAGE_SIZE, KCORE_USER);
>   
>   	preallocate_vmalloc_pages();
> -
> -	mem_init_print_info(NULL);
>   }
>   
>   #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> diff --git a/arch/xtensa/mm/init.c b/arch/xtensa/mm/init.c
> index 2daeba9e454e..6a32b2cf2718 100644
> --- a/arch/xtensa/mm/init.c
> +++ b/arch/xtensa/mm/init.c
> @@ -119,7 +119,6 @@ void __init mem_init(void)
>   
>   	memblock_free_all();
>   
> -	mem_init_print_info(NULL);
>   	pr_info("virtual kernel memory layout:\n"
>   #ifdef CONFIG_KASAN
>   		"    kasan   : 0x%08lx - 0x%08lx  (%5lu MB)\n"
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 89314651dd62..c2e0b3495c5a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2373,7 +2373,7 @@ extern unsigned long free_reserved_area(void *start, void *end,
>   					int poison, const char *s);
>   
>   extern void adjust_managed_page_count(struct page *page, long count);
> -extern void mem_init_print_info(const char *str);
> +extern void mem_init_print_info(void);
>   
>   extern void reserve_bootmem_region(phys_addr_t start, phys_addr_t end);
>   
> diff --git a/init/main.c b/init/main.c
> index 53b278845b88..5581af5b4cb7 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -830,6 +830,7 @@ static void __init mm_init(void)
>   	report_meminit();
>   	stack_depot_init();
>   	mem_init();
> +	mem_init_print_info();
>   	/* page_owner must be initialized after buddy is ready */
>   	page_ext_init_flatmem_late();
>   	kmem_cache_init();
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 55d938297ce6..b5fe5962837c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7728,7 +7728,7 @@ unsigned long free_reserved_area(void *start, void *end, int poison, const char
>   	return pages;
>   }
>   
> -void __init mem_init_print_info(const char *str)
> +void __init mem_init_print_info(void)
>   {
>   	unsigned long physpages, codesize, datasize, rosize, bss_size;
>   	unsigned long init_code_size, init_data_size;
> @@ -7767,17 +7767,17 @@ void __init mem_init_print_info(const char *str)
>   #ifdef	CONFIG_HIGHMEM
>   		", %luK highmem"
>   #endif
> -		"%s%s)\n",
> +		")\n",
>   		nr_free_pages() << (PAGE_SHIFT - 10),
>   		physpages << (PAGE_SHIFT - 10),
>   		codesize >> 10, datasize >> 10, rosize >> 10,
>   		(init_data_size + init_code_size) >> 10, bss_size >> 10,
>   		(physpages - totalram_pages() - totalcma_pages) << (PAGE_SHIFT - 10),
> -		totalcma_pages << (PAGE_SHIFT - 10),
> +		totalcma_pages << (PAGE_SHIFT - 10)
>   #ifdef	CONFIG_HIGHMEM
> -		totalhigh_pages() << (PAGE_SHIFT - 10),
> +		, totalhigh_pages() << (PAGE_SHIFT - 10)
>   #endif
> -		str ? ", " : "", str ? str : "");
> +		);
>   }
>   
>   /**
> 

As I had roughly the same patch laying around here when playing with 
adjustments of the managed page counters

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb


^ permalink raw reply

* Re: [PATCH -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-17 11:16 UTC (permalink / raw)
  To: He Ying, mpe, benh, paulus, npiggin, msuchanek, peterz,
	geert+renesas, kernelfans, frederic
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210317103438.177428-1-heying24@huawei.com>



Le 17/03/2021 à 11:34, He Ying a écrit :
> We found these warnings in arch/powerpc/kernel/time.c as follows:
> warning: symbol 'decrementer_max' was not declared. Should it be static?
> warning: symbol 'rtc_lock' was not declared. Should it be static?
> warning: symbol 'dtl_consumer' was not declared. Should it be static?
> 
> Declare 'decrementer_max' in arch/powerpc/include/asm/time.h. And include
> proper header in which 'rtc_lock' is declared. Move 'dtl_consumer'
> definition behind "include <asm/dtl.h>" because 'dtl_consumer' is declared
> there.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>
> ---
>   arch/powerpc/include/asm/time.h | 1 +
>   arch/powerpc/kernel/time.c      | 7 +++----
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>   extern unsigned long tb_ticks_per_usec;
>   extern unsigned long tb_ticks_per_sec;
>   extern struct clock_event_device decrementer_clockevent;
> +extern u64 decrementer_max;
>   
>   
>   extern void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..409967713ca6 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,6 +55,7 @@
>   #include <linux/sched/cputime.h>
>   #include <linux/sched/clock.h>
>   #include <linux/processor.h>
> +#include <linux/mc146818rtc.h>

I don't think that's the good place. It has no link to powerpc, it is only by chance that it has the 
same name.

As rtc_lock is defined in powerpc time.c, I think you should declare it in powerpc asm/time.h


>   #include <asm/trace.h>
>   
>   #include <asm/interrupt.h>
> @@ -150,10 +151,6 @@ bool tb_invalid;
>   u64 __cputime_usec_factor;
>   EXPORT_SYMBOL(__cputime_usec_factor);
>   
> -#ifdef CONFIG_PPC_SPLPAR
> -void (*dtl_consumer)(struct dtl_entry *, u64);
> -#endif
> -
>   static void calc_cputime_factors(void)
>   {
>   	struct div_result res;
> @@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>   
>   #include <asm/dtl.h>
>   
> +void (*dtl_consumer)(struct dtl_entry *, u64);
> +
>   /*
>    * Scan the dispatch trace log and count up the stolen time.
>    * Should be called with interrupts disabled.
> 

^ permalink raw reply

* [PATCH -next] powerpc: kernel/time.c - cleanup warnings
From: He Ying @ 2021-03-17 10:34 UTC (permalink / raw)
  To: mpe, benh, paulus, christophe.leroy, npiggin, msuchanek, heying24,
	peterz, geert+renesas, kernelfans, frederic
  Cc: linuxppc-dev, linux-kernel

We found these warnings in arch/powerpc/kernel/time.c as follows:
warning: symbol 'decrementer_max' was not declared. Should it be static?
warning: symbol 'rtc_lock' was not declared. Should it be static?
warning: symbol 'dtl_consumer' was not declared. Should it be static?

Declare 'decrementer_max' in arch/powerpc/include/asm/time.h. And include
proper header in which 'rtc_lock' is declared. Move 'dtl_consumer'
definition behind "include <asm/dtl.h>" because 'dtl_consumer' is declared
there.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: He Ying <heying24@huawei.com>
---
 arch/powerpc/include/asm/time.h | 1 +
 arch/powerpc/kernel/time.c      | 7 +++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 8dd3cdb25338..2cd2b50bedda 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
 extern unsigned long tb_ticks_per_usec;
 extern unsigned long tb_ticks_per_sec;
 extern struct clock_event_device decrementer_clockevent;
+extern u64 decrementer_max;
 
 
 extern void generic_calibrate_decr(void);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b67d93a609a2..409967713ca6 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -55,6 +55,7 @@
 #include <linux/sched/cputime.h>
 #include <linux/sched/clock.h>
 #include <linux/processor.h>
+#include <linux/mc146818rtc.h>
 #include <asm/trace.h>
 
 #include <asm/interrupt.h>
@@ -150,10 +151,6 @@ bool tb_invalid;
 u64 __cputime_usec_factor;
 EXPORT_SYMBOL(__cputime_usec_factor);
 
-#ifdef CONFIG_PPC_SPLPAR
-void (*dtl_consumer)(struct dtl_entry *, u64);
-#endif
-
 static void calc_cputime_factors(void)
 {
 	struct div_result res;
@@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
 
 #include <asm/dtl.h>
 
+void (*dtl_consumer)(struct dtl_entry *, u64);
+
 /*
  * Scan the dispatch trace log and count up the stolen time.
  * Should be called with interrupts disabled.
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 04/11] powerpc/64e/interrupt: use new interrupt return
From: Nicholas Piggin @ 2021-03-17 10:05 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <53a3a1e1-b2c5-116c-174b-dd4beefa6515@csgroup.eu>

Excerpts from Christophe Leroy's message of March 16, 2021 8:49 pm:
> 
> 
> Le 16/03/2021 à 11:41, Nicholas Piggin a écrit :
>> Update the new C and asm interrupt return code to account for 64e
>> specifics, switch over to use it.
>> 
>> The now-unused old ret_from_except code, that was moved to 64e after the
>> 64s conversion, is removed.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   arch/powerpc/include/asm/asm-prototypes.h |   2 -
>>   arch/powerpc/include/asm/ppc_asm.h        |  20 --
>>   arch/powerpc/kernel/asm-offsets.c         |  10 -
>>   arch/powerpc/kernel/exceptions-64e.S      | 321 ++--------------------
>>   arch/powerpc/kernel/irq.c                 |  76 -----
>>   5 files changed, 25 insertions(+), 404 deletions(-)
>> 
> 
>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>> index 85ba2b0bc8d8..c880ad18b851 100644
>> --- a/arch/powerpc/kernel/asm-offsets.c
>> +++ b/arch/powerpc/kernel/asm-offsets.c
>> @@ -282,21 +282,11 @@ int main(void)
>>   	OFFSET(PACAHWCPUID, paca_struct, hw_cpu_id);
>>   	OFFSET(PACAKEXECSTATE, paca_struct, kexec_state);
>>   	OFFSET(PACA_DSCR_DEFAULT, paca_struct, dscr_default);
>> -	OFFSET(ACCOUNT_STARTTIME, paca_struct, accounting.starttime);
>> -	OFFSET(ACCOUNT_STARTTIME_USER, paca_struct, accounting.starttime_user);
>> -	OFFSET(ACCOUNT_USER_TIME, paca_struct, accounting.utime);
>> -	OFFSET(ACCOUNT_SYSTEM_TIME, paca_struct, accounting.stime);
>>   #ifdef CONFIG_PPC_BOOK3E
>>   	OFFSET(PACA_TRAP_SAVE, paca_struct, trap_save);
>>   #endif
>>   	OFFSET(PACA_SPRG_VDSO, paca_struct, sprg_vdso);
>>   #else /* CONFIG_PPC64 */
> 
> The #else is useless

Thanks. I'll hold off re posting until there is some more activity.

Thanks,
Nick

^ permalink raw reply

* [PATCH 00/36] [Set 4] Rid W=1 warnings in SCSI
From: Lee Jones @ 2021-03-17  9:11 UTC (permalink / raw)
  To: lee.jones
  Cc: Uma Krishnan, Brian Macy, Hannes Reinecke, Anil Ravindranath,
	Tyrel Datwyler, willy, Le Moal, Dave Boutcher, Marvell,
	Jirka Hanika, Linda Xie, C.L. Huang, target-devel, Drew Eckhardt,
	Brian King, Christoph Hellwig, Alan Cox, linux-drivers,
	Nicholas A. Bellinger, Linux GmbH, linux-scsi, Shaun Tancheff,
	Subbu Seetharaman, Sathya Prakash, Doug Ledford,
	Leonard N. Zubkoff, Ketan Mukadam, Dave Boutcher, Colin DeVilbiss,
	Karan Tilak Kumar, Badari Pulavarty, Bryant G. Ly,
	Douglas Gilbert, Jamie Lenehan, MPT-FusionLinux.pdl,
	Richard Gooch, MPT-FusionLinux.pdl, Bas Vermeulen,
	Artur Paszkiewicz, Michael Cyr, dc395x, Satish Kharat,
	Suganath Prabu Subramani, James E.J. Bottomley, Luben Tuikov,
	Ali Akcaagac, Kurt Garloff, Jitendra Bhivare, Brian King,
	Hannes Reinecke, Erich Chen, David Chaw, Santiago Leon,
	Matthew R. Ochs, Manoj N. Kumar, Sreekanth Reddy,
	Martin K. Petersen, Eric Youngdale, Oliver Neukum, linux-kernel,
	Sesidhar Baddela, Alex Davis, Torben Mathiasen, Paul Mackerras,
	FUJITA Tomonori, linuxppc-dev

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

Lee Jones (36):
  scsi: myrb: Demote non-conformant kernel-doc headers and fix others
  scsi: ipr: Fix incorrect function names in their headers
  scsi: mvumi: Fix formatting and doc-rot issues
  scsi: sd_zbc: Place function name into header
  scsi: pmcraid: Fix a whole host of kernel-doc issues
  scsi: sd: Fix function name in header
  scsi: aic94xx: aic94xx_dump: Correct misspelling of function
    asd_dump_seq_state()
  scsi: be2iscsi: be_main: Ensure function follows directly after its
    header
  scsi: dc395x: Fix some function param descriptions
  scsi: initio: Fix a few kernel-doc misdemeanours
  scsi: a100u2w: Fix some misnaming and formatting issues
  scsi: myrs: Add missing ':' to make the kernel-doc checker happy
  scsi: pmcraid: Correct function name pmcraid_show_adapter_id() in
    header
  scsi: mpt3sas: mpt3sas_scs: Fix a few kernel-doc issues
  scsi: be2iscsi: be_main: Demote incomplete/non-conformant kernel-doc
    header
  scsi: isci: phy: Fix a few different kernel-doc related issues
  scsi: fnic: fnic_scsi: Demote non-conformant kernel-doc headers
  scsi: fnic: fnic_fcs: Kernel-doc headers must contain the function
    name
  scsi: isci: phy: Provide function name and demote non-conforming
    header
  scsi: isci: request: Fix a myriad of kernel-doc issues
  scsi: isci: host: Fix bunch of kernel-doc related issues
  scsi: isci: task: Demote non-conformant header and remove superfluous
    param
  scsi: isci: remote_node_table: Fix a bunch of kernel-doc misdemeanours
  scsi: isci: remote_node_context: Fix one function header and demote a
    couple more
  scsi: isci: port_config: Fix a bunch of doc-rot and demote abuses
  scsi: isci: remote_device: Fix a bunch of doc-rot issues
  scsi: isci: request: Fix doc-rot issue relating to 'ireq' param
  scsi: isci: port: Fix a bunch of kernel-doc issues
  scsi: isci: remote_node_context: Demote kernel-doc abuse
  scsi: isci: remote_node_table: Provide some missing params and remove
    others
  scsi: cxlflash: main: Fix a little do-rot
  scsi: cxlflash: superpipe: Fix a few misnaming issues
  scsi: ibmvscsi: Fix a bunch of kernel-doc related issues
  scsi: ibmvscsi: ibmvfc: Fix a bunch of misdocumentation
  scsi: ibmvscsi_tgt: ibmvscsi_tgt: Remove duplicate section 'NOTE'
  scsi: cxlflash: vlun: Fix some misnaming related doc-rot

 drivers/scsi/a100u2w.c                   |  8 +--
 drivers/scsi/aic94xx/aic94xx_dump.c      |  2 +-
 drivers/scsi/be2iscsi/be_main.c          |  5 +-
 drivers/scsi/cxlflash/main.c             |  8 +--
 drivers/scsi/cxlflash/superpipe.c        |  6 +-
 drivers/scsi/cxlflash/vlun.c             |  8 +--
 drivers/scsi/dc395x.c                    |  3 +-
 drivers/scsi/fnic/fnic_fcs.c             |  2 +-
 drivers/scsi/fnic/fnic_scsi.c            |  6 +-
 drivers/scsi/ibmvscsi/ibmvfc.c           | 29 ++++++----
 drivers/scsi/ibmvscsi/ibmvscsi.c         | 70 ++++++++++++------------
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c |  8 +--
 drivers/scsi/initio.c                    | 13 ++---
 drivers/scsi/ipr.c                       |  8 +--
 drivers/scsi/isci/host.c                 | 37 ++++++-------
 drivers/scsi/isci/phy.c                  | 34 ++++++------
 drivers/scsi/isci/port.c                 | 58 ++++++++++----------
 drivers/scsi/isci/port_config.c          | 37 +++++++------
 drivers/scsi/isci/remote_device.c        | 31 ++++++-----
 drivers/scsi/isci/remote_node_context.c  | 13 +----
 drivers/scsi/isci/remote_node_table.c    | 64 +++++++++++-----------
 drivers/scsi/isci/request.c              | 60 ++++++++++----------
 drivers/scsi/isci/task.c                 |  3 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c     | 18 +++---
 drivers/scsi/mvumi.c                     |  5 +-
 drivers/scsi/myrb.c                      | 47 ++++++++--------
 drivers/scsi/myrs.c                      |  6 +-
 drivers/scsi/pmcraid.c                   | 70 ++++++++++++------------
 drivers/scsi/sd.c                        |  2 +-
 drivers/scsi/sd_zbc.c                    |  2 +-
 30 files changed, 329 insertions(+), 334 deletions(-)

Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Alex Davis <letmein@erols.com>
Cc: Ali Akcaagac <aliakc@web.de>
Cc: Anil Ravindranath <anil_ravindranath@pmc-sierra.com>
Cc: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
Cc: Badari Pulavarty <pbadari@us.ibm.com>
Cc: Bas Vermeulen <bvermeul@blackstar.xs4all.nl>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Brian King <brking@linux.vnet.ibm.com>
Cc: Brian King <brking@us.ibm.com>
Cc: Brian Macy <bmacy@sunshinecomputing.com>
Cc: "Bryant G. Ly" <bryantly@linux.vnet.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "C.L. Huang" <ching@tekram.com.tw>
Cc: Colin DeVilbiss <devilbis@us.ibm.com>
Cc: Dave Boutcher <boutcher@us.ibm.com>
Cc: Dave Boutcher <sleddog@us.ibm.com>
Cc: David Chaw <david_chaw@adaptec.com>
Cc: dc395x@twibble.org
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Drew Eckhardt <drew@colorado.edu>
Cc: Erich Chen <erich@tekram.com.tw>
Cc: Eric Youngdale <eric@andante.org>
Cc: FUJITA Tomonori <tomof@acm.org>
Cc: Hannes Reinecke <hare@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: Jamie Lenehan <lenehan@twibble.org>
Cc: Jirka Hanika <geo@ff.cuni.cz>
Cc: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
Cc: Karan Tilak Kumar <kartilak@cisco.com>
Cc: Ketan Mukadam <ketan.mukadam@broadcom.com>
Cc: Kurt Garloff <garloff@suse.de>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Le Moal <damien.lemoal@hgst.com>
Cc: "Leonard N. Zubkoff" <lnz@dandelion.com>
Cc: Linda Xie <lxie@us.ibm.com>
Cc: linux-drivers@broadcom.com
Cc: Linux GmbH <hare@suse.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-scsi@vger.kernel.org
Cc: Luben Tuikov <luben_tuikov@adaptec.com>
Cc: "Manoj N. Kumar" <manoj@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Marvell <jyli@marvell.com>
Cc: "Matthew R. Ochs" <mrochs@linux.ibm.com>
Cc: Michael Cyr <mikecyr@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: MPT-FusionLinux.pdl@avagotech.com
Cc: MPT-FusionLinux.pdl@broadcom.com
Cc: "Nicholas A. Bellinger" <nab@kernel.org>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Richard Gooch <rgooch@atnf.csiro.au>
Cc: Santiago Leon <santil@us.ibm.com>
Cc: Sathya Prakash <sathya.prakash@broadcom.com>
Cc: Satish Kharat <satishkh@cisco.com>
Cc: Sesidhar Baddela <sebaddel@cisco.com>
Cc: Shaun Tancheff <shaun.tancheff@seagate.com>
Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
Cc: Subbu Seetharaman <subbu.seetharaman@broadcom.com>
Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>
Cc: target-devel@vger.kernel.org
Cc: Torben Mathiasen <tmm@image.dk>
Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
Cc: Uma Krishnan <ukrishn@linux.ibm.com>
Cc: willy@debian.org
-- 
2.27.0


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox