* 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
* Re: [PATCH] selftests/powerpc: Purge extra count_pmc() calls of ebb selftests
From: Sachin Sant @ 2020-07-08 10:38 UTC (permalink / raw)
To: Desnes A. Nunes do Rosario; +Cc: shuah, linuxppc-dev
In-Reply-To: <20200626164737.21943-1-desnesn@linux.ibm.com>
> On 26-Jun-2020, at 10:17 PM, Desnes A. Nunes do Rosario <desnesn@linux.ibm.com> wrote:
>
> An extra count on ebb_state.stats.pmc_count[PMC_INDEX(pmc)] is being per-
> formed when count_pmc() is used to reset PMCs on a few selftests. This
> extra pmc_count can occasionally invalidate results, such as the ones from
> cycles_test shown hereafter. The ebb_check_count() failed with an above
> the upper limit error due to the extra value on ebb_state.stats.pmc_count.
>
> Furthermore, this extra count is also indicated by extra PMC1 trace_log on
> the output of the cycle test (as well as on pmc56_overflow_test):
>
> ==========
> ...
> [21]: counter = 8
> [22]: register SPRN_MMCR0 = 0x0000000080000080
> [23]: register SPRN_PMC1 = 0x0000000080000004
> [24]: counter = 9
> [25]: register SPRN_MMCR0 = 0x0000000080000080
> [26]: register SPRN_PMC1 = 0x0000000080000004
> [27]: counter = 10
> [28]: register SPRN_MMCR0 = 0x0000000080000080
> [29]: register SPRN_PMC1 = 0x0000000080000004
>>> [30]: register SPRN_PMC1 = 0x000000004000051e
> PMC1 count (0x280000546) above upper limit 0x2800003e8 (+0x15e)
> [FAIL] Test FAILED on line 52
> failure: cycles
> ==========
>
> Signed-off-by: Desnes A. Nunes do Rosario <desnesn@linux.ibm.com>
> ---
I too have run into similar failure with cycles_test. I will add that the failure
is inconsistent. I have run into this issue 1 out of 25 times. The failure always
happen at first instance. Subsequent tries work correctly.
With this patch applied the test completes successfully 25 out of 25 times.
# ./cycles_test
test: cycles
…..
…..
[25]: register SPRN_MMCR0 = 0x0000000080000080
[26]: register SPRN_PMC1 = 0x0000000080000004
[27]: counter = 10
[28]: register SPRN_MMCR0 = 0x0000000080000080
[29]: register SPRN_PMC1 = 0x0000000080000004
[30]: register SPRN_PMC1 = 0x000000004000048f
PMC1 count (0x2800004b7) above upper limit 0x2800003e8 (+0xcf)
[FAIL] Test FAILED on line 52
failure: cycles
With the patch
# ./cycles_test
test: cycles
…..
…..
[25]: register SPRN_MMCR0 = 0x0000000080000080
[26]: register SPRN_PMC1 = 0x0000000080000004
[27]: counter = 10
[28]: register SPRN_MMCR0 = 0x0000000080000080
[29]: register SPRN_PMC1 = 0x0000000080000004
PMC1 count (0x280000028) is between 0x27ffffc18 and 0x2800003e8 delta +0x410/-0x3c0
success: cycles
#
FWIW Tested-by : Sachin Sant <sachinp@linux.vnet.ibm.com>
Thanks
-Sachin
^ permalink raw reply
* Re: [PATCH v2 06/10] powerpc/perf: power10 Performance Monitoring support
From: Athira Rajeev @ 2020-07-08 10:56 UTC (permalink / raw)
To: Michael Neuling; +Cc: maddy, linuxppc-dev
In-Reply-To: <babc98c50100bb2cc925a6518bdb885909f0c473.camel@neuling.org>
> On 07-Jul-2020, at 12:20 PM, Michael Neuling <mikey@neuling.org> wrote:
>
>
>> @@ -480,6 +520,7 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>> mmcr[1] = mmcr1;
>> mmcr[2] = mmcra;
>> mmcr[3] = mmcr2;
>> + mmcr[4] = mmcr3;
>
> This is fragile like the kvm vcpu case I commented on before but it gets passed
> in via a function parameter?! Can you create a struct to store these in rather
> than this odd ball numbering?
Mikey,
Yes, it gets passed as cpuhw->mmcr array
I will check on these cleanup changes for the kvm vcpu case as well as cpu_hw_events mmcr array
Thanks
Athira
>
> The cleanup should start in patch 1/10 here:
>
> /*
> * The order of the MMCR array is:
> - * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
> + * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3
> * - 32-bit, MMCR0, MMCR1, MMCR2
> */
> - unsigned long mmcr[4];
> + unsigned long mmcr[5];
>
>
>
> mikey
^ permalink raw reply
* Re: [PATCH v2 01/10] powerpc/perf: Add support for ISA3.1 PMU SPRs
From: Michael Ellerman @ 2020-07-08 11:02 UTC (permalink / raw)
To: Athira Rajeev; +Cc: mikey, maddy, linuxppc-dev
In-Reply-To: <1593595262-1433-2-git-send-email-atrajeev@linux.vnet.ibm.com>
Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
...
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index cd6a742..5c64bd3 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -39,10 +39,10 @@ struct cpu_hw_events {
> unsigned int flags[MAX_HWEVENTS];
> /*
> * The order of the MMCR array is:
> - * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
> + * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3
> * - 32-bit, MMCR0, MMCR1, MMCR2
> */
> - unsigned long mmcr[4];
> + unsigned long mmcr[5];
> struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];
> u8 limited_hwidx[MAX_LIMITED_HWCOUNTERS];
> u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
...
> @@ -1310,6 +1326,10 @@ static void power_pmu_enable(struct pmu *pmu)
> if (!cpuhw->n_added) {
> mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
> mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
> +#ifdef CONFIG_PPC64
> + if (ppmu->flags & PPMU_ARCH_310S)
> + mtspr(SPRN_MMCR3, cpuhw->mmcr[4]);
> +#endif /* CONFIG_PPC64 */
> goto out_enable;
> }
>
> @@ -1353,6 +1373,11 @@ static void power_pmu_enable(struct pmu *pmu)
> if (ppmu->flags & PPMU_ARCH_207S)
> mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);
>
> +#ifdef CONFIG_PPC64
> + if (ppmu->flags & PPMU_ARCH_310S)
> + mtspr(SPRN_MMCR3, cpuhw->mmcr[4]);
> +#endif /* CONFIG_PPC64 */
I don't think you need the #ifdef CONFIG_PPC64?
cheers
^ permalink raw reply
* Re: [PATCH v2 03/10] powerpc/xmon: Add PowerISA v3.1 PMU SPRs
From: Michael Ellerman @ 2020-07-08 11:04 UTC (permalink / raw)
To: Athira Rajeev; +Cc: mikey, maddy, linuxppc-dev
In-Reply-To: <1593595262-1433-4-git-send-email-atrajeev@linux.vnet.ibm.com>
Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
> From: Madhavan Srinivasan <maddy@linux.ibm.com>
>
> PowerISA v3.1 added three new perfromance
> monitoring unit (PMU) speical purpose register (SPR).
> They are Monitor Mode Control Register 3 (MMCR3),
> Sampled Instruction Event Register 2 (SIER2),
> Sampled Instruction Event Register 3 (SIER3).
>
> Patch here adds a new dump function dump_310_sprs
> to print these SPR values.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> ---
> arch/powerpc/xmon/xmon.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 7efe4bc..8917fe8 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2022,6 +2022,20 @@ static void dump_300_sprs(void)
> #endif
> }
>
> +static void dump_310_sprs(void)
> +{
> +#ifdef CONFIG_PPC64
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + return;
> +
> + printf("mmcr3 = %.16lx\n",
> + mfspr(SPRN_MMCR3));
> +
> + printf("sier2 = %.16lx sier3 = %.16lx\n",
> + mfspr(SPRN_SIER2), mfspr(SPRN_SIER3));
Why not all on one line like many of the others?
cheers
^ permalink raw reply
* Re: [PATCH v2 04/10] powerpc/perf: Add power10_feat to dt_cpu_ftrs
From: Michael Ellerman @ 2020-07-08 11:15 UTC (permalink / raw)
To: Athira Rajeev; +Cc: mikey, maddy, linuxppc-dev
In-Reply-To: <1593595262-1433-5-git-send-email-atrajeev@linux.vnet.ibm.com>
Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
> From: Madhavan Srinivasan <maddy@linux.ibm.com>
>
> Add power10 feature function to dt_cpu_ftrs.c along
> with a power10 specific init() to initialize pmu sprs.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> ---
> arch/powerpc/include/asm/reg.h | 3 +++
> arch/powerpc/kernel/cpu_setup_power.S | 7 +++++++
> arch/powerpc/kernel/dt_cpu_ftrs.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 21a1b2d..900ada1 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -1068,6 +1068,9 @@
> #define MMCR0_PMC2_LOADMISSTIME 0x5
> #endif
>
> +/* BHRB disable bit for PowerISA v3.10 */
> +#define MMCRA_BHRB_DISABLE 0x0000002000000000
> +
> /*
> * SPRG usage:
> *
> diff --git a/arch/powerpc/kernel/cpu_setup_power.S b/arch/powerpc/kernel/cpu_setup_power.S
> index efdcfa7..e8b3370c 100644
> --- a/arch/powerpc/kernel/cpu_setup_power.S
> +++ b/arch/powerpc/kernel/cpu_setup_power.S
> @@ -233,3 +233,10 @@ __init_PMU_ISA207:
> li r5,0
> mtspr SPRN_MMCRS,r5
> blr
> +
> +__init_PMU_ISA31:
> + li r5,0
> + mtspr SPRN_MMCR3,r5
> + LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE)
> + mtspr SPRN_MMCRA,r5
> + blr
This doesn't seem like it belongs in this patch. It's not called?
cheers
^ permalink raw reply
* [PATCH] arch: powerpc: Remove unnecessary cast in kfree()
From: Xu Wang @ 2020-07-08 7:22 UTC (permalink / raw)
To: mpe, benh, paulus, vulab, linuxppc-dev; +Cc: linux-kernel
Remove unnecassary casts in the argument to kfree.
Signed-off-by: Xu Wang <vulab@iscas.ac.cn>
---
arch/powerpc/platforms/pseries/dlpar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 16e86ba8aa20..1f3d26806295 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -379,7 +379,7 @@ static void pseries_hp_work_fn(struct work_struct *work)
handle_dlpar_errorlog(hp_work->errlog);
kfree(hp_work->errlog);
- kfree((void *)work);
+ kfree(work);
}
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 07/10] powerpc/perf: support BHRB disable bit and new filtering modes
From: Athira Rajeev @ 2020-07-08 7:41 UTC (permalink / raw)
To: Michael Neuling; +Cc: Vaidyanathan Srinivasan, maddy, linuxppc-dev, ego
In-Reply-To: <0cf26e42a3b190d5ea69d1ba61ae71bcaeee1973.camel@neuling.org>
[-- Attachment #1: Type: text/plain, Size: 8382 bytes --]
> On 07-Jul-2020, at 12:47 PM, Michael Neuling <mikey@neuling.org> 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).
>
Sure Mikey, will add these information in commit message
Thanks
Athira
>> 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(-)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index fad5159..9709606 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -466,9 +466,13 @@ static void power_pmu_bhrb_read(struct perf_event *event,
>> struct cpu_hw_events *
>> * addresses at this point. Check the privileges before
>> * exporting it to userspace (avoid exposure of regions
>> * where we could have speculative execution)
>> + * Incase of ISA 310, BHRB will capture only user-space
>> + * address,hence include a check before filtering code
>> */
>> - if (is_kernel_addr(addr) && perf_allow_kernel(&event-
>>> attr) != 0)
>> - continue;
>> + if (!(ppmu->flags & PPMU_ARCH_310S))
>> + if (is_kernel_addr(addr) &&
>> + perf_allow_kernel(&event->attr) != 0)
>> + continue;
>>
>> /* Branches are read most recent first (ie. mfbhrb 0 is
>> * the most recent branch).
>> @@ -1212,7 +1216,7 @@ static void write_mmcr0(struct cpu_hw_events *cpuhw,
>> unsigned long mmcr0)
>> static void power_pmu_disable(struct pmu *pmu)
>> {
>> struct cpu_hw_events *cpuhw;
>> - unsigned long flags, mmcr0, val;
>> + unsigned long flags, mmcr0, val, mmcra = 0;
>>
>> if (!ppmu)
>> return;
>> @@ -1245,12 +1249,23 @@ static void power_pmu_disable(struct pmu *pmu)
>> mb();
>> isync();
>>
>> + val = mmcra = cpuhw->mmcr[2];
>> +
>> /*
>> * Disable instruction sampling if it was enabled
>> */
>> - if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
>> - mtspr(SPRN_MMCRA,
>> - cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
>> + if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE)
>> + mmcra = cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE;
>> +
>> + /* Disable BHRB via mmcra [:26] for p10 if needed */
>> + if (!(cpuhw->mmcr[2] & MMCRA_BHRB_DISABLE))
>> + mmcra |= MMCRA_BHRB_DISABLE;
>> +
>> + /* Write SPRN_MMCRA if mmcra has either disabled
>> + * instruction sampling or BHRB
>> + */
>> + if (val != mmcra) {
>> + mtspr(SPRN_MMCRA, mmcra);
>> mb();
>> isync();
>> }
>> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-
>> common.c
>> index 7d4839e..463d925 100644
>> --- a/arch/powerpc/perf/isa207-common.c
>> +++ b/arch/powerpc/perf/isa207-common.c
>> @@ -404,6 +404,12 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>>
>> mmcra = mmcr1 = mmcr2 = mmcr3 = 0;
>>
>> + /* Disable bhrb unless explicitly requested
>> + * by setting MMCRA [:26] bit.
>> + */
>> + if (cpu_has_feature(CPU_FTR_ARCH_31))
>> + mmcra |= MMCRA_BHRB_DISABLE;
>> +
>> /* Second pass: assign PMCs, set all MMCR1 fields */
>> for (i = 0; i < n_ev; ++i) {
>> pmc = (event[i] >> EVENT_PMC_SHIFT) & EVENT_PMC_MASK;
>> @@ -475,10 +481,17 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>> }
>>
>> if (event[i] & EVENT_WANTS_BHRB) {
>> + /* set MMCRA[:26] to 0 for Power10 to enable BHRB */
>> + if (cpu_has_feature(CPU_FTR_ARCH_31))
>> + mmcra &= ~MMCRA_BHRB_DISABLE;
>> val = (event[i] >> EVENT_IFM_SHIFT) & EVENT_IFM_MASK;
>> mmcra |= val << MMCRA_IFM_SHIFT;
>> }
>>
>> + /* set MMCRA[:26] to 0 if there is user request for BHRB */
>> + if (cpu_has_feature(CPU_FTR_ARCH_31) &&
>> has_branch_stack(pevents[i]))
>> + mmcra &= ~MMCRA_BHRB_DISABLE;
>> +
>> if (pevents[i]->attr.exclude_user)
>> mmcr2 |= MMCR2_FCP(pmc);
>>
>> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
>> index d64d69d..07fb919 100644
>> --- a/arch/powerpc/perf/power10-pmu.c
>> +++ b/arch/powerpc/perf/power10-pmu.c
>> @@ -82,6 +82,8 @@
>>
>> /* MMCRA IFM bits - POWER10 */
>> #define POWER10_MMCRA_IFM1 0x0000000040000000UL
>> +#define POWER10_MMCRA_IFM2 0x0000000080000000UL
>> +#define POWER10_MMCRA_IFM3 0x00000000C0000000UL
>> #define POWER10_MMCRA_BHRB_MASK 0x00000000C0000000UL
>>
>> /* Table of alternatives, sorted by column 0 */
>> @@ -233,8 +235,15 @@ static u64 power10_bhrb_filter_map(u64
>> branch_sample_type)
>> if (branch_sample_type & PERF_SAMPLE_BRANCH_ANY_RETURN)
>> return -1;
>>
>> - if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL)
>> - return -1;
>> + if (branch_sample_type & PERF_SAMPLE_BRANCH_IND_CALL) {
>> + pmu_bhrb_filter |= POWER10_MMCRA_IFM2;
>> + return pmu_bhrb_filter;
>> + }
>> +
>> + if (branch_sample_type & PERF_SAMPLE_BRANCH_COND) {
>> + pmu_bhrb_filter |= POWER10_MMCRA_IFM3;
>> + return pmu_bhrb_filter;
>> + }
>>
>> if (branch_sample_type & PERF_SAMPLE_BRANCH_CALL)
>> return -1;
>> 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;
>> 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.
>> + */
>> + mmcra_bhrb = mfspr(SPRN_MMCRA);
>
>
> Why is the bhrb bit of mmcra special here?
This to save/restore BHRB disable bit in state-loss idle state to make sure
we keep BHRB disabled if it was not enabled on request at runtime.
>
>> + }
>> +
>> 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.
[-- Attachment #2: Type: text/html, Size: 37042 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox