LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 8/9] powerpc/watchpoint: Return available watchpoints dynamically
From: Ravi Bangoria @ 2020-07-08  4:50 UTC (permalink / raw)
  To: mpe, mikey
  Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
	miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
	naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>

So far Book3S Powerpc supported only one watchpoint. Power10 is
introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 arch/powerpc/include/asm/cputable.h      | 4 +++-
 arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 3445c86e1f6f..36a0851a7a9b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -633,7 +633,9 @@ enum {
  * Maximum number of hw breakpoint supported on powerpc. Number of
  * breakpoints supported by actual hw might be less than this.
  */
-#define HBP_NUM_MAX	1
+#define HBP_NUM_MAX	2
+#define HBP_NUM_ONE	1
+#define HBP_NUM_TWO	2
 
 #endif /* !__ASSEMBLY__ */
 
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index cb424799da0d..d4eab1694bcd 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -5,10 +5,11 @@
  * Copyright 2010, IBM Corporation.
  * Author: K.Prasad <prasad@linux.vnet.ibm.com>
  */
-
 #ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
 #define _PPC_BOOK3S_64_HW_BREAKPOINT_H
 
+#include <asm/cpu_has_feature.h>
+
 #ifdef	__KERNEL__
 struct arch_hw_breakpoint {
 	unsigned long	address;
@@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
 
 static inline int nr_wp_slots(void)
 {
-	return HBP_NUM_MAX;
+	return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
 }
 
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
-- 
2.26.2


^ permalink raw reply related

* [PATCH v3 9/9] powerpc/watchpoint: Remove 512 byte boundary
From: Ravi Bangoria @ 2020-07-08  4:50 UTC (permalink / raw)
  To: mpe, mikey
  Cc: christophe.leroy, ravi.bangoria, apopple, peterz, fweisbec,
	miltonm, oleg, npiggin, linux-kernel, paulus, jolsa, pedromfc,
	naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-1-ravi.bangoria@linux.ibm.com>

Power10 has removed 512 bytes boundary from match criteria. i.e. The watch
range can cross 512 bytes boundary.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 7a66c370a105..270fbb4d01ce 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -418,8 +418,9 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
 
 	if (dawr_enabled()) {
 		max_len = DAWR_MAX_LEN;
-		/* DAWR region can't cross 512 bytes boundary */
-		if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512))
+		/* DAWR region can't cross 512 bytes boundary on p10 predecessors */
+		if (!cpu_has_feature(CPU_FTR_ARCH_31) &&
+		    (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512)))
 			return -EINVAL;
 	} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
 		/* 8xx can setup a range without limitation */
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Nicholas Piggin @ 2020-07-08  5:10 UTC (permalink / raw)
  To: linuxppc-dev, Waiman Long
  Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
	virtualization, Ingo Molnar, Will Deacon
In-Reply-To: <de3ead58-7f81-8ebd-754d-244f6be24af4@redhat.com>

Excerpts from Waiman Long's message of July 8, 2020 1:33 pm:
> On 7/7/20 1:57 AM, Nicholas Piggin wrote:
>> Yes, powerpc could certainly get more performance out of the slow
>> paths, and then there are a few parameters to tune.
>>
>> We don't have a good alternate patching for function calls yet, but
>> that would be something to do for native vs pv.
>>
>> And then there seem to be one or two tunable parameters we could
>> experiment with.
>>
>> The paravirt locks may need a bit more tuning. Some simple testing
>> under KVM shows we might be a bit slower in some cases. Whether this
>> is fairness or something else I'm not sure. The current simple pv
>> spinlock code can do a directed yield to the lock holder CPU, whereas
>> the pv qspl here just does a general yield. I think we might actually
>> be able to change that to also support directed yield. Though I'm
>> not sure if this is actually the cause of the slowdown yet.
> 
> Regarding the paravirt lock, I have taken a further look into the 
> current PPC spinlock code. There is an equivalent of pv_wait() but no 
> pv_kick(). Maybe PPC doesn't really need that.

So powerpc has two types of wait, either undirected "all processors" or 
directed to a specific processor which has been preempted by the 
hypervisor.

The simple spinlock code does a directed wait, because it knows the CPU 
which is holding the lock. In this case, there is a sequence that is 
used to ensure we don't wait if the condition has become true, and the
target CPU does not need to kick the waiter it will happen automatically
(see splpar_spin_yield). This is preferable because we only wait as 
needed and don't require the kick operation.

The pv spinlock code I did uses the undirected wait, because we don't
know the CPU number which we are waiting on. This is undesirable because 
it's higher overhead and the wait is not so accurate.

I think perhaps we could change things so we wait on the correct CPU 
when queued, which might be good enough (we could also put the lock
owner CPU in the spinlock word, if we add another format).

> Attached are two 
> additional qspinlock patches that adds a CONFIG_PARAVIRT_QSPINLOCKS_LITE 
> option to not require pv_kick(). There is also a fixup patch to be 
> applied after your patchset.
> 
> I don't have access to a PPC LPAR with shared processor at the moment, 
> so I can't test the performance of the paravirt code. Would you mind 
> adding my patches and do some performance test on your end to see if it 
> gives better result?

Great, I'll do some tests. Any suggestions for what to try?

Thanks,
Nick

^ permalink raw reply

* Re: Using Firefox hangs system
From: Nicholas Piggin @ 2020-07-08  5:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
	Paul Menzel
  Cc: linuxppc-dev
In-Reply-To: <604fa0d7-ddb5-7302-fd28-b9fee57ce015@molgen.mpg.de>

Excerpts from Paul Menzel's message of July 8, 2020 3:42 am:
> Dear Nicholas,
> 
> 
> Am 07.07.20 um 09:03 schrieb Nicholas Piggin:
>> Excerpts from Paul Menzel's message of July 6, 2020 3:20 pm:
> 
>>> Am 06.07.20 um 02:41 schrieb Nicholas Piggin:
>>>> Excerpts from Paul Menzel's message of July 5, 2020 8:30 pm:
>>>
>>>>> Am 05.07.20 um 11:22 schrieb Paul Menzel:
>>>>>> [  572.253008] Oops: Exception in kernel mode, sig: 5 [#1]
>>>>>> [  572.253198] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
>>>>>> [  572.253232] Modules linked in: tcp_diag inet_diag unix_diag xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables nfnetlink ip6table_filter ip6_tables iptable_filter bridge stp llc overlay xfs kvm_hv kvm binfmt_misc joydev uas usb_storage vmx_crypto bnx2x crct10dif_vpmsum ofpart cmdlinepart powernv_flash mtd mdio ibmpowernv at24 ipmi_powernv ipmi_devintf ipmi_msghandler opal_prd powernv_rng sch_fq_codel parport_pc ppdev lp nfsd parport auth_rpcgss nfs_acl lockd grace sunrpc ip_tables x_tables autofs4 btrfs blake2b_generic libcrc32c xor zstd_compress raid6_pq input_leds mac_hid hid_generic ast drm_vram_helper drm_ttm_helper i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm ahci drm_panel_orientation_quirks libahci usbhid hid crc32c_vpmsum uio_pdrv_genirq uio
>>>>>> [  572.253639] CPU: 4 PID: 6728 Comm: Web Content Not tainted 5.8.0-rc3+ #1
>>>>>> [  572.253659] NIP:  c00000000000ff5c LR: c00000000001a8f8 CTR: c0000000001d5f00
>>>>>> [  572.253835] REGS: c000007f31f0f420 TRAP: 1500   Not tainted  (5.8.0-rc3+)
>>>>>> [  572.253854] MSR:  900000000290b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 28c48482  XER: 20000000
>>>>>> [  572.253888] CFAR: c00000000000fecc IRQMASK: 1
>>>>>> [  572.253888] GPR00: c00000000001b228 c000007f31f0f6b0 c000000001f9a900 c000007f351544d0
>>>>>> [  572.253888] GPR04: 0000000000000000 c000007f31f0fe90 c000007f351544f0 c000007f32e522b0
>>>>>> [  572.253888] GPR08: 0000000000000000 0000000000002000 9000000000009033 c000007fbcd85800
>>>>>> [  572.253888] GPR12: 0000000000008800 c000007fffffb680 0000000000000005 0000000000000004
>>>>>> [  572.253888] GPR16: c000007f35153800 c000007f35154130 0000000000000005 0000000000000001
>>>>>> [  572.253888] GPR20: 0000000000000024 c000007f32e51e68 c000007f35154028 0000007fd8da0000
>>>>>> [  572.253888] GPR24: 0000007fd8da0000 c000007f351544d0 c000007e9a4024d0 c000000001665f18
>>>>>> [  572.253888] GPR28: c000007f351544d0 c000007f35153800 900000000290f033 c000007f35153800
>>>>>> [  572.254079] NIP [c00000000000ff5c] save_fpu+0xa8/0x2ac
>>>>>> [  572.254098] LR [c00000000001a8f8] __giveup_fpu+0x28/0x80
>>>>>> [  572.254114] Call Trace:
>>>>>> [  572.254128] [c000007f31f0f6b0] [c000007f35153980] 0xc000007f35153980 (unreliable)
>>>>>> [  572.254156] [c000007f31f0f6e0] [c00000000001b228] giveup_all+0x128/0x150
>>>>>> [  572.254327] [c000007f31f0f710] [c00000000001c124] __switch_to+0x104/0x490
>>>>>> [  572.254352] [c000007f31f0f770] [c0000000010d2e34] __schedule+0x2e4/0xa10
>>>>>> [  572.254374] [c000007f31f0f840] [c0000000010d35d4] schedule+0x74/0x140
>>>>>> [  572.254397] [c000007f31f0f870] [c0000000010d9478] schedule_timeout+0x358/0x5d0
>>>>>> [  572.254424] [c000007f31f0f980] [c0000000010d5638] wait_for_completion+0xc8/0x210
>>>>>> [  572.254451] [c000007f31f0fa00] [c000000000608ed4] do_coredump+0x3a4/0xd60
>>>>>> [  572.254625] [c000007f31f0fba0] [c00000000018d1cc] get_signal+0x1dc/0xd00
>>>>>> [  572.254648] [c000007f31f0fcc0] [c00000000001f088] do_notify_resume+0x158/0x450
>>>>>> [  572.254672] [c000007f31f0fda0] [c000000000037d04] interrupt_exit_user_prepare+0x1c4/0x230
>>>>>> [  572.254699] [c000007f31f0fe20] [c00000000000f2b4] interrupt_return+0x14/0x1c0
>>>>>> [  572.254720] Instruction dump:
>>>>>> [  572.254882] dae60170 db060180 db260190 db4601a0 db6601b0 db8601c0 dba601d0 dbc601e0
>>>>>> [  572.254912] dbe601f0 48000204 38800000 f0000250 <7c062798> f0000250 38800010 f0210a50
>>>>>> [  572.254946] ---[ end trace ba4452ee5c77d58e ]---
>>>>>
>>>>> Please find all the messages attached.
>>>>
>>>> "Oops: Exception in kernel mode, sig: 5 [#1]"
>>>>
>>>> Unfortunately it's a very poor error message. I think it is a 0x1500
>>>> exception triggering in the kernel FP register saving. Do you have the
>>>> CONFIG_PPC_DENORMALISATION config option set?
>>>
>>> Yes, as it’s set in the Ubuntu Linux kernel configuration, I have it set
>>> too.
>>>
>>>       $ grep DENORMALI /boot/config-*
>>>       /boot/config-4.15.0-23-generic:CONFIG_PPC_DENORMALISATION=y
>>>       /boot/config-5.4.0-40-generic:CONFIG_PPC_DENORMALISATION=y
>>>       /boot/config-5.7.0-rc5+:CONFIG_PPC_DENORMALISATION=y
>>>       /boot/config-5.8.0-rc3+:CONFIG_PPC_DENORMALISATION=y
>> 
>> Ah thanks I was able to reproduce with a little denorm test case.
>> 
>> The denorm interrupt handler got broken by some careless person.
>> 
>> This patch should hopefully fix it for you?
> 
> Yes, it does. Thank you.
> 
>> ---
>>   arch/powerpc/kernel/exceptions-64s.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index fa080694e581..0fc8bad878b2 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -2551,7 +2551,7 @@ EXC_VIRT_NONE(0x5400, 0x100)
>>   INT_DEFINE_BEGIN(denorm_exception)
>>   	IVEC=0x1500
>>   	IHSRR=1
>> -	IBRANCH_COMMON=0
>> +	IBRANCH_TO_COMMON=0
>>   	IKVM_REAL=1
>>   INT_DEFINE_END(denorm_exception)
> 
> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>

Thanks for reporting and testing, sorry for breaking it for you!

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Nicholas Piggin @ 2020-07-08  5:17 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: linux-arch, linuxppc-dev
In-Reply-To: <638683144.970.1594121101349.JavaMail.zimbra@efficios.com>

Excerpts from Mathieu Desnoyers's message of July 7, 2020 9:25 pm:
> ----- On Jul 7, 2020, at 1:50 AM, Nicholas Piggin npiggin@gmail.com wrote:
> 
>> Excerpts from Christophe Leroy's message of July 6, 2020 7:53 pm:
>>> 
>>> 
>>> Le 06/07/2020 à 04:18, Nicholas Piggin a écrit :
>>>> diff --git a/arch/powerpc/include/asm/exception-64s.h
>>>> b/arch/powerpc/include/asm/exception-64s.h
>>>> index 47bd4ea0837d..b88cb3a989b6 100644
>>>> --- a/arch/powerpc/include/asm/exception-64s.h
>>>> +++ b/arch/powerpc/include/asm/exception-64s.h
>>>> @@ -68,6 +68,10 @@
>>>>    *
>>>>    * The nop instructions allow us to insert one or more instructions to flush the
>>>>    * L1-D cache when returning to userspace or a guest.
>>>> + *
>>>> + * powerpc relies on return from interrupt/syscall being context synchronising
>>>> + * (which hrfid, rfid, and rfscv are) to support ARCH_HAS_MEMBARRIER_SYNC_CORE
>>>> + * without additional additional synchronisation instructions.
>>> 
>>> This file is dedicated to BOOK3S/64. What about other ones ?
>>> 
>>> On 32 bits, this is also valid as 'rfi' is also context synchronising,
>>> but then why just add some comment in exception-64s.h and only there ?
>> 
>> Yeah you're right, I basically wanted to keep a note there just in case,
>> because it's possible we would get a less synchronising return (maybe
>> unlikely with meltdown) or even return from a kernel interrupt using a
>> something faster (e.g., bctar if we don't use tar register in the kernel
>> anywhere).
>> 
>> So I wonder where to add the note, entry_32.S and 64e.h as well?
>> 
> 
> For 64-bit powerpc, I would be tempted to either place the comment in the header
> implementing the RFI_TO_USER and RFI_TO_USER_OR_KERNEL macros or the .S files
> using them, e.g. either:
> 
> arch/powerpc/include/asm/exception-64e.h
> arch/powerpc/include/asm/exception-64s.h
> 
> or
> 
> arch/powerpc/kernel/exceptions-64s.S
> arch/powerpc/kernel/entry_64.S
> 
> And for 32-bit powerpc, AFAIU
> 
> arch/powerpc/kernel/entry_32.S
> 
> uses SYNC + RFI to return to user-space. RFI is defined in
> 
> arch/powerpc/include/asm/ppc_asm.h
> 
> So a comment either near the RFI define and its uses should work.
> 
>> I should actually change the comment for 64-bit because soft masked
>> interrupt replay is an interesting case. I thought it was okay (because
>> the IPI would cause a hard interrupt which does do the rfi) but that
>> should at least be written.
> 
> Yes.
> 
>> The context synchronisation happens before
>> the Linux IPI function is called, but for the purpose of membarrier I
>> think that is okay (the membarrier just needs to have caused a memory
>> barrier + context synchronistaion by the time it has done).
> 
> Can you point me to the code implementing this logic ?

It's mostly in arch/powerpc/kernel/exception-64s.S and 
powerpc/kernel/irq.c, but a lot of asm so easier to explain.

When any Linux code does local_irq_disable(), we set interrupts as 
software-masked in a per-cpu flag. When interrupts (including IPIs) come
in, the first thing we do is check that flag and if we are masked, then
record that the interrupt needs to be "replayed" in another per-cpu 
flag. The interrupt handler then exits back using RFI (which is context 
synchronising the CPU). Later, when the kernel code does 
local_irq_enable(), it checks the replay flag to see if anything needs 
to be done. At that point we basically just call the interrupt handler 
code like a normal function, and when that returns there is no context
synchronising instruction.

So membarrier IPI will always cause target CPUs to perform a context
synchronising instruction, but sometimes it happens before the IPI 
handler function runs.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH 06/20] Documentation: gpu/komeda-kms: eliminate duplicated word
From: james qian wang (Arm Technology China) @ 2020-07-08  5:08 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
	Liviu Dudau, dri-devel, Douglas Anderson, Paul Cercueil, keyrings,
	Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
	Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
	linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
	Halil Pasic, Jarkko Sakkinen, Mali DP Maintainers, linux-input,
	Derek Kiernan, linux-mips, Dragan Cvetic, Wu Hao, Tony Krowiak,
	linux-kbuild, James E.J. Bottomley, Jiri Kosina, Hannes Reinecke,
	linux-block, Thomas Bogendoerfer, Jacek Anaszewski, linux-mm,
	Dan Williams, nd, Andrew Morton, Mimi Zohar, Jens Axboe,
	Michal Marek, Martin K. Petersen, Pierre Morel, linux-kernel,
	Wolfram Sang, Daniel Vetter, Jason Wessel, Paolo Bonzini,
	linux-integrity, linuxppc-dev, Mike Rapoport, Dan Murphy
In-Reply-To: <20200707180414.10467-7-rdunlap@infradead.org>

Hi Randy

On Tue, Jul 07, 2020 at 11:04:00AM -0700, Randy Dunlap wrote:
> Drop the doubled word "and".
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: James (Qian) Wang <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mihail Atanassov <mihail.atanassov@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> ---
>  Documentation/gpu/komeda-kms.rst |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-next-20200701.orig/Documentation/gpu/komeda-kms.rst
> +++ linux-next-20200701/Documentation/gpu/komeda-kms.rst
> @@ -41,7 +41,7 @@ Compositor blends multiple layers or pix
>  frame. its output frame can be fed into post image processor for showing it on
>  the monitor or fed into wb_layer and written to memory at the same time.
>  user can also insert a scaler between compositor and wb_layer to down scale
> -the display frame first and and then write to memory.
> +the display frame first and then write to memory.

Thank you for the patch.

Reviewed-by: James Qian Wang <james.qian.wang@arm.com>

>  Writeback Layer (wb_layer)
>  --------------------------

^ permalink raw reply

* RE: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
From: Madalin Bucur (OSS) @ 2020-07-08  6:03 UTC (permalink / raw)
  To: Christian Zigotzky, Madalin Bucur (OSS)
  Cc: Darren Stevens, mad skateman, netdev@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Camelia Alexandra Groza, R.T.Dickinson
In-Reply-To: <4E3069C3-B777-4460-A781-84214C4539DA@xenosoft.de>

> From: Christian Zigotzky <chzigotzky@xenosoft.de> 
> Sent: Tuesday, July 7, 2020 9:26 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: mad skateman <madskateman@gmail.com>; Camelia Alexandra Groza <camelia.groza@nxp.com>;
> linuxppc-dev@ozlabs.org; netdev@vger.kernel.org; R.T.Dickinson <rtd2@xtra.co.nz>;
> Darren Stevens <darren@stevens-zone.net>
> Subject: Re: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
>
>
> On 7. Jul 2020, at 17:53, Madalin Bucur (OSS) <mailto:madalin.bucur@oss.nxp.com> wrote:
> Was DPAA functional before commit A?
> How about after commit A and before commit B?

> The DPAA Ethernet works from  the kernel 5.6-rc4 [1] till the Git kernel from the
> 11 of June [2]. It doesn’t work since the commit “fix bitmap_parse” [3].

> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=49936#p49936
> [2] https://forum.hyperion-entertainment.com/viewtopic.php?p=50848#p50848
> [3] https://forum.hyperion-entertainment.com/viewtopic.php?p=50980#p50980

Hi,

can you please try to disable the network manager (see [1]), then boot with
the latest kernel, that does not work, and setup the interfaces manually?

Madalin

[1] https://help.ubuntu.com/community/NetworkManager#Stopping_and_Disabling_NetworkManager


^ permalink raw reply

* Re: [PATCH 00/20] Documentation: eliminate duplicated words
From: Martin K. Petersen @ 2020-07-08  6:06 UTC (permalink / raw)
  To: linux-kernel, Randy Dunlap
  Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
	Liviu Dudau, Jarkko Sakkinen, linux-mips, Paul Cercueil, keyrings,
	Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
	Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
	linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
	Halil Pasic, James Wang, linux-input, Mali DP Maintainers,
	Derek Kiernan, Dragan Cvetic, Wu Hao, Jens Axboe, linux-kbuild,
	James E.J. Bottomley, Jiri Kosina, Hannes Reinecke, linux-block,
	Thomas Bogendoerfer, Jacek Anaszewski, linux-mm, Dan Williams,
	Andrew Morton, Mimi Zohar, Tony Krowiak, Michal Marek,
	Pierre Morel, Martin K . Petersen, dri-devel, Douglas Anderson,
	Wolfram Sang, Daniel Vetter, Jason Wessel, Paolo Bonzini,
	linux-integrity, linuxppc-dev, Mike Rapoport, Dan Murphy
In-Reply-To: <20200707180414.10467-1-rdunlap@infradead.org>

On Tue, 7 Jul 2020 11:03:54 -0700, Randy Dunlap wrote:

> Drop doubled words in various parts of Documentation/.
> 
> [...]

Applied to 5.9/scsi-queue, thanks!

[17/20] scsi: advansys: docs: Eliminate duplicated word
        https://git.kernel.org/mkp/scsi/c/3010dfb0b77c

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
From: Christian Zigotzky @ 2020-07-08  6:25 UTC (permalink / raw)
  To: Madalin Bucur (OSS)
  Cc: Darren Stevens, mad skateman, netdev@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Camelia Alexandra Groza, R.T.Dickinson
In-Reply-To: <AM6PR04MB3976996912A9342D7BB7C1FFEC670@AM6PR04MB3976.eurprd04.prod.outlook.com>

On 08 July 2020 at 08:03 am, Madalin Bucur (OSS) wrote:
>> From: Christian Zigotzky <chzigotzky@xenosoft.de>
>> Sent: Tuesday, July 7, 2020 9:26 PM
>> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
>> Cc: mad skateman <madskateman@gmail.com>; Camelia Alexandra Groza <camelia.groza@nxp.com>;
>> linuxppc-dev@ozlabs.org; netdev@vger.kernel.org; R.T.Dickinson <rtd2@xtra.co.nz>;
>> Darren Stevens <darren@stevens-zone.net>
>> Subject: Re: FSL P5020/P5040: DPAA Ethernet issue with the latest Git kernel
>>
>>
>> On 7. Jul 2020, at 17:53, Madalin Bucur (OSS) <mailto:madalin.bucur@oss.nxp.com> wrote:
>> Was DPAA functional before commit A?
>> How about after commit A and before commit B?
>> The DPAA Ethernet works from  the kernel 5.6-rc4 [1] till the Git kernel from the
>> 11 of June [2]. It doesn’t work since the commit “fix bitmap_parse” [3].
>> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=49936#p49936
>> [2] https://forum.hyperion-entertainment.com/viewtopic.php?p=50848#p50848
>> [3] https://forum.hyperion-entertainment.com/viewtopic.php?p=50980#p50980
> Hi,
>
> can you please try to disable the network manager (see [1]), then boot with
> the latest kernel, that does not work, and setup the interfaces manually?
>
> Madalin
>
> [1] https://help.ubuntu.com/community/NetworkManager#Stopping_and_Disabling_NetworkManager
>
@Skateman
I will compile the latest Git kernel after the 17th. Could you please 
test it without the NetworkManager?

Thanks

^ permalink raw reply

* Re: [PATCH v2 4/4] powerpc/mm/radix: Create separate mappings for hot-plugged memory
From: Aneesh Kumar K.V @ 2020-07-08  6:24 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: Bharata B Rao
In-Reply-To: <877dve4nvj.fsf@mpe.ellerman.id.au>

On 7/8/20 10:14 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> To enable memory unplug without splitting kernel page table
>> mapping, we force the max mapping size to the LMB size. LMB
>> size is the unit in which hypervisor will do memory add/remove
>> operation.
>>
>> This implies on pseries system, we now end up mapping
> 
> Please expand on why it "implies" that for pseries.
> 
>> memory with 2M page size instead of 1G. To improve
>> that we want hypervisor to hint the kernel about the hotplug
>> memory range.  This was added that as part of
>                   That
>>
>> commit b6eca183e23e ("powerpc/kernel: Enables memory
>> hot-remove after reboot on pseries guests")
>>
>> But we still don't do that on PowerVM. Once we get PowerVM
> 
> I think you mean PowerVM doesn't provide that hint yet?
> 
> Realistically it won't until P10. So this means we'll always use 2MB on
> Power9 PowerVM doesn't it?
> 
> What about KVM?
> 
> Have you done any benchmarking on the impact of switching the linear
> mapping to 2MB pages?
> 

The TLB impact should be minimal because with a 256M LMB size partition 
scoped entries are still 2M and hence we end up with TLBs of 2M size.


>> updated, we can then force the 2M mapping only to hot-pluggable
>> memory region using memblock_is_hotpluggable(). Till then
>> let's depend on LMB size for finding the mapping page size
>> for linear range.
>>

updated


powerpc/mm/radix: Create separate mappings for hot-plugged memory

To enable memory unplug without splitting kernel page table
mapping, we force the max mapping size to the LMB size. LMB
size is the unit in which hypervisor will do memory add/remove
operation.

Pseries systems supports max LMB size of 256MB. Hence on pseries,
we now end up mapping memory with 2M page size instead of 1G. To improve
that we want hypervisor to hint the kernel about the hotplug
memory range.  That was added that as part of

commit b6eca18 ("powerpc/kernel: Enables memory
hot-remove after reboot on pseries guests")

But PowerVM doesn't provide that hint yet. Once we get PowerVM
updated, we can then force the 2M mapping only to hot-pluggable
memory region using memblock_is_hotpluggable(). Till then
let's depend on LMB size for finding the mapping page size
for linear range.

With this change KVM guest will also be doing linear mapping with
2M page size.



>> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   arch/powerpc/mm/book3s64/radix_pgtable.c | 83 ++++++++++++++++++++----
>>   arch/powerpc/platforms/powernv/setup.c   | 10 ++-
>>   2 files changed, 81 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> index 78ad11812e0d..a4179e4da49d 100644
>> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
>> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> @@ -15,6 +15,7 @@
>>   #include <linux/mm.h>
>>   #include <linux/hugetlb.h>
>>   #include <linux/string_helpers.h>
>> +#include <linux/memory.h>
>>   
>>   #include <asm/pgtable.h>
>>   #include <asm/pgalloc.h>
>> @@ -34,6 +35,7 @@
>>   
>>   unsigned int mmu_pid_bits;
>>   unsigned int mmu_base_pid;
>> +unsigned int radix_mem_block_size;
> 
> static? __ro_after_init?
> 

Added __ro_after_iit

>> @@ -266,6 +268,7 @@ static unsigned long next_boundary(unsigned long addr, unsigned long end)
>>   
>>   static int __meminit create_physical_mapping(unsigned long start,
>>   					     unsigned long end,
>> +					     unsigned long max_mapping_size,
>>   					     int nid, pgprot_t _prot)
>>   {
>>   	unsigned long vaddr, addr, mapping_size = 0;
>> @@ -279,6 +282,8 @@ static int __meminit create_physical_mapping(unsigned long start,
>>   		int rc;
>>   
>>   		gap = next_boundary(addr, end) - addr;
>> +		if (gap > max_mapping_size)
>> +			gap = max_mapping_size;
>>   		previous_size = mapping_size;
>>   		prev_exec = exec;
>>   
>> @@ -329,8 +334,9 @@ static void __init radix_init_pgtable(void)
>>   
>>   	/* We don't support slb for radix */
>>   	mmu_slb_size = 0;
>> +
>>   	/*
>> -	 * Create the linear mapping, using standard page size for now
>> +	 * Create the linear mapping
>>   	 */
>>   	for_each_memblock(memory, reg) {
>>   		/*
>> @@ -346,6 +352,7 @@ static void __init radix_init_pgtable(void)
>>   
>>   		WARN_ON(create_physical_mapping(reg->base,
>>   						reg->base + reg->size,
>> +						radix_mem_block_size,
>>   						-1, PAGE_KERNEL));
>>   	}
>>   
>> @@ -486,6 +493,49 @@ static int __init radix_dt_scan_page_sizes(unsigned long node,
>>   	return 1;
>>   }
>>   
>> +static int __init probe_memory_block_size(unsigned long node, const char *uname, int
>> +					  depth, void *data)
>> +{
>> +	const __be32 *block_size;
>> +	int len;
>> +
>> +	if (depth != 1)
>> +		return 0;
>> +
>> +	if (!strcmp(uname, "ibm,dynamic-reconfiguration-memory")) {
> 
> 	if (strcmp(uname, "ibm,dynamic-reconfiguration-memory") != 0)
>          	return 0;
> 
> Then you can de-indent the rest of the body.
> 

updated

> 
>> +		block_size = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>> +		if (!block_size || len < dt_root_size_cells * sizeof(__be32))
> 
> This will give us a sparse warning, please do the endian conversion
> before looking at the value.
> 
>> +			/*
>> +			 * Nothing in the device tree
>> +			 */
>> +			return MIN_MEMORY_BLOCK_SIZE;
> 
>> +
>> +		return dt_mem_next_cell(dt_root_size_cells, &block_size);
> 
> Just of_read_number() ?
> 
> This is misusing the return value, as I explained on one of your other
> recent patches. You should return !0 to indicate that scanning should
> stop, and the actual value can go via the data pointer, or better just
> set the global.
> 


updated

> 
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static unsigned long radix_memory_block_size(void)
>> +{
>> +	unsigned long mem_block_size = MIN_MEMORY_BLOCK_SIZE;
> 
>> +
>> +	if (firmware_has_feature(FW_FEATURE_OPAL)) {
>> +
>> +		mem_block_size = 1UL * 1024 * 1024 * 1024;
> 
> We have 1GB hardcoded here and also in pnv_memory_block_size().
> 
> Shouldn't pnv_memory_block_size() at least be using radix_mem_block_size?


updated

> 
>> +	} else if (firmware_has_feature(FW_FEATURE_LPAR)) {
>> +		mem_block_size = of_scan_flat_dt(probe_memory_block_size, NULL);
>> +		if (!mem_block_size)
>> +			mem_block_size = MIN_MEMORY_BLOCK_SIZE;
>> +	}
>> +
>> +	return mem_block_size;
>> +}
> 
> It would probably be simpler if that was just inlined below.
> 
>> +
>> +
>>   void __init radix__early_init_devtree(void)
>>   {
>>   	int rc;
>> @@ -494,17 +544,27 @@ void __init radix__early_init_devtree(void)
>>   	 * Try to find the available page sizes in the device-tree
>>   	 */
>>   	rc = of_scan_flat_dt(radix_dt_scan_page_sizes, NULL);
>> -	if (rc != 0)  /* Found */
>> -		goto found;
>> +	if (rc == 0) {
>> +		/*
>> +		 * no page size details found in device tree
>> +		 * let's assume we have page 4k and 64k support
> 
> Capitals and punctuation please?
> 
>> +		 */
>> +		mmu_psize_defs[MMU_PAGE_4K].shift = 12;
>> +		mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
>> +
>> +		mmu_psize_defs[MMU_PAGE_64K].shift = 16;
>> +		mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
>> +	}
> 
> Moving that seems like an unrelated change. It's a reasonable change but
> I'd rather you did it in a standalone patch.
> 

we needed that change so that we can call radix_memory_block_size() for 
both found and !found case.

>>   	/*
>> -	 * let's assume we have page 4k and 64k support
>> +	 * Max mapping size used when mapping pages. We don't use
>> +	 * ppc_md.memory_block_size() here because this get called
>> +	 * early and we don't have machine probe called yet. Also
>> +	 * the pseries implementation only check for ibm,lmb-size.
>> +	 * All hypervisor supporting radix do expose that device
>> +	 * tree node.
>>   	 */
>> -	mmu_psize_defs[MMU_PAGE_4K].shift = 12;
>> -	mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
>> -
>> -	mmu_psize_defs[MMU_PAGE_64K].shift = 16;
>> -	mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
>> -found:
>> +	radix_mem_block_size = radix_memory_block_size();
> 
> If you did that earlier in the function, before
> radix_dt_scan_page_sizes(), the logic would be simpler.
> 
>>   	return;
>>   }
>>   
>> @@ -856,7 +916,8 @@ int __meminit radix__create_section_mapping(unsigned long start,
>>   		return -1;
>>   	}
>>   
>> -	return create_physical_mapping(__pa(start), __pa(end), nid, prot);
>> +	return create_physical_mapping(__pa(start), __pa(end),
>> +				       radix_mem_block_size, nid, prot);
>>   }
>>   
>>   int __meminit radix__remove_section_mapping(unsigned long start, unsigned long end)
>> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
>> index 3bc188da82ba..6e2f614590a3 100644
>> --- a/arch/powerpc/platforms/powernv/setup.c
>> +++ b/arch/powerpc/platforms/powernv/setup.c
>> @@ -399,7 +399,15 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
>>   #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
>>   static unsigned long pnv_memory_block_size(void)
>>   {
>> -	return 256UL * 1024 * 1024;
>> +	/*
>> +	 * We map the kernel linear region with 1GB large pages on radix. For
>> +	 * memory hot unplug to work our memory block size must be at least
>> +	 * this size.
>> +	 */
>> +	if (radix_enabled())
>> +		return 1UL * 1024 * 1024 * 1024;
>> +	else
>> +		return 256UL * 1024 * 1024;
>>   }
>>   #endif
>>   
>> -- 
>> 2.26.2
> 
> 
> cheers
> 


-aneesh

^ permalink raw reply

* Re: [PATCH 06/20] Documentation: gpu/komeda-kms: eliminate duplicated word
From: james qian wang (Arm Technology China) @ 2020-07-08  7:19 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
	Liviu Dudau, dri-devel, Douglas Anderson, Paul Cercueil, keyrings,
	Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
	Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
	linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
	Halil Pasic, Jarkko Sakkinen, Mali DP Maintainers, linux-input,
	Derek Kiernan, linux-mips, Dragan Cvetic, Wu Hao, Tony Krowiak,
	linux-kbuild, James E.J. Bottomley, Jiri Kosina, Hannes Reinecke,
	linux-block, Thomas Bogendoerfer, Jacek Anaszewski, linux-mm,
	Dan Williams, nd, Andrew Morton, Mimi Zohar, Jens Axboe,
	Michal Marek, Martin K. Petersen, Pierre Morel, linux-kernel,
	Wolfram Sang, Daniel Vetter, Jason Wessel, Paolo Bonzini,
	linux-integrity, linuxppc-dev, Mike Rapoport, Dan Murphy
In-Reply-To: <20200707180414.10467-7-rdunlap@infradead.org>

On Tue, Jul 07, 2020 at 11:04:00AM -0700, Randy Dunlap wrote:
> Drop the doubled word "and".
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: James (Qian) Wang <james.qian.wang@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Mihail Atanassov <mihail.atanassov@arm.com>
> Cc: Mali DP Maintainers <malidp@foss.arm.com>
> ---
>  Documentation/gpu/komeda-kms.rst |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-next-20200701.orig/Documentation/gpu/komeda-kms.rst
> +++ linux-next-20200701/Documentation/gpu/komeda-kms.rst
> @@ -41,7 +41,7 @@ Compositor blends multiple layers or pix
>  frame. its output frame can be fed into post image processor for showing it on
>  the monitor or fed into wb_layer and written to memory at the same time.
>  user can also insert a scaler between compositor and wb_layer to down scale
> -the display frame first and and then write to memory.
> +the display frame first and then write to memory.
>

Thank you Randy

Reviewed-by: James Qian Wang <james.qian.wang@arm.com>

>  Writeback Layer (wb_layer)
>  --------------------------

^ permalink raw reply

* Re: [PATCH 16/20] Documentation: s390/vfio-ap: eliminate duplicated word
From: Pierre Morel @ 2020-07-08  6:57 UTC (permalink / raw)
  To: Randy Dunlap, linux-kernel
  Cc: kvm, linux-doc, David Airlie, kgdb-bugreport, linux-fpga,
	Liviu Dudau, dri-devel, linux-mips, Paul Cercueil, keyrings,
	Paul Mackerras, linux-i2c, Pavel Machek, Srinivas Pandruvada,
	Mihail Atanassov, linux-leds, linux-s390, Daniel Thompson,
	linux-scsi, Jonathan Corbet, Masahiro Yamada, Matthew Wilcox,
	Halil Pasic, Jarkko Sakkinen, James Wang, linux-input,
	Mali DP Maintainers, Derek Kiernan, Dragan Cvetic, Wu Hao,
	Tony Krowiak, linux-kbuild, James E.J. Bottomley, Jiri Kosina,
	Hannes Reinecke, linux-block, Thomas Bogendoerfer,
	Jacek Anaszewski, linux-mm, Dan Williams, Andrew Morton,
	Mimi Zohar, Jens Axboe, Michal Marek, Martin K. Petersen,
	Douglas Anderson, Wolfram Sang, Daniel Vetter, Jason Wessel,
	Paolo Bonzini, linux-integrity, linuxppc-dev, Mike Rapoport,
	Dan Murphy
In-Reply-To: <20200707180414.10467-17-rdunlap@infradead.org>



On 2020-07-07 20:04, Randy Dunlap wrote:
> Drop the doubled word "the".
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> ---
>   Documentation/s390/vfio-ap.rst |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-next-20200701.orig/Documentation/s390/vfio-ap.rst
> +++ linux-next-20200701/Documentation/s390/vfio-ap.rst
> @@ -361,7 +361,7 @@ matrix device.
>       assign_domain / unassign_domain:
>         Write-only attributes for assigning/unassigning an AP usage domain to/from
>         the mediated matrix device. To assign/unassign a domain, the domain
> -      number of the the usage domain is echoed to the respective attribute
> +      number of the usage domain is echoed to the respective attribute
>         file.
>       matrix:
>         A read-only file for displaying the APQNs derived from the cross product
> 


Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

Thanks Randy,

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

^ permalink raw reply

* Re: [PATCH v2 07/10] powerpc/perf: support BHRB disable bit and new filtering modes
From: Gautham R Shenoy @ 2020-07-08  7:43 UTC (permalink / raw)
  To: Michael Neuling
  Cc: ego, Athira Rajeev, Vaidyanathan Srinivasan, maddy, linuxppc-dev
In-Reply-To: <0cf26e42a3b190d5ea69d1ba61ae71bcaeee1973.camel@neuling.org>

On Tue, Jul 07, 2020 at 05:17:55PM +1000, Michael Neuling wrote:
> On Wed, 2020-07-01 at 05:20 -0400, Athira Rajeev wrote:
> > PowerISA v3.1 has few updates for the Branch History Rolling Buffer(BHRB).
> > First is the addition of BHRB disable bit and second new filtering
> > modes for BHRB.
> > 
> > BHRB disable is controlled via Monitor Mode Control Register A (MMCRA)
> > bit 26, namely "BHRB Recording Disable (BHRBRD)". This field controls
> > whether BHRB entries are written when BHRB recording is enabled by other
> > bits. Patch implements support for this BHRB disable bit.
> 
> Probably good to note here that this is backwards compatible. So if you have a
> kernel that doesn't know about this bit, it'll clear it and hence you still get
> BHRB. 
> 
> You should also note why you'd want to do disable this (ie. the core will run
> faster).
> 
> > Secondly PowerISA v3.1 introduce filtering support for
> > PERF_SAMPLE_BRANCH_IND_CALL/COND. The patch adds BHRB filter support
> > for "ind_call" and "cond" in power10_bhrb_filter_map().
> > 
> > 'commit bb19af816025 ("powerpc/perf: Prevent kernel address leak to userspace
> > via BHRB buffer")'
> > added a check in bhrb_read() to filter the kernel address from BHRB buffer.
> > Patch here modified
> > it to avoid that check for PowerISA v3.1 based processors, since PowerISA v3.1
> > allows
> > only MSR[PR]=1 address to be written to BHRB buffer.
> > 
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/perf/core-book3s.c       | 27 +++++++++++++++++++++------
> >  arch/powerpc/perf/isa207-common.c     | 13 +++++++++++++
> >  arch/powerpc/perf/power10-pmu.c       | 13 +++++++++++--
> >  arch/powerpc/platforms/powernv/idle.c | 14 ++++++++++++++
> 
> This touches the idle code so we should get those guys on CC (adding Vaidy and
> Ego).
> 
> >  4 files changed, 59 insertions(+), 8 deletions(-)
> > 

[..snip..]


> > diff --git a/arch/powerpc/platforms/powernv/idle.c
> > b/arch/powerpc/platforms/powernv/idle.c
> > index 2dd4673..7db99c7 100644
> > --- a/arch/powerpc/platforms/powernv/idle.c
> > +++ b/arch/powerpc/platforms/powernv/idle.c
> > @@ -611,6 +611,7 @@ static unsigned long power9_idle_stop(unsigned long psscr,
> > bool mmu_on)
> >  	unsigned long srr1;
> >  	unsigned long pls;
> >  	unsigned long mmcr0 = 0;
> > +	unsigned long mmcra_bhrb = 0;

We are saving the whole of MMCRA aren't we ? We might want to just
name it mmcra in that case.

> >  	struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
> >  	bool sprs_saved = false;
> >  
> > @@ -657,6 +658,15 @@ static unsigned long power9_idle_stop(unsigned long
> > psscr, bool mmu_on)
> >  		  */
> >  		mmcr0		= mfspr(SPRN_MMCR0);
> >  	}
> > +
> > +	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> > +		/* POWER10 uses MMCRA[:26] as BHRB disable bit
> > +		 * to disable BHRB logic when not used. Hence Save and
> > +		 * restore MMCRA after a state-loss idle.
> > +		 */

Multi-line comment usually has the first line blank.

		/*
	         * Line 1
		 * Line 2
		 * .
		 * .
		 * .
		 * Line N
		 */

> > +		mmcra_bhrb		= mfspr(SPRN_MMCRA);
> 
> 
> Why is the bhrb bit of mmcra special here?

The comment above could include the consequence of not saving and
restoring MMCRA i.e

- If the user hasn't asked for the BHRB to be
  written the value of MMCRA[BHRBD] = 1.

- On wakeup from stop, MMCRA[BHRBD] will be 0, since MMCRA is not a
  previleged resource and will be lost.

- Thus, if we do not save and restore the MMCRA[BHRBD], the hardware
  will be needlessly writing to the BHRB in the problem mode.

> 
> > +	}
> > +
> >  	if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
> >  		sprs.lpcr	= mfspr(SPRN_LPCR);
> >  		sprs.hfscr	= mfspr(SPRN_HFSCR);
> > @@ -721,6 +731,10 @@ static unsigned long power9_idle_stop(unsigned long
> > psscr, bool mmu_on)
> >  			mtspr(SPRN_MMCR0, mmcr0);
> >  		}
> >  
> > +		/* Reload MMCRA to restore BHRB disable bit for POWER10 */
> > +		if (cpu_has_feature(CPU_FTR_ARCH_31))
> > +			mtspr(SPRN_MMCRA, mmcra_bhrb);
> > +
> >  		/*
> >  		 * DD2.2 and earlier need to set then clear bit 60 in MMCRA
> >  		 * to ensure the PMU starts running.
> 

--
Thanks and Regards
gautham.

^ permalink raw reply

* Re: [PATCH v3 1/9] powerpc/watchpoint: Fix 512 byte boundary limit
From: Jordan Niethe @ 2020-07-08  7:44 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Christophe Leroy, apopple, mikey, miltonm, peterz, oleg,
	Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
	pedromfc, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200708045046.135702-2-ravi.bangoria@linux.ibm.com>

On Wed, Jul 8, 2020 at 2:53 PM Ravi Bangoria
<ravi.bangoria@linux.ibm.com> wrote:
>
> Milton Miller reported that we are aligning start and end address to
> wrong size SZ_512M. It should be SZ_512. Fix that.
>
> While doing this change I also found a case where ALIGN() comparison
> fails. Within a given aligned range, ALIGN() of two addresses does not
> match when start address is pointing to the first byte and end address
> is pointing to any other byte except the first one. But that's not true
> for ALIGN_DOWN(). ALIGN_DOWN() of any two addresses within that range
> will always point to the first byte. So use ALIGN_DOWN() instead of
> ALIGN().
>
> Fixes: e68ef121c1f4 ("powerpc/watchpoint: Use builtin ALIGN*() macros")
> Reported-by: Milton Miller <miltonm@us.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
>  arch/powerpc/kernel/hw_breakpoint.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index 0000daf0e1da..031e6defc08e 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -419,7 +419,7 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
>         if (dawr_enabled()) {
>                 max_len = DAWR_MAX_LEN;
>                 /* DAWR region can't cross 512 bytes boundary */
> -               if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
> +               if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512))
I wonder if you should use end_addr - 1, but rather end_addr. For example:
512 -> 1023, because of the -1, 1024 will now be included in this
range meaning 513 bytes?

