LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/22] PCI: Add PCI_ERROR_RESPONSE and it's related defintions
From: Naveen Naidu @ 2021-10-11 17:37 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-samsung-soc, linux-rockchip, linux-pci, linuxppc-dev,
	linux-kernel, linux-renesas-soc, Naveen Naidu,
	bcm-kernel-feedback-list, linux-mediatek, linux-kernel-mentees,
	linux-arm-kernel
In-Reply-To: <cover.1633972263.git.naveennaidu479@gmail.com>

An MMIO read from a PCI device that doesn't exist or doesn't respond
causes a PCI error.  There's no real data to return to satisfy the
CPU read, so most hardware fabricates ~0 data.

Add a PCI_ERROR_RESPONSE definition for that and use it where
appropriate to make these checks consistent and easier to find.

Also add helper definitions SET_PCI_ERROR_RESPONSE and
RESPONSE_IS_PCI_ERROR to make the code more readable.

Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
---
 include/linux/pci.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index cd8aa6fce204..928c589bb5c4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -154,6 +154,15 @@ enum pci_interrupt_pin {
 /* The number of legacy PCI INTx interrupts */
 #define PCI_NUM_INTX	4
 
+/*
+ * Reading from a device that doesn't respond typically returns ~0.  A
+ * successful read from a device may also return ~0, so you need additional
+ * information to reliably identify errors.
+ */
+#define PCI_ERROR_RESPONSE			(~0ULL)
+#define SET_PCI_ERROR_RESPONSE(val)	(*val = ((typeof(*val)) PCI_ERROR_RESPONSE))
+#define RESPONSE_IS_PCI_ERROR(val)	(*val == ((typeof(*val)) PCI_ERROR_RESPONSE))
+
 /*
  * pci_power_t values must match the bits in the Capabilities PME_Support
  * and Control/Status PowerState fields in the Power Management capability.
-- 
2.25.1


^ permalink raw reply related

* [PATCH 00/22] PCI: Unify PCI error response checking
From: Naveen Naidu @ 2021-10-11 17:35 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-samsung-soc, linux-rockchip, linux-pci, linuxppc-dev,
	linux-kernel, linux-renesas-soc, Naveen Naidu,
	bcm-kernel-feedback-list, linux-mediatek, linux-kernel-mentees,
	linux-arm-kernel

An MMIO read from a PCI device that doesn't exist or doesn't respond
causes a PCI error.  There's no real data to return to satisfy the 
CPU read, so most hardware fabricates ~0 data.

This patch series adds PCI_ERROR_RESPONSE definition and other helper
defintion SET_PCI_ERROR_RESPONSE and RESPONSE_IS_PCI_ERROR and uses it
where appropriate to make these checks consistent and easier to find.

This helps unify PCI error response checking and make error check
consistent and easier to find.

Patch 1:
  - Adds the PCI_ERROR_RESPONSE and other related defintions
  - All other patches are dependent on this patch. This patch needs to
    be applied first, before the others

Patch 2 - 13
  - Uses SET_PCI_ERROR_RESPONSE() when device is not found

Patch 14 - 19
  - Uses RESPONSE_IS_PCI_ERROR() to check the reads from hardware

Patch 20 - 22
  - Edits the comments to include PCI_ERROR_RESPONSE alsong with
    0xFFFFFFFF, so that it becomes easier to grep for faulty hardware
    reads.

Thanks,
Naveen

Naveen Naidu (22):
  [PATCH 1/22] PCI: Add PCI_ERROR_RESPONSE and it's related defintions
  [PATCH 2/22] PCI: Unify PCI error response checking
  [PATCH 3/22] PCI: thunder: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 4/22] PCI: iproc: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 5/22] PCI: mediatek: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 6/22] PCI: exynos: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 7/22] PCI: histb: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 8/22] PCI: kirin: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 9/22] PCI: aardvark: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 10/22] PCI: mvebu: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 11/22] PCI: altera: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 12/22] PCI: rcar: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 13/22] PCI: rockchip: Use SET_PCI_ERROR_RESPONSE() when device not found
  [PATCH 14/22] PCI/ERR: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 15/22] PCI: vmd: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 16/22] PCI: pciehp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 17/22] PCI/DPC: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 18/22] PCI/PME: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 19/22] PCI: cpqphp: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
  [PATCH 20/22] PCI: keystone: Use PCI_ERROR_RESPONSE to specify hardware error
  [PATCH 21/22] PCI: hv: Use PCI_ERROR_RESPONSE to specify hardware read error
  [PATCH 22/22] PCI: xgene: Use PCI_ERROR_RESPONSE to specify hardware error

 drivers/pci/access.c                        | 22 ++++++++++-----------
 drivers/pci/controller/dwc/pci-exynos.c     |  2 +-
 drivers/pci/controller/dwc/pci-keystone.c   |  4 ++--
 drivers/pci/controller/dwc/pcie-histb.c     |  2 +-
 drivers/pci/controller/dwc/pcie-kirin.c     |  2 +-
 drivers/pci/controller/pci-aardvark.c       |  8 ++++----
 drivers/pci/controller/pci-hyperv.c         |  2 +-
 drivers/pci/controller/pci-mvebu.c          |  4 ++--
 drivers/pci/controller/pci-thunder-ecam.c   | 20 +++++++++----------
 drivers/pci/controller/pci-thunder-pem.c    |  2 +-
 drivers/pci/controller/pci-xgene.c          |  8 ++++----
 drivers/pci/controller/pcie-altera.c        |  2 +-
 drivers/pci/controller/pcie-iproc.c         |  2 +-
 drivers/pci/controller/pcie-mediatek.c      |  4 ++--
 drivers/pci/controller/pcie-rcar-host.c     |  2 +-
 drivers/pci/controller/pcie-rockchip-host.c |  2 +-
 drivers/pci/controller/vmd.c                |  2 +-
 drivers/pci/hotplug/cpqphp_ctrl.c           |  4 ++--
 drivers/pci/hotplug/pciehp_hpc.c            | 10 +++++-----
 drivers/pci/pci.c                           | 10 +++++-----
 drivers/pci/pcie/dpc.c                      |  4 ++--
 drivers/pci/pcie/pme.c                      |  4 ++--
 drivers/pci/probe.c                         | 10 +++++-----
 include/linux/pci.h                         |  9 +++++++++
 24 files changed, 75 insertions(+), 66 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor'
to know whether arch has function descriptors.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 4 ++--
 arch/parisc/include/asm/sections.h  | 6 ++++--
 arch/powerpc/include/asm/sections.h | 6 ++++--
 include/asm-generic/sections.h      | 3 ++-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 35f24e52149a..80f5868afb06 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -7,6 +7,8 @@
  *	David Mosberger-Tang <davidm@hpl.hp.com>
  */
 
+#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+
 #include <linux/elf.h>
 #include <linux/uaccess.h>
 #include <asm-generic/sections.h>
@@ -27,8 +29,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
 extern char __start_unwind[], __end_unwind[];
 extern char __start_ivt_text[], __end_ivt_text[];
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 static inline void *dereference_function_descriptor(void *ptr)
 {
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index bb52aea0cb21..2e781ee19b66 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -2,6 +2,10 @@
 #ifndef _PARISC_SECTIONS_H
 #define _PARISC_SECTIONS_H
 
+#ifdef CONFIG_64BIT
+#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+#endif
+
 /* nothing to see, move along */
 #include <asm-generic/sections.h>
 
@@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
 
 #ifdef CONFIG_64BIT
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 void *dereference_function_descriptor(void *);
 
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 32e7035863ac..b7f1ba04e756 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -8,6 +8,10 @@
 
 #define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
 
+#ifdef PPC64_ELF_ABI_v1
+#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+#endif
+
 #include <asm-generic/sections.h>
 
 extern bool init_mem_is_free;
@@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
 
 #ifdef PPC64_ELF_ABI_v1
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 static inline void *dereference_function_descriptor(void *ptr)
 {
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index d16302d3eb59..1db5cfd69817 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
 extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
-#ifndef dereference_function_descriptor
+#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
+#else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
 #endif
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 01/10] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h

It was initially in module_64.c and commit 2d291e902791 ("Fix compile
failure with non modular builds") moved it into asm/elf.h

But it was by mistake added outside of __KERNEL__ section,
therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
arch/powerpc/include/asm") moved it to uapi/asm/elf.h

Move it back into asm/elf.h, this brings it back in line with
IA64 and PARISC architectures.

Fixes: 2d291e902791 ("Fix compile failure with non modular builds")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/elf.h      | 7 +++++++
 arch/powerpc/include/uapi/asm/elf.h | 8 --------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index b8425e3cfd81..64b523848cd7 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -176,4 +176,11 @@ do {									\
 /* Relocate the kernel image to @final_address */
 void relocate(unsigned long final_address);
 
+/* There's actually a third entry here, but it's unused */
+struct ppc64_opd_entry
+{
+	unsigned long funcaddr;
+	unsigned long r2;
+};
+
 #endif /* _ASM_POWERPC_ELF_H */
diff --git a/arch/powerpc/include/uapi/asm/elf.h b/arch/powerpc/include/uapi/asm/elf.h
index 860c59291bfc..308857123a08 100644
--- a/arch/powerpc/include/uapi/asm/elf.h
+++ b/arch/powerpc/include/uapi/asm/elf.h
@@ -289,12 +289,4 @@ typedef elf_fpreg_t elf_vsrreghalf_t32[ELF_NVSRHALFREG];
 /* Keep this the last entry.  */
 #define R_PPC64_NUM		253
 
-/* There's actually a third entry here, but it's unused */
-struct ppc64_opd_entry
-{
-	unsigned long funcaddr;
-	unsigned long r2;
-};
-
-
 #endif /* _UAPI_ASM_POWERPC_ELF_H */
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 05/10] asm-generic: Define 'funct_descr_t' to commonly describe function descriptors
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

We have three architectures using function descriptors, each with its
own name.

Add a common typedef that can be used in generic code.

Also add a stub typedef for architecture without function descriptors,
to avoid a forest of #ifdefs.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 1 +
 arch/parisc/include/asm/sections.h  | 1 +
 arch/powerpc/include/asm/sections.h | 1 +
 include/asm-generic/sections.h      | 3 +++
 4 files changed, 6 insertions(+)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 80f5868afb06..929b5c535620 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -8,6 +8,7 @@
  */
 
 #define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+typedef struct fdesc funct_descr_t;
 
 #include <linux/elf.h>
 #include <linux/uaccess.h>
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 2e781ee19b66..329e80f7af0a 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -4,6 +4,7 @@
 
 #ifdef CONFIG_64BIT
 #define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+typedef Elf64_Fdesc funct_descr_t;
 #endif
 
 /* nothing to see, move along */
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index b7f1ba04e756..d0d5287fa568 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -10,6 +10,7 @@
 
 #ifdef PPC64_ELF_ABI_v1
 #define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+typedef struct ppc64_opd_entry funct_descr_t;
 #endif
 
 #include <asm-generic/sections.h>
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 1db5cfd69817..436412d94054 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
 #else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
+typedef struct {
+	unsigned long addr;
+} funct_descr_t;
 #endif
 
 /* random extra sections (if any).  Override
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 08/10] lkdtm: Really write into kernel text in WRITE_KERN
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

WRITE_KERN is supposed to overwrite some kernel text, namely
do_overwritten() function.

But at the time being it overwrites do_overwritten() function
descriptor, not function text.

Fix it by dereferencing the function descriptor to obtain
function text pointer.

And make do_overwritten() noinline so that it is really
do_overwritten() which is called by lkdtm_WRITE_KERN().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/perms.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 60b3b2fe929d..442d60ed25ef 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -5,6 +5,7 @@
  * even non-readable regions.
  */
 #include "lkdtm.h"
+#include <linux/kallsyms.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/mman.h>
@@ -37,7 +38,7 @@ static noinline void do_nothing(void)
 }
 
 /* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
+static noinline void do_overwritten(void)
 {
 	pr_info("do_overwritten wasn't overwritten!\n");
 	return;
@@ -113,8 +114,9 @@ void lkdtm_WRITE_KERN(void)
 	size_t size;
 	volatile unsigned char *ptr;
 
-	size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
-	ptr = (unsigned char *)do_overwritten;
+	size = (unsigned long)dereference_symbol_descriptor(do_overwritten) -
+	       (unsigned long)dereference_symbol_descriptor(do_nothing);
+	ptr = dereference_symbol_descriptor(do_overwritten);
 
 	pr_info("attempting bad %zu byte write at %px\n", size, ptr);
 	memcpy((void *)ptr, (unsigned char *)do_nothing, size);
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 00/10] Fix LKDTM for PPC64/IA64/PARISC
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev

PPC64/IA64/PARISC have function descriptors. LKDTM doesn't work
on those three architectures because LKDTM messes up function
descriptors with functions.

This series does some cleanup in the three architectures and
refactors function descriptors so that it can then easily use it
in a generic way in LKDTM.

Patch 6 is not absolutely necessary but it is a good trivial cleanup.

Christophe Leroy (10):
  powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
  powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
  ia64: Rename 'ip' to 'addr' in 'struct fdesc'
  asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define
    associated stubs
  asm-generic: Define 'funct_descr_t' to commonly describe function
    descriptors
  asm-generic: Refactor dereference_[kernel]_function_descriptor()
  lkdtm: Force do_nothing() out of line
  lkdtm: Really write into kernel text in WRITE_KERN
  lkdtm: Fix lkdtm_EXEC_RODATA()
  lkdtm: Fix execute_[user]_location()

 arch/ia64/include/asm/elf.h         |  2 +-
 arch/ia64/include/asm/sections.h    | 24 ++---------
 arch/ia64/kernel/module.c           |  6 +--
 arch/parisc/include/asm/sections.h  | 16 +++----
 arch/parisc/kernel/process.c        | 21 ---------
 arch/powerpc/include/asm/elf.h      |  7 +++
 arch/powerpc/include/asm/sections.h | 30 +++----------
 arch/powerpc/include/uapi/asm/elf.h |  8 ----
 arch/powerpc/kernel/module_64.c     |  6 +--
 drivers/misc/lkdtm/perms.c          | 66 +++++++++++++++++++++++------
 include/asm-generic/sections.h      | 24 ++++++++++-
 11 files changed, 102 insertions(+), 108 deletions(-)

-- 
2.31.1


^ permalink raw reply

* [PATCH v1 07/10] lkdtm: Force do_nothing() out of line
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

LKDTM tests display that the run do_nothing() at a given
address, but in reality do_nothing() is inlined into the
caller.

Force it out of line so that it really runs text at the
displayed address.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/perms.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f..60b3b2fe929d 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -21,7 +21,7 @@
 /* This is non-const, so it will end up in the .data section. */
 static u8 data_area[EXEC_SIZE];
 
-/* This is cost, so it will end up in the .rodata section. */
+/* This is const, so it will end up in the .rodata section. */
 static const unsigned long rodata = 0xAA55AA55;
 
 /* This is marked __ro_after_init, so it should ultimately be .rodata. */
@@ -31,7 +31,7 @@ static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
  */
-static void do_nothing(void)
+static noinline void do_nothing(void)
 {
 	return;
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 06/10] asm-generic: Refactor dereference_[kernel]_function_descriptor()
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

dereference_function_descriptor() and
dereference_kernel_function_descriptor() are identical on the
three architectures implementing them.

Make it common.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 19 -------------------
 arch/parisc/include/asm/sections.h  |  9 ---------
 arch/parisc/kernel/process.c        | 21 ---------------------
 arch/powerpc/include/asm/sections.h | 23 -----------------------
 include/asm-generic/sections.h      | 18 ++++++++++++++++++
 5 files changed, 18 insertions(+), 72 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 929b5c535620..d9addaea8339 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -30,23 +30,4 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
 extern char __start_unwind[], __end_unwind[];
 extern char __start_ivt_text[], __end_ivt_text[];
 
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
-	struct fdesc *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
-		return ptr;
-	return dereference_function_descriptor(ptr);
-}
-
 #endif /* _ASM_IA64_SECTIONS_H */
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 329e80f7af0a..b041fb32a300 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -12,13 +12,4 @@ typedef Elf64_Fdesc funct_descr_t;
 
 extern char __alt_instructions[], __alt_instructions_end[];
 
-#ifdef CONFIG_64BIT
-
-#undef dereference_function_descriptor
-void *dereference_function_descriptor(void *);
-
-#undef dereference_kernel_function_descriptor
-void *dereference_kernel_function_descriptor(void *);
-#endif
-
 #endif
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 38ec4ae81239..7382576b52a8 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -266,27 +266,6 @@ get_wchan(struct task_struct *p)
 	return 0;
 }
 
-#ifdef CONFIG_64BIT
-void *dereference_function_descriptor(void *ptr)
-{
-	Elf64_Fdesc *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd ||
-			ptr >= (void *)__end_opd)
-		return ptr;
-
-	return dereference_function_descriptor(ptr);
-}
-#endif
-
 static inline unsigned long brk_rnd(void)
 {
 	return (get_random_int() & BRK_RND_MASK) << PAGE_SHIFT;
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index d0d5287fa568..8f8e95f234e2 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -72,29 +72,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
 		(unsigned long)_stext < end;
 }
 
-#ifdef PPC64_ELF_ABI_v1
-
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
-	struct ppc64_opd_entry *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
-		return ptr;
-
-	return dereference_function_descriptor(ptr);
-}
-#endif /* PPC64_ELF_ABI_v1 */
-
 #endif
 
 #endif /* __KERNEL__ */
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 436412d94054..5baaf9d7c671 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -60,6 +60,24 @@ extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
 #ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
+static inline void *dereference_function_descriptor(void *ptr)
+{
+	funct_descr_t *desc = ptr;
+	void *p;
+
+	if (!get_kernel_nofault(p, (void *)&desc->addr))
+		ptr = p;
+	return ptr;
+}
+
+static inline void *dereference_kernel_function_descriptor(void *ptr)
+{
+	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
+		return ptr;
+
+	return dereference_function_descriptor(ptr);
+}
+
 #else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 03/10] ia64: Rename 'ip' to 'addr' in 'struct fdesc'
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.

powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'

Vote for 'addr' and update 'struct fdesc' accordingly.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/elf.h      | 2 +-
 arch/ia64/include/asm/sections.h | 2 +-
 arch/ia64/kernel/module.c        | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/include/asm/elf.h b/arch/ia64/include/asm/elf.h
index 6629301a2620..2ef5f9966ad1 100644
--- a/arch/ia64/include/asm/elf.h
+++ b/arch/ia64/include/asm/elf.h
@@ -226,7 +226,7 @@ struct got_entry {
  * Layout of the Function Descriptor
  */
 struct fdesc {
-	uint64_t ip;
+	uint64_t addr;
 	uint64_t gp;
 };
 
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 3a033d2008b3..35f24e52149a 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -35,7 +35,7 @@ static inline void *dereference_function_descriptor(void *ptr)
 	struct fdesc *desc = ptr;
 	void *p;
 
-	if (!get_kernel_nofault(p, (void *)&desc->ip))
+	if (!get_kernel_nofault(p, (void *)&desc->addr))
 		ptr = p;
 	return ptr;
 }
diff --git a/arch/ia64/kernel/module.c b/arch/ia64/kernel/module.c
index 2cba53c1da82..4f6400cbf79e 100644
--- a/arch/ia64/kernel/module.c
+++ b/arch/ia64/kernel/module.c
@@ -602,15 +602,15 @@ get_fdesc (struct module *mod, uint64_t value, int *okp)
 		return value;
 
 	/* Look for existing function descriptor. */
-	while (fdesc->ip) {
-		if (fdesc->ip == value)
+	while (fdesc->addr) {
+		if (fdesc->addr == value)
 			return (uint64_t)fdesc;
 		if ((uint64_t) ++fdesc >= mod->arch.opd->sh_addr + mod->arch.opd->sh_size)
 			BUG();
 	}
 
 	/* Create new one */
-	fdesc->ip = value;
+	fdesc->addr = value;
 	fdesc->gp = mod->arch.gp;
 	return (uint64_t) fdesc;
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 02/10] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.

powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'

Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/elf.h      | 2 +-
 arch/powerpc/include/asm/sections.h | 2 +-
 arch/powerpc/kernel/module_64.c     | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index 64b523848cd7..90c3259a81ab 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -179,7 +179,7 @@ void relocate(unsigned long final_address);
 /* There's actually a third entry here, but it's unused */
 struct ppc64_opd_entry
 {
-	unsigned long funcaddr;
+	unsigned long addr;
 	unsigned long r2;
 };
 
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 6e4af4492a14..32e7035863ac 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr)
 	struct ppc64_opd_entry *desc = ptr;
 	void *p;
 
-	if (!get_kernel_nofault(p, (void *)&desc->funcaddr))
+	if (!get_kernel_nofault(p, (void *)&desc->addr))
 		ptr = p;
 	return ptr;
 }
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..82908c9be627 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr)
 }
 static unsigned long func_addr(unsigned long addr)
 {
-	return func_desc(addr).funcaddr;
+	return func_desc(addr).addr;
 }
 static unsigned long stub_func_addr(func_desc_t func)
 {
-	return func.funcaddr;
+	return func.addr;
 }
 static unsigned int local_entry_offset(const Elf64_Sym *sym)
 {
@@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y)
 static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 				    const Elf64_Shdr *sechdrs)
 {
-	/* One extra reloc so it's always 0-funcaddr terminated */
+	/* One extra reloc so it's always 0-addr terminated */
 	unsigned long relocs = 1;
 	unsigned i;
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 10/10] lkdtm: Fix execute_[user]_location()
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

execute_location() and execute_user_location() intent
to copy do_nothing() text and execute it at a new location.
However, at the time being it doesn't copy do_nothing() function
but do_nothing() function descriptor which still points to the
original text. So at the end it still executes do_nothing() at
its original location allthough using a copied function descriptor.

So, fix that by really copying do_nothing() text and build a new
function descriptor by copying do_nothing() function descriptor and
updating the target address with the new location.

Also fix the displayed addresses by dereferencing do_nothing()
function descriptor.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/perms.c | 45 +++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index da16564e1ecd..1d03cd44c21d 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -44,19 +44,42 @@ static noinline void do_overwritten(void)
 	return;
 }
 
+static void *setup_function_descriptor(funct_descr_t *fdesc, void *dst)
+{
+	int err;
+
+	if (!__is_defined(HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR))
+		return dst;
+
+	err = copy_from_kernel_nofault(fdesc, do_nothing, sizeof(*fdesc));
+	if (err < 0)
+		return ERR_PTR(err);
+
+	fdesc->addr = (unsigned long)dst;
+	barrier();
+
+	return fdesc;
+}
+
 static noinline void execute_location(void *dst, bool write)
 {
-	void (*func)(void) = dst;
+	void (*func)(void);
+	funct_descr_t fdesc;
+	void *do_nothing_text = dereference_symbol_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing_text);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing, EXEC_SIZE);
+		memcpy(dst, do_nothing_text, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
-	pr_info("attempting bad execution at %px\n", func);
+	func = setup_function_descriptor(&fdesc, dst);
+	if (IS_ERR(func))
+		return;
+
+	pr_info("attempting bad execution at %px\n", dst);
 	func();
 	pr_err("FAIL: func returned\n");
 }
@@ -66,16 +89,22 @@ static void execute_user_location(void *dst)
 	int copied;
 
 	/* Intentionally crossing kernel/user memory boundary. */
-	void (*func)(void) = dst;
+	void (*func)(void);
+	funct_descr_t fdesc;
+	void *do_nothing_text = dereference_symbol_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing_text);
 	do_nothing();
 
-	copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+	copied = access_process_vm(current, (unsigned long)dst, do_nothing_text,
 				   EXEC_SIZE, FOLL_WRITE);
 	if (copied < EXEC_SIZE)
 		return;
-	pr_info("attempting bad execution at %px\n", func);
+	func = setup_function_descriptor(&fdesc, dst);
+	if (IS_ERR(func))
+		return;
+
+	pr_info("attempting bad execution at %px\n", dst);
 	func();
 	pr_err("FAIL: func returned\n");
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v1 09/10] lkdtm: Fix lkdtm_EXEC_RODATA()
From: Christophe Leroy @ 2021-10-11 15:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1633964380.git.christophe.leroy@csgroup.eu>

Behind a location, lkdtm_EXEC_RODATA() executes a real function,
not a copy of do_nothing().

So do it directly instead of using execute_location().

And fix displayed addresses by dereferencing the function descriptors.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/perms.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 442d60ed25ef..da16564e1ecd 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -153,7 +153,14 @@ void lkdtm_EXEC_VMALLOC(void)
 
 void lkdtm_EXEC_RODATA(void)
 {
-	execute_location(lkdtm_rodata_do_nothing, CODE_AS_IS);
+	pr_info("attempting ok execution at %px\n",
+		dereference_symbol_descriptor(do_nothing));
+	do_nothing();
+
+	pr_info("attempting bad execution at %px\n",
+		dereference_symbol_descriptor(lkdtm_rodata_do_nothing));
+	lkdtm_rodata_do_nothing();
+	pr_err("FAIL: func returned\n");
 }
 
 void lkdtm_EXEC_USERSPACE(void)
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH] KVM: PPC: Book3S HV: H_ENTER filter out reserved HPTE[B] value
From: Fabiano Rosas @ 2021-10-11 14:47 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20211004145749.1331331-1-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> The HPTE B field is a 2-bit field with values 0b10 and 0b11 reserved.
> This field is also taken from the HPTE and used when KVM executes
> TLBIEs to set the B field of those instructions.
>
> Disallow the guest setting B to a reserved value with H_ENTER by
> rejecting it. This is the same approach already taken for rejecting
> reserved (unsupported) LLP values. This prevents the guest from being
> able to induce the host to execute TLBIE with reserved values, which
> is not known to be a problem with current processors but in theory it
> could prevent the TLBIE from working correctly in a future processor.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

The ISA says:

B Segment Size Selector
0b00 - 256 MB (s=28)
0b01 - 1 TB (s=40)
0b10 - reserved
0b11 - reserved

So that looks good. I couldn't find any other guest initiated PTE
modifications, so I think we're covered.

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  arch/powerpc/include/asm/kvm_book3s_64.h | 4 ++++
>  arch/powerpc/kvm/book3s_hv_rm_mmu.c      | 9 +++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 19b6942c6969..fff391b9b97b 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -378,6 +378,10 @@ static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
>  		rb |= 1;		/* L field */
>  		rb |= r & 0xff000 & ((1ul << a_pgshift) - 1); /* LP field */
>  	}
> +	/*
> +	 * This sets both bits of the B field in the PTE. 0b1x values are
> +	 * reserved, but those will have been filtered by kvmppc_do_h_enter.
> +	 */
>  	rb |= (v >> HPTE_V_SSIZE_SHIFT) << 8;	/* B field */
>  	return rb;
>  }
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> index 632b2545072b..2c1f3c6e72d1 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -207,6 +207,15 @@ long kvmppc_do_h_enter(struct kvm *kvm, unsigned long flags,
>
>  	if (kvm_is_radix(kvm))
>  		return H_FUNCTION;
> +	/*
> +	 * The HPTE gets used by compute_tlbie_rb() to set TLBIE bits, so
> +	 * these functions should work together -- must ensure a guest can not
> +	 * cause problems with the TLBIE that KVM executes.
> +	 */
> +	if ((pteh >> HPTE_V_SSIZE_SHIFT) & 0x2) {
> +		/* B=0b1x is a reserved value, disallow it. */
> +		return H_PARAMETER;
> +	}
>  	psize = kvmppc_actual_pgsz(pteh, ptel);
>  	if (!psize)
>  		return H_PARAMETER;

^ permalink raw reply

* Re: [V3 0/4] powerpc/perf: Add instruction and data address registers to extended regs
From: kajoljain @ 2021-10-11 13:09 UTC (permalink / raw)
  To: Athira Rajeev, mpe, acme, jolsa; +Cc: maddy, linuxppc-dev, rnsastry
In-Reply-To: <20211007065505.27809-1-atrajeev@linux.vnet.ibm.com>



On 10/7/21 12:25 PM, Athira Rajeev wrote:
> Patch set adds PMU registers namely Sampled Instruction Address Register
> (SIAR) and Sampled Data Address Register (SDAR) as part of extended regs
> in PowerPC. These registers provides the instruction/data address and
> adding these to extended regs helps in debug purposes.
> 
> Patch 1/4 and 2/4 refactors the existing macro definition of
> PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to make it more
> readable.
> Patch 3/4 adds SIAR and SDAR as part of the extended regs mask.
> Patch 4/4 includes perf tools side changes to add the SPRs to
> sample_reg_mask to use with -I? option.
> 
> Changelog:
> Change from v2 -> v3:
> Addressed review comments from Michael Ellerman
> - Fixed the macro definition to use "unsigned long long"
>   which otherwise will cause build error with perf on
>   32-bit.
> - Added Reviewed-by from Daniel Axtens for patch3.
> 
> Change from v1 -> v2:
> Addressed review comments from Michael Ellerman
> - Refactored the perf reg extended mask value macros for
>   PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to
>   make it more readable. Also moved PERF_REG_EXTENDED_MAX
>   along with enum definition similar to PERF_REG_POWERPC_MAX.
> 
> Athira Rajeev (4):
>   powerpc/perf: Refactor the code definition of perf reg extended mask
>   tools/perf: Refactor the code definition of perf reg extended mask in
>     tools side header file
>   powerpc/perf: Expose instruction and data address registers as part of
>     extended regs
>   tools/perf: Add perf tools support to expose instruction and data
>     address registers as part of extended regs

Patch-set looks good to me.

Reviewed-by: Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol Jain
> 
>  arch/powerpc/include/uapi/asm/perf_regs.h     | 28 ++++++++++++-------
>  arch/powerpc/perf/perf_regs.c                 |  4 +++
>  .../arch/powerpc/include/uapi/asm/perf_regs.h | 28 ++++++++++++-------
>  tools/perf/arch/powerpc/include/perf_regs.h   |  2 ++
>  tools/perf/arch/powerpc/util/perf_regs.c      |  2 ++
>  5 files changed, 44 insertions(+), 20 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v2 4/4] powerpc/pseries/cpuhp: remove obsolete comment from pseries_cpu_die
From: Michael Ellerman @ 2021-10-11 12:34 UTC (permalink / raw)
  To: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
  Cc: tyreld, ldufour, aneesh.kumar
