* RE: [PATCH 06/12] openrisc: Use of_get_cpu_hwid()
From: David Laight @ 2021-10-07 7:53 UTC (permalink / raw)
To: 'Segher Boessenkool', Stafford Horne
Cc: Rich Felker, Rafael J. Wysocki, Catalin Marinas, x86@kernel.org,
Guo Ren, H. Peter Anvin, linux-riscv@lists.infradead.org,
Frank Rowand, Jonas Bonn, Rob Herring, Florian Fainelli,
Will Deacon, linux-sh@vger.kernel.org, Russell King,
linux-csky@vger.kernel.org, Ingo Molnar,
bcm-kernel-feedback-list@broadcom.com, Palmer Dabbelt,
devicetree@vger.kernel.org, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc@lists.librecores.org,
Borislav Petkov, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel@lists.infradead.org, Scott Branden,
Yoshinori Sato, linux-kernel@vger.kernel.org, James Morse,
Greg Kroah-Hartman, Paul Mackerras, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20211006212728.GM10333@gate.crashing.org>
From: Segher Boessenkool
> Sent: 06 October 2021 22:27
>
> On Thu, Oct 07, 2021 at 05:44:00AM +0900, Stafford Horne wrote:
> > You have defined of_get_cpu_hwid to return u64, will this create compiler
> > warnings when since we are storing a u64 into a u32?
> >
> > It seems only if we make with W=3.
>
> Yes. This is done by -Wconversion, "Warn for implicit conversions that
> may alter a value."
>
> > I thought we usually warned on this.
The microsoft compiler does - best to turn all those warnings off.
> This warning is not in -Wall or -Wextra either, it suffers too much from
> false positives. It is very natural to just ignore the high bits of
> modulo types (which is what "unsigned" types *are*). Or the bits that
> "fall off" on a conversion. The C standard makes this required
> behaviour, it is useful, and it is the only convenient way of getting
> this!
I've also seen a compiler convert:
struct->char_member = (char)(int_val & 0xff);
into:
reg = int_val;
reg &= 0xff; // for the & 0xff
reg &= 0xff; // for the cast
struct->char_member = low_8bits(reg);
You really don't want the extra noise.
I'll bet that (char)int_val is actually an arithmetic expression.
So its type will be 'int'.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 04/12] arm64: Use of_get_cpu_hwid()
From: Will Deacon @ 2021-10-07 8:07 UTC (permalink / raw)
To: Rob Herring
Cc: Rich Felker, Rafael J. Wysocki, Guo Ren, H. Peter Anvin,
linux-riscv, Frank Rowand, Stafford Horne, Jonas Bonn,
Florian Fainelli, Yoshinori Sato, linux-sh, x86, Russell King,
linux-csky, Ingo Molnar, bcm-kernel-feedback-list,
Catalin Marinas, Palmer Dabbelt, devicetree, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc, Borislav Petkov, Paul Walmsley,
Thomas Gleixner, linux-arm-kernel, Scott Branden,
Greg Kroah-Hartman, linux-kernel, James Morse, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20211006164332.1981454-5-robh@kernel.org>
On Wed, Oct 06, 2021 at 11:43:24AM -0500, Rob Herring wrote:
> Replace the open coded parsing of CPU nodes' 'reg' property with
> of_get_cpu_hwid().
>
> This change drops an error message for missing 'reg' property, but that
> should not be necessary as the DT tools will ensure 'reg' is present.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> arch/arm64/kernel/smp.c | 31 ++-----------------------------
> 1 file changed, 2 insertions(+), 29 deletions(-)
Acked-by: Will Deacon <will@kernel.org>
It's a shame INVALID_HWID can't be removed too, but looks like it's still
used in a couple of places.
Will
^ permalink raw reply
* Re: [PATCH v1 1/6] mm/memory_hotplug: remove CONFIG_X86_64_ACPI_NUMA dependency from CONFIG_MEMORY_HOTPLUG
From: Oscar Salvador @ 2021-10-07 8:09 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-2-david@redhat.com>
On Wed, Sep 29, 2021 at 04:35:55PM +0200, David Hildenbrand wrote:
> SPARSEMEM is the only possible memory model for x86-64, FLATMEM is not
> possible:
> config ARCH_FLATMEM_ENABLE
> def_bool y
> depends on X86_32 && !NUMA
>
> And X86_64_ACPI_NUMA (obviously) only supports x86-64:
> config X86_64_ACPI_NUMA
> def_bool y
> depends on X86_64 && NUMA && ACPI && PCI
>
> Let's just remove the CONFIG_X86_64_ACPI_NUMA dependency, as it does no
> longer make sense.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> ---
> mm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index d16ba9249bc5..b7fb3f0b485e 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -123,7 +123,7 @@ config ARCH_ENABLE_MEMORY_HOTPLUG
> config MEMORY_HOTPLUG
> bool "Allow for memory hot-add"
> select MEMORY_ISOLATION
> - depends on SPARSEMEM || X86_64_ACPI_NUMA
> + depends on SPARSEMEM
> depends on ARCH_ENABLE_MEMORY_HOTPLUG
> depends on 64BIT || BROKEN
> select NUMA_KEEP_MEMINFO if NUMA
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 2/6] mm/memory_hotplug: remove CONFIG_MEMORY_HOTPLUG_SPARSE
From: Oscar Salvador @ 2021-10-07 8:20 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-3-david@redhat.com>
On Wed, Sep 29, 2021 at 04:35:56PM +0200, David Hildenbrand wrote:
> CONFIG_MEMORY_HOTPLUG depends on CONFIG_SPARSEMEM, so there is no need for
> CONFIG_MEMORY_HOTPLUG_SPARSE anymore; adjust all instances to use
> CONFIG_MEMORY_HOTPLUG and remove CONFIG_MEMORY_HOTPLUG_SPARSE.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Oscar Salvador <osalvador@suse.de>
> ---
> arch/powerpc/include/asm/machdep.h | 2 +-
> arch/powerpc/kernel/setup_64.c | 2 +-
> arch/powerpc/platforms/powernv/setup.c | 4 ++--
> arch/powerpc/platforms/pseries/setup.c | 2 +-
> drivers/base/Makefile | 2 +-
> drivers/base/node.c | 9 ++++-----
> drivers/virtio/Kconfig | 2 +-
> include/linux/memory.h | 18 +++++++-----------
> include/linux/node.h | 4 ++--
> lib/Kconfig.debug | 2 +-
> mm/Kconfig | 4 ----
> mm/memory_hotplug.c | 2 --
> tools/testing/selftests/memory-hotplug/config | 1 -
> 13 files changed, 21 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
> index 764f2732a821..d8a2ca007082 100644
> --- a/arch/powerpc/include/asm/machdep.h
> +++ b/arch/powerpc/include/asm/machdep.h
> @@ -32,7 +32,7 @@ struct machdep_calls {
> void (*iommu_save)(void);
> void (*iommu_restore)(void);
> #endif
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> unsigned long (*memory_block_size)(void);
> #endif
> #endif /* CONFIG_PPC64 */
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index eaa79a0996d1..21f15d82f062 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -912,7 +912,7 @@ void __init setup_per_cpu_areas(void)
> }
> #endif
>
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> unsigned long memory_block_size_bytes(void)
> {
> if (ppc_md.memory_block_size)
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index a8db3f153063..ad56a54ac9c5 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -440,7 +440,7 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
> }
> #endif /* CONFIG_KEXEC_CORE */
>
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> static unsigned long pnv_memory_block_size(void)
> {
> /*
> @@ -553,7 +553,7 @@ define_machine(powernv) {
> #ifdef CONFIG_KEXEC_CORE
> .kexec_cpu_down = pnv_kexec_cpu_down,
> #endif
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> .memory_block_size = pnv_memory_block_size,
> #endif
> };
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index f79126f16258..d29f6f1f7f37 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -1089,7 +1089,7 @@ define_machine(pseries) {
> .machine_kexec = pSeries_machine_kexec,
> .kexec_cpu_down = pseries_kexec_cpu_down,
> #endif
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> .memory_block_size = pseries_memory_block_size,
> #endif
> };
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index ef8e44a7d288..02f7f1358e86 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -13,7 +13,7 @@ obj-y += power/
> obj-$(CONFIG_ISA_BUS_API) += isa.o
> obj-y += firmware_loader/
> obj-$(CONFIG_NUMA) += node.o
> -obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
> +obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
> ifeq ($(CONFIG_SYSFS),y)
> obj-$(CONFIG_MODULES) += module.o
> endif
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index c56d34f8158f..b5a4ba18f9f9 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -629,7 +629,7 @@ static void node_device_release(struct device *dev)
> {
> struct node *node = to_node(dev);
>
> -#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
> +#if defined(CONFIG_MEMORY_HOTPLUG) && defined(CONFIG_HUGETLBFS)
> /*
> * We schedule the work only when a memory section is
> * onlined/offlined on this node. When we come here,
> @@ -782,7 +782,7 @@ int unregister_cpu_under_node(unsigned int cpu, unsigned int nid)
> return 0;
> }
>
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifdef CONFIG_MEMORY_HOTPLUG
> static int __ref get_nid_for_pfn(unsigned long pfn)
> {
> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> @@ -958,10 +958,9 @@ static int node_memory_callback(struct notifier_block *self,
> return NOTIFY_OK;
> }
> #endif /* CONFIG_HUGETLBFS */
> -#endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
> +#endif /* CONFIG_MEMORY_HOTPLUG */
>
> -#if !defined(CONFIG_MEMORY_HOTPLUG_SPARSE) || \
> - !defined(CONFIG_HUGETLBFS)
> +#if !defined(CONFIG_MEMORY_HOTPLUG) || !defined(CONFIG_HUGETLBFS)
> static inline int node_memory_callback(struct notifier_block *self,
> unsigned long action, void *arg)
> {
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index ce1b3f6ec325..3654def9915c 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -98,7 +98,7 @@ config VIRTIO_MEM
> default m
> depends on X86_64
> depends on VIRTIO
> - depends on MEMORY_HOTPLUG_SPARSE
> + depends on MEMORY_HOTPLUG
> depends on MEMORY_HOTREMOVE
> depends on CONTIG_ALLOC
> help
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index 7efc0a7c14c9..dd6e608c3e0b 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -110,7 +110,7 @@ struct mem_section;
> #define SLAB_CALLBACK_PRI 1
> #define IPC_CALLBACK_PRI 10
>
> -#ifndef CONFIG_MEMORY_HOTPLUG_SPARSE
> +#ifndef CONFIG_MEMORY_HOTPLUG
> static inline void memory_dev_init(void)
> {
> return;
> @@ -126,7 +126,11 @@ static inline int memory_notify(unsigned long val, void *v)
> {
> return 0;
> }
> -#else
> +#define hotplug_memory_notifier(fn, pri) ({ 0; })
> +/* These aren't inline functions due to a GCC bug. */
> +#define register_hotmemory_notifier(nb) ({ (void)(nb); 0; })
> +#define unregister_hotmemory_notifier(nb) ({ (void)(nb); })
> +#else /* CONFIG_MEMORY_HOTPLUG */
> extern int register_memory_notifier(struct notifier_block *nb);
> extern void unregister_memory_notifier(struct notifier_block *nb);
> int create_memory_block_devices(unsigned long start, unsigned long size,
> @@ -149,9 +153,6 @@ struct memory_group *memory_group_find_by_id(int mgid);
> typedef int (*walk_memory_groups_func_t)(struct memory_group *, void *);
> int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t func,
> struct memory_group *excluded, void *arg);
> -#endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
> -
> -#ifdef CONFIG_MEMORY_HOTPLUG
> #define hotplug_memory_notifier(fn, pri) ({ \
> static __meminitdata struct notifier_block fn##_mem_nb =\
> { .notifier_call = fn, .priority = pri };\
> @@ -159,12 +160,7 @@ int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t func,
> })
> #define register_hotmemory_notifier(nb) register_memory_notifier(nb)
> #define unregister_hotmemory_notifier(nb) unregister_memory_notifier(nb)
> -#else
> -#define hotplug_memory_notifier(fn, pri) ({ 0; })
> -/* These aren't inline functions due to a GCC bug. */
> -#define register_hotmemory_notifier(nb) ({ (void)(nb); 0; })
> -#define unregister_hotmemory_notifier(nb) ({ (void)(nb); })
> -#endif
> +#endif /* CONFIG_MEMORY_HOTPLUG */
>
> /*
> * Kernel text modification mutex, used for code patching. Users of this lock
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 8e5a29897936..bb21fd631b16 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -85,7 +85,7 @@ struct node {
> struct device dev;
> struct list_head access_list;
>
> -#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
> +#if defined(CONFIG_MEMORY_HOTPLUG) && defined(CONFIG_HUGETLBFS)
> struct work_struct node_work;
> #endif
> #ifdef CONFIG_HMEM_REPORTING
> @@ -98,7 +98,7 @@ struct memory_block;
> extern struct node *node_devices[];
> typedef void (*node_registration_func_t)(struct node *);
>
> -#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
> +#if defined(CONFIG_MEMORY_HOTPLUG) && defined(CONFIG_NUMA)
> void link_mem_sections(int nid, unsigned long start_pfn,
> unsigned long end_pfn,
> enum meminit_context context);
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 2a9b6dcdac4f..669fee1d26b8 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -877,7 +877,7 @@ config DEBUG_MEMORY_INIT
>
> config MEMORY_NOTIFIER_ERROR_INJECT
> tristate "Memory hotplug notifier error injection module"
> - depends on MEMORY_HOTPLUG_SPARSE && NOTIFIER_ERROR_INJECTION
> + depends on MEMORY_HOTPLUG && NOTIFIER_ERROR_INJECTION
> help
> This option provides the ability to inject artificial errors to
> memory hotplug notifier chain callbacks. It is controlled through
> diff --git a/mm/Kconfig b/mm/Kconfig
> index b7fb3f0b485e..ea8762cd8e1e 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -128,10 +128,6 @@ config MEMORY_HOTPLUG
> depends on 64BIT || BROKEN
> select NUMA_KEEP_MEMINFO if NUMA
>
> -config MEMORY_HOTPLUG_SPARSE
> - def_bool y
> - depends on SPARSEMEM && MEMORY_HOTPLUG
> -
> config MEMORY_HOTPLUG_DEFAULT_ONLINE
> bool "Online the newly added memory blocks by default"
> depends on MEMORY_HOTPLUG
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 9fd0be32a281..8d7b2b593a26 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -220,7 +220,6 @@ static void release_memory_resource(struct resource *res)
> kfree(res);
> }
>
> -#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> const char *reason)
> {
> @@ -1163,7 +1162,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
> mem_hotplug_done();
> return ret;
> }
> -#endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
>
> static void reset_node_present_pages(pg_data_t *pgdat)
> {
> diff --git a/tools/testing/selftests/memory-hotplug/config b/tools/testing/selftests/memory-hotplug/config
> index a7e8cd5bb265..1eef042a31e1 100644
> --- a/tools/testing/selftests/memory-hotplug/config
> +++ b/tools/testing/selftests/memory-hotplug/config
> @@ -1,5 +1,4 @@
> CONFIG_MEMORY_HOTPLUG=y
> -CONFIG_MEMORY_HOTPLUG_SPARSE=y
> CONFIG_NOTIFIER_ERROR_INJECTION=y
> CONFIG_MEMORY_NOTIFIER_ERROR_INJECT=m
> CONFIG_MEMORY_HOTREMOVE=y
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 04/10] powerpc/bpf: Fix BPF_SUB when imm == 0x80000000
From: Naveen N. Rao @ 2021-10-07 8:47 UTC (permalink / raw)
To: Alexei Starovoitov, Christophe Leroy, Daniel Borkmann,
Jordan Niethe, Johan Almbladh, Michael Ellerman, Nicholas Piggin,
Song Liu
Cc: bpf, linuxppc-dev
In-Reply-To: <90494652-7551-7ecb-e44d-a2adbb6a1afe@csgroup.eu>
Christophe Leroy wrote:
>
>
> Le 05/10/2021 à 22:25, Naveen N. Rao a écrit :
>> We aren't handling subtraction involving an immediate value of
>> 0x80000000 properly. Fix the same.
>>
>> Fixes: 156d0e290e969c ("powerpc/ebpf/jit: Implement JIT compiler for extended BPF")
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> Changelog:
>> - Split up BPF_ADD and BPF_SUB cases per Christophe's comments
>>
>> arch/powerpc/net/bpf_jit_comp64.c | 27 +++++++++++++++++----------
>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
>> index d67f6d62e2e1ff..6626e6c17d4ed2 100644
>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
>> @@ -330,18 +330,25 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
>> EMIT(PPC_RAW_SUB(dst_reg, dst_reg, src_reg));
>> goto bpf_alu32_trunc;
>> case BPF_ALU | BPF_ADD | BPF_K: /* (u32) dst += (u32) imm */
>> - case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */
>> case BPF_ALU64 | BPF_ADD | BPF_K: /* dst += imm */
>> + if (!imm) {
>> + goto bpf_alu32_trunc;
>> + } else if (imm >= -32768 && imm < 32768) {
>> + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm)));
>> + } else {
>> + PPC_LI32(b2p[TMP_REG_1], imm);
>> + EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1]));
>> + }
>> + goto bpf_alu32_trunc;
>> + case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */
>> case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */
>> - if (BPF_OP(code) == BPF_SUB)
>> - imm = -imm;
>> - if (imm) {
>> - if (imm >= -32768 && imm < 32768)
>> - EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm)));
>> - else {
>> - PPC_LI32(b2p[TMP_REG_1], imm);
>> - EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1]));
>> - }
>> + if (!imm) {
>> + goto bpf_alu32_trunc;
>> + } else if (imm > -32768 && imm < 32768) {
>
> Why do you exclude imm == 32768 ?
>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Good catch -- that was from an earlier version where this was shared
across BPF_ADD and BPF_SUB. I missed updating this section before
posting.
Michael, please consider squashing in the below diff into this patch.
Thanks!
- Naveen
---
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index f5a804d8c95bc1..0fdc1ff86e4f1c 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -368,7 +368,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */
if (!imm) {
goto bpf_alu32_trunc;
- } else if (imm > -32768 && imm < 32768) {
+ } else if (imm > -32768 && imm <= 32768) {
EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(-imm)));
} else {
PPC_LI32(b2p[TMP_REG_1], imm);
^ permalink raw reply related
* Re: [PATCH v1 3/6] mm/memory_hotplug: restrict CONFIG_MEMORY_HOTPLUG to 64 bit
From: Oscar Salvador @ 2021-10-07 9:02 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-4-david@redhat.com>
On Wed, Sep 29, 2021 at 04:35:57PM +0200, David Hildenbrand wrote:
> 32 bit support is broken in various ways: for example, we can online
> memory that should actually go to ZONE_HIGHMEM to ZONE_MOVABLE or in
> some cases even to one of the other kernel zones.
>
> We marked it BROKEN in commit b59d02ed0869 ("mm/memory_hotplug: disable the
> functionality for 32b") almost one year ago. According to that commit
> it might be broken at least since 2017. Further, there is hardly a sane use
> case nowadays.
>
> Let's just depend completely on 64bit, dropping the "BROKEN" dependency to
> make clear that we are not going to support it again. Next, we'll remove
> some HIGHMEM leftovers from memory hotplug code to clean up.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> ---
> mm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ea8762cd8e1e..88273dd5c6d6 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -125,7 +125,7 @@ config MEMORY_HOTPLUG
> select MEMORY_ISOLATION
> depends on SPARSEMEM
> depends on ARCH_ENABLE_MEMORY_HOTPLUG
> - depends on 64BIT || BROKEN
> + depends on 64BIT
> select NUMA_KEEP_MEMINFO if NUMA
>
> config MEMORY_HOTPLUG_DEFAULT_ONLINE
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 4/6] mm/memory_hotplug: remove HIGHMEM leftovers
From: Oscar Salvador @ 2021-10-07 9:13 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-5-david@redhat.com>
On Wed, Sep 29, 2021 at 04:35:58PM +0200, David Hildenbrand wrote:
> We don't support CONFIG_MEMORY_HOTPLUG on 32 bit and consequently not
> HIGHMEM. Let's remove any leftover code -- including the unused
> "status_change_nid_high" field part of the memory notifier.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> ---
> Documentation/core-api/memory-hotplug.rst | 3 --
> .../zh_CN/core-api/memory-hotplug.rst | 4 ---
> include/linux/memory.h | 1 -
> mm/memory_hotplug.c | 36 ++-----------------
> 4 files changed, 2 insertions(+), 42 deletions(-)
>
> diff --git a/Documentation/core-api/memory-hotplug.rst b/Documentation/core-api/memory-hotplug.rst
> index de7467e48067..682259ee633a 100644
> --- a/Documentation/core-api/memory-hotplug.rst
> +++ b/Documentation/core-api/memory-hotplug.rst
> @@ -57,7 +57,6 @@ The third argument (arg) passes a pointer of struct memory_notify::
> unsigned long start_pfn;
> unsigned long nr_pages;
> int status_change_nid_normal;
> - int status_change_nid_high;
> int status_change_nid;
> }
>
> @@ -65,8 +64,6 @@ The third argument (arg) passes a pointer of struct memory_notify::
> - nr_pages is # of pages of online/offline memory.
> - status_change_nid_normal is set node id when N_NORMAL_MEMORY of nodemask
> is (will be) set/clear, if this is -1, then nodemask status is not changed.
> -- status_change_nid_high is set node id when N_HIGH_MEMORY of nodemask
> - is (will be) set/clear, if this is -1, then nodemask status is not changed.
> - status_change_nid is set node id when N_MEMORY of nodemask is (will be)
> set/clear. It means a new(memoryless) node gets new memory by online and a
> node loses all memory. If this is -1, then nodemask status is not changed.
> diff --git a/Documentation/translations/zh_CN/core-api/memory-hotplug.rst b/Documentation/translations/zh_CN/core-api/memory-hotplug.rst
> index 161f4d2c18cc..9a204eb196f2 100644
> --- a/Documentation/translations/zh_CN/core-api/memory-hotplug.rst
> +++ b/Documentation/translations/zh_CN/core-api/memory-hotplug.rst
> @@ -63,7 +63,6 @@ memory_notify结构体的指针::
> unsigned long start_pfn;
> unsigned long nr_pages;
> int status_change_nid_normal;
> - int status_change_nid_high;
> int status_change_nid;
> }
>
> @@ -74,9 +73,6 @@ memory_notify结构体的指针::
> - status_change_nid_normal是当nodemask的N_NORMAL_MEMORY被设置/清除时设置节
> 点id，如果是-1，则nodemask状态不改变。
>
> -- status_change_nid_high是当nodemask的N_HIGH_MEMORY被设置/清除时设置的节点
> - id，如果这个值为-1，那么nodemask状态不会改变。
> -
> - status_change_nid是当nodemask的N_MEMORY被（将）设置/清除时设置的节点id。这
> 意味着一个新的（没上线的）节点通过联机获得新的内存，而一个节点失去了所有的内
> 存。如果这个值为-1，那么nodemask的状态就不会改变。
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index dd6e608c3e0b..c46ff374d48d 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -96,7 +96,6 @@ struct memory_notify {
> unsigned long start_pfn;
> unsigned long nr_pages;
> int status_change_nid_normal;
> - int status_change_nid_high;
> int status_change_nid;
> };
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 8d7b2b593a26..95c927c8bfb8 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -21,7 +21,6 @@
> #include <linux/memory.h>
> #include <linux/memremap.h>
> #include <linux/memory_hotplug.h>
> -#include <linux/highmem.h>
> #include <linux/vmalloc.h>
> #include <linux/ioport.h>
> #include <linux/delay.h>
> @@ -585,10 +584,6 @@ void generic_online_page(struct page *page, unsigned int order)
> debug_pagealloc_map_pages(page, 1 << order);
> __free_pages_core(page, order);
> totalram_pages_add(1UL << order);
> -#ifdef CONFIG_HIGHMEM
> - if (PageHighMem(page))
> - totalhigh_pages_add(1UL << order);
> -#endif
> }
> EXPORT_SYMBOL_GPL(generic_online_page);
>
> @@ -625,16 +620,11 @@ static void node_states_check_changes_online(unsigned long nr_pages,
>
> arg->status_change_nid = NUMA_NO_NODE;
> arg->status_change_nid_normal = NUMA_NO_NODE;
> - arg->status_change_nid_high = NUMA_NO_NODE;
>
> if (!node_state(nid, N_MEMORY))
> arg->status_change_nid = nid;
> if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY))
> arg->status_change_nid_normal = nid;
> -#ifdef CONFIG_HIGHMEM
> - if (zone_idx(zone) <= ZONE_HIGHMEM && !node_state(nid, N_HIGH_MEMORY))
> - arg->status_change_nid_high = nid;
> -#endif
> }
>
> static void node_states_set_node(int node, struct memory_notify *arg)
> @@ -642,9 +632,6 @@ static void node_states_set_node(int node, struct memory_notify *arg)
> if (arg->status_change_nid_normal >= 0)
> node_set_state(node, N_NORMAL_MEMORY);
>
> - if (arg->status_change_nid_high >= 0)
> - node_set_state(node, N_HIGH_MEMORY);
> -
> if (arg->status_change_nid >= 0)
> node_set_state(node, N_MEMORY);
> }
> @@ -1801,7 +1788,6 @@ static void node_states_check_changes_offline(unsigned long nr_pages,
>
> arg->status_change_nid = NUMA_NO_NODE;
> arg->status_change_nid_normal = NUMA_NO_NODE;
> - arg->status_change_nid_high = NUMA_NO_NODE;
>
> /*
> * Check whether node_states[N_NORMAL_MEMORY] will be changed.
> @@ -1816,24 +1802,9 @@ static void node_states_check_changes_offline(unsigned long nr_pages,
> if (zone_idx(zone) <= ZONE_NORMAL && nr_pages >= present_pages)
> arg->status_change_nid_normal = zone_to_nid(zone);
>
> -#ifdef CONFIG_HIGHMEM
> - /*
> - * node_states[N_HIGH_MEMORY] contains nodes which
> - * have normal memory or high memory.
> - * Here we add the present_pages belonging to ZONE_HIGHMEM.
> - * If the zone is within the range of [0..ZONE_HIGHMEM), and
> - * we determine that the zones in that range become empty,
> - * we need to clear the node for N_HIGH_MEMORY.
> - */
> - present_pages += pgdat->node_zones[ZONE_HIGHMEM].present_pages;
> - if (zone_idx(zone) <= ZONE_HIGHMEM && nr_pages >= present_pages)
> - arg->status_change_nid_high = zone_to_nid(zone);
> -#endif
> -
> /*
> - * We have accounted the pages from [0..ZONE_NORMAL), and
> - * in case of CONFIG_HIGHMEM the pages from ZONE_HIGHMEM
> - * as well.
> + * We have accounted the pages from [0..ZONE_NORMAL); ZONE_HIGHMEM
> + * does not apply as we don't support 32bit.
> * Here we count the possible pages from ZONE_MOVABLE.
> * If after having accounted all the pages, we see that the nr_pages
> * to be offlined is over or equal to the accounted pages,
> @@ -1851,9 +1822,6 @@ static void node_states_clear_node(int node, struct memory_notify *arg)
> if (arg->status_change_nid_normal >= 0)
> node_clear_state(node, N_NORMAL_MEMORY);
>
> - if (arg->status_change_nid_high >= 0)
> - node_clear_state(node, N_HIGH_MEMORY);
> -
> if (arg->status_change_nid >= 0)
> node_clear_state(node, N_MEMORY);
> }
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 5/6] mm/memory_hotplug: remove stale function declarations
From: Oscar Salvador @ 2021-10-07 9:14 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-6-david@redhat.com>
On Wed, Sep 29, 2021 at 04:35:59PM +0200, David Hildenbrand wrote:
> These functions no longer exist.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> ---
> include/linux/memory_hotplug.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index e5a867c950b2..be48e003a518 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -98,9 +98,6 @@ static inline void zone_seqlock_init(struct zone *zone)
> {
> seqlock_init(&zone->span_seqlock);
> }
> -extern int zone_grow_free_lists(struct zone *zone, unsigned long new_nr_pages);
> -extern int zone_grow_waitqueues(struct zone *zone, unsigned long nr_pages);
> -extern int add_one_highpage(struct page *page, int pfn, int bad_ppro);
> extern void adjust_present_page_count(struct page *page,
> struct memory_group *group,
> long nr_pages);
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 6/6] x86: remove memory hotplug support on X86_32
From: Oscar Salvador @ 2021-10-07 9:15 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210929143600.49379-7-david@redhat.com>
On Wed, Sep 29, 2021 at 04:36:00PM +0200, David Hildenbrand wrote:
> CONFIG_MEMORY_HOTPLUG was marked BROKEN over one year and we just
> restricted it to 64 bit. Let's remove the unused x86 32bit implementation
> and simplify the Kconfig.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
> ---
> arch/x86/Kconfig | 6 +++---
> arch/x86/mm/init_32.c | 31 -------------------------------
> 2 files changed, 3 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ab83c22d274e..85f4762429f1 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -62,7 +62,7 @@ config X86
> select ARCH_32BIT_OFF_T if X86_32
> select ARCH_CLOCKSOURCE_INIT
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if X86_64 && HUGETLB_PAGE && MIGRATION
> - select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64 || (X86_32 && HIGHMEM)
> + select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64
> select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
> select ARCH_ENABLE_SPLIT_PMD_PTLOCK if (PGTABLE_LEVELS > 2) && (X86_64 || X86_PAE)
> select ARCH_ENABLE_THP_MIGRATION if X86_64 && TRANSPARENT_HUGEPAGE
> @@ -1615,7 +1615,7 @@ config ARCH_SELECT_MEMORY_MODEL
>
> config ARCH_MEMORY_PROBE
> bool "Enable sysfs memory/probe interface"
> - depends on X86_64 && MEMORY_HOTPLUG
> + depends on MEMORY_HOTPLUG
> help
> This option enables a sysfs memory/probe interface for testing.
> See Documentation/admin-guide/mm/memory-hotplug.rst for more information.
> @@ -2395,7 +2395,7 @@ endmenu
>
> config ARCH_HAS_ADD_PAGES
> def_bool y
> - depends on X86_64 && ARCH_ENABLE_MEMORY_HOTPLUG
> + depends on ARCH_ENABLE_MEMORY_HOTPLUG
>
> config ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
> def_bool y
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index bd90b8fe81e4..5cd7ea6d645c 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -779,37 +779,6 @@ void __init mem_init(void)
> test_wp_bit();
> }
>
> -#ifdef CONFIG_MEMORY_HOTPLUG
> -int arch_add_memory(int nid, u64 start, u64 size,
> - struct mhp_params *params)
> -{
> - unsigned long start_pfn = start >> PAGE_SHIFT;
> - unsigned long nr_pages = size >> PAGE_SHIFT;
> - int ret;
> -
> - /*
> - * The page tables were already mapped at boot so if the caller
> - * requests a different mapping type then we must change all the
> - * pages with __set_memory_prot().
> - */
> - if (params->pgprot.pgprot != PAGE_KERNEL.pgprot) {
> - ret = __set_memory_prot(start, nr_pages, params->pgprot);
> - if (ret)
> - return ret;
> - }
> -
> - return __add_pages(nid, start_pfn, nr_pages, params);
> -}
> -
> -void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> -{
> - unsigned long start_pfn = start >> PAGE_SHIFT;
> - unsigned long nr_pages = size >> PAGE_SHIFT;
> -
> - __remove_pages(start_pfn, nr_pages, altmap);
> -}
> -#endif
> -
> int kernel_set_to_readonly __read_mostly;
>
> static void mark_nxdata_nx(void)
> --
> 2.31.1
>
>
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1 6/6] x86: remove memory hotplug support on X86_32
From: David Hildenbrand @ 2021-10-07 9:27 UTC (permalink / raw)
To: Oscar Salvador
Cc: Michal Hocko, Michael S. Tsirkin, Peter Zijlstra, Jason Wang,
Dave Hansen, virtualization, linux-mm, Paul Mackerras,
linux-kselftest, H. Peter Anvin, Shuah Khan, Alex Shi,
Jonathan Corbet, x86, Ingo Molnar, Borislav Petkov,
Andy Lutomirski, Thomas Gleixner, Greg Kroah-Hartman, linux-doc,
linux-kernel, Rafael J. Wysocki, Andrew Morton, linuxppc-dev,
Mike Rapoport
In-Reply-To: <YV66zoLEP3niIHEu@localhost.localdomain>
On 07.10.21 11:15, Oscar Salvador wrote:
> On Wed, Sep 29, 2021 at 04:36:00PM +0200, David Hildenbrand wrote:
>> CONFIG_MEMORY_HOTPLUG was marked BROKEN over one year and we just
>> restricted it to 64 bit. Let's remove the unused x86 32bit implementation
>> and simplify the Kconfig.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
Thanks for the review Oscar!
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH] video: fbdev: use memset_io() instead of memset()
From: Michael Ellerman @ 2021-10-07 11:28 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Andrew Morton
Cc: linux-fbdev, Finn Thain, Stan Johnson, linux-kernel, dri-devel,
linuxppc-dev
In-Reply-To: <884a54f1e5cb774c1d9b4db780209bee5d4f6718.1631712563.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> While investigating a lockup at startup on Powerbook 3400C, it was
> identified that the fbdev driver generates alignment exception at
> startup:
...
>
> Use fb_memset() instead of memset(). fb_memset() is defined as
> memset_io() for powerpc.
>
> Fixes: 8c8709334cec ("[PATCH] ppc32: Remove CONFIG_PMAC_PBOOK")
> Reported-by: Stan Johnson <userm57@yahoo.com>
> Cc: Finn Thain <fthain@linux-m68k.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> drivers/video/fbdev/chipsfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks like drivers/video/fbdev is orphaned, so I'll pick this up via
powerpc.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/mce: check if event info is valid
From: Michael Ellerman @ 2021-10-07 12:09 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mahesh, npiggin
In-Reply-To: <4f9431fd-74b0-2ce4-807e-9796b326d729@linux.ibm.com>
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/6/21 6:53 PM, Ganesh Goudar wrote:
>
>> Check if the event info is valid before printing the
>> event information. When a fwnmi enabled nested kvm guest
>> hits a machine check exception L0 and L2 would generate
>> machine check event info, But L1 would not generate any
>> machine check event info as it won't go through 0x200
>> vector and prints some unwanted message.
>>
>> To fix this, 'in_use' variable in machine check event info is
>> no more in use, rename it to 'valid' and check if the event
>> information is valid before logging the event information.
>>
>> without this patch L1 would print following message for
>> exceptions encountered in L2, as event structure will be
>> empty in L1.
>>
>> "Machine Check Exception, Unknown event version 0".
>>
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>
> Hi mpe, Any comments on this patch.
The variable rename is a bit of a distraction.
But ignoring that, how do we end up processing a machine_check_event
that has in_use = 0?
You don't give much detail on what call path you're talking about. I
guess we're coming in via the calls in the KVM code?
In the definition of kvm_vcpu_arch we have:
struct machine_check_event mce_evt; /* Valid if trap == 0x200 */
And you said we're not going via 0x200 in L1. But so shouldn't we be
teaching the KVM code not to use mce_evt when trap is not 0x200?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/eeh:Fix some mistakes in comments
From: Michael Ellerman @ 2021-10-07 12:55 UTC (permalink / raw)
To: Daniel Axtens, Kai Song, linuxppc-dev
Cc: paulus, Kai Song, oohall, linux-kernel
In-Reply-To: <878rze60by.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> Hi Kai,
>
> Thank you for your contribution to the powerpc kernel!
>
>> Get rid of warning:
>> arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead
>
> You haven't said where this warning is from. I thought it might be from
> sparse but I couldn't seem to reproduce it - is my version of sparse too
> old or are you using a different tool?
>
>> /**
>> - * eeh_set_pe_freset - Check the required reset for the indicated device
>> - * @data: EEH device
>> + * eeh_set_dev_freset - Check the required reset for the indicated device
>> + * @edev: EEH device
>> * @flag: return value
>> *
>> * Each device might have its preferred reset type: fundamental or
>
> This looks like a good and correct change.
>
> I checked through git history with git blame to see when the function
> was renamed. There are 2 commits that should have updated the comment:
> one renamed the function and one renamed an argument. So, I think this
> commit could have:
>
> Fixes: d6c4932fbf24 ("powerpc/eeh: Strengthen types of eeh traversal functions")
> Fixes: c270a24c59bd ("powerpc/eeh: Do reset based on PE")
>
> But I don't know if an out of date comment is enough of a 'bug' to
> justify a Fixes: tag? (mpe, I'm sure I've asked this before, sorry!)
It depends. If you think it's important that the fix gets backported
then you should add the Fixes tag.
In this case I would say no. The comments have been broken for years,
and it's a pretty obscure API.
cheers
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Laurent Vivier @ 2021-10-07 13:45 UTC (permalink / raw)
To: Greg Kurz; +Cc: linux-kernel, Nicholas Piggin, kvm-ppc, linuxppc-dev
In-Reply-To: <20211006124204.4741bb5c@bahia.huguette>
On 06/10/2021 12:42, Greg Kurz wrote:
> On Wed, 6 Oct 2021 09:37:45 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> Commit 61bd0f66ff92 has moved guest_enter() out of the interrupt
>> protected area to be able to have updated tick counters, but
>> commit 112665286d08 moved back to this area to avoid wrong
>> context warning (or worse).
>>
>> None of them are correct, to fix the problem port to POWER
>> the x86 fix 160457140187 ("KVM: x86: Defer vtime accounting 'til
>> after IRQ handling"):
>>
>> "Defer the call to account guest time until after servicing any IRQ(s)
>> that happened in the guest or immediately after VM-Exit. Tick-based
>> accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
>> handler runs, and IRQs are blocked throughout the main sequence of
>> vcpu_enter_guest(), including the call into vendor code to actually
>> enter and exit the guest."
>>
>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2009312
>> Fixes: 61bd0f66ff92 ("KVM: PPC: Book3S HV: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN")
>
> This patch was merged in linux 4.16 and thus is on the 4.16.y
> stable branch and it was backported to stable 4.14.y. It would
> make sense to Cc: stable # v4.14 also, but...
>
>> Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
>
> ... this one, which was merged in linux 5.12, was never backported
> anywhere because it wasn't considered as a fix, as commented here:
>
> https://lore.kernel.org/linuxppc-dev/1610793296.fjhomer31g.astroid@bobo.none/
>
> AFAICT commit 61bd0f66ff92 was never mentioned anywhere in a bug. The
> first Fixes: tag thus looks a bit misleading. I'd personally drop it
> and Cc: stable # v5.12.
>
Ok, I update the comment.
>> Cc: npiggin@gmail.com
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 2acb1c96cfaf..43e1ce853785 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>>
>> srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
>>
>> + context_tracking_guest_exit();
>> +
>> set_irq_happened(trap);
>>
>> spin_lock(&vc->lock);
>> @@ -3726,9 +3728,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>>
>> kvmppc_set_host_core(pcpu);
>>
>> - guest_exit_irqoff();
>> -
>
>
> Change looks ok but it might be a bit confusing for the
> occasional reader that guest_enter_irqoff() isn't matched
> by a guest_exit_irqoff().
>
>> local_irq_enable();
>> + vtime_account_guest_exit();
>>
>
> Maybe add a comment like in x86 ?
>
done
>> /* Let secondaries go back to the offline loop */
>> for (i = 0; i < controlled_threads; ++i) {
>> @@ -4506,13 +4507,14 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>>
>> srcu_read_unlock(&kvm->srcu, srcu_idx);
>>
>> + context_tracking_guest_exit();
>> +
>> set_irq_happened(trap);
>>
>> kvmppc_set_host_core(pcpu);
>>
>> - guest_exit_irqoff();
>> -
>> local_irq_enable();
>> + vtime_account_guest_exit();
>>
>> cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
>>
>
> Same remarks. FWIW a followup was immediately added to x86 to
> encapsulate the enter/exit logic in common helpers :
>
> ommit bc908e091b3264672889162733020048901021fb
> Author: Sean Christopherson <seanjc@google.com>
> Date: Tue May 4 17:27:35 2021 -0700
>
> KVM: x86: Consolidate guest enter/exit logic to common helpers
>
> This makes the code nicer. Maybe do something similar for POWER ?
I don't like to modify kernel code when it's not needed. I just want to fix a bug, if
someone wants this nicer I let this to him...
Thanks,
Laurent
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Paul A. Clarke @ 2021-10-07 13:47 UTC (permalink / raw)
To: Kajol Jain
Cc: maddy, rnsastry, linuxppc-dev, linux-kernel, acme,
linux-perf-users, atrajeev, jolsa
In-Reply-To: <20211006173248.GA7389@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
> > + {
> > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>
> I'm not sure what that means.
After doing a bit of research, I think it might be a bit more clear as:
"Average cycles per instruction when the NTF instruction finishes at dispatch"
> > + "MetricExpr": "PM_EXEC_STALL_FIN_AT_DISP / PM_RUN_INST_CMPL",
> > + "MetricGroup": "CPI",
> > + "MetricName": "FIN_AT_DISP_STALL_CPI"
> > + },
PC
^ permalink raw reply
* [PATCH v2] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Laurent Vivier @ 2021-10-07 14:28 UTC (permalink / raw)
To: kvm-ppc
Cc: Laurent Vivier, Greg Kurz, Nicholas Piggin, linux-kernel, stable,
linuxppc-dev
Commit 112665286d08 moved guest_exit() in the interrupt protected
area to avoid wrong context warning (or worse), but the tick counter
cannot be updated and the guest time is accounted to the system time.
To fix the problem port to POWER the x86 fix
160457140187 ("Defer vtime accounting 'til after IRQ handling"):
"Defer the call to account guest time until after servicing any IRQ(s)
that happened in the guest or immediately after VM-Exit. Tick-based
accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
handler runs, and IRQs are blocked throughout the main sequence of
vcpu_enter_guest(), including the call into vendor code to actually
enter and exit the guest."
Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
Cc: npiggin@gmail.com
Cc: <stable@vger.kernel.org> # 5.12
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Notes:
v2: remove reference to commit 61bd0f66ff92
cc stable 5.12
add the same comment in the code as for x86
arch/powerpc/kvm/book3s_hv.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2acb1c96cfaf..a694d1a8f6ce 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
+ context_tracking_guest_exit();
+
set_irq_happened(trap);
spin_lock(&vc->lock);
@@ -3726,9 +3728,15 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
-
local_irq_enable();
+ /*
+ * Wait until after servicing IRQs to account guest time so that any
+ * ticks that occurred while running the guest are properly accounted
+ * to the guest. Waiting until IRQs are enabled degrades the accuracy
+ * of accounting via context tracking, but the loss of accuracy is
+ * acceptable for all known use cases.
+ */
+ vtime_account_guest_exit();
/* Let secondaries go back to the offline loop */
for (i = 0; i < controlled_threads; ++i) {
@@ -4506,13 +4514,21 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
srcu_read_unlock(&kvm->srcu, srcu_idx);
+ context_tracking_guest_exit();
+
set_irq_happened(trap);
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
-
local_irq_enable();
+ /*
+ * Wait until after servicing IRQs to account guest time so that any
+ * ticks that occurred while running the guest are properly accounted
+ * to the guest. Waiting until IRQs are enabled degrades the accuracy
+ * of accounting via context tracking, but the loss of accuracy is
+ * acceptable for all known use cases.
+ */
+ vtime_account_guest_exit();
cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
--
2.31.1
^ permalink raw reply related
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Segher Boessenkool @ 2021-10-07 18:43 UTC (permalink / raw)
To: Paul A. Clarke
Cc: maddy, rnsastry, Kajol Jain, jolsa, linux-kernel, acme,
linux-perf-users, atrajeev, linuxppc-dev
In-Reply-To: <20211007134750.GA243632@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On Thu, Oct 07, 2021 at 08:47:50AM -0500, Paul A. Clarke wrote:
> On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
> > > + {
> > > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
> >
> > I'm not sure what that means.
>
> After doing a bit of research, I think it might be a bit more clear as:
> "Average cycles per instruction when the NTF instruction finishes at dispatch"
Is "next to finish" some defined and/or sensible term in this context?
Or do you mean NTC here? Or what :-)
Instructions do not finish in order at all generally.
Segher
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS eb8257a12192f43ffd41bd90932c39dade958042
From: kernel test robot @ 2021-10-08 2:58 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: eb8257a12192f43ffd41bd90932c39dade958042 pseries/eeh: Fix the kdump kernel crash during eeh_pseries_init
elapsed time: 731m
configs tested: 96
configs skipped: 96
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
powerpc motionpro_defconfig
arc nsimosci_hs_smp_defconfig
m68k atari_defconfig
powerpc ppc40x_defconfig
powerpc stx_gp3_defconfig
m68k multi_defconfig
sh landisk_defconfig
sh microdev_defconfig
powerpc ppc64_defconfig
mips ath25_defconfig
arm orion5x_defconfig
arm collie_defconfig
powerpc ps3_defconfig
h8300 allyesconfig
powerpc pcm030_defconfig
powerpc pasemi_defconfig
powerpc64 defconfig
mips capcella_defconfig
arm davinci_all_defconfig
sh polaris_defconfig
xtensa iss_defconfig
arm multi_v4t_defconfig
powerpc ksi8560_defconfig
mips bcm63xx_defconfig
x86_64 randconfig-c001-20211003
i386 randconfig-c001-20211003
arm randconfig-c002-20211003
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nds32 defconfig
csky defconfig
alpha defconfig
alpha allyesconfig
nios2 allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
s390 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allnoconfig
powerpc allyesconfig
powerpc allmodconfig
x86_64 randconfig-a015-20211004
x86_64 randconfig-a012-20211004
x86_64 randconfig-a016-20211004
x86_64 randconfig-a014-20211004
x86_64 randconfig-a013-20211004
x86_64 randconfig-a011-20211004
i386 randconfig-a013-20211004
i386 randconfig-a016-20211004
i386 randconfig-a014-20211004
i386 randconfig-a011-20211004
i386 randconfig-a012-20211004
i386 randconfig-a015-20211004
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
um x86_64_defconfig
um i386_defconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a003-20211004
x86_64 randconfig-a005-20211004
x86_64 randconfig-a001-20211004
x86_64 randconfig-a002-20211004
x86_64 randconfig-a004-20211004
x86_64 randconfig-a006-20211004
i386 randconfig-a001-20211004
i386 randconfig-a003-20211004
i386 randconfig-a005-20211004
i386 randconfig-a002-20211004
i386 randconfig-a004-20211004
i386 randconfig-a006-20211004
hexagon randconfig-r045-20211007
hexagon randconfig-r041-20211007
s390 randconfig-r044-20211007
riscv randconfig-r042-20211007
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 880c5cbb35b2c6e7d9627a53d26799f7affb6472
From: kernel test robot @ 2021-10-08 3:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 880c5cbb35b2c6e7d9627a53d26799f7affb6472 powerpc/paravirt: correct preempt debug splat in vcpu_is_preempted()
elapsed time: 772m
configs tested: 121
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allmodconfig
arm allyesconfig
powerpc motionpro_defconfig
arc nsimosci_hs_smp_defconfig
m68k atari_defconfig
powerpc ppc40x_defconfig
powerpc stx_gp3_defconfig
m68k multi_defconfig
sh landisk_defconfig
sh microdev_defconfig
powerpc ppc64_defconfig
mips ath25_defconfig
arm orion5x_defconfig
arm collie_defconfig
s390 zfcpdump_defconfig
m68k m5475evb_defconfig
sparc alldefconfig
sparc sparc64_defconfig
sh sh7757lcr_defconfig
arc axs101_defconfig
sh kfr2r09_defconfig
mips maltaup_defconfig
mips loongson3_defconfig
powerpc ps3_defconfig
h8300 allyesconfig
powerpc pcm030_defconfig
powerpc pasemi_defconfig
powerpc64 defconfig
mips capcella_defconfig
arm moxart_defconfig
arm sama5_defconfig
arc nsimosci_hs_defconfig
m68k stmark2_defconfig
arm davinci_all_defconfig
sh polaris_defconfig
xtensa iss_defconfig
arm multi_v4t_defconfig
powerpc ksi8560_defconfig
mips bcm63xx_defconfig
x86_64 randconfig-c001-20211003
i386 randconfig-c001-20211003
arm randconfig-c002-20211003
x86_64 randconfig-c001-20211004
i386 randconfig-c001-20211004
arm randconfig-c002-20211004
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k defconfig
m68k allmodconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
csky defconfig
alpha defconfig
alpha allyesconfig
nios2 allyesconfig
arc defconfig
sh allmodconfig
xtensa allyesconfig
parisc defconfig
parisc allyesconfig
s390 defconfig
s390 allyesconfig
s390 allmodconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allmodconfig
powerpc allnoconfig
powerpc allyesconfig
x86_64 randconfig-a015-20211004
x86_64 randconfig-a012-20211004
x86_64 randconfig-a016-20211004
x86_64 randconfig-a014-20211004
x86_64 randconfig-a013-20211004
x86_64 randconfig-a011-20211004
i386 randconfig-a013-20211004
i386 randconfig-a016-20211004
i386 randconfig-a014-20211004
i386 randconfig-a011-20211004
i386 randconfig-a012-20211004
i386 randconfig-a015-20211004
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
um x86_64_defconfig
um i386_defconfig
x86_64 allyesconfig
x86_64 rhel-8.3-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a003-20211004
x86_64 randconfig-a005-20211004
x86_64 randconfig-a001-20211004
x86_64 randconfig-a002-20211004
x86_64 randconfig-a004-20211004
x86_64 randconfig-a006-20211004
i386 randconfig-a001-20211004
i386 randconfig-a003-20211004
i386 randconfig-a005-20211004
i386 randconfig-a002-20211004
i386 randconfig-a004-20211004
i386 randconfig-a006-20211004
hexagon randconfig-r045-20211007
hexagon randconfig-r041-20211007
s390 randconfig-r044-20211007
riscv randconfig-r042-20211007
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] powerpc/eeh:Fix docstrings in eeh
From: Daniel Axtens @ 2021-10-08 5:28 UTC (permalink / raw)
To: Kai Song, linuxppc-dev; +Cc: paulus, Kai Song, oohall, linux-kernel
In-Reply-To: <20211004085842.255813-1-songkai01@inspur.com>
Hi Kai,
Thank you for your patch! I have 3 very minor tweaks and I am otherwise
very happy with it.
Firstly, in your commit name, there should be a space between
"powerpc/eeh:" and "Fix docstrings". You might also want to say "in
eeh.c" rather than "in eeh" because there is eeh code in a number of
other files too.
> We fix the following warnings when building kernel with W=1:
> arch/powerpc/kernel/eeh.c:598: warning: Function parameter or member 'function' not described in 'eeh_pci_enable'
> arch/powerpc/kernel/eeh.c:774: warning: Function parameter or member 'edev' not described in 'eeh_set_dev_freset'
> arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead
> arch/powerpc/kernel/eeh.c:814: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset_full'
> arch/powerpc/kernel/eeh.c:944: warning: Function parameter or member 'ops' not described in 'eeh_init'
> arch/powerpc/kernel/eeh.c:1451: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset'
> arch/powerpc/kernel/eeh.c:1526: warning: Function parameter or member 'func' not described in 'eeh_pe_inject_err'
> arch/powerpc/kernel/eeh.c:1526: warning: Excess function parameter 'function' described in 'eeh_pe_inject_err'
>
> Signed-off-by: Kai Song <songkai01@inspur.com>
> ---
> arch/powerpc/kernel/eeh.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index e9b597ed423c..57a6868a41ab 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -589,6 +589,7 @@ EXPORT_SYMBOL(eeh_check_failure);
> /**
> * eeh_pci_enable - Enable MMIO or DMA transfers for this slot
> * @pe: EEH PE
> + * @function : EEH function
I don't think there should be a space between '@function' and ':'.
I know the parameter is called 'function' and I think "EEH function" was
a good guess for the docstring. However, if we look at the way
'function' is used, it is compared with EEH_OPT_xxx constants, and then
it's passed to eeh_ops->set_option(), eeh_pci_enable() is also called in
eeh_pe_set_option() with a parameter called 'option'. So I think maybe
'function' should be described as the "EEH option"?
This is still very unsatisfactory but it's not the fault of your patch -
the EEH codebase is very messy and it's worth fixing the W=1 warnings
even if we don't fully clean up the EEH codebase.
> - * eeh_set_pe_freset - Check the required reset for the indicated device
> - * @data: EEH device
> + * eeh_set_dev_freset - Check the required reset for the indicated device
> + * @edev: EEH device
> * @flag: return value
> *
This is good.
> /**
> * eeh_pe_reset_full - Complete a full reset process on the indicated PE
> * @pe: EEH PE
> + * @include_passed: include passed-through devices?
> *
> * This function executes a full reset procedure on a PE, including setting
> * the appropriate flags, performing a fundamental or hot reset, and then
> @@ -937,6 +939,7 @@ static struct notifier_block eeh_device_nb = {
This is OK.
> /**
> * eeh_init - System wide EEH initialization
> + * @ops: struct to trace EEH operation callback functions
I think "@ops: platform-specific functions for EEH operations" is
probably clearer?
> *
> * It's the platform's job to call this from an arch_initcall().
> */
> @@ -1442,6 +1445,7 @@ static int eeh_pe_reenable_devices(struct eeh_pe *pe, bool include_passed)
> * eeh_pe_reset - Issue PE reset according to specified type
> * @pe: EEH PE
> * @option: reset type
> + * @include_passed: include passed-through devices?
> *
This is OK.
> * The routine is called to reset the specified PE with the
> * indicated type, either fundamental reset or hot reset.
> @@ -1513,12 +1517,12 @@ EXPORT_SYMBOL_GPL(eeh_pe_configure);
> * eeh_pe_inject_err - Injecting the specified PCI error to the indicated PE
> * @pe: the indicated PE
> * @type: error type
> - * @function: error function
> + * @func: error function
This is good.
> * @addr: address
> * @mask: address mask
> *
> * The routine is called to inject the specified PCI error, which
> - * is determined by @type and @function, to the indicated PE for
> + * is determined by @type and @func, to the indicated PE for
This is good.
When you resend, you can include:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* linux-next: build warnings in Linus' tree
From: Stephen Rothwell @ 2021-10-08 5:47 UTC (permalink / raw)
To: Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
Hi all,
After merging the origin tree, today's linux-next build (powerpc
allyesconfig) produced these warnings (along with many others):
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
arch/powerpc/boot/dts/mpc5200b.dtsi:182.18-186.5: Warning (spi_bus_bridge): /soc5200@f0000000/psc@2000: node name for SPI buses should be 'spi'
arch/powerpc/boot/dts/mpc5200b.dtsi:267.20-280.4: Warning (pci_bridge): /pci@f0000d00: missing ranges for PCI bridge (or not a bridge)
Given that arch/powerpc/boot/dts/mpc5200b.dtsi is oncluded by several
other dts files, fixing this one file would go quite a long way to
silencing our allyesoncig build. Unfotunatley, I have no idea how to
fix this file (ad maybe some fo the interactions it has with other files).
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] KVM: PPC: Defer vtime accounting 'til after IRQ handling
From: Greg Kurz @ 2021-10-08 5:53 UTC (permalink / raw)
To: Laurent Vivier
Cc: linux-kernel, Nicholas Piggin, kvm-ppc, stable, linuxppc-dev
In-Reply-To: <20211007142856.41205-1-lvivier@redhat.com>
On Thu, 7 Oct 2021 16:28:56 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> Commit 112665286d08 moved guest_exit() in the interrupt protected
> area to avoid wrong context warning (or worse), but the tick counter
> cannot be updated and the guest time is accounted to the system time.
>
> To fix the problem port to POWER the x86 fix
> 160457140187 ("Defer vtime accounting 'til after IRQ handling"):
>
> "Defer the call to account guest time until after servicing any IRQ(s)
> that happened in the guest or immediately after VM-Exit. Tick-based
> accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
> handler runs, and IRQs are blocked throughout the main sequence of
> vcpu_enter_guest(), including the call into vendor code to actually
> enter and exit the guest."
>
> Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
> Cc: npiggin@gmail.com
> Cc: <stable@vger.kernel.org> # 5.12
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>
> Notes:
> v2: remove reference to commit 61bd0f66ff92
> cc stable 5.12
> add the same comment in the code as for x86
>
Works for me. As you stated in your answer, someone can polish the
code later on.
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/kvm/book3s_hv.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 2acb1c96cfaf..a694d1a8f6ce 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3695,6 +3695,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>
> srcu_read_unlock(&vc->kvm->srcu, srcu_idx);
>
> + context_tracking_guest_exit();
> +
> set_irq_happened(trap);
>
> spin_lock(&vc->lock);
> @@ -3726,9 +3728,15 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>
> kvmppc_set_host_core(pcpu);
>
> - guest_exit_irqoff();
> -
> local_irq_enable();
> + /*
> + * Wait until after servicing IRQs to account guest time so that any
> + * ticks that occurred while running the guest are properly accounted
> + * to the guest. Waiting until IRQs are enabled degrades the accuracy
> + * of accounting via context tracking, but the loss of accuracy is
> + * acceptable for all known use cases.
> + */
> + vtime_account_guest_exit();
>
> /* Let secondaries go back to the offline loop */
> for (i = 0; i < controlled_threads; ++i) {
> @@ -4506,13 +4514,21 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>
> srcu_read_unlock(&kvm->srcu, srcu_idx);
>
> + context_tracking_guest_exit();
> +
> set_irq_happened(trap);
>
> kvmppc_set_host_core(pcpu);
>
> - guest_exit_irqoff();
> -
> local_irq_enable();
> + /*
> + * Wait until after servicing IRQs to account guest time so that any
> + * ticks that occurred while running the guest are properly accounted
> + * to the guest. Waiting until IRQs are enabled degrades the accuracy
> + * of accounting via context tracking, but the loss of accuracy is
> + * acceptable for all known use cases.
> + */
> + vtime_account_guest_exit();
>
> cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
>
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: Michael Ellerman @ 2021-10-08 5:58 UTC (permalink / raw)
To: Segher Boessenkool, Paul A. Clarke
Cc: maddy, rnsastry, Kajol Jain, jolsa, linux-kernel, acme,
linux-perf-users, atrajeev, linuxppc-dev
In-Reply-To: <20211007184337.GN10333@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Oct 07, 2021 at 08:47:50AM -0500, Paul A. Clarke wrote:
>> On Wed, Oct 06, 2021 at 12:32:48PM -0500, Paul A. Clarke wrote:
>> > > + {
>> > > + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>> >
>> > I'm not sure what that means.
>>
>> After doing a bit of research, I think it might be a bit more clear as:
>> "Average cycles per instruction when the NTF instruction finishes at dispatch"
>
> Is "next to finish" some defined and/or sensible term in this context?
> Or do you mean NTC here? Or what :-)
Yeah I am also more familiar with NTC.
But those descriptions originally come from a spreadsheet we're given,
and they do include NTF as an acronym.
eg. search for NTF in here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
From the context it does presumably mean "next to finish".
cheers
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Add metric events json file for power10 platform
From: kajoljain @ 2021-10-08 7:48 UTC (permalink / raw)
To: Paul A. Clarke
Cc: maddy, rnsastry, jolsa, linux-kernel, acme, linux-perf-users,
atrajeev, linuxppc-dev
In-Reply-To: <20211006173248.GA7389@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On 10/6/21 11:02 PM, Paul A. Clarke wrote:
> Kajol,
>
> On Wed, Oct 06, 2021 at 01:01:19PM +0530, Kajol Jain wrote:
>> Add pmu metric json file for power10 platform.
>
> Thanks for producing this! A few minor corrections, plus a number of
> stylistic comments below...
Hi Paul,
I will make mentioned nit changes and send a v2.
Thanks,
Kajol Jain
>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> .../arch/powerpc/power10/metrics.json | 772 ++++++++++++++++++
>> 1 file changed, 772 insertions(+)
>> create mode 100644 tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>>
>> diff --git a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>> new file mode 100644
>> index 000000000000..028c9777a516
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
>> @@ -0,0 +1,772 @@
>> +[
>> + {
>> + "BriefDescription": "Percentage of cycles that are run cycles",
>> + "MetricExpr": "PM_RUN_CYC / PM_CYC * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_CYCLES_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per completed instruction",
>> + "MetricExpr": "PM_CYC / PM_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "CYCLES_PER_INSTRUCTION"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled for any reason",
>> + "MetricExpr": "PM_DISP_STALL_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because there was a flush",
>> + "MetricExpr": "PM_DISP_STALL_FLUSH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_FLUSH_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because the MMU was handling a translation miss",
>> + "MetricExpr": "PM_DISP_STALL_TRANSLATION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_TRANSLATION_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled waiting to resolve an instruction ERAT miss",
>> + "MetricExpr": "PM_DISP_STALL_IERAT_ONLY_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IERAT_ONLY_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled waiting to resolve an instruction TLB miss",
>> + "MetricExpr": "PM_DISP_STALL_ITLB_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_ITLB_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to an icache miss",
>> + "MetricExpr": "PM_DISP_STALL_IC_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched form the local L2",
>
> s/form/from/
>
>> + "MetricExpr": "PM_DISP_STALL_IC_L2 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L2_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched form the local L3",
>
> s/form/from/
>
>> + "MetricExpr": "PM_DISP_STALL_IC_L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L3_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while the instruction was fetched from any source beyond the local L3",
>> + "MetricExpr": "PM_DISP_STALL_IC_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_IC_L3MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to an icache miss after a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_ICMISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_ICMISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from the local L2 after suffering a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L2 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L2_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from the local L3 after suffering a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L3_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled while instruction was fetched from any source beyond the local L3 after suffering a branch mispredict",
>
> extra space after "beyond"
>
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_IC_L3MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled due to a branch mispredict",
>> + "MetricExpr": "PM_DISP_STALL_BR_MPRED / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_BR_MPRED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch for any reason",
>
> s/ntc/NTC/ or "next-to-complete"
> I do see uses of "NTC" below.
> Same comment for other instances of "ntc", below...
>
>> + "MetricExpr": "PM_DISP_STALL_HELD_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because of a synchronizing instruction that requires the ICT to be empty before dispatch",
>> + "MetricExpr": "PM_DISP_STALL_HELD_SYNC_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_SYNC_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch while waiting on the scoreboard",
>> + "MetricExpr": "PM_DISP_STALL_HELD_SCOREBOARD_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_SCOREBOARD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch due to issue q full",
>
> s/q/queue/
>
>> + "MetricExpr": "PM_DISP_STALL_HELD_ISSQ_FULL_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISP_HELD_STALL_ISSQ_FULL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_RENAME_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_RENAME_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the STF mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_STF_MAPPER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_STF_MAPPER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because the XVFC mapper/SRB was full",
>> + "MetricExpr": "PM_DISP_STALL_HELD_XVFC_MAPPER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_XVFC_MAPPER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch for any other reason",
>> + "MetricExpr": "PM_DISP_STALL_HELD_OTHER_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_OTHER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction has been dispatched but not issued for any reason",
>> + "MetricExpr": "PM_ISSUE_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "ISSUE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting to be finished in one of the execution units",
>> + "MetricExpr": "PM_EXEC_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "EXECUTION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction spent executing an NTC instruction that gets flushed some time after dispatch",
>> + "MetricExpr": "PM_EXEC_STALL_NTC_FLUSH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "NTC_FLUSH_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the instruction finishes at dispatch",
>
> I'm not sure what that means.
>
>> + "MetricExpr": "PM_EXEC_STALL_FIN_AT_DISP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "FIN_AT_DISP_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the branch unit",
>> + "MetricExpr": "PM_EXEC_STALL_BRU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "BRU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a simple fixed point instr that is executing in the lsu unit",
>
> s/instr/instruction/
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_SIMPLE_FX / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "SIMPLE_FX_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the vsu unit",
>
> s/vsu unit/VSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_VSU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "VSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting to be finished in one of the execution units",
>> + "MetricExpr": "PM_EXEC_STALL_TRANSLATION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TRANSLATION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a load or store that suffered a translation miss",
>> + "MetricExpr": "PM_EXEC_STALL_DERAT_ONLY_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DERAT_ONLY_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is recovering from a TLB miss",
>> + "MetricExpr": "PM_EXEC_STALL_DERAT_DTLB_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DERAT_DTLB_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_LSU / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a load that is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_LOAD / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LOAD_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3, with an RC dispatch conflict",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3_CONFLICT / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_CONFLICT_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from either the local L2 or local L3, without an RC dispatch conflict",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L2L3_NOCONFLICT / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L2L3_NOCONFLICT_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a source beyond the local L2 and local L3",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L3MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L3MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a neighbor chiplet's L2 or L3 in the same chip",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_L21_L31 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_L21_L31_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from local memory, L4 or OpenCapp chip",
>
> Most descriptions put L4 before memory.
> (My preference is to use an "Oxford comma", as in "memory, L4, or ..."
> (comma after "L4"), but acknowledge there are those who prefer otherwise.)
>
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_LMEM / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_LMEM_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a remote chip (cache, L4, memory or CAPP) in the same group",
>
> Is there a distinction between "OpenCapp" and "CAPP"? If not, pick one throughout.
> Is this supposed to be "OpenCAPI"?
>
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_OFF_CHIP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_OFF_CHIP_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is waiting for a load miss to resolve from a distant chip (cache, L4, memory or CAPP chip)",
>> + "MetricExpr": "PM_EXEC_STALL_DMISS_OFF_NODE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DMISS_OFF_NODE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing a TLBIEL instruction",
>> + "MetricExpr": "PM_EXEC_STALL_TLBIEL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TLBIEL_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is finishing a load after its data has been reloaded from a data source beyond the local L1, OR when the LSU is processing an L1-hit, OR when the NTF instruction merged with another load in the LMQ",
>> + "MetricExpr": "PM_EXEC_STALL_LOAD_FINISH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LOAD_FINISH_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a store that is executing in the lsu unit",
>
> s/lsu unit/LSU/
>
>> + "MetricExpr": "PM_EXEC_STALL_STORE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is in the store unit outside of handling store misses or other special store operations",
>
> s/store unit/LSU/ ?
>
>> + "MetricExpr": "PM_EXEC_STALL_STORE_PIPE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_PIPE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a store whose cache line was not resident in the L1 and had to wait for allocation of the missing line into the L1",
>> + "MetricExpr": "PM_EXEC_STALL_STORE_MISS / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STORE_MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a TLBIE instruction waiting for a response from the L2",
>> + "MetricExpr": "PM_EXEC_STALL_TLBIE / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "TLBIE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is executing a PTESYNC instruction",
>> + "MetricExpr": "PM_EXEC_STALL_PTESYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "PTESYNC_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete because the thread was blocked",
>> + "MetricExpr": "PM_CMPL_STALL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete because it was interrupted by ANY exception",
>> + "MetricExpr": "PM_CMPL_STALL_EXCEPTION / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "EXCEPTION_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is stuck at finish waiting for the non-speculative finish of either a stcx waiting for its result or a load waiting for non-critical sectors of data and ECC",
>> + "MetricExpr": "PM_CMPL_STALL_MEM_ECC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "MEM_ECC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction cannot complete the instruction is a stcx waiting for resolution from the nest",
>> + "MetricExpr": "PM_CMPL_STALL_STCX / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "STCX_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a LWSYNC instruction waiting to complete",
>
> Sometimes instruction mnemonics are ALL CAPS, like here, and sometimes not,
> like "stcx", above. Pick one style. Also pick whether the mnemonic is
> followed by "instruction" or not. I prefer including "instruction" for
> clarity.
>
>> + "MetricExpr": "PM_CMPL_STALL_LWSYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "LWSYNC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction is a HWSYNC instruction stuck at finish waiting for a response from the L2",
>> + "MetricExpr": "PM_CMPL_STALL_HWSYNC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "HWSYNC_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction required special handling before completion",
>> + "MetricExpr": "PM_CMPL_STALL_SPECIAL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "SPECIAL_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_translation or children are miscounting",
>
> Are these "Should equal 0" metrics generally useful?
>
>> + "MetricExpr": "DISPATCHED_TRANSLATION_CPI - (DISPATCHED_IERAT_ONLY_MISS_CPI + DISPATCHED_ITLB_MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_TRANSLATION_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_ic_miss or children are miscounting",
>> + "MetricExpr": "DISPATCHED_IC_MISS_CPI - (DISPATCHED_IC_L2_CPI + DISPATCHED_IC_L3_CPI + DISPATCHED_IC_L3MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_IC_MISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_br_mpred_icmiss or children are miscounting",
>> + "MetricExpr": "DISPATCHED_BR_MPRED_ICMISS_CPI - (DISPATCHED_BR_MPRED_IC_L2_CPI + DISPATCHED_BR_MPRED_IC_L3_CPI + DISPATCHED_BR_MPRED_IC_L3MISS_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_BR_MPRED_ICMISS_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_held_rename or children are miscounting",
>> + "MetricExpr": "DISPATCHED_HELD_RENAME_CPI - (DISPATCHED_HELD_STF_MAPPER_CPI + DISPATCHED_HELD_XVFC_MAPPER_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_HELD_RENAME_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall_held or children are miscounting",
>> + "MetricExpr": "DISPATCHED_HELD_CPI - (DISP_HELD_STALL_SYNC_CPI + DISP_HELD_STALL_SCOREBOARD_CPI + DISP_HELD_STALL_ISSQ_FULL_CPI + DISPATCHED_HELD_RENAME_CPI + DISPATCHED_HELD_OTHER_CPI + DISPATCHED_HELD_HALT_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_HELD_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, disp_stall or children are miscounting",
>> + "MetricExpr": "DISPATCHED_CPI - (DISPATCHED_FLUSH_CPI + DISPATCHED_TRANSLATION_CPI + DISPATCHED_IC_MISS_CPI + DISPATCHED_BR_MPRED_ICMISS_CPI + DISPATCHED_BR_MPRED_CPI + DISPATCHED_HELD_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DISPATCHED_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_translation or children are miscounting",
>> + "MetricExpr": "TRANSLATION_STALL_CPI - (DERAT_ONLY_MISS_STALL_CPI + DERAT_DTLB_MISS_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_TRANSLATION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_dmiss_l2l3 or children are miscounting",
>> + "MetricExpr": "DMISS_L2L3_STALL_CPI - (DMISS_L2L3_CONFLICT_STALL_CPI + DMISS_L2L3_NOCONFLICT_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DMISS_L2L3_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_dmiss_l3miss or children are miscounting",
>> + "MetricExpr": "DMISS_L3MISS_STALL_CPI - (DMISS_L21_L31_STALL_CPI + DMISS_LMEM_STALL_CPI + DMISS_OFF_CHIP_STALL_CPI + DMISS_OFF_NODE_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_DMISS_L3MISS_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_load or children are miscounting",
>> + "MetricExpr": "LOAD_STALL_CPI - (DMISS_L2L3_STALL_CPI + DMISS_L3MISS_STALL_CPI + TLBIEL_STALL_CPI + LOAD_FINISH_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_LOAD_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_store or children are miscounting",
>> + "MetricExpr": "STORE_STALL_CPI - (STORE_PIPE_STALL_CPI + STORE_MISS_STALL_CPI + TLBIE_STALL_CPI + PTESYNC_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_STORE_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall_lsu or children are miscounting",
>> + "MetricExpr": "LSU_STALL_CPI - (LOAD_STALL_CPI + STORE_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_LSU_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, cmpl_stall or children are miscounting",
>> + "MetricExpr": "COMPLETION_STALL_CPI - (EXCEPTION_COMPLETION_STALL_CPI + MEM_ECC_COMPLETION_STALL_CPI + STCX_COMPLETION_STALL_CPI + LWSYNC_COMPLETION_STALL_CPI + HWSYNC_COMPLETION_STALL_CPI + SPECIAL_COMPLETION_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_COMPLETION_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, exec_stall or children are miscounting",
>> + "MetricExpr": "EXECUTION_STALL_CPI - (NTC_FLUSH_STALL_CPI + FIN_AT_DISP_STALL_CPI + BRU_STALL_CPI + SIMPLE_FX_STALL_CPI + VSU_STALL_CPI + TRANSLATION_STALL_CPI + LSU_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_STALL_CPI"
>> + },
>> + {
>> + "BriefDescription": "Should equal 0. If not, pm_cyc or children are miscounting",
>> + "MetricExpr": "CYCLES_PER_INSTRUCTION - (DISPATCHED_CPI + ISSUE_STALL_CPI + EXECUTION_STALL_CPI + COMPLETION_STALL_CPI)",
>> + "MetricGroup": "CPI",
>> + "MetricName": "OTHER_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when dispatch was stalled because Fetch was being held, so there was nothing in the pipeline for this thread",
>
> s/Fetch/fetch/
> extra space after "held,"
>
>> + "MetricExpr": "PM_DISP_STALL_FETCH / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_FETCH_CPI"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntc instruction was held at dispatch because of power management",
>> + "MetricExpr": "PM_DISP_STALL_HELD_HALT_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "CPI",
>> + "MetricName": "DISPATCHED_HELD_HALT_CPI"
>> + },
>> + {
>> + "BriefDescription": "Percentage of flushes per completed instruction",
>> + "MetricExpr": "PM_FLUSH / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "FLUSH_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of flushes due to a branch mispredict per instruction",
>> + "MetricExpr": "PM_FLUSH_MPRED / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "BR_MPRED_FLUSH_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of branch mispredictions per completed instruction",
>> + "MetricExpr": "PM_BR_MPRED_CMPL / PM_RUN_INST_CMPL",
>> + "MetricGroup": "Others",
>> + "MetricName": "BRANCH_MISPREDICTION_RATE"
>> + },
>> + {
>> + "BriefDescription": "Percentage of finished loads that missed in the L1",
>> + "MetricExpr": "PM_LD_MISS_L1 / PM_LD_REF_L1 * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "L1_LD_MISS_RATIO",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were loads that missed the L1",
>> + "MetricExpr": "PM_LD_MISS_L1 / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "L1_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of instructions when the DPTEG required for the load/store instruction in execution was missing from the TLB",
>> + "MetricExpr": "PM_DTLB_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Others",
>> + "MetricName": "DTLB_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of instruction dispatched per instruction completed",
>
> s/instruction/instrucions/
>
>> + "MetricExpr": "PM_INST_DISP / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "DISPATCH_PER_INST_CMPL"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were a demand load that did not hit in the L1 or L2",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L2_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were demand fetches that missed the L1 instruction cache",
>> + "MetricExpr": "PM_L1_ICACHE_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "L1_INST_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were demand fetches that reloaded from beyond the L3 instruction cache",
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L3_INST_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of completed instructions per cycle",
>> + "MetricExpr": "PM_INST_CMPL / PM_CYC",
>> + "MetricGroup": "General",
>> + "MetricName": "IPC"
>> + },
>> + {
>> + "BriefDescription": "Average number of cycles per completed instruction group",
>> + "MetricExpr": "PM_CYC / PM_1PLUS_PPC_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "CYCLES_PER_COMPLETED_INSTRUCTIONS_SET"
>> + },
>> + {
>> + "BriefDescription": "Percentage of cycles when at least 1 instruction dispatched",
>> + "MetricExpr": "PM_1PLUS_PPC_DISP / PM_RUN_CYC * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "CYCLES_ATLEAST_ONE_INST_DISPATCHED",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished loads per completed instruction",
>
> Most similar "rate" metrics are using the phrase "average number of".
> Do we want to use that here as well? (Applies to all "rate" metrics.)
>
>> + "MetricExpr": "PM_LD_REF_L1 / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LOADS_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished stores per completed instruction",
>> + "MetricExpr": "PM_ST_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "STORES_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand loads that reloaded from beyond the L2 per completed instruction",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L2_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand loads that reloaded from beyond the L3 per completed instruction",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L3_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 4k page size per completed run instruction",
>
> When PM_RUN_INST_CMPL is used, sometimes we say "run instruction",
> and sometimes we say "completed instruction". Let's pick one.
>
>> + "MetricExpr": "PM_DERAT_MISS_4K / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_4K_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 64k page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_64K / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_64K_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of run cycles per completed run instruction",
>
> Here we cover our bases and say "completed run instruction". ;-)
> Let's convert this one to whichever phrase is chosen for PM_RUN_INST_CMPL.
> Seen below, too.
>
>> + "MetricExpr": "PM_RUN_CYC / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_CPI"
>> + },
>> + {
>> + "BriefDescription": "Total number of run cycles",
>> + "MetricExpr": "PM_RUN_CYC",
>
> Isn't this more an event than a metric?
> Does it need to be included here?
>
>> + "MetricGroup": "General",
>> + "MetricName": "TOTAL_RUN_CYCLES"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Average number of completed run instructions per run cycle",
>> + "MetricExpr": "PM_RUN_INST_CMPL / PM_RUN_CYC",
>> + "MetricGroup": "General",
>> + "MetricName": "RUN_IPC"
>> + },
>> + {
>> + "BriefDescription": "Average number of instruction completed per instruction group",
>
> s/instruction/instructions/
>
>> + "MetricExpr": "PM_RUN_INST_CMPL / PM_1PLUS_PPC_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "AVERAGE_COMPLETED_INSTRUCTION_SET_SIZE"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished instructions per completed instructions",
>
>
>> + "MetricExpr": "PM_INST_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "INST_FIN_PER_CMPL"
>> + },
>> + {
>> + "BriefDescription": "Average cycles per instruction when the ntf instruction is completing and the finish was overlooked",
>
> s/ntf/NTF/
> "overlooked" seems like an odd term.
>
>> + "MetricExpr": "PM_EXEC_STALL_UNKNOWN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "EXEC_STALL_UNKOWN_CPI"
>> + },
>> + {
>> + "BriefDescription": "Percentage of finished branches that were taken",
>> + "MetricExpr": "PM_BR_TAKEN_CMPL / PM_BR_FIN * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "TAKEN_BRANCHES",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of completed instructions that were a demand load that did not hit in the L1, L2, or the L3",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "L3_LD_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Rate of finished branches per completed instruction",
>> + "MetricExpr": "PM_BR_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "BRANCHES_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of instructions finished in the LSU per completed instruction",
>> + "MetricExpr": "PM_LSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LSU_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of instructions finished in the VSU per completed instruction",
>> + "MetricExpr": "PM_VSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "VSU_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of TLBIE instructions finished in the LSU per completed instruction",
>> + "MetricExpr": "PM_TLBIE_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "TLBIE_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of STCX instructions finshed per completed instruction",
>> + "MetricExpr": "PM_STCX_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "STXC_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of LARX instructions finshed per completed instruction",
>> + "MetricExpr": "PM_LARX_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "LARX_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of ptesync instructions finshed per completed instruction",
>> + "MetricExpr": "PM_PTESYNC_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "PTESYNC_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Rate of simple fixed-point instructions finshed in the store unit per completed instruction",
>
> s/store unit/LSU/ ?
>
>> + "MetricExpr": "PM_FX_LSU_FIN / PM_RUN_INST_CMPL",
>> + "MetricGroup": "General",
>> + "MetricName": "FX_PER_INST"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded the L1 cache",
>> + "MetricExpr": "PM_LD_DEMAND_MISS_L1 / PM_LD_MISS_L1 * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "DL1_MISS_RELOADS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded from beyond the local L2",
>> + "MetricExpr": "PM_DATA_FROM_L2MISS / PM_LD_DEMAND_MISS_L1 * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L2_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of demand load misses that reloaded from beyond the local L3",
>> + "MetricExpr": "PM_DATA_FROM_L3MISS / PM_LD_DEMAND_MISS_L1 * 100",
>> + "MetricGroup": "dL1_Reloads",
>> + "MetricName": "DL1_RELOAD_FROM_L3_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of cycles stalled due to the ntc instruction waiting for a load miss to resolve from a source beyond the local L2 and local L3",
>> + "MetricExpr": "DMISS_L3MISS_STALL_CPI / RUN_CPI * 100",
>> + "MetricGroup": "General",
>> + "MetricName": "DCACHE_MISS_CPI",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 2M page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_2M / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_2M_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses with 16M page size per completed run instruction",
>> + "MetricExpr": "PM_DERAT_MISS_16M / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_16M_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 4K page size",
>> + "MetricExpr": "PM_DERAT_MISS_4K / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_4K_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 2M page size",
>> + "MetricExpr": "PM_DERAT_MISS_2M / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_2M_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 16M page size",
>> + "MetricExpr": "PM_DERAT_MISS_16M / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_16M_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "DERAT miss ratio for 64K page size",
>> + "MetricExpr": "PM_DERAT_MISS_64K / PM_DERAT_MISS",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_64K_MISS_RATIO"
>> + },
>> + {
>> + "BriefDescription": "Percentage of DERAT misses that resulted in TLB reloads",
>> + "MetricExpr": "PM_DTLB_MISS / PM_DERAT_MISS * 100",
>> + "MetricGroup": "Translation",
>> + "MetricName": "DERAT_MISS_RELOAD",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of ICache misses that were reloaded from beyond the local L3",
>
> Sometimes we use "ICache" and sometimes "icache". Pick one.
>
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_L1_ICACHE_MISS * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "INST_FROM_L3_MISS",
>> + "ScaleUnit": "1%"
>> + },
>> + {
>> + "BriefDescription": "Percentage of ICache reloads from the beyond the L3 per completed run instruction",
>> + "MetricExpr": "PM_INST_FROM_L3MISS / PM_RUN_INST_CMPL * 100",
>> + "MetricGroup": "Instruction_Misses",
>> + "MetricName": "INST_FROM_L3_MISS_RATE",
>> + "ScaleUnit": "1%"
>> + }
>> +]
>
> PC
>
^ permalink raw reply
* Re: [PATCH v6 19/22] powerpc/book3s64/hash/kuap: Enable kuap on hash
From: Christophe Leroy @ 2021-10-08 9:32 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Sandipan Das
In-Reply-To: <20201125051634.509286-20-aneesh.kumar@linux.ibm.com>
Le 25/11/2020 à 06:16, Aneesh Kumar K.V a écrit :
> Reviewed-by: Sandipan Das <sandipan@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/pkeys.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index f747d66cc87d..84f8664ffc47 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -257,7 +257,12 @@ void __init setup_kuep(bool disabled)
> #ifdef CONFIG_PPC_KUAP
> void __init setup_kuap(bool disabled)
> {
> - if (disabled || !early_radix_enabled())
> + if (disabled)
> + return;
> + /*
> + * On hash if PKEY feature is not enabled, disable KUAP too.
> + */
> + if (!early_radix_enabled() && !early_mmu_has_feature(MMU_FTR_PKEY))
pkey_early_init_devtree() bails out without setting MMU_FTR_PKEY with
the following:
/*
* Only P7 and above supports SPRN_AMR update with MSR[PR] = 1
*/
if (!early_cpu_has_feature(CPU_FTR_ARCH_206))
return;
Why would it be impossible to do KUAP in that case ? KUAP doesn't
require updating SPRN_AMR with MSR[PR] = 1
> return;
>
> if (smp_processor_id() == boot_cpuid) {
>
^ 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