>                         return -EINVAL;
>         } else if (IS_ENABLED(CONFIG_PPC_8xx)) {
>                 /* 8xx can setup a range without limitation */
> --
> 2.26.2
>

^ permalink raw reply

* [PATCH] powerpc/64s/exception: Fix 0x1500 interrupt handler crash
From: Nicholas Piggin @ 2020-07-08  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Menzel, Nicholas Piggin

A typo caused the interrupt handler to branch immediately to the common
"unknown interrupt" handler and skip the special case test for denormal
cause.

This does not affect KVM softpatch handling (e.g., for POWER9 TM assist)
because the KVM test was moved to common code by commit 9600f261acaa
("powerpc/64s/exception: Move KVM test to common code") just before this
bug was introduced.

Fixes: 3f7fbd97d07d ("powerpc/64s/exception: Clean up SRR specifiers")
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64s.S          |  2 +-
 tools/testing/selftests/powerpc/math/Makefile |  2 +-
 .../selftests/powerpc/math/fpu_denormal.c     | 38 +++++++++++++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/math/fpu_denormal.c

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index fa080694e581..0fc8bad878b2 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -2551,7 +2551,7 @@ EXC_VIRT_NONE(0x5400, 0x100)
 INT_DEFINE_BEGIN(denorm_exception)
 	IVEC=0x1500
 	IHSRR=1
-	IBRANCH_COMMON=0
+	IBRANCH_TO_COMMON=0
 	IKVM_REAL=1
 INT_DEFINE_END(denorm_exception)
 
diff --git a/tools/testing/selftests/powerpc/math/Makefile b/tools/testing/selftests/powerpc/math/Makefile
index 11a10d7a2bbd..4e2049d2fd8d 100644
--- a/tools/testing/selftests/powerpc/math/Makefile
+++ b/tools/testing/selftests/powerpc/math/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-TEST_GEN_PROGS := fpu_syscall fpu_preempt fpu_signal vmx_syscall vmx_preempt vmx_signal vsx_preempt
+TEST_GEN_PROGS := fpu_syscall fpu_preempt fpu_signal fpu_denormal vmx_syscall vmx_preempt vmx_signal vsx_preempt
 
 top_srcdir = ../../../../..
 include ../../lib.mk
diff --git a/tools/testing/selftests/powerpc/math/fpu_denormal.c b/tools/testing/selftests/powerpc/math/fpu_denormal.c
new file mode 100644
index 000000000000..5f96682abaa8
--- /dev/null
+++ b/tools/testing/selftests/powerpc/math/fpu_denormal.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright IBM Corp. 2020
+ *
+ * This test attempts to cause a FP denormal exception on POWER8 CPUs. Unfortunately
+ * if the denormal handler is not configured or working properly, this can cause a bad
+ * crash in kernel mode when the kernel tries to save FP registers when the process
+ * exits.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "utils.h"
+
+static int test_denormal_fpu(void)
+{
+	unsigned int m32;
+	unsigned long m64;
+	volatile float f;
+	volatile double d;
+
+	/* try to induce lfs <denormal> ; stfd */
+
+	m32 = 0x00715fcf; /* random denormal */
+	memcpy((float *)&f, &m32, sizeof(f));
+	d = f;
+	memcpy(&m64, (double *)&d, sizeof(d));
+
+	FAIL_IF((long)(m64 != 0x380c57f3c0000000)); /* renormalised value */
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	return test_harness(test_denormal_fpu, "fpu_denormal");
+}
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH v3 1/9] powerpc/watchpoint: Fix 512 byte boundary limit
From: Christophe Leroy @ 2020-07-08  7:52 UTC (permalink / raw)
  To: Jordan Niethe, Ravi Bangoria
  Cc: Christophe Leroy, apopple, mikey, miltonm, peterz, oleg,
	Nicholas Piggin, linux-kernel, Paul Mackerras, jolsa, fweisbec,
	pedromfc, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <CACzsE9qka0j7vKm186Z70fhNy9L4dNR3B97-jQ2oZZAwYduaRw@mail.gmail.com>



Le 08/07/2020 à 09:44, Jordan Niethe a écrit :
> On Wed, Jul 8, 2020 at 2:53 PM Ravi Bangoria
> <ravi.bangoria@linux.ibm.com> wrote:
>>
>> Milton Miller reported that we are aligning start and end address to
>> wrong size SZ_512M. It should be SZ_512. Fix that.
>>
>> While doing this change I also found a case where ALIGN() comparison
>> fails. Within a given aligned range, ALIGN() of two addresses does not
>> match when start address is pointing to the first byte and end address
>> is pointing to any other byte except the first one. But that's not true
>> for ALIGN_DOWN(). ALIGN_DOWN() of any two addresses within that range
>> will always point to the first byte. So use ALIGN_DOWN() instead of
>> ALIGN().
>>
>> Fixes: e68ef121c1f4 ("powerpc/watchpoint: Use builtin ALIGN*() macros")
>> Reported-by: Milton Miller <miltonm@us.ibm.com>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>>   arch/powerpc/kernel/hw_breakpoint.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
>> index 0000daf0e1da..031e6defc08e 100644
>> --- a/arch/powerpc/kernel/hw_breakpoint.c
>> +++ b/arch/powerpc/kernel/hw_breakpoint.c
>> @@ -419,7 +419,7 @@ static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
>>          if (dawr_enabled()) {
>>                  max_len = DAWR_MAX_LEN;
>>                  /* DAWR region can't cross 512 bytes boundary */
>> -               if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
>> +               if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1, SZ_512))
> I wonder if you should use end_addr - 1, but rather end_addr. For example:
> 512 -> 1023, because of the -1, 1024 will now be included in this
> range meaning 513 bytes?

end_addr is not part of the range.

If you want the range [512;1023], it means addr 512 len 512, that is 
end_addr = addr + len = 1024

Christophe

^ permalink raw reply

* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Giuseppe Sacco @ 2020-07-08  7:59 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <990279c219476c4d513df52454adf583de32641a.camel@sguazz.it>

Hello Cristophe,

Il giorno mar, 07/07/2020 alle 17.34 +0200, Giuseppe Sacco ha scritto:
> Il giorno mar, 07/07/2020 alle 16.52 +0200, Christophe Leroy ha
> scritto:
> > Le 07/07/2020 à 16:03, Giuseppe Sacco a écrit :
> > > Hello Cristophe,
> > > 
> > > > Can you tell which defconfig you use or provide your .config
> > > 
> > > You may get the standard one from debian or a reduced one that I
> > > made on purpose. The latter is here:
> > > https://eppesuigoccas.homedns.org/~giuseppe/config-5.4.50.gz
> > > 
> > >  (boot)
> > > https://eppesuigoccas.homedns.org/~giuseppe/config-5.6.19.gz
> > > 
> > >  (no boot)
> > 
> > Thanks
> > 
> > Can you provide the complete output when it works, so that I can
> > see 
> > what is after the place it stops when it fails.
> 
> Here it is:
> https://eppesuigoccas.homedns.org/~giuseppe/dmesg-5.4.40-minimo.gz
> 
> 
> > And can you try without CONFIG_VMAP_STACK on 5.6.19
> 
> Sure, I'll let you know.

No, this change did not make the kernel boot. I only changed the option
you proposed:

$ grep VMAP .config 
CONFIG_HAVE_ARCH_VMAP_STACK=y
# CONFIG_VMAP_STACK is not set

Thanks,
Giuseppe


^ permalink raw reply

* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Peter Zijlstra @ 2020-07-08  8:32 UTC (permalink / raw)
  To: Waiman Long
  Cc: linux-arch, Will Deacon, Boqun Feng, linux-kernel, kvm-ppc,
	virtualization, Ingo Molnar, Nicholas Piggin, linuxppc-dev
In-Reply-To: <de3ead58-7f81-8ebd-754d-244f6be24af4@redhat.com>

On Tue, Jul 07, 2020 at 11:33:45PM -0400, Waiman Long wrote:
> From 5d7941a498935fb225b2c7a3108cbf590114c3db Mon Sep 17 00:00:00 2001
> From: Waiman Long <longman@redhat.com>
> Date: Tue, 7 Jul 2020 22:29:16 -0400
> Subject: [PATCH 2/9] locking/pvqspinlock: Introduce
>  CONFIG_PARAVIRT_QSPINLOCKS_LITE
> 
> Add a new PARAVIRT_QSPINLOCKS_LITE config option that allows
> architectures to use the PV qspinlock code without the need to use or
> implement a pv_kick() function, thus eliminating the atomic unlock
> overhead. The non-atomic queued_spin_unlock() can be used instead.
> The pv_wait() function will still be needed, but it can be a dummy
> function.
> 
> With that option set, the hybrid PV queued/unfair locking code should
> still be able to make it performant enough in a paravirtualized