In-Reply-To: <87k0izfux7.fsf@linux.ibm.com>

Nathan Lynch <nathanl@linux.ibm.com> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Daniel Henrique Barboza <danielhb413@gmail.com> writes:
>>> This is enough to say that we can't easily see the history behind this comment.
>>> I also believe that we're better of without it since it doesn't make sense
>>> with the current codebase.
>>
>> It was added by the original CPU hotplug commit for ppc64::
>>
>> https://github.com/mpe/linux-fullhistory/commit/0e9fd9441cd2113b67b14e739267c9e69761489b
>>
>>
>> The code was fairly similar:
>>
>> void __cpu_die(unsigned int cpu)
>> {
>> 	int tries;
>> 	int cpu_status;
>> 	unsigned int pcpu = get_hard_smp_processor_id(cpu);
>>
>> 	for (tries = 0; tries < 5; tries++) {
>> 		cpu_status = query_cpu_stopped(pcpu);
>>
>> 		if (cpu_status == 0)
>> 			break;
>> 		set_current_state(TASK_UNINTERRUPTIBLE);
>> 		schedule_timeout(HZ);
>> 	}
>> 	if (cpu_status != 0) {
>> 		printk("Querying DEAD? cpu %i (%i) shows %i\n",
>> 		       cpu, pcpu, cpu_status);
>> 	}
>>
>> 	/* Isolation and deallocation are definatly done by
>> 	 * drslot_chrp_cpu.  If they were not they would be
>> 	 * done here.  Change isolate state to Isolate and
>> 	 * change allocation-state to Unusable.
>> 	 */
>> 	paca[cpu].xProcStart = 0;
>>
>> 	/* So we can recognize if it fails to come up next time. */
>> 	cpu_callin_map[cpu] = 0;
>> }
>>
>>
>> drslot_chrp_cpu() still exists in drmgr:
>>
>>   https://github.com/ibm-power-utilities/powerpc-utils/blob/e798c4a09fbf0fa0f421e624cfa366a6c405c9fe/src/drmgr/drslot_chrp_cpu.c#L406
>>
>>
>> I agree the comment is no longer meaningful and can be removed.
>
> Thanks for providing this background.
>
>> It might be good to then add a comment explaining why we need to set
>> cpu_start = 0.
>
> Sure, I can take that as a follow-up. Or perhaps it should be moved to
> the online path.

Yeah possibly.

>> It's not immediately clear why we need to. When we bring a CPU back
>> online in smp_pSeries_kick_cpu() we ask RTAS to start it and then
>> immediately set cpu_start = 1, ie. there isn't a separate step that sets
>> cpu_start = 1 for hotplugged CPUs.
>
> Hmm I'm not following the distinction you seem to be drawing between
> bringing a CPU back online and a hotplugged CPU. kick_cpu is used in all
> cases AFAIK.

Yeah that wasn't very clear, reading it back I have half confused myself.

At boot we need the paca->cpu_start flag because some CPUs can be
spinning in generic_secondary_common_init, in head_64.S.

ie. they're not in RTAS, they're spinning in kernel code, and the only
thing that stops them coming "online" in the Linux sense is
paca->cpu_start.

You can see that in pseries/smp.c:

