* [PATCH 17/20] crypto: nx: nx-aes-cbc: Headers comments should not be kernel-doc
From: Lee Jones @ 2021-02-04 11:09 UTC (permalink / raw)
To: lee.jones
Cc: Herbert Xu, Kent Yoder, Nayna Jain, linux-kernel,
Paulo Flabiano Smorigo, linux-crypto, Breno Leitão,
Paul Mackerras, linuxppc-dev, David S. Miller
In-Reply-To: <20210204111000.2800436-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'tfm' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'in_key' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: Function parameter or member 'key_len' not described in 'cbc_aes_nx_set_key'
drivers/crypto/nx/nx-aes-cbc.c:24: warning: expecting prototype for Nest Accelerators driver(). Prototype was for cbc_aes_nx_set_key() instead
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kent Yoder <yoder1@us.ibm.com>
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/crypto/nx/nx-aes-cbc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 92e921eceed75..d6314ea9ae896 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES CBC routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
--
2.25.1
^ permalink raw reply related
* [PATCH 19/20] crypto: nx: Demote header comment and add description for 'nbytes'
From: Lee Jones @ 2021-02-04 11:09 UTC (permalink / raw)
To: lee.jones
Cc: Herbert Xu, Kent Yoder, Nayna Jain, linux-kernel,
Paulo Flabiano Smorigo, linux-crypto, Breno Leitão,
Paul Mackerras, linuxppc-dev, David S. Miller
In-Reply-To: <20210204111000.2800436-1-lee.jones@linaro.org>
Fixes the following W=1 kernel build warning(s):
drivers/crypto/nx/nx.c:31: warning: Incorrect use of kernel-doc format: * nx_hcall_sync - make an H_COP_OP hcall for the passed in op structure
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'nx_ctx' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'op' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: Function parameter or member 'may_sleep' not described in 'nx_hcall_sync'
drivers/crypto/nx/nx.c:43: warning: expecting prototype for Nest Accelerators driver(). Prototype was for nx_hcall_sync() instead
drivers/crypto/nx/nx.c:209: warning: Function parameter or member 'nbytes' not described in 'trim_sg_list'
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kent Yoder <yoder1@us.ibm.com>
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/crypto/nx/nx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 0d2dc5be7f192..010be6793c9fc 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* Routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
@@ -200,7 +200,8 @@ struct nx_sg *nx_walk_and_build(struct nx_sg *nx_dst,
* @sg: sg list head
* @end: sg lisg end
* @delta: is the amount we need to crop in order to bound the list.
- *
+ * @nbytes: length of data in the scatterlists or data length - whichever
+ * is greater.
*/
static long int trim_sg_list(struct nx_sg *sg,
struct nx_sg *end,
--
2.25.1
^ permalink raw reply related
* [PATCH 00/20] Rid W=1 warnings in Crypto
From: Lee Jones @ 2021-02-04 11:09 UTC (permalink / raw)
To: lee.jones
Cc: Alexandre Belloni, Aymen Sghaier, Takashi Iwai, Kent Yoder,
Ayush Sawal, Joakim Bech, Gustavo A. R. Silva, Paul Mackerras,
Andreas Westin, Breno Leitão, Atul Gupta, Niklas Hernaeus,
M R Gowda, Herbert Xu, Horia Geantă, Rohit Maheshwari,
Nayna Jain, Manoj Malviya, Ludovic Desroches, Jonas Linde,
Rob Rice, Zaibo Xu, Harsh Jain, Declan Murphy, Vinay Kumar Yadav,
Tudor Ambarus, Nicolas Ferre, Shujuan Chen, Henrique Cerri,
Daniele Alessandrelli, linux-arm-kernel, Jonathan Cameron,
linux-kernel, Berne Hebark, linux-crypto, Jitendra Lulla,
Paulo Flabiano Smorigo, linuxppc-dev, David S. Miller
This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.
This is set 1 of 2 sets required to fully clean Crypto.
Lee Jones (20):
crypto: hisilicon: sec_drv: Supply missing description for
'sec_queue_empty()'s 'queue' param
crypto: bcm: util: Repair a couple of documentation formatting issues
crypto: chelsio: chcr_core: File headers are not good candidates for
kernel-doc
crypto: ux500: hash: hash_core: Fix worthy kernel-doc headers and
remove others
crypto: bcm: spu: Fix formatting and misspelling issues
crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
crypto: bcm: spu2: Fix a whole host of kernel-doc misdemeanours
crypto: ux500: cryp: Demote some conformant non-kernel headers fix
another
crypto: ux500: cryp_irq: File headers are not good kernel-doc
candidates
crypto: chelsio: chcr_algo: Fix a couple of kernel-doc issues caused
by doc-rot
crypto: ux500: cryp_core: Fix formatting issue and add description for
'session_id'
crypto: atmel-ecc: Struct headers need to start with keyword 'struct'
crypto: bcm: cipher: Provide description for 'req' and fix formatting
issues
crypto: caam: caampkc: Provide the name of the function
crypto: caam: caamalg_qi2: Supply a couple of 'fallback' related
descriptions
crypto: vmx: Source headers are not good kernel-doc candidates
crypto: nx: nx-aes-cbc: Headers comments should not be kernel-doc
crypto: nx: nx_debugfs: Header comments should not be kernel-doc
crypto: nx: Demote header comment and add description for 'nbytes'
crypto: cavium: nitrox_isr: Demote non-compliant kernel-doc headers
drivers/crypto/atmel-ecc.c | 2 +-
drivers/crypto/bcm/cipher.c | 7 ++--
drivers/crypto/bcm/spu.c | 16 ++++-----
drivers/crypto/bcm/spu2.c | 43 +++++++++++++----------
drivers/crypto/bcm/util.c | 4 +--
drivers/crypto/caam/caamalg_qi2.c | 2 ++
drivers/crypto/caam/caampkc.c | 3 +-
drivers/crypto/cavium/nitrox/nitrox_isr.c | 4 +--
drivers/crypto/chelsio/chcr_algo.c | 8 ++---
drivers/crypto/chelsio/chcr_core.c | 2 +-
drivers/crypto/hisilicon/sec/sec_drv.c | 1 +
drivers/crypto/keembay/ocs-hcu.c | 6 ++--
drivers/crypto/nx/nx-aes-cbc.c | 2 +-
drivers/crypto/nx/nx.c | 5 +--
drivers/crypto/nx/nx_debugfs.c | 2 +-
drivers/crypto/ux500/cryp/cryp.c | 5 +--
drivers/crypto/ux500/cryp/cryp_core.c | 5 +--
drivers/crypto/ux500/cryp/cryp_irq.c | 2 +-
drivers/crypto/ux500/hash/hash_core.c | 15 +++-----
drivers/crypto/vmx/vmx.c | 2 +-
20 files changed, 71 insertions(+), 65 deletions(-)
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Andreas Westin <andreas.westin@stericsson.com>
Cc: Atul Gupta <atul.gupta@chelsio.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Ayush Sawal <ayush.sawal@chelsio.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Berne Hebark <berne.herbark@stericsson.com>
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Declan Murphy <declan.murphy@intel.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Harsh Jain <harsh@chelsio.com>
Cc: Henrique Cerri <mhcerri@br.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Jitendra Lulla <jlulla@chelsio.com>
Cc: Joakim Bech <joakim.xx.bech@stericsson.com>
Cc: Jonas Linde <jonas.linde@stericsson.com>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: Kent Yoder <yoder1@us.ibm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-crypto@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
Cc: Manoj Malviya <manojmalviya@chelsio.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: M R Gowda <yeshaswi@chelsio.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Niklas Hernaeus <niklas.hernaeus@stericsson.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Rob Rice <rob.rice@broadcom.com>
Cc: Rohit Maheshwari <rohitm@chelsio.com>
Cc: Shujuan Chen <shujuan.chen@stericsson.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>
Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Cc: Zaibo Xu <xuzaibo@huawei.com>
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-02-04 10:49 UTC (permalink / raw)
To: mpe
Cc: Ravi Bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <20210204104703.273429-1-ravi.bangoria@linux.ibm.com>
On 2/4/21 4:17 PM, Ravi Bangoria wrote:
> Don't allow Uprobe on 2nd word of a prefixed instruction. As per
> ISA 3.1, prefixed instruction should not cross 64-byte boundary.
> So don't allow Uprobe on such prefixed instruction as well.
>
> There are two ways probed instruction is changed in mapped pages.
> First, when Uprobe is activated, it searches for all the relevant
> pages and replace instruction in them. In this case, if we notice
> that probe is on the 2nd word of prefixed instruction, error out
> directly. Second, when Uprobe is already active and user maps a
> relevant page via mmap(), instruction is replaced via mmap() code
> path. But because Uprobe is invalid, entire mmap() operation can
> not be stopped. In this case just print an error and continue.
@mpe,
arch_uprobe_analyze_insn() can return early if
cpu_has_feature(CPU_FTR_ARCH_31) is not set. But that will
miss out a rare scenario of user running binary with prefixed
instruction on p10 predecessors. Please let me know if I
should add cpu_has_feature(CPU_FTR_ARCH_31) or not.
- Ravi
^ permalink raw reply
* [PATCH v2] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-02-04 10:47 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
sandipan, naveen.n.rao, linuxppc-dev
Don't allow Uprobe on 2nd word of a prefixed instruction. As per
ISA 3.1, prefixed instruction should not cross 64-byte boundary.
So don't allow Uprobe on such prefixed instruction as well.
There are two ways probed instruction is changed in mapped pages.
First, when Uprobe is activated, it searches for all the relevant
pages and replace instruction in them. In this case, if we notice
that probe is on the 2nd word of prefixed instruction, error out
directly. Second, when Uprobe is already active and user maps a
relevant page via mmap(), instruction is replaced via mmap() code
path. But because Uprobe is invalid, entire mmap() operation can
not be stopped. In this case just print an error and continue.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
v1: http://lore.kernel.org/r/20210119091234.76317-1-ravi.bangoria@linux.ibm.com
v1->v2:
- Instead of introducing new arch hook from verify_opcode(), use
existing hook arch_uprobe_analyze_insn().
- Add explicit check for prefixed instruction crossing 64-byte
boundary. If probe is on such instruction, throw an error.
arch/powerpc/kernel/uprobes.c | 66 ++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..485d19a2a31f 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -7,6 +7,7 @@
* Adapted from the x86 port by Ananth N Mavinakayanahalli <ananth@in.ibm.com>
*/
#include <linux/kernel.h>
+#include <linux/highmem.h>
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <linux/uprobes.h>
@@ -28,6 +29,69 @@ bool is_trap_insn(uprobe_opcode_t *insn)
return (is_trap(*insn));
}
+#ifdef CONFIG_PPC64
+static int get_instr(struct mm_struct *mm, unsigned long addr, u32 *instr)
+{
+ struct page *page;
+ struct vm_area_struct *vma;
+ void *kaddr;
+ unsigned int gup_flags = FOLL_FORCE | FOLL_SPLIT_PMD;
+
+ if (get_user_pages_remote(mm, addr, 1, gup_flags, &page, &vma, NULL) <= 0)
+ return -EINVAL;
+
+ kaddr = kmap_atomic(page);
+ *instr = *((u32 *)(kaddr + (addr & ~PAGE_MASK)));
+ kunmap_atomic(kaddr);
+ put_page(page);
+ return 0;
+}
+
+static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr)
+{
+ struct ppc_inst inst;
+ u32 prefix, suffix;
+
+ /*
+ * No need to check if addr is pointing to beginning of the
+ * page. Even if probe is on a suffix of page-unaligned
+ * prefixed instruction, hw will raise exception and kernel
+ * will send SIGBUS.
+ */
+ if (!(addr & ~PAGE_MASK))
+ return 0;
+
+ if (get_instr(mm, addr, &prefix) < 0)
+ return -EINVAL;
+ if (get_instr(mm, addr + 4, &suffix) < 0)
+ return -EINVAL;
+
+ inst = ppc_inst_prefix(prefix, suffix);
+ if (ppc_inst_prefixed(inst) && (addr & 0x3F) == 0x3C) {
+ printk_ratelimited("Cannot register a uprobe on 64 byte "
+ "unaligned prefixed instruction\n");
+ return -EINVAL;
+ }
+
+ suffix = prefix;
+ if (get_instr(mm, addr - 4, &prefix) < 0)
+ return -EINVAL;
+
+ inst = ppc_inst_prefix(prefix, suffix);
+ if (ppc_inst_prefixed(inst)) {
+ printk_ratelimited("Cannot register a uprobe on the second "
+ "word of prefixed instruction\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+#else
+static int validate_prefixed_instr(struct mm_struct *mm, unsigned long addr)
+{
+ return 0;
+}
+#endif
+
/**
* arch_uprobe_analyze_insn
* @mm: the probed address space.
@@ -41,7 +105,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
if (addr & 0x03)
return -EINVAL;
- return 0;
+ return validate_prefixed_instr(mm, addr);
}
/*
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module
From: Michael Ellerman @ 2021-02-04 10:44 UTC (permalink / raw)
To: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, Jessica Yu, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
dri-devel, live-patching, linuxppc-dev
In-Reply-To: <20210202121334.1361503-2-hch@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> The static inline get_cxl_module function is entirely unused since commit
> 8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel
> api on the real phb"), so remove it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-cxl.c | 22 ----------------------
> 1 file changed, 22 deletions(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* RE: [PATCH v2 1/3] powerpc: sstep: Fix load and update emulation
From: David Laight @ 2021-02-04 10:29 UTC (permalink / raw)
To: 'Segher Boessenkool', Naveen N. Rao
Cc: ravi.bangoria@linux.ibm.com, ananth@linux.ibm.com,
jniethe5@gmail.com, paulus@samba.org, Sandipan Das,
linuxppc-dev@lists.ozlabs.org, dja@axtens.net
In-Reply-To: <20210203211732.GD30983@gate.crashing.org>
From: Segher Boessenkool
> Sent: 03 February 2021 21:18
...
> Power9 does:
>
> Load with Update Instructions (RA = 0)
> EA is placed into R0.
Does that change the value of 0?
Rather reminds me of some 1960s era systems that had the small integers
at fixed (global) addresses.
FORTRAN always passes by reference, pass 0 and the address of the global
zero location was passed, the called function could change 0 to 1 for
the entire computer!
> Load with Update Instructions (RA = RT)
> The storage operand addressed by EA is accessed. The displacement
> field is added to the data returned by the load and placed into RT.
Shame that isn't standard - could be used to optimise some code.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH v7 39/42] powerpc: move NMI entry/exit code into wrapper
From: Michael Ellerman @ 2021-02-04 10:15 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Athira Rajeev, Nicholas Piggin
In-Reply-To: <20210130130852.2952424-40-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This moves the common NMI entry and exit code into the interrupt handler
> wrappers.
>
> This changes the behaviour of soft-NMI (watchdog) and HMI interrupts, and
> also MCE interrupts on 64e, by adding missing parts of the NMI entry to
> them.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 28 ++++++++++++++++++++++
> arch/powerpc/kernel/mce.c | 11 ---------
> arch/powerpc/kernel/traps.c | 35 +++++-----------------------
> arch/powerpc/kernel/watchdog.c | 10 ++++----
> 4 files changed, 38 insertions(+), 46 deletions(-)
This is unhappy when injecting SLB multi-hits:
root@p86-2:~# echo PPC_SLB_MULTIHIT > /sys/kernel/debug/provoke-crash/DIRECT
[ 312.496026][ T1344] kernel BUG at arch/powerpc/include/asm/interrupt.h:152!
[ 312.496037][ T1344] Oops: Exception in kernel mode, sig: 5 [#1]
[ 312.496045][ T1344] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 312.496053][ T1344] Modules linked in: squashfs dm_multipath scsi_dh_rdac scsi_dh_alua pseries_rng rng_core vmx_crypto gf128mul crc32c_vpmsum ip_tables x_tables autofs4
[ 312.496085][ T1344] CPU: 19 PID: 1344 Comm: bash Not tainted 5.11.0-rc2-gcc-8.2.0-00123-g3fadced17474-dirty #638
[ 312.496096][ T1344] NIP: c000000000ef1618 LR: c000000000ef1600 CTR: 0000000000000000
[ 312.496104][ T1344] REGS: c00000001eb4ba00 TRAP: 0700 Not tainted (5.11.0-rc2-gcc-8.2.0-00123-g3fadced17474-dirty)
[ 312.496113][ T1344] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 48428284 XER: 00000000
[ 312.496132][ T1344] CFAR: c000000000ef28b8 IRQMASK: 3
[ 312.496132][ T1344] GPR00: c000000000ef15e4 c00000001eb4bca0 c000000001a53900 0000000000000001
[ 312.496132][ T1344] GPR04: c0000000017e7230 4000000000000000 3ffffffffffffffe 0000000000006d66
[ 312.496132][ T1344] GPR08: c00000001ec6fe80 0000000000000001 c00000000e72d380 0000000000001001
[ 312.496132][ T1344] GPR12: 0000000000000010 c00000001ec6fe80 00000100235ad1d0 000000013c2fb738
[ 312.496132][ T1344] GPR16: 000000013c210ae0 0000000000000000 0000000022000000 00000100235af740
[ 312.496132][ T1344] GPR20: 0000000000000000 0000000000000001 000000013c2a3ca0 c000000033c50040
[ 312.496132][ T1344] GPR24: c0000000100fbd80 0000000000000000 c000000001a7dc78 0000000000000001
[ 312.496132][ T1344] GPR28: c00000001eb4bd60 0000000000000001 0000000000000000 0000000000000001
[ 312.496229][ T1344] NIP [c000000000ef1618] machine_check_early+0x138/0x1f0
[ 312.496241][ T1344] LR [c000000000ef1600] machine_check_early+0x120/0x1f0
[ 312.496249][ T1344] Call Trace:
[ 312.496254][ T1344] [c00000001eb4bca0] [c000000000ef15e4] machine_check_early+0x104/0x1f0 (unreliable)
[ 312.496267][ T1344] [c00000001eb4bcf0] [c000000000008394] machine_check_early_common+0x134/0x1f0
[ 312.496279][ T1344] --- interrupt: 200 at lkdtm_PPC_SLB_MULTIHIT+0x128/0x138
[ 312.496290][ T1344] NIP: c0000000009cfea8 LR: c0000000009cfea0 CTR: 0000000000000000
[ 312.496298][ T1344] REGS: c00000001eb4bd60 TRAP: 0200 Not tainted (5.11.0-rc2-gcc-8.2.0-00123-g3fadced17474-dirty)
[ 312.496307][ T1344] MSR: 8000000000209033 <SF,EE,ME,IR,DR,RI,LE> CR: 24428284 XER: 00000000
[ 312.496326][ T1344] CFAR: 000000000000021c DAR: c008000003880000 DSISR: 00000080 IRQMASK: 0
[ 312.496326][ T1344] GPR00: c0000000009cfea0 c0000000100fbb80 c000000001a53900 c008000003880000
[ 312.496326][ T1344] GPR04: 00000000000000b0 c000000401023440 8e01c533000000c0 0000000000bf50d9
[ 312.496326][ T1344] GPR08: 000000000fffffff 0000000000000021 000000000000001c c008000004880000
[ 312.496326][ T1344] GPR12: 0000000048428222 c00000001ec6fe80 00000100235ad1d0 000000013c2fb738
[ 312.496326][ T1344] GPR16: 000000013c210ae0 0000000000000000 0000000022000000 00000100235af740
[ 312.496326][ T1344] GPR20: 0000000000000000 0000000000000001 000000013c2a3ca0 c000000033c50040
[ 312.496326][ T1344] GPR24: c0000000100fbd80 0000000000000000 0000000000000011 c000000001a2ffb8
[ 312.496326][ T1344] GPR28: 00000000000004b0 c000000033c50000 c000000001105298 c008000003880000
[ 312.496427][ T1344] NIP [c0000000009cfea8] lkdtm_PPC_SLB_MULTIHIT+0x128/0x138
[ 312.496437][ T1344] LR [c0000000009cfea0] lkdtm_PPC_SLB_MULTIHIT+0x120/0x138
[ 312.496446][ T1344] --- interrupt: 200
[ 312.496451][ T1344] [c0000000100fbbf0] [c0000000009cb530] lkdtm_do_action+0x40/0x80
[ 312.496462][ T1344] [c0000000100fbc10] [c0000000009cbdfc] direct_entry+0x16c/0x350
[ 312.496473][ T1344] [c0000000100fbcc0] [c0000000007a7590] full_proxy_write+0x90/0xe0
[ 312.496484][ T1344] [c0000000100fbd10] [c00000000046b090] vfs_write+0xf0/0x310
[ 312.496496][ T1344] [c0000000100fbd60] [c00000000046b48c] ksys_write+0x7c/0x140
[ 312.496507][ T1344] [c0000000100fbdb0] [c000000000036340] system_call_exception+0x1a0/0x2e0
[ 312.496518][ T1344] [c0000000100fbe10] [c00000000000d360] system_call_common+0xf0/0x27c
[ 312.496528][ T1344] Instruction dump:
[ 312.496534][ T1344] 7c7b1b78 e93a0000 75290040 41820008 480000a8 4800125d 60000000 e94d0968
[ 312.496552][ T1344] 812a0000 55290216 7d290034 5529d97e <0b090000> 812a0000 3d29ffef 912a0000
[ 312.496571][ T1344] ---[ end trace 1cd2275de93cc3c3 ]---
[ 312.500581][ T1344]
[ 312.500705][ C19] RTAS: event: 1, Type: Unknown (0), Severity: 3
[ 312.501068][ T0] ------------[ cut here ]------------
[ 312.501081][ T0] WARNING: CPU: 19 PID: 0 at kernel/rcu/tree.c:632 rcu_eqs_enter.isra.59+0x140/0x160
[ 312.501103][ T0] Modules linked in: squashfs dm_multipath scsi_dh_rdac scsi_dh_alua pseries_rng rng_core vmx_crypto gf128mul crc32c_vpmsum ip_tables x_tables autofs4
[ 312.501152][ T0] CPU: 19 PID: 0 Comm: swapper/19 Tainted: G D 5.11.0-rc2-gcc-8.2.0-00123-g3fadced17474-dirty #638
[ 312.501173][ T0] NIP: c000000000ef2830 LR: c000000000c287c0 CTR: 0000000000000000
[ 312.501187][ T0] REGS: c000000008d5fa50 TRAP: 0700 Tainted: G D (5.11.0-rc2-gcc-8.2.0-00123-g3fadced17474-dirty)
[ 312.501203][ T0] MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28000248 XER: 20000009
[ 312.501248][ T0] CFAR: c000000000ef2728 IRQMASK: 1
[ 312.501248][ T0] GPR00: c000000000c287c0 c000000008d5fcf0 c000000001a53900 00000048c283bf7e
[ 312.501248][ T0] GPR04: c00000000199fae0 0000000000000001 0000000000000800 4000000000000000
[ 312.501248][ T0] GPR08: c00000001ec6fe80 c0000003fdb93980 3ffffffffffffffe 000000000098967f
[ 312.501248][ T0] GPR12: 00000000ffffffff c00000001ec6fe80 0000000000000000 000000001ef2fa60
[ 312.501248][ T0] GPR16: 0000000000000000 0000000000000000 c0000007fffebff8 c000000000051ca0
[ 312.501248][ T0] GPR20: c0000007fffec628 c00000000199fae0 0000000000000001 c0000003fdb917c8
[ 312.501248][ T0] GPR24: 0000000000000001 00000048c283bf7e 0000000000000000 0000000000000001
[ 312.501248][ T0] GPR28: c0000003fdb917c8 c00000000199fae0 0000000000000001 c0000000013d3980
[ 312.501428][ T0] NIP [c000000000ef2830] rcu_eqs_enter.isra.59+0x140/0x160
[ 312.501445][ T0] LR [c000000000c287c0] cpuidle_enter_state+0x2f0/0x540
[ 312.501469][ T0] Call Trace:
[ 312.501481][ T0] [c000000008d5fcf0] [c000000008d5fd40] 0xc000000008d5fd40 (unreliable)
[ 312.501508][ T0] [c000000008d5fd20] [c0000003fdb917c8] 0xc0000003fdb917c8
[ 312.501524][ T0] [c000000008d5fd80] [c000000000c28ab0] cpuidle_enter+0x50/0x70
[ 312.501544][ T0] [c000000008d5fdc0] [c00000000019000c] call_cpuidle+0x4c/0x80
[ 312.501564][ T0] [c000000008d5fde0] [c0000000001906a0] do_idle+0x390/0x3e0
[ 312.501581][ T0] [c000000008d5fe70] [c00000000019096c] cpu_startup_entry+0x3c/0x40
[ 312.501602][ T0] [c000000008d5fea0] [c000000000054838] start_secondary+0x5b8/0x9b0
[ 312.501619][ T0] [c000000008d5ff90] [c00000000000c654] start_secondary_prolog+0x10/0x14
[ 312.501642][ T0] Instruction dump:
[ 312.501657][ T0] 60000000 e8010040 7c0803a6 e94d0030 39200000 38210030 7fff5214 f93f00f0
[ 312.501692][ T0] ebe1fff8 4bfffd14 60000000 60000000 <0fe00000> 4bfffef8 60000000 60000000
[ 312.501721][ T0] ---[ end trace 1cd2275de93cc3c4 ]---
147 static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct interrupt_nmi_state *state)
148 {
149 if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64) ||
150 !firmware_has_feature(FW_FEATURE_LPAR) ||
151 radix_enabled() || (mfmsr() & MSR_DR))
152 nmi_exit();
So presumably it's:
#define __nmi_exit() \
do { \
BUG_ON(!in_nmi()); \
cheers
^ permalink raw reply
* Re: [PATCH] powerpc64/idle: Fix SP offsets when saving GPRs
From: Nicholas Piggin @ 2021-02-04 9:05 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev, Michael Ellerman
In-Reply-To: <C90JVYFOGWU0.1C2DRATSDH0FM@geist>
Excerpts from Christopher M. Riedl's message of February 4, 2021 4:59 pm:
> On Sat Jan 30, 2021 at 7:44 AM CST, Nicholas Piggin wrote:
>> Excerpts from Michael Ellerman's message of January 30, 2021 9:32 pm:
>> > "Christopher M. Riedl" <cmr@codefail.de> writes:
>> >> The idle entry/exit code saves/restores GPRs in the stack "red zone"
>> >> (Protected Zone according to PowerPC64 ELF ABI v2). However, the offset
>> >> used for the first GPR is incorrect and overwrites the back chain - the
>> >> Protected Zone actually starts below the current SP. In practice this is
>> >> probably not an issue, but it's still incorrect so fix it.
>> >
>> > Nice catch.
>> >
>> > Corrupting the back chain means you can't backtrace from there, which
>> > could be confusing for debugging one day.
>>
>> Yeah, we seem to have got away without noticing because the CPU will
>> wake up and return out of here before it tries to unwind the stack,
>> but if you tried to walk it by hand if the CPU got stuck in idle or
>> something, then we'd get confused.
>>
>> > It does make me wonder why we don't just create a stack frame and use
>> > the normal macros? It would use a bit more stack space, but we shouldn't
>> > be short of stack space when going idle.
>> >
>> > Nick, was there a particular reason for using the red zone?
>>
>> I don't recall a particular reason, I think a normal stack frame is
>> probably a good idea.
>
> I'll send a version using STACKFRAMESIZE - I assume that's the "normal"
> stack frame :)
>
I think STACKFRAMESIZE is STACK_FRAME_OVERHEAD + NVGPRs. LR and CR can
be saved in the caller's frame so that should be okay.
> I admit I am a bit confused when I saw the similar but much smaller
> STACK_FRAME_OVERHEAD which is also used in _some_ cases to save/restore
> a few registers.
Yeah if you don't need to save all nvgprs you can use caller's frame
plus a few bytes in the minimum frame as volatile storage.
STACK_FRAME_OVERHEAD should be 32 on LE, but I think the problem is a
lot of asm uses it and hasn't necessarily been audited to make sure it's
not assuming it's bigger. You could actually use STACK_FRAME_MIN_SIZE
for new code, maybe we add a STACK_FRAME_MIN_NVGPR_SIZE to match and
use that.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 28/32] powerpc/64s: interrupt implement exit logic in C
From: Nicholas Piggin @ 2021-02-04 8:53 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: Michal Suchanek
In-Reply-To: <65686b53-feb4-2788-88e1-76c3714d3e97@csgroup.eu>
Excerpts from Christophe Leroy's message of February 4, 2021 6:03 pm:
>
>
> Le 04/02/2021 à 04:27, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of February 4, 2021 2:25 am:
>>>
>>>
>>> Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
>>>> Implement the bulk of interrupt return logic in C. The asm return code
>>>> must handle a few cases: restoring full GPRs, and emulating stack store.
>>>>
>>>
>>>
>>>> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
>>>> +{
>>>> + unsigned long *ti_flagsp = ¤t_thread_info()->flags;
>>>> + unsigned long flags;
>>>> +
>>>> + if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
>>>> + unrecoverable_exception(regs);
>>>> + BUG_ON(regs->msr & MSR_PR);
>>>> + BUG_ON(!FULL_REGS(regs));
>>>> +
>>>> + local_irq_save(flags);
>>>> +
>>>> + if (regs->softe == IRQS_ENABLED) {
>>>> + /* Returning to a kernel context with local irqs enabled. */
>>>> + WARN_ON_ONCE(!(regs->msr & MSR_EE));
>>>> +again:
>>>> + if (IS_ENABLED(CONFIG_PREEMPT)) {
>>>> + /* Return to preemptible kernel context */
>>>> + if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
>>>> + if (preempt_count() == 0)
>>>> + preempt_schedule_irq();
>>>> + }
>>>> + }
>>>> +
>>>> + trace_hardirqs_on();
>>>> + __hard_EE_RI_disable();
>>>> + if (unlikely(lazy_irq_pending())) {
>>>> + __hard_RI_enable();
>>>> + irq_soft_mask_set(IRQS_ALL_DISABLED);
>>>> + trace_hardirqs_off();
>>>> + local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>>>> + /*
>>>> + * Can't local_irq_enable in case we are in interrupt
>>>> + * context. Must replay directly.
>>>> + */
>>>> + replay_soft_interrupts();
>>>> + irq_soft_mask_set(flags);
>>>> + /* Took an interrupt, may have more exit work to do. */
>>>> + goto again;
>>>> + }
>>>> + local_paca->irq_happened = 0;
>>>> + irq_soft_mask_set(IRQS_ENABLED);
>>>> + } else {
>>>> + /* Returning to a kernel context with local irqs disabled. */
>>>> + trace_hardirqs_on();
>>>> + __hard_EE_RI_disable();
>>>> + if (regs->msr & MSR_EE)
>>>> + local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
>>>> + }
>>>> +
>>>> +
>>>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>>>> + local_paca->tm_scratch = regs->msr;
>>>> +#endif
>>>> +
>>>> + /*
>>>> + * We don't need to restore AMR on the way back to userspace for KUAP.
>>>> + * The value of AMR only matters while we're in the kernel.
>>>> + */
>>>> + kuap_restore_amr(regs);
>>>
>>> Is that correct to restore KUAP state here ? Shouldn't we have it at lower level in assembly ?
>>>
>>> Isn't there a risk that someone manages to call interrupt_exit_kernel_prepare() or the end of it in
>>> a way or another, and get the previous KUAP state restored by this way ?
>>
>> I'm not sure if there much more risk if it's here rather than the
>> instruction being in another place in the code.
>>
>> There's a lot of user access around the kernel too if you want to find a
>> gadget to unlock KUAP then I suppose there is a pretty large attack
>> surface.
>
> My understanding is that user access scope is strictly limited, for instance we enforce the
> begin/end of user access to be in the same function, and we refrain from calling any other function
> inside the user access window. x86 even have 'objtool' to enforce it at build time. So in theory
> there is no way to get out of the function while user access is open.
>
> Here with the interrupt exit function it is free beer. You have a place where you re-open user
> access and return with a simple blr. So that's open bar. If someone manages to just call the
> interrupt exit function, then user access remains open
Hmm okay maybe that's a good point.
>>> Also, it looks a bit strange to have kuap_save_amr_and_lock() done at lowest level in assembly, and
>>> kuap_restore_amr() done in upper level. That looks unbalanced.
>>
>> I'd like to bring the entry assembly into C.
>>
>
> I really think it's not a good idea. We'll get better control and readability by keeping it at the
> lowest possible level in assembly.
>
> x86 only save and restore SMAC state in assembly.
Okay we could go the other way and move the unlock into asm then.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH RFC v1 5/6] xen-swiotlb: convert variables to arrays
From: Christoph Hellwig @ 2021-02-04 8:40 UTC (permalink / raw)
To: Dongli Zhang
Cc: ulf.hansson, airlied, joonas.lahtinen, dri-devel, linux-kernel,
bhelgaas, paulus, hpa, hch, m.szyprowski, sstabellini,
adrian.hunter, x86, joe.jin, mingo, peterz, mingo, bskeggs,
linux-pci, xen-devel, matthew.auld, thomas.lendacky, konrad.wilk,
intel-gfx, jani.nikula, bp, rodrigo.vivi, nouveau,
boris.ostrovsky, chris, jgross, tsbogend, robin.murphy, linux-mmc,
linux-mips, iommu, tglx, bauerman, daniel, akpm, linuxppc-dev,
rppt
In-Reply-To: <20210203233709.19819-6-dongli.zhang@oracle.com>
So one thing that has been on my mind for a while: I'd really like
to kill the separate dma ops in Xen swiotlb. If we compare xen-swiotlb
to swiotlb the main difference seems to be:
- additional reasons to bounce I/O vs the plain DMA capable
- the possibility to do a hypercall on arm/arm64
- an extra translation layer before doing the phys_to_dma and vice
versa
- an special memory allocator
I wonder if inbetween a few jump labels or other no overhead enablement
options and possibly better use of the dma_range_map we could kill
off most of swiotlb-xen instead of maintaining all this code duplication?
^ permalink raw reply
* Re: [PATCH v4 1/2] powerpc: sstep: Fix load-store and update emulation
From: Naveen N. Rao @ 2021-02-04 8:31 UTC (permalink / raw)
To: Sandipan Das; +Cc: ravi.bangoria, ananth, jniethe5, paulus, linuxppc-dev, dja
In-Reply-To: <20210204080744.135785-1-sandipan@linux.ibm.com>
On 2021/02/04 01:37PM, Sandipan Das wrote:
> The Power ISA says that the fixed-point load and update
> instructions must neither use R0 for the base address (RA)
> nor have the destination (RT) and the base address (RA) as
> the same register. Similarly, for fixed-point stores and
> floating-point loads and stores, the instruction is invalid
> when R0 is used as the base address (RA).
>
> This is applicable to the following instructions.
> * Load Byte and Zero with Update (lbzu)
> * Load Byte and Zero with Update Indexed (lbzux)
> * Load Halfword and Zero with Update (lhzu)
> * Load Halfword and Zero with Update Indexed (lhzux)
> * Load Halfword Algebraic with Update (lhau)
> * Load Halfword Algebraic with Update Indexed (lhaux)
> * Load Word and Zero with Update (lwzu)
> * Load Word and Zero with Update Indexed (lwzux)
> * Load Word Algebraic with Update Indexed (lwaux)
> * Load Doubleword with Update (ldu)
> * Load Doubleword with Update Indexed (ldux)
> * Load Floating Single with Update (lfsu)
> * Load Floating Single with Update Indexed (lfsux)
> * Load Floating Double with Update (lfdu)
> * Load Floating Double with Update Indexed (lfdux)
> * Store Byte with Update (stbu)
> * Store Byte with Update Indexed (stbux)
> * Store Halfword with Update (sthu)
> * Store Halfword with Update Indexed (sthux)
> * Store Word with Update (stwu)
> * Store Word with Update Indexed (stwux)
> * Store Doubleword with Update (stdu)
> * Store Doubleword with Update Indexed (stdux)
> * Store Floating Single with Update (stfsu)
> * Store Floating Single with Update Indexed (stfsux)
> * Store Floating Double with Update (stfdu)
> * Store Floating Double with Update Indexed (stfdux)
>
> E.g. the following behaviour is observed for an invalid
> load and update instruction having RA = RT.
>
> While a userspace program having an instruction word like
> 0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
> receiving a SIGILL on a Power system (observed on P8 and
> P9), the outcome of executing that instruction word varies
> and its behaviour can be considered to be undefined.
>
> Attaching an uprobe at that instruction's address results
> in emulation which currently performs the load as well as
> writes the effective address back to the base register.
> This might not match the outcome from hardware.
>
> To remove any inconsistencies, this adds additional checks
> for the aforementioned instructions to make sure that the
> emulation infrastructure treats them as unknown. The kernel
> can then fallback to executing such instructions on hardware.
>
> Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in emulate_step()")
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
> Previous versions can be found at:
> v3: https://lore.kernel.org/linuxppc-dev/20210204071432.116439-1-sandipan@linux.ibm.com/
> v2: https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandipan@linux.ibm.com/
> v1: https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandipan@linux.ibm.com/
>
> Changes in v4:
> - Fixed grammar and switch-case alignment.
>
> Changes in v3:
> - Consolidated the checks as suggested by Naveen.
> - Consolidated load/store changes into a single patch.
> - Included floating-point load/store and update instructions.
>
> Changes in v2:
> - Jump to unknown_opcode instead of returning -1 for invalid
> instruction forms.
>
> ---
> arch/powerpc/lib/sstep.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
For the series:
Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc: sstep: Fix load and update emulation
From: Naveen N. Rao @ 2021-02-04 8:27 UTC (permalink / raw)
To: Segher Boessenkool
Cc: ravi.bangoria, ananth, jniethe5, paulus, Sandipan Das,
linuxppc-dev, dja
In-Reply-To: <20210203211732.GD30983@gate.crashing.org>
On 2021/02/03 03:17PM, Segher Boessenkool wrote:
> On Wed, Feb 03, 2021 at 03:19:09PM +0530, Naveen N. Rao wrote:
> > On 2021/02/03 12:08PM, Sandipan Das wrote:
> > > The Power ISA says that the fixed-point load and update
> > > instructions must neither use R0 for the base address (RA)
> > > nor have the destination (RT) and the base address (RA) as
> > > the same register. In these cases, the instruction is
> > > invalid.
>
> > > However, the following behaviour is observed using some
> > > invalid opcodes where RA = RT.
> > >
> > > An userspace program using an invalid instruction word like
> > > 0xe9ce0001, i.e. "ldu r14, 0(r14)", runs and exits without
> > > getting terminated abruptly. The instruction performs the
> > > load operation but does not write the effective address to
> > > the base address register.
> >
> > While the processor (p8 in my test) doesn't seem to be throwing an
> > exception, I don't think it is necessarily loading the value. Qemu
> > throws an exception though. It's probably best to term the behavior as
> > being undefined.
>
> Power8 does:
>
> Load with Update Instructions (RA = 0)
> EA is placed into R0.
> Load with Update Instructions (RA = RT)
> EA is placed into RT. The storage operand addressed by EA is
> accessed, but the data returned by the load is discarded.
I'm actually not seeing that. This is what I am testing with:
li 8,0xaaa
mr 6,1
std 8,64(6)
#ldu 6,64(6)
.long 0xe8c60041
And, r6 always ends up with 0xaea. It changes with the value I put into
r6 though.
Granted, this is all up in the air, but it does look like there is more
going on and the value isn't the EA or the value at the address.
>
> Power9 does:
>
> Load with Update Instructions (RA = 0)
> EA is placed into R0.
> Load with Update Instructions (RA = RT)
> The storage operand addressed by EA is accessed. The displacement
> field is added to the data returned by the load and placed into RT.
>
> Both UMs also say
>
> Invalid Forms
> In general, the POWER9 core handles invalid forms of instructions in
> the manner that is most convenient for the particular case (within
> the scope of meeting the boundedly-undefined definition described in
> the Power ISA). This document specifies the behavior for these
> cases. However, it is not recommended that software or other system
> facilities make use of the POWER9 behavior in these cases because
> such behavior might be different in another processor that
> implements the Power ISA.
>
> (or POWER8 instead of POWER9 of course). Always complaining about most
> invalid forms seems wise, certainly if not all recent CPUs behave the
> same :-)
Agreed.
- Naveen
^ permalink raw reply
* Re: [PATCH v2 0/5] shoot lazy tlbs
From: Nicholas Piggin @ 2021-02-04 8:21 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-arch, linux-mm, linuxppc-dev, Andy Lutomirski
In-Reply-To: <20201214065312.270062-1-npiggin@gmail.com>
I'll ask Andrew to put this in -mm if no objections.
The series now doesn't touch other archs in non-trivial ways, and core code
is functionally not changed much / at all if the option is not selected so
it's actually pretty simple aside from the powerpc change.
Thanks,
Nick
Excerpts from Nicholas Piggin's message of December 14, 2020 4:53 pm:
> This is another rebase, on top of mainline now (don't need the
> asm-generic tree), and without any x86 or membarrier changes.
> This makes the series far smaller and more manageable and
> without the controversial bits.
>
> Thanks,
> Nick
>
> Nicholas Piggin (5):
> lazy tlb: introduce lazy mm refcount helper functions
> lazy tlb: allow lazy tlb mm switching to be configurable
> lazy tlb: shoot lazies, a non-refcounting lazy tlb option
> powerpc: use lazy mm refcount helper functions
> powerpc/64s: enable MMU_LAZY_TLB_SHOOTDOWN
>
> arch/Kconfig | 30 ++++++++++
> arch/arm/mach-rpc/ecard.c | 2 +-
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/kernel/smp.c | 2 +-
> arch/powerpc/mm/book3s64/radix_tlb.c | 4 +-
> fs/exec.c | 4 +-
> include/linux/sched/mm.h | 20 +++++++
> kernel/cpu.c | 2 +-
> kernel/exit.c | 2 +-
> kernel/fork.c | 52 ++++++++++++++++
> kernel/kthread.c | 11 ++--
> kernel/sched/core.c | 88 ++++++++++++++++++++--------
> kernel/sched/sched.h | 4 +-
> 13 files changed, 184 insertions(+), 38 deletions(-)
>
> --
> 2.23.0
>
>
^ permalink raw reply
* [PATCH v4 2/2] powerpc: sstep: Fix darn emulation
From: Sandipan Das @ 2021-02-04 8:07 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, ananth, jniethe5, paulus, naveen.n.rao,
linuxppc-dev, dja
In-Reply-To: <20210204080744.135785-1-sandipan@linux.ibm.com>
Commit 8813ff49607e ("powerpc/sstep: Check instruction
validity against ISA version before emulation") introduced
a proper way to skip unknown instructions. This makes sure
that the same is used for the darn instruction when the
range selection bits have a reserved value.
Fixes: a23987ef267a ("powerpc: sstep: Add support for darn instruction")
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
arch/powerpc/lib/sstep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 11f14b209d7f..683f7c20f74b 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1916,7 +1916,7 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
goto compute_done;
}
- return -1;
+ goto unknown_opcode;
#ifdef __powerpc64__
case 777: /* modsd */
if (!cpu_has_feature(CPU_FTR_ARCH_300))
--
2.25.1
^ permalink raw reply related
* [PATCH v4 1/2] powerpc: sstep: Fix load-store and update emulation
From: Sandipan Das @ 2021-02-04 8:07 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, ananth, jniethe5, paulus, naveen.n.rao,
linuxppc-dev, dja
The Power ISA says that the fixed-point load and update
instructions must neither use R0 for the base address (RA)
nor have the destination (RT) and the base address (RA) as
the same register. Similarly, for fixed-point stores and
floating-point loads and stores, the instruction is invalid
when R0 is used as the base address (RA).
This is applicable to the following instructions.
* Load Byte and Zero with Update (lbzu)
* Load Byte and Zero with Update Indexed (lbzux)
* Load Halfword and Zero with Update (lhzu)
* Load Halfword and Zero with Update Indexed (lhzux)
* Load Halfword Algebraic with Update (lhau)
* Load Halfword Algebraic with Update Indexed (lhaux)
* Load Word and Zero with Update (lwzu)
* Load Word and Zero with Update Indexed (lwzux)
* Load Word Algebraic with Update Indexed (lwaux)
* Load Doubleword with Update (ldu)
* Load Doubleword with Update Indexed (ldux)
* Load Floating Single with Update (lfsu)
* Load Floating Single with Update Indexed (lfsux)
* Load Floating Double with Update (lfdu)
* Load Floating Double with Update Indexed (lfdux)
* Store Byte with Update (stbu)
* Store Byte with Update Indexed (stbux)
* Store Halfword with Update (sthu)
* Store Halfword with Update Indexed (sthux)
* Store Word with Update (stwu)
* Store Word with Update Indexed (stwux)
* Store Doubleword with Update (stdu)
* Store Doubleword with Update Indexed (stdux)
* Store Floating Single with Update (stfsu)
* Store Floating Single with Update Indexed (stfsux)
* Store Floating Double with Update (stfdu)
* Store Floating Double with Update Indexed (stfdux)
E.g. the following behaviour is observed for an invalid
load and update instruction having RA = RT.
While a userspace program having an instruction word like
0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
receiving a SIGILL on a Power system (observed on P8 and
P9), the outcome of executing that instruction word varies
and its behaviour can be considered to be undefined.
Attaching an uprobe at that instruction's address results
in emulation which currently performs the load as well as
writes the effective address back to the base register.
This might not match the outcome from hardware.
To remove any inconsistencies, this adds additional checks
for the aforementioned instructions to make sure that the
emulation infrastructure treats them as unknown. The kernel
can then fallback to executing such instructions on hardware.
Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in emulate_step()")
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
Previous versions can be found at:
v3: https://lore.kernel.org/linuxppc-dev/20210204071432.116439-1-sandipan@linux.ibm.com/
v2: https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandipan@linux.ibm.com/
v1: https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandipan@linux.ibm.com/
Changes in v4:
- Fixed grammar and switch-case alignment.
Changes in v3:
- Consolidated the checks as suggested by Naveen.
- Consolidated load/store changes into a single patch.
- Included floating-point load/store and update instructions.
Changes in v2:
- Jump to unknown_opcode instead of returning -1 for invalid
instruction forms.
---
arch/powerpc/lib/sstep.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index e96cff845ef7..11f14b209d7f 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3017,6 +3017,20 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
}
+ if (OP_IS_LOAD_STORE(op->type) && (op->type & UPDATE)) {
+ switch (GETTYPE(op->type)) {
+ case LOAD:
+ if (ra == rd)
+ goto unknown_opcode;
+ fallthrough;
+ case STORE:
+ case LOAD_FP:
+ case STORE_FP:
+ if (ra == 0)
+ goto unknown_opcode;
+ }
+ }
+
#ifdef CONFIG_VSX
if ((GETTYPE(op->type) == LOAD_VSX ||
GETTYPE(op->type) == STORE_VSX) &&
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 28/32] powerpc/64s: interrupt implement exit logic in C
From: Christophe Leroy @ 2021-02-04 8:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Michal Suchanek
In-Reply-To: <1612409077.fadt3kvld9.astroid@bobo.none>
Le 04/02/2021 à 04:27, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of February 4, 2021 2:25 am:
>>
>>
>> Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
>>> Implement the bulk of interrupt return logic in C. The asm return code
>>> must handle a few cases: restoring full GPRs, and emulating stack store.
>>>
>>
>>
>>> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
>>> +{
>>> + unsigned long *ti_flagsp = ¤t_thread_info()->flags;
>>> + unsigned long flags;
>>> +
>>> + if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
>>> + unrecoverable_exception(regs);
>>> + BUG_ON(regs->msr & MSR_PR);
>>> + BUG_ON(!FULL_REGS(regs));
>>> +
>>> + local_irq_save(flags);
>>> +
>>> + if (regs->softe == IRQS_ENABLED) {
>>> + /* Returning to a kernel context with local irqs enabled. */
>>> + WARN_ON_ONCE(!(regs->msr & MSR_EE));
>>> +again:
>>> + if (IS_ENABLED(CONFIG_PREEMPT)) {
>>> + /* Return to preemptible kernel context */
>>> + if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
>>> + if (preempt_count() == 0)
>>> + preempt_schedule_irq();
>>> + }
>>> + }
>>> +
>>> + trace_hardirqs_on();
>>> + __hard_EE_RI_disable();
>>> + if (unlikely(lazy_irq_pending())) {
>>> + __hard_RI_enable();
>>> + irq_soft_mask_set(IRQS_ALL_DISABLED);
>>> + trace_hardirqs_off();
>>> + local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>>> + /*
>>> + * Can't local_irq_enable in case we are in interrupt
>>> + * context. Must replay directly.
>>> + */
>>> + replay_soft_interrupts();
>>> + irq_soft_mask_set(flags);
>>> + /* Took an interrupt, may have more exit work to do. */
>>> + goto again;
>>> + }
>>> + local_paca->irq_happened = 0;
>>> + irq_soft_mask_set(IRQS_ENABLED);
>>> + } else {
>>> + /* Returning to a kernel context with local irqs disabled. */
>>> + trace_hardirqs_on();
>>> + __hard_EE_RI_disable();
>>> + if (regs->msr & MSR_EE)
>>> + local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
>>> + }
>>> +
>>> +
>>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>>> + local_paca->tm_scratch = regs->msr;
>>> +#endif
>>> +
>>> + /*
>>> + * We don't need to restore AMR on the way back to userspace for KUAP.
>>> + * The value of AMR only matters while we're in the kernel.
>>> + */
>>> + kuap_restore_amr(regs);
>>
>> Is that correct to restore KUAP state here ? Shouldn't we have it at lower level in assembly ?
>>
>> Isn't there a risk that someone manages to call interrupt_exit_kernel_prepare() or the end of it in
>> a way or another, and get the previous KUAP state restored by this way ?
>
> I'm not sure if there much more risk if it's here rather than the
> instruction being in another place in the code.
>
> There's a lot of user access around the kernel too if you want to find a
> gadget to unlock KUAP then I suppose there is a pretty large attack
> surface.
My understanding is that user access scope is strictly limited, for instance we enforce the
begin/end of user access to be in the same function, and we refrain from calling any other function
inside the user access window. x86 even have 'objtool' to enforce it at build time. So in theory
there is no way to get out of the function while user access is open.
Here with the interrupt exit function it is free beer. You have a place where you re-open user
access and return with a simple blr. So that's open bar. If someone manages to just call the
interrupt exit function, then user access remains open
>
>> Also, it looks a bit strange to have kuap_save_amr_and_lock() done at lowest level in assembly, and
>> kuap_restore_amr() done in upper level. That looks unbalanced.
>
> I'd like to bring the entry assembly into C.
>
I really think it's not a good idea. We'll get better control and readability by keeping it at the
lowest possible level in assembly.
x86 only save and restore SMAC state in assembly.
Christophe
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc: sstep: Fix load-store and update emulation
From: Sandipan Das @ 2021-02-04 7:59 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: ravi.bangoria, ananth, jniethe5, paulus, linuxppc-dev, dja
In-Reply-To: <20210204073954.GH210@DESKTOP-TDPLP67.localdomain>
On 04/02/21 1:09 pm, Naveen N. Rao wrote:
> [...]
>
> I'm afraid there is one more thing. scripts/checkpatch.pl reports:
>
> WARNING: 'an userspace' may be misspelled - perhaps 'a userspace'?
> #52:
> While an userspace program having an instruction word like
> ^^^^^^^^^^^^
>
> ERROR: switch and case should be at the same indent
> #96: FILE: arch/powerpc/lib/sstep.c:3021:
> + switch (GETTYPE(op->type)) {
> + case LOAD:
> [...]
> + case STORE:
> + case LOAD_FP:
> + case STORE_FP:
>
>
Yikes! Thanks for pointing that out. Sending v4.
- Sandipan
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc: sstep: Fix load-store and update emulation
From: Naveen N. Rao @ 2021-02-04 7:39 UTC (permalink / raw)
To: Sandipan Das; +Cc: ravi.bangoria, ananth, jniethe5, paulus, linuxppc-dev, dja
In-Reply-To: <20210204071432.116439-1-sandipan@linux.ibm.com>
On 2021/02/04 12:44PM, Sandipan Das wrote:
> The Power ISA says that the fixed-point load and update
> instructions must neither use R0 for the base address (RA)
> nor have the destination (RT) and the base address (RA) as
> the same register. Similarly, for fixed-point stores and
> floating-point loads and stores, the instruction is invalid
> when R0 is used as the base address (RA).
>
> This is applicable to the following instructions.
> * Load Byte and Zero with Update (lbzu)
> * Load Byte and Zero with Update Indexed (lbzux)
> * Load Halfword and Zero with Update (lhzu)
> * Load Halfword and Zero with Update Indexed (lhzux)
> * Load Halfword Algebraic with Update (lhau)
> * Load Halfword Algebraic with Update Indexed (lhaux)
> * Load Word and Zero with Update (lwzu)
> * Load Word and Zero with Update Indexed (lwzux)
> * Load Word Algebraic with Update Indexed (lwaux)
> * Load Doubleword with Update (ldu)
> * Load Doubleword with Update Indexed (ldux)
> * Load Floating Single with Update (lfsu)
> * Load Floating Single with Update Indexed (lfsux)
> * Load Floating Double with Update (lfdu)
> * Load Floating Double with Update Indexed (lfdux)
> * Store Byte with Update (stbu)
> * Store Byte with Update Indexed (stbux)
> * Store Halfword with Update (sthu)
> * Store Halfword with Update Indexed (sthux)
> * Store Word with Update (stwu)
> * Store Word with Update Indexed (stwux)
> * Store Doubleword with Update (stdu)
> * Store Doubleword with Update Indexed (stdux)
> * Store Floating Single with Update (stfsu)
> * Store Floating Single with Update Indexed (stfsux)
> * Store Floating Double with Update (stfdu)
> * Store Floating Double with Update Indexed (stfdux)
>
> E.g. the following behaviour is observed for an invalid
> load and update instruction having RA = RT.
>
> While an userspace program having an instruction word like
> 0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
> receiving a SIGILL on a Power system (observed on P8 and
> P9), the outcome of executing that instruction word varies
> and its behaviour can be considered to be undefined.
>
> Attaching an uprobe at that instruction's address results
> in emulation which currently performs the load as well as
> writes the effective address back to the base register.
> This might not match the outcome from hardware.
>
> To remove any inconsistencies, this adds additional checks
> for the aforementioned instructions to make sure that the
> emulation infrastructure treats them as unknown. The kernel
> can then fallback to executing such instructions on hardware.
>
> Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in emulate_step()")
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
> Previous versions can be found at:
> v2: https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandipan@linux.ibm.com/
> v1: https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandipan@linux.ibm.com/
>
> Changes in v3:
> - Dropped CONFIG_PPC_FPU check as suggested by Michael.
> - Consolidated the checks as suggested by Naveen.
>
> Changes in v2:
> - Jump to unknown_opcode instead of returning -1 for invalid
> instruction forms.
>
> ---
> arch/powerpc/lib/sstep.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index e96cff845ef7..9138967eb82e 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -3017,6 +3017,20 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>
> }
>
> + if (OP_IS_LOAD_STORE(op->type) && (op->type & UPDATE)) {
> + switch (GETTYPE(op->type)) {
> + case LOAD:
> + if (ra == rd)
> + goto unknown_opcode;
> + fallthrough;
> + case STORE:
> + case LOAD_FP:
> + case STORE_FP:
> + if (ra == 0)
> + goto unknown_opcode;
> + }
> + }
> +
> #ifdef CONFIG_VSX
> if ((GETTYPE(op->type) == LOAD_VSX ||
> GETTYPE(op->type) == STORE_VSX) &&
I'm afraid there is one more thing. scripts/checkpatch.pl reports:
WARNING: 'an userspace' may be misspelled - perhaps 'a userspace'?
#52:
While an userspace program having an instruction word like
^^^^^^^^^^^^
ERROR: switch and case should be at the same indent
#96: FILE: arch/powerpc/lib/sstep.c:3021:
+ switch (GETTYPE(op->type)) {
+ case LOAD:
[...]
+ case STORE:
+ case LOAD_FP:
+ case STORE_FP:
- Naveen
^ permalink raw reply
* Re: [PATCH RFC v1 2/6] swiotlb: convert variables to arrays
From: Christoph Hellwig @ 2021-02-04 7:29 UTC (permalink / raw)
To: Dongli Zhang
Cc: ulf.hansson, airlied, joonas.lahtinen, dri-devel, linux-kernel,
bhelgaas, paulus, hpa, hch, m.szyprowski, sstabellini,
adrian.hunter, x86, joe.jin, mingo, peterz, mingo, bskeggs,
linux-pci, xen-devel, matthew.auld, thomas.lendacky, konrad.wilk,
intel-gfx, jani.nikula, bp, rodrigo.vivi, nouveau, Claire Chang,
boris.ostrovsky, chris, jgross, tsbogend, robin.murphy, linux-mmc,
linux-mips, iommu, tglx, bauerman, daniel, akpm, linuxppc-dev,
rppt
In-Reply-To: <20210203233709.19819-3-dongli.zhang@oracle.com>
On Wed, Feb 03, 2021 at 03:37:05PM -0800, Dongli Zhang wrote:
> This patch converts several swiotlb related variables to arrays, in
> order to maintain stat/status for different swiotlb buffers. Here are
> variables involved:
>
> - io_tlb_start and io_tlb_end
> - io_tlb_nslabs and io_tlb_used
> - io_tlb_list
> - io_tlb_index
> - max_segment
> - io_tlb_orig_addr
> - no_iotlb_memory
>
> There is no functional change and this is to prepare to enable 64-bit
> swiotlb.
Claire Chang (on Cc) already posted a patch like this a month ago,
which looks much better because it actually uses a struct instead
of all the random variables.
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc: sstep: Fix load-store and update emulation
From: Sandipan Das @ 2021-02-04 7:23 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, ananth, jniethe5, paulus, naveen.n.rao,
linuxppc-dev, dja
In-Reply-To: <20210204071432.116439-1-sandipan@linux.ibm.com>
On 04/02/21 12:44 pm, Sandipan Das wrote:
> The Power ISA says that the fixed-point load and update
> instructions must neither use R0 for the base address (RA)
> nor have the destination (RT) and the base address (RA) as
> the same register. Similarly, for fixed-point stores and
> floating-point loads and stores, the instruction is invalid
> when R0 is used as the base address (RA).
>
> This is applicable to the following instructions.
> * Load Byte and Zero with Update (lbzu)
> * Load Byte and Zero with Update Indexed (lbzux)
> * Load Halfword and Zero with Update (lhzu)
> * Load Halfword and Zero with Update Indexed (lhzux)
> * Load Halfword Algebraic with Update (lhau)
> * Load Halfword Algebraic with Update Indexed (lhaux)
> * Load Word and Zero with Update (lwzu)
> * Load Word and Zero with Update Indexed (lwzux)
> * Load Word Algebraic with Update Indexed (lwaux)
> * Load Doubleword with Update (ldu)
> * Load Doubleword with Update Indexed (ldux)
> * Load Floating Single with Update (lfsu)
> * Load Floating Single with Update Indexed (lfsux)
> * Load Floating Double with Update (lfdu)
> * Load Floating Double with Update Indexed (lfdux)
> * Store Byte with Update (stbu)
> * Store Byte with Update Indexed (stbux)
> * Store Halfword with Update (sthu)
> * Store Halfword with Update Indexed (sthux)
> * Store Word with Update (stwu)
> * Store Word with Update Indexed (stwux)
> * Store Doubleword with Update (stdu)
> * Store Doubleword with Update Indexed (stdux)
> * Store Floating Single with Update (stfsu)
> * Store Floating Single with Update Indexed (stfsux)
> * Store Floating Double with Update (stfdu)
> * Store Floating Double with Update Indexed (stfdux)
>
> E.g. the following behaviour is observed for an invalid
> load and update instruction having RA = RT.
>
> While an userspace program having an instruction word like
> 0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
> receiving a SIGILL on a Power system (observed on P8 and
> P9), the outcome of executing that instruction word varies
> and its behaviour can be considered to be undefined.
>
> Attaching an uprobe at that instruction's address results
> in emulation which currently performs the load as well as
> writes the effective address back to the base register.
> This might not match the outcome from hardware.
>
> To remove any inconsistencies, this adds additional checks
> for the aforementioned instructions to make sure that the
> emulation infrastructure treats them as unknown. The kernel
> can then fallback to executing such instructions on hardware.
>
> Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in emulate_step()")
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
> Previous versions can be found at:
> v2: https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandipan@linux.ibm.com/
> v1: https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandipan@linux.ibm.com/
>
> Changes in v3:
> - Dropped CONFIG_PPC_FPU check as suggested by Michael.
> - Consolidated the checks as suggested by Naveen.
Sorry. Missed these in the changelog:
- Consolidated load/store changes into a single patch.
- Included floating-point load/store and update instructions.
- Sandipan
^ permalink raw reply
* [PATCH v3 2/2] powerpc: sstep: Fix darn emulation
From: Sandipan Das @ 2021-02-04 7:14 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, ananth, jniethe5, paulus, naveen.n.rao,
linuxppc-dev, dja
In-Reply-To: <20210204071432.116439-1-sandipan@linux.ibm.com>
Commit 8813ff49607e ("powerpc/sstep: Check instruction
validity against ISA version before emulation") introduced
a proper way to skip unknown instructions. This makes sure
that the same is used for the darn instruction when the
range selection bits have a reserved value.
Fixes: a23987ef267a ("powerpc: sstep: Add support for darn instruction")
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
arch/powerpc/lib/sstep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 9138967eb82e..e7e8bc974c0f 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1916,7 +1916,7 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
goto compute_done;
}
- return -1;
+ goto unknown_opcode;
#ifdef __powerpc64__
case 777: /* modsd */
if (!cpu_has_feature(CPU_FTR_ARCH_300))
--
2.25.1
^ permalink raw reply related
* [PATCH v3 1/2] powerpc: sstep: Fix load-store and update emulation
From: Sandipan Das @ 2021-02-04 7:14 UTC (permalink / raw)
To: mpe
Cc: ravi.bangoria, ananth, jniethe5, paulus, naveen.n.rao,
linuxppc-dev, dja
The Power ISA says that the fixed-point load and update
instructions must neither use R0 for the base address (RA)
nor have the destination (RT) and the base address (RA) as
the same register. Similarly, for fixed-point stores and
floating-point loads and stores, the instruction is invalid
when R0 is used as the base address (RA).
This is applicable to the following instructions.
* Load Byte and Zero with Update (lbzu)
* Load Byte and Zero with Update Indexed (lbzux)
* Load Halfword and Zero with Update (lhzu)
* Load Halfword and Zero with Update Indexed (lhzux)
* Load Halfword Algebraic with Update (lhau)
* Load Halfword Algebraic with Update Indexed (lhaux)
* Load Word and Zero with Update (lwzu)
* Load Word and Zero with Update Indexed (lwzux)
* Load Word Algebraic with Update Indexed (lwaux)
* Load Doubleword with Update (ldu)
* Load Doubleword with Update Indexed (ldux)
* Load Floating Single with Update (lfsu)
* Load Floating Single with Update Indexed (lfsux)
* Load Floating Double with Update (lfdu)
* Load Floating Double with Update Indexed (lfdux)
* Store Byte with Update (stbu)
* Store Byte with Update Indexed (stbux)
* Store Halfword with Update (sthu)
* Store Halfword with Update Indexed (sthux)
* Store Word with Update (stwu)
* Store Word with Update Indexed (stwux)
* Store Doubleword with Update (stdu)
* Store Doubleword with Update Indexed (stdux)
* Store Floating Single with Update (stfsu)
* Store Floating Single with Update Indexed (stfsux)
* Store Floating Double with Update (stfdu)
* Store Floating Double with Update Indexed (stfdux)
E.g. the following behaviour is observed for an invalid
load and update instruction having RA = RT.
While an userspace program having an instruction word like
0xe9ce0001, i.e. ldu r14, 0(r14), runs without getting
receiving a SIGILL on a Power system (observed on P8 and
P9), the outcome of executing that instruction word varies
and its behaviour can be considered to be undefined.
Attaching an uprobe at that instruction's address results
in emulation which currently performs the load as well as
writes the effective address back to the base register.
This might not match the outcome from hardware.
To remove any inconsistencies, this adds additional checks
for the aforementioned instructions to make sure that the
emulation infrastructure treats them as unknown. The kernel
can then fallback to executing such instructions on hardware.
Fixes: 0016a4cf5582 ("powerpc: Emulate most Book I instructions in emulate_step()")
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
Previous versions can be found at:
v2: https://lore.kernel.org/linuxppc-dev/20210203063841.431063-1-sandipan@linux.ibm.com/
v1: https://lore.kernel.org/linuxppc-dev/20201119054139.244083-1-sandipan@linux.ibm.com/
Changes in v3:
- Dropped CONFIG_PPC_FPU check as suggested by Michael.
- Consolidated the checks as suggested by Naveen.
Changes in v2:
- Jump to unknown_opcode instead of returning -1 for invalid
instruction forms.
---
arch/powerpc/lib/sstep.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index e96cff845ef7..9138967eb82e 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3017,6 +3017,20 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
}
+ if (OP_IS_LOAD_STORE(op->type) && (op->type & UPDATE)) {
+ switch (GETTYPE(op->type)) {
+ case LOAD:
+ if (ra == rd)
+ goto unknown_opcode;
+ fallthrough;
+ case STORE:
+ case LOAD_FP:
+ case STORE_FP:
+ if (ra == 0)
+ goto unknown_opcode;
+ }
+ }
+
#ifdef CONFIG_VSX
if ((GETTYPE(op->type) == LOAD_VSX ||
GETTYPE(op->type) == STORE_VSX) &&
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] powerpc64/idle: Fix SP offsets when saving GPRs
From: Christopher M. Riedl @ 2021-02-04 6:59 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Michael Ellerman
In-Reply-To: <1612014056.e1qcnzac7c.astroid@bobo.none>
On Sat Jan 30, 2021 at 7:44 AM CST, Nicholas Piggin wrote:
> Excerpts from Michael Ellerman's message of January 30, 2021 9:32 pm:
> > "Christopher M. Riedl" <cmr@codefail.de> writes:
> >> The idle entry/exit code saves/restores GPRs in the stack "red zone"
> >> (Protected Zone according to PowerPC64 ELF ABI v2). However, the offset
> >> used for the first GPR is incorrect and overwrites the back chain - the
> >> Protected Zone actually starts below the current SP. In practice this is
> >> probably not an issue, but it's still incorrect so fix it.
> >
> > Nice catch.
> >
> > Corrupting the back chain means you can't backtrace from there, which
> > could be confusing for debugging one day.
>
> Yeah, we seem to have got away without noticing because the CPU will
> wake up and return out of here before it tries to unwind the stack,
> but if you tried to walk it by hand if the CPU got stuck in idle or
> something, then we'd get confused.
>
> > It does make me wonder why we don't just create a stack frame and use
> > the normal macros? It would use a bit more stack space, but we shouldn't
> > be short of stack space when going idle.
> >
> > Nick, was there a particular reason for using the red zone?
>
> I don't recall a particular reason, I think a normal stack frame is
> probably a good idea.
I'll send a version using STACKFRAMESIZE - I assume that's the "normal"
stack frame :)
I admit I am a bit confused when I saw the similar but much smaller
STACK_FRAME_OVERHEAD which is also used in _some_ cases to save/restore
a few registers.
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH v2] powerpc/64s: Fix pte update for kernel memory on radix
From: Naveen N. Rao @ 2021-02-04 6:41 UTC (permalink / raw)
To: Jordan Niethe; +Cc: linuxppc-dev, npiggin, cmr
In-Reply-To: <20210203235907.961243-1-jniethe5@gmail.com>
Hi Jordan,
On 2021/02/04 10:59AM, Jordan Niethe wrote:
> When adding a pte a ptesync is needed to order the update of the pte
> with subsequent accesses otherwise a spurious fault may be raised.
>
> radix__set_pte_at() does not do this for performance gains. For
> non-kernel memory this is not an issue as any faults of this kind are
> corrected by the page fault handler. For kernel memory these faults are
> not handled. The current solution is that there is a ptesync in
> flush_cache_vmap() which should be called when mapping from the vmalloc
> region.
>
> However, map_kernel_page() does not call flush_cache_vmap(). This is
> troublesome in particular for code patching with Strict RWX on radix. In
> do_patch_instruction() the page frame that contains the instruction to
> be patched is mapped and then immediately patched. With no ordering or
> synchronization between setting up the pte and writing to the page it is
> possible for faults.
>
> As the code patching is done using __put_user_asm_goto() the resulting
> fault is obscured - but using a normal store instead it can be seen:
>
> [ 418.498768][ T757] BUG: Unable to handle kernel data access on write at 0xc008000008f24a3c
> [ 418.498790][ T757] Faulting instruction address: 0xc00000000008bd74
> [ 418.498805][ T757] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 418.498828][ T757] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
> [ 418.498843][ T757] Modules linked in: nop_module(PO+) [last unloaded: nop_module]
> [ 418.498872][ T757] CPU: 4 PID: 757 Comm: sh Tainted: P O 5.10.0-rc5-01361-ge3c1b78c8440-dirty #43
> [ 418.498936][ T757] NIP: c00000000008bd74 LR: c00000000008bd50 CTR: c000000000025810
> [ 418.498979][ T757] REGS: c000000016f634a0 TRAP: 0300 Tainted: P O (5.10.0-rc5-01361-ge3c1b78c8440-dirty)
> [ 418.499033][ T757] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 44002884 XER: 00000000
> [ 418.499084][ T757] CFAR: c00000000007c68c DAR: c008000008f24a3c DSISR: 42000000 IRQMASK: 1
>
> This results in the kind of issue reported here:
> https://lore.kernel.org/linuxppc-dev/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw/
>
Thanks for fixing this!
- Naveen
^ 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