How is this supposed to work? If there is no kick, you have no control
over who wakes up and fairness goes out the window entirely.

You don't even begin to explain...

^ permalink raw reply

* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Christophe Leroy @ 2020-07-08  8:38 UTC (permalink / raw)
  To: Giuseppe Sacco, linuxppc-dev
In-Reply-To: <211a35b02193ae79a201d4d567fe1d7a53a979f5.camel@sguazz.it>

Hi Giuseppe,

Le 08/07/2020 à 09:59, Giuseppe Sacco a écrit :
> Hello Cristophe,
> 
> Il giorno mar, 07/07/2020 alle 17.34 +0200, Giuseppe Sacco ha scritto:
>> Il giorno mar, 07/07/2020 alle 16.52 +0200, Christophe Leroy ha
>> scritto:
>>> Le 07/07/2020 à 16:03, Giuseppe Sacco a écrit :
>>>> Hello Cristophe,
>>>>
>>>>> Can you tell which defconfig you use or provide your .config
>>>>
>>>> You may get the standard one from debian or a reduced one that I
>>>> made on purpose. The latter is here:
>>>> https://eppesuigoccas.homedns.org/~giuseppe/config-5.4.50.gz
>>>>
>>>>   (boot)
>>>> https://eppesuigoccas.homedns.org/~giuseppe/config-5.6.19.gz
>>>>
>>>>   (no boot)
>>>
>>> Thanks
>>>
>>> Can you provide the complete output when it works, so that I can
>>> see
>>> what is after the place it stops when it fails.
>>
>> Here it is:
>> https://eppesuigoccas.homedns.org/~giuseppe/dmesg-5.4.40-minimo.gz
>>
>>
>>> And can you try without CONFIG_VMAP_STACK on 5.6.19
>>
>> Sure, I'll let you know.
> 
> No, this change did not make the kernel boot. I only changed the option
> you proposed:
> 
> $ grep VMAP .config
> CONFIG_HAVE_ARCH_VMAP_STACK=y
> # CONFIG_VMAP_STACK is not set
> 

Ok, at least it is not that. I wanted to be sure because this is a huge 
change added recently that is selected by default.

I tried on QEMU with your config, it boots properly.

So I think the only way now will be to bisect. Hope you were able to 
setup a cross compiler on a speedy machine.

Christophe

^ permalink raw reply

* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Peter Zijlstra @ 2020-07-08  8:41 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: linux-arch, Will Deacon, Boqun Feng, linux-kernel, kvm-ppc,
	virtualization, Ingo Molnar, Waiman Long, linuxppc-dev
In-Reply-To: <1594101082.hfq9x5yact.astroid@bobo.none>

On Tue, Jul 07, 2020 at 03:57:06PM +1000, Nicholas Piggin wrote:
> Yes, powerpc could certainly get more performance out of the slow
> paths, and then there are a few parameters to tune.

Can you clarify? The slow path is already in use on ARM64 which is weak,
so I doubt there's superfluous serialization present. And Will spend a
fair amount of time on making that thing guarantee forward progressm, so
there just isn't too much room to play.

> We don't have a good alternate patching for function calls yet, but
> that would be something to do for native vs pv.

Going by your jump_label implementation, support for static_call should
be fairly straight forward too, no?

  https://lkml.kernel.org/r/20200624153024.794671356@infradead.org

^ permalink raw reply

* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Giuseppe Sacco @ 2020-07-08  8:56 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <639a48d1-815b-33f1-3c9e-cd9ca8ec41b1@csgroup.eu>

Salut Cristophe,

Il giorno mer, 08/07/2020 alle 10.38 +0200, Christophe Leroy ha
scritto:
> Hi Giuseppe,
> 
> Le 08/07/2020 à 09:59, Giuseppe Sacco a écrit :
> > Hello Cristophe,
> > 
> > Il giorno mar, 07/07/2020 alle 17.34 +0200, Giuseppe Sacco ha
> > scritto:
[...]
> > > > And can you try without CONFIG_VMAP_STACK on 5.6.19
> > > 
> > > Sure, I'll let you know.
> > 
> > No, this change did not make the kernel boot. I only changed the
> > option
> > you proposed:
> > 
> > $ grep VMAP .config
> > CONFIG_HAVE_ARCH_VMAP_STACK=y
> > # CONFIG_VMAP_STACK is not set
> > 
> 
> Ok, at least it is not that. I wanted to be sure because this is a
> huge 
> change added recently that is selected by default.
> 
> I tried on QEMU with your config, it boots properly.
> 
> So I think the only way now will be to bisect. Hope you were able to 
> setup a cross compiler on a speedy machine.

thank you for your time; indeed I may now crosscompile the kernel in 15
minutes instead of about 390... I hope to have a final result in a
couple of days.

Bye,
Giuseppe


^ permalink raw reply

* [PATCH v4 1/2] powerpc/perf/hv-24x7: Add cpu hotplug support
From: Kajol Jain @ 2020-07-08  8:59 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: nathanl, ego, maddy, kjain, suka, anju
In-Reply-To: <20200708085955.655686-1-kjain@linux.ibm.com>

Patch here adds cpu hotplug functions to hv_24x7 pmu.
A new cpuhp_state "CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE" enum
is added.

The online callback function updates the cpumask only if its
empty. As the primary intention of adding hotplug support
is to designate a CPU to make HCALL to collect the
counter data.

The offline function test and clear corresponding cpu in a cpumask
and update cpumask to any other active cpu.

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 arch/powerpc/perf/hv-24x7.c | 46 +++++++++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h  |  1 +
 2 files changed, 47 insertions(+)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index db213eb7cb02..93b4700dcf8c 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -31,6 +31,8 @@ static int interface_version;
 /* Whether we have to aggregate result data for some domains. */
 static bool aggregate_result_elements;
 