static inline int smp_startup_cpu(unsigned int lcpu)
{
	...
	if (cpumask_test_cpu(lcpu, of_spin_mask))
		/* Already started by OF and sitting in spin loop */
		return 1;


We also hit that case when kexec'ing, where all the secondaries come in
that way.


On the other hand when we offline a CPU, we set paca->cpu_start = 0, in
pseries_cpu_die(), and then we return the CPU to RTAS.

The only way it *should* come back online is via smp_pSeries_kick_cpu(),
which calls smp_startup_cpu() to bring the CPU out of RTAS, and then
smp_pSeries_kick_cpu() immediately sets cpu_start = 1.

So the sequence is:

	CPU goes offline from Linux POV
	paca->cpu_start = 0;
        CPU offline in RTAS
        ...
        CPU brought out of RTAS
	paca->cpu_start = 1;
	CPU comes back online from Linux POV


But I guess I kind of answered my own question above, where I said
*should*. Clearing paca->cpu_start when we offline the CPU gives us a
little bit of backup if the CPU comes out of RTAS unexpectedly. ie. it
would then spin on paca->cpu_start, rather than spontaneously coming
back into Linux when we weren't expecting it.

cheers

^ permalink raw reply

* Re: [PATCH 0/1] Arch use of pci_dev_is_added()
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: Niklas Schnelle, Bjorn Helgaas
  Cc: linux-arch, linux-s390, linux-pci, linux-kernel,
	Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20210910141940.2598035-1-schnelle@linux.ibm.com>

On Fri, 10 Sep 2021 16:19:39 +0200, Niklas Schnelle wrote:
> In my proposal to make pci_dev_is_added() more regularly usable by arch code
> you mentioned[0] that you believe the uses in arch/powerpc are not necessary
> anymore. From code reading I agree and so does Oliver O'Halloran[1].
> 
> So as promised here is a patch removing them. I only compile tested this as
> I don't have access to a powerpc system.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: Drop superfluous pci_dev_is_added() calls
      https://git.kernel.org/powerpc/c/452f145eca73945222923525a7eba59cf37909cc

cheers

^ permalink raw reply

* Re: [PATCH v2 0/2] powerpc:85xx: fix timebase sync issue when CONFIG_HOTPLUG_CPU=n
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: paul.gortmaker, linuxppc-dev, mpe, Xiaoming Ni, stable, oss, benh,
	chenhui.zhao, linux-kernel, gregkh, paulus, Yuantian.Tang
  Cc: liuwenliang, wangle6, chenjianguo3
In-Reply-To: <20210929033646.39630-1-nixiaoming@huawei.com>

On Wed, 29 Sep 2021 11:36:44 +0800, Xiaoming Ni wrote:
> When CONFIG_SMP=y, timebase synchronization is required for mpc8572 when
>  the second kernel is started
> 	arch/powerpc/kernel/smp.c:
> 	int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> 	{
> 		...
> 		if (smp_ops->give_timebase)
> 			smp_ops->give_timebase();
> 		...
> 	}
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc:85xx:Fix oops when mpc85xx_smp_guts_ids node cannot be found
      https://git.kernel.org/powerpc/c/3c2172c1c47b4079c29f0e6637d764a99355ebcd
[2/2] powerpc:85xx: fix timebase sync issue when CONFIG_HOTPLUG_CPU=n
      https://git.kernel.org/powerpc/c/c45361abb9185b1e172bd75eff51ad5f601ccae4

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: clean up UPD_CONSTR
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: Nick Desaulniers, Michael Ellerman
  Cc: Michal Hocko, David Hildenbrand, Peter Zijlstra, linuxppc-dev,
	Boqun Feng, linux-kernel, kvm-ppc, Christopher M. Riedl,
	Nathan Chancellor, Paul Mackerras, Dan Williams, Will Deacon,
	Daniel Axtens
In-Reply-To: <20210914161712.2463458-1-ndesaulniers@google.com>

On Tue, 14 Sep 2021 09:17:04 -0700, Nick Desaulniers wrote:
> UPD_CONSTR was previously a preprocessor define for an old GCC 4.9 inline
> asm bug with m<> constraints.
> 
> 
> 
> 

Applied to powerpc/next.

[1/1] powerpc: clean up UPD_CONSTR
      https://git.kernel.org/powerpc/c/2a24d80fc86bcd70c8e780078254e873ea217379

cheers

^ permalink raw reply

* Re: [PATCH v2 0/4] CPU DLPAR/hotplug for v5.16
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: linuxppc-dev, Nathan Lynch; +Cc: aneesh.kumar, ldufour, danielhb413, tyreld
In-Reply-To: <20210927201933.76786-1-nathanl@linux.ibm.com>

On Mon, 27 Sep 2021 15:19:29 -0500, Nathan Lynch wrote:
> Fixes for some vintage bugs in handling cache node addition and removal, a
> miscellaneous BUG->WARN conversion, and removal of the fragile "by count"
> CPU DLPAR code that probably has no users.
> 
> Changes since v1:
> * Remove set but unused local variable (0day)
> * Additional comment cleanup patch
> 
> [...]

Applied to powerpc/next.

[1/4] powerpc/pseries/cpuhp: cache node corrections
      https://git.kernel.org/powerpc/c/7edd5c9a8820bedb22870b34a809d45f2a86a35a
[2/4] powerpc/cpuhp: BUG -> WARN conversion in offline path
      https://git.kernel.org/powerpc/c/983f9101740641434cea4f2e172175ff4b0276ad
[3/4] powerpc/pseries/cpuhp: delete add/remove_by_count code
      https://git.kernel.org/powerpc/c/fa2a5dfe2ddd0e7c77e5f608e1fa374192e5be97
[4/4] powerpc/pseries/cpuhp: remove obsolete comment from pseries_cpu_die
      https://git.kernel.org/powerpc/c/f9473a65719e59c45f1638cc04db7c80de8fcc1a

cheers

^ permalink raw reply

* Re: [PATCH v2 0/2] powerpc/paravirt: vcpu_is_preempted() tweaks
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: linuxppc-dev, Nathan Lynch; +Cc: srikar, npiggin
In-Reply-To: <20210928214147.312412-1-nathanl@linux.ibm.com>

On Tue, 28 Sep 2021 16:41:45 -0500, Nathan Lynch wrote:
> Minor changes arising from discovering that this code throws warnings with
> DEBUG_PREEMPT kernels.
> 
> Changes since v1:
> * Additional commentary to (1) distinguish hypervisor dispatch and preempt
>   behavior from kernel scheduler preemption; and (2) more clearly justify
>   the use of raw_smp_processor_id().
> * Additional patch to update existing comments before making the functional
>   change.
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/paravirt: vcpu_is_preempted() commentary
      https://git.kernel.org/powerpc/c/799f9b51db688608b50e630a57bee5f699b268ca
[2/2] powerpc/paravirt: correct preempt debug splat in vcpu_is_preempted()
      https://git.kernel.org/powerpc/c/fda0eb220021a97c1d656434b9340ebf3fc4704a

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: fix unbalanced node refcount in check_kvm_guest()
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: linuxppc-dev, Nathan Lynch; +Cc: srikar, npiggin
In-Reply-To: <20210928124550.132020-1-nathanl@linux.ibm.com>

On Tue, 28 Sep 2021 07:45:50 -0500, Nathan Lynch wrote:
> When check_kvm_guest() succeeds in looking up a /hypervisor OF node, it
> returns without performing a matching put for the lookup, leaving the
> node's reference count elevated.
> 
> Add the necessary call to of_node_put(), rearranging the code slightly to
> avoid repetition or goto.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: fix unbalanced node refcount in check_kvm_guest()
      https://git.kernel.org/powerpc/c/56537faf8821e361d739fc5ff58c9c40f54a1d4c

cheers

^ permalink raw reply

* Re: [PATCH trivial v2] powerpc/powernv/dump: Fix typo in comment
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: linuxppc-dev, Vasant Hegde; +Cc: joel
In-Reply-To: <20210914143802.54325-1-hegdevasant@linux.vnet.ibm.com>

On Tue, 14 Sep 2021 20:08:02 +0530, Vasant Hegde wrote:
> 


Applied to powerpc/next.

[1/1] powerpc/powernv/dump: Fix typo in comment
      https://git.kernel.org/powerpc/c/ee87843795ec5dc2f3bb315fade3ec098c88f639

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: Remove unused prototype for of_show_percpuinfo
From: Michael Ellerman @ 2021-10-11 12:06 UTC (permalink / raw)
  To: linuxppc-dev, Daniel Axtens
In-Reply-To: <20210903063246.70691-1-dja@axtens.net>

On Fri, 3 Sep 2021 16:32:46 +1000, Daniel Axtens wrote:
> commit 6d7f58b04d82 ("[PATCH] powerpc: Some minor cleanups to setup_32.c")
> removed of_show_percpuinfo but didn't remove the prototype.
> 
> Remove it.
> 
> 
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: Remove unused prototype for of_show_percpuinfo
      https://git.kernel.org/powerpc/c/93fa8e9d88118bd2bdf2c61f9b63e7d4d9b648fe

cheers

^ permalink raw reply

* Re: [PATCH] video: fbdev: use memset_io() instead of memset()
From: Michael Ellerman @ 2021-10-11 12:06 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>

On Wed, 15 Sep 2021 15:34:35 +0200, Christophe Leroy wrote:
> While investigating a lockup at startup on Powerbook 3400C, it was
> identified that the fbdev driver generates alignment exception at
> startup:
> 
> 	--- interrupt: 600 at memset+0x60/0xc0
> 	NIP:  c0021414 LR: c03fc49c CTR: 00007fff
> 	REGS: ca021c10 TRAP: 0600   Tainted: G        W          (5.14.2-pmac-00727-g12a41fa69492)
> 	MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 44008442  XER: 20000100
> 	DAR: cab80020 DSISR: 00017c07
> 	GPR00: 00000007 ca021cd0 c14412e0 cab80000 00000000 00100000 cab8001c 00000004
> 	GPR08: 00100000 00007fff 00000000 00000000 84008442 00000000 c0006fb4 00000000
> 	GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00100000
> 	GPR24: 00000000 81800000 00000320 c15fa400 c14d1878 00000000 c14d1800 c094e19c
> 	NIP [c0021414] memset+0x60/0xc0
> 	LR [c03fc49c] chipsfb_pci_init+0x160/0x580
> 	--- interrupt: 600
> 	[ca021cd0] [c03fc46c] chipsfb_pci_init+0x130/0x580 (unreliable)
> 	[ca021d20] [c03a3a70] pci_device_probe+0xf8/0x1b8
> 	[ca021d50] [c043d584] really_probe.part.0+0xac/0x388
> 	[ca021d70] [c043d914] __driver_probe_device+0xb4/0x170
> 	[ca021d90] [c043da18] driver_probe_device+0x48/0x144
> 	[ca021dc0] [c043e318] __driver_attach+0x11c/0x1c4
> 	[ca021de0] [c043ad30] bus_for_each_dev+0x88/0xf0
> 	[ca021e10] [c043c724] bus_add_driver+0x190/0x22c
> 	[ca021e40] [c043ee94] driver_register+0x9c/0x170
> 	[ca021e60] [c0006c28] do_one_initcall+0x54/0x1ec
> 	[ca021ed0] [c08246e4] kernel_init_freeable+0x1c0/0x270
> 	[ca021f10] [c0006fdc] kernel_init+0x28/0x11c
> 	[ca021f30] [c0017148] ret_from_kernel_thread+0x14/0x1c
> 	Instruction dump:
> 	7d4601a4 39490777 7d4701a4 39490888 7d4801a4 39490999 7d4901a4 39290aaa
> 	7d2a01a4 4c00012c 4bfffe88 0fe00000 <4bfffe80> 9421fff0 38210010 48001970
> 
> [...]

Applied to powerpc/next.

[1/1] video: fbdev: use memset_io() instead of memset()
      https://git.kernel.org/powerpc/c/f2719b26ae27282c145202ffd656d5ff1fe737cc

cheers

^ permalink raw reply


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