+static cpumask_t hv_24x7_cpumask;
+
 static bool domain_is_valid(unsigned domain)
 {
 	switch (domain) {
@@ -1641,6 +1643,45 @@ static struct pmu h_24x7_pmu = {
 	.capabilities = PERF_PMU_CAP_NO_EXCLUDE,
 };
 
+static int ppc_hv_24x7_cpu_online(unsigned int cpu)
+{
+	if (cpumask_empty(&hv_24x7_cpumask))
+		cpumask_set_cpu(cpu, &hv_24x7_cpumask);
+
+	return 0;
+}
+
+static int ppc_hv_24x7_cpu_offline(unsigned int cpu)
+{
+	int target;
+
+	/* Check if exiting cpu is used for collecting 24x7 events */
+	if (!cpumask_test_and_clear_cpu(cpu, &hv_24x7_cpumask))
+		return 0;
+
+	/* Find a new cpu to collect 24x7 events */
+	target = cpumask_last(cpu_active_mask);
+
+	if (target < 0 || target >= nr_cpu_ids) {
+		pr_err("hv_24x7: CPU hotplug init failed\n");
+		return -1;
+	}
+
+	/* Migrate 24x7 events to the new target */
+	cpumask_set_cpu(target, &hv_24x7_cpumask);
+	perf_pmu_migrate_context(&h_24x7_pmu, cpu, target);
+
+	return 0;
+}
+
+static int hv_24x7_cpu_hotplug_init(void)
+{
+	return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE,
+			  "perf/powerpc/hv_24x7:online",
+			  ppc_hv_24x7_cpu_online,
+			  ppc_hv_24x7_cpu_offline);
+}
+
 static int hv_24x7_init(void)
 {
 	int r;
@@ -1685,6 +1726,11 @@ static int hv_24x7_init(void)
 	if (r)
 		return r;
 
+	/* init cpuhotplug */
+	r = hv_24x7_cpu_hotplug_init();
+	if (r)
+		return r;
+
 	r = perf_pmu_register(&h_24x7_pmu, h_24x7_pmu.name, -1);
 	if (r)
 		return r;
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 191772d4a4d7..a2710e654b64 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -181,6 +181,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_POWERPC_CORE_IMC_ONLINE,
 	CPUHP_AP_PERF_POWERPC_THREAD_IMC_ONLINE,
 	CPUHP_AP_PERF_POWERPC_TRACE_IMC_ONLINE,
+	CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE,
 	CPUHP_AP_WATCHDOG_ONLINE,
 	CPUHP_AP_WORKQUEUE_ONLINE,
 	CPUHP_AP_RCUTREE_ONLINE,
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 2/2] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show cpumask
From: Kajol Jain @ 2020-07-08  8:59 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: nathanl, ego, maddy, kjain, suka, anju
In-Reply-To: <20200708085955.655686-1-kjain@linux.ibm.com>

Patch here adds a cpumask attr to hv_24x7 pmu along with ABI documentation.

Primary use to expose the cpumask is for the perf tool which has the
capability to parse the driver sysfs folder and understand the
cpumask file. Having cpumask file will reduce the number of perf command
line parameters (will avoid "-C" option in the perf tool
command line). It can also notify the user which is
the current cpu used to retrieve the counter data.

command:# cat /sys/devices/hv_24x7/cpumask
0

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 .../sysfs-bus-event_source-devices-hv_24x7    |  7 ++++
 arch/powerpc/perf/hv-24x7.c                   | 33 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
index e8698afcd952..ee89d0e94602 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
@@ -43,6 +43,13 @@ Description:	read only
 		This sysfs interface exposes the number of cores per chip
 		present in the system.
 
+What:		/sys/devices/hv_24x7/cpumask
+Date:		June 2020
+Contact:	Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description:	read only
+		This sysfs file exposes the cpumask which is designated to make
+		HCALLs to retrieve hv-24x7 pmu event counter data.
+
 What:		/sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
 Date:		February 2014
 Contact:	Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 93b4700dcf8c..3f769bb2d06a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -448,6 +448,12 @@ static ssize_t device_show_string(struct device *dev,
 	return sprintf(buf, "%s\n", (char *)d->var);
 }
 
+static ssize_t cpumask_get_attr(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	return cpumap_print_to_pagebuf(true, buf, &hv_24x7_cpumask);
+}
+
 static ssize_t sockets_show(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
@@ -1116,6 +1122,17 @@ static DEVICE_ATTR_RO(sockets);
 static DEVICE_ATTR_RO(chipspersocket);
 static DEVICE_ATTR_RO(coresperchip);
 
+static DEVICE_ATTR(cpumask, S_IRUGO, cpumask_get_attr, NULL);
+
+static struct attribute *cpumask_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group cpumask_attr_group = {
+	.attrs = cpumask_attrs,
+};
+
 static struct bin_attribute *if_bin_attrs[] = {
 	&bin_attr_catalog,
 	NULL,
@@ -1143,6 +1160,11 @@ static const struct attribute_group *attr_groups[] = {
 	&event_desc_group,
 	&event_long_desc_group,
 	&if_group,
+	/*
+	 * This NULL is a placeholder for the cpumask attr which will update
+	 * onlyif cpuhotplug registration is successful
+	 */
+	NULL,
 	NULL,
 };
 
@@ -1684,7 +1706,7 @@ static int hv_24x7_cpu_hotplug_init(void)
 
 static int hv_24x7_init(void)
 {
-	int r;
+	int r, i = -1;
 	unsigned long hret;
 	struct hv_perf_caps caps;
 
@@ -1731,6 +1753,15 @@ static int hv_24x7_init(void)
 	if (r)
 		return r;
 
+	/*
+	 * Cpu hotplug init is successful, add the
+	 * cpumask file as part of pmu attr group and
+	 * assign it to very first NULL location.
+	 */
+	while (attr_groups[++i])
+		/* nothing */;
+	attr_groups[i] = &cpumask_attr_group;
+
 	r = perf_pmu_register(&h_24x7_pmu, h_24x7_pmu.name, -1);
 	if (r)
 		return r;
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 0/2] Add cpu hotplug support for powerpc/perf/hv-24x7
From: Kajol Jain @ 2020-07-08  8:59 UTC (permalink / raw)
  To: linuxppc-dev, mpe; +Cc: nathanl, ego, maddy, kjain, suka, anju

This patchset add cpu hotplug support for hv_24x7 driver by adding
online/offline cpu hotplug function. It also add sysfs file
"cpumask" to expose current online cpu that can be used for
hv_24x7 event count.

Changelog:
v3 -> v4
- Make PMU initialization fail incase hotplug init failed. Rather then
  just printing error msg.
- Did some nits changes like removing extra comment and initialising
  target value part as suggested by Michael Ellerman
- Retained Reviewd-by tag because the changes were fixes to some nits.

- Incase we sequentially offline multiple cpus, taking cpumask_first() may
  add some latency in that scenario.

  So, I was trying to test benchmark in power9 lpar with 16 cpu,
  by off-lining cpu 0-14

With cpumask_last: This is what I got.

real	0m2.812s
user	0m0.002s
sys	0m0.003s

With cpulast_any:
real	0m3.690s
user	0m0.002s
sys	0m0.062s

That's why I just went with cpumask_last thing.

v2 -> v3
- Corrected some of the typo mistakes and update commit message
  as suggested by Gautham R Shenoy.
- Added Reviewed-by tag for the first patch in the patchset.

v1 -> v2
- Changed function to pick active cpu incase of offline
  from "cpumask_any_but" to "cpumask_last", as
  cpumask_any_but function pick very next online cpu and incase where
  we are sequentially off-lining multiple cpus, "pmu_migrate_context"
  can add extra latency.
  - Suggested by: Gautham R Shenoy.

- Change documentation for cpumask and rather then hardcode the
  initialization for cpumask_attr_group, add loop to get very first
  NULL as suggested by Gautham R Shenoy.

Kajol Jain (2):
  powerpc/perf/hv-24x7: Add cpu hotplug support
  powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show cpumask

 .../sysfs-bus-event_source-devices-hv_24x7    |  7 ++
 arch/powerpc/perf/hv-24x7.c                   | 79 ++++++++++++++++++-
 include/linux/cpuhotplug.h                    |  1 +
 3 files changed, 86 insertions(+), 1 deletion(-)

-- 
2.26.2


^ permalink raw reply

* Re: [PATCH v4 2/2] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show cpumask
From: Madhavan Srinivasan @ 2020-07-08  9:34 UTC (permalink / raw)
  To: Kajol Jain, linuxppc-dev, mpe; +Cc: nathanl, ego, suka, maddy, anju
In-Reply-To: <20200708085955.655686-3-kjain@linux.ibm.com>



On 7/8/20 2:29 PM, Kajol Jain wrote:
> Patch here adds a cpumask attr to hv_24x7 pmu along with ABI documentation.
>
> Primary use to expose the cpumask is for the perf tool which has the
> capability to parse the driver sysfs folder and understand the
> cpumask file. Having cpumask file will reduce the number of perf command
> line parameters (will avoid "-C" option in the perf tool
> command line). It can also notify the user which is
> the current cpu used to retrieve the counter data.
>
> command:# cat /sys/devices/hv_24x7/cpumask
> 0
>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>   .../sysfs-bus-event_source-devices-hv_24x7    |  7 ++++
>   arch/powerpc/perf/hv-24x7.c                   | 33 ++++++++++++++++++-
>   2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
> index e8698afcd952..ee89d0e94602 100644
> --- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
> +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
> @@ -43,6 +43,13 @@ Description:	read only
>   		This sysfs interface exposes the number of cores per chip
>   		present in the system.
>
> +What:		/sys/devices/hv_24x7/cpumask
> +Date:		June 2020
> +Contact:	Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
> +Description:	read only
> +		This sysfs file exposes the cpumask which is designated to make
> +		HCALLs to retrieve hv-24x7 pmu event counter data.
> +
>   What:		/sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
>   Date:		February 2014
>   Contact:	Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 93b4700dcf8c..3f769bb2d06a 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -448,6 +448,12 @@ static ssize_t device_show_string(struct device *dev,
>   	return sprintf(buf, "%s\n", (char *)d->var);
>   }
>
> +static ssize_t cpumask_get_attr(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	return cpumap_print_to_pagebuf(true, buf, &hv_24x7_cpumask);
> +}
> +
>   static ssize_t sockets_show(struct device *dev,
>   			    struct device_attribute *attr, char *buf)
>   {
> @@ -1116,6 +1122,17 @@ static DEVICE_ATTR_RO(sockets);
>   static DEVICE_ATTR_RO(chipspersocket);
>   static DEVICE_ATTR_RO(coresperchip);
>
> +static DEVICE_ATTR(cpumask, S_IRUGO, cpumask_get_attr, NULL);
> +
> +static struct attribute *cpumask_attrs[] = {
> +	&dev_attr_cpumask.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group cpumask_attr_group = {
> +	.attrs = cpumask_attrs,
> +};
> +
>   static struct bin_attribute *if_bin_attrs[] = {
>   	&bin_attr_catalog,
>   	NULL,
> @@ -1143,6 +1160,11 @@ static const struct attribute_group *attr_groups[] = {
>   	&event_desc_group,
>   	&event_long_desc_group,
>   	&if_group,
> +	/*
> +	 * This NULL is a placeholder for the cpumask attr which will update
> +	 * onlyif cpuhotplug registration is successful
> +	 */
> +	NULL,
>   	NULL,
>   };
>
> @@ -1684,7 +1706,7 @@ static int hv_24x7_cpu_hotplug_init(void)
>
>   static int hv_24x7_init(void)
>   {
> -	int r;
> +	int r, i = -1;
>   	unsigned long hret;
>   	struct hv_perf_caps caps;
>
> @@ -1731,6 +1753,15 @@ static int hv_24x7_init(void)
>   	if (r)
>   		return r;
>
> +	/*
> +	 * Cpu hotplug init is successful, add the
> +	 * cpumask file as part of pmu attr group and
> +	 * assign it to very first NULL location.
> +	 */
> +	while (attr_groups[++i])
> +		/* nothing */;
> +	attr_groups[i] = &cpumask_attr_group;
> +

We can avoid this complex stuff right.
Now that if the cpuhotplug init fail, we fail the pmu
registration right, with that, we dont need this dance.

Cant we just add the cpumask_attr_group right next to if_group
in "attr_group"?

Maddy

>   	r = perf_pmu_register(&h_24x7_pmu, h_24x7_pmu.name, -1);
>   	if (r)
>   		return r;


^ 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