From 13f62e84385fa0241fc6a2178da50af02189121b Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 2 Nov 2022 15:16:43 +0100 Subject: [PATCH 1/8] s390/cmpxchg: use symbolic names for inline assembly operands Make cmpxchg() inline assemblies more readable by using symbolic names for operands. Link: https://lore.kernel.org/r/Y2J7yzQYt/bjLQXY@osiris Signed-off-by: Heiko Carstens --- arch/s390/include/asm/cmpxchg.h | 76 ++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h index 84c3f0d576c5..56fb8aa08945 100644 --- a/arch/s390/include/asm/cmpxchg.h +++ b/arch/s390/include/asm/cmpxchg.h @@ -96,56 +96,64 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, shift = (3 ^ (address & 3)) << 3; address ^= address & 3; asm volatile( - " l %0,%2\n" - "0: nr %0,%5\n" - " lr %1,%0\n" - " or %0,%3\n" - " or %1,%4\n" - " cs %0,%1,%2\n" - " jnl 1f\n" - " xr %1,%0\n" - " nr %1,%5\n" - " jnz 0b\n" + " l %[prev],%[address]\n" + "0: nr %[prev],%[mask]\n" + " lr %[tmp],%[prev]\n" + " or %[prev],%[old]\n" + " or %[tmp],%[new]\n" + " cs %[prev],%[tmp],%[address]\n" + " jnl 1f\n" + " xr %[tmp],%[prev]\n" + " nr %[tmp],%[mask]\n" + " jnz 0b\n" "1:" - : "=&d" (prev), "=&d" (tmp), "+Q" (*(int *) address) - : "d" ((old & 0xff) << shift), - "d" ((new & 0xff) << shift), - "d" (~(0xff << shift)) + : [prev] "=&d" (prev), + [tmp] "=&d" (tmp), + [address] "+Q" (*(int *)address) + : [old] "d" ((old & 0xff) << shift), + [new] "d" ((new & 0xff) << shift), + [mask] "d" (~(0xff << shift)) : "memory", "cc"); return prev >> shift; case 2: shift = (2 ^ (address & 2)) << 3; address ^= address & 2; asm volatile( - " l %0,%2\n" - "0: nr %0,%5\n" - " lr %1,%0\n" - " or %0,%3\n" - " or %1,%4\n" - " cs %0,%1,%2\n" - " jnl 1f\n" - " xr %1,%0\n" - " nr %1,%5\n" - " jnz 0b\n" + " l %[prev],%[address]\n" + "0: nr %[prev],%[mask]\n" + " lr %[tmp],%[prev]\n" + " or %[prev],%[old]\n" + " or %[tmp],%[new]\n" + " cs %[prev],%[tmp],%[address]\n" + " jnl 1f\n" + " xr %[tmp],%[prev]\n" + " nr %[tmp],%[mask]\n" + " jnz 0b\n" "1:" - : "=&d" (prev), "=&d" (tmp), "+Q" (*(int *) address) - : "d" ((old & 0xffff) << shift), - "d" ((new & 0xffff) << shift), - "d" (~(0xffff << shift)) + : [prev] "=&d" (prev), + [tmp] "=&d" (tmp), + [address] "+Q" (*(int *)address) + : [old] "d" ((old & 0xffff) << shift), + [new] "d" ((new & 0xffff) << shift), + [mask] "d" (~(0xffff << shift)) : "memory", "cc"); return prev >> shift; case 4: asm volatile( - " cs %0,%3,%1\n" - : "=&d" (prev), "+Q" (*(int *) address) - : "0" (old), "d" (new) + " cs %[prev],%[new],%[address]\n" + : [prev] "=&d" (prev), + [address] "+Q" (*(int *)address) + : "0" (old), + [new] "d" (new) : "memory", "cc"); return prev; case 8: asm volatile( - " csg %0,%3,%1\n" - : "=&d" (prev), "+QS" (*(long *) address) - : "0" (old), "d" (new) + " csg %[prev],%[new],%[address]\n" + : [prev] "=&d" (prev), + [address] "+QS" (*(long *)address) + : "0" (old), + [new] "d" (new) : "memory", "cc"); return prev; } From ce968f654570dbd9cac7de694681640061559d3b Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 2 Nov 2022 15:17:28 +0100 Subject: [PATCH 2/8] s390/cmpxchg: make variables local to each case label Make variables local to each case label. This limits the scope of variables and allows to use proper types everywhere. Link: https://lore.kernel.org/r/Y2J7+HqgAZwnfxsh@osiris Signed-off-by: Heiko Carstens --- arch/s390/include/asm/cmpxchg.h | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h index 56fb8aa08945..02165acdaa93 100644 --- a/arch/s390/include/asm/cmpxchg.h +++ b/arch/s390/include/asm/cmpxchg.h @@ -88,11 +88,10 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, unsigned long old, unsigned long new, int size) { - unsigned long prev, tmp; - int shift; - switch (size) { - case 1: + case 1: { + unsigned int prev, tmp, shift; + shift = (3 ^ (address & 3)) << 3; address ^= address & 3; asm volatile( @@ -115,7 +114,10 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, [mask] "d" (~(0xff << shift)) : "memory", "cc"); return prev >> shift; - case 2: + } + case 2: { + unsigned int prev, tmp, shift; + shift = (2 ^ (address & 2)) << 3; address ^= address & 2; asm volatile( @@ -138,16 +140,22 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, [mask] "d" (~(0xffff << shift)) : "memory", "cc"); return prev >> shift; - case 4: + } + case 4: { + unsigned int prev; + asm volatile( " cs %[prev],%[new],%[address]\n" : [prev] "=&d" (prev), [address] "+Q" (*(int *)address) - : "0" (old), + : "0" ((unsigned int)old), [new] "d" (new) : "memory", "cc"); return prev; - case 8: + } + case 8: { + unsigned long prev; + asm volatile( " csg %[prev],%[new],%[address]\n" : [prev] "=&d" (prev), @@ -157,6 +165,7 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, : "memory", "cc"); return prev; } + } __cmpxchg_called_with_bad_pointer(); return old; } From e388d66f032166c8e8c4efd9e6c6f43610378903 Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 2 Nov 2022 15:18:07 +0100 Subject: [PATCH 3/8] s390/cmpxchg: remove digits from input constraints Instead of using a digit for input constraints simply initialize the corresponding output operand in C code and use a "+" constraint modifier. Link: https://lore.kernel.org/r/Y2J8H82B6JhJhrp2@osiris Signed-off-by: Heiko Carstens --- arch/s390/include/asm/cmpxchg.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h index 02165acdaa93..1c5785b851ec 100644 --- a/arch/s390/include/asm/cmpxchg.h +++ b/arch/s390/include/asm/cmpxchg.h @@ -142,26 +142,24 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, return prev >> shift; } case 4: { - unsigned int prev; + unsigned int prev = old; asm volatile( " cs %[prev],%[new],%[address]\n" - : [prev] "=&d" (prev), + : [prev] "+&d" (prev), [address] "+Q" (*(int *)address) - : "0" ((unsigned int)old), - [new] "d" (new) + : [new] "d" (new) : "memory", "cc"); return prev; } case 8: { - unsigned long prev; + unsigned long prev = old; asm volatile( " csg %[prev],%[new],%[address]\n" - : [prev] "=&d" (prev), + : [prev] "+&d" (prev), [address] "+QS" (*(long *)address) - : "0" (old), - [new] "d" (new) + : [new] "d" (new) : "memory", "cc"); return prev; } From f39a8c4a22ea104c368361b9ae0f550da161db2d Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 2 Nov 2022 15:18:45 +0100 Subject: [PATCH 4/8] s390/extable: add EX_TABLE_UA_LOAD_REGPAIR() macro Add new exception table type which is able to handle register pairs. If an exception is recognized on such an instruction the specified register pair will be zeroed, and the specified error register will be modified so it contains -EFAULT, similar to the existing EX_TABLE_UA_LOAD_REG() macro. Link: https://lore.kernel.org/r/Y2J8RSW2khWLgpPo@osiris Signed-off-by: Heiko Carstens --- arch/s390/include/asm/asm-extable.h | 4 ++++ arch/s390/mm/extable.c | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/s390/include/asm/asm-extable.h b/arch/s390/include/asm/asm-extable.h index b74f1070ddb2..55a02a153dfc 100644 --- a/arch/s390/include/asm/asm-extable.h +++ b/arch/s390/include/asm/asm-extable.h @@ -12,6 +12,7 @@ #define EX_TYPE_UA_STORE 3 #define EX_TYPE_UA_LOAD_MEM 4 #define EX_TYPE_UA_LOAD_REG 5 +#define EX_TYPE_UA_LOAD_REGPAIR 6 #define EX_DATA_REG_ERR_SHIFT 0 #define EX_DATA_REG_ERR GENMASK(3, 0) @@ -85,4 +86,7 @@ #define EX_TABLE_UA_LOAD_REG(_fault, _target, _regerr, _regzero) \ __EX_TABLE_UA(__ex_table, _fault, _target, EX_TYPE_UA_LOAD_REG, _regerr, _regzero, 0) +#define EX_TABLE_UA_LOAD_REGPAIR(_fault, _target, _regerr, _regzero) \ + __EX_TABLE_UA(__ex_table, _fault, _target, EX_TYPE_UA_LOAD_REGPAIR, _regerr, _regzero, 0) + #endif /* __ASM_EXTABLE_H */ diff --git a/arch/s390/mm/extable.c b/arch/s390/mm/extable.c index 1e4d2187541a..fe87291df95d 100644 --- a/arch/s390/mm/extable.c +++ b/arch/s390/mm/extable.c @@ -47,13 +47,16 @@ static bool ex_handler_ua_load_mem(const struct exception_table_entry *ex, struc return true; } -static bool ex_handler_ua_load_reg(const struct exception_table_entry *ex, struct pt_regs *regs) +static bool ex_handler_ua_load_reg(const struct exception_table_entry *ex, + bool pair, struct pt_regs *regs) { unsigned int reg_zero = FIELD_GET(EX_DATA_REG_ADDR, ex->data); unsigned int reg_err = FIELD_GET(EX_DATA_REG_ERR, ex->data); regs->gprs[reg_err] = -EFAULT; regs->gprs[reg_zero] = 0; + if (pair) + regs->gprs[reg_zero + 1] = 0; regs->psw.addr = extable_fixup(ex); return true; } @@ -75,7 +78,9 @@ bool fixup_exception(struct pt_regs *regs) case EX_TYPE_UA_LOAD_MEM: return ex_handler_ua_load_mem(ex, regs); case EX_TYPE_UA_LOAD_REG: - return ex_handler_ua_load_reg(ex, regs); + return ex_handler_ua_load_reg(ex, false, regs); + case EX_TYPE_UA_LOAD_REGPAIR: + return ex_handler_ua_load_reg(ex, true, regs); } panic("invalid exception table entry"); } From 4148575abe1e14af3cb9fd1a3c9c2a708ec0b1f9 Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 2 Nov 2022 15:19:23 +0100 Subject: [PATCH 5/8] s390/uaccess: add cmpxchg_user_key() Add cmpxchg_user_key() which allows to execute a compare and exchange on a user space address. This allows also to specify a storage key which makes sure that key-controlled protection is considered. This is based on a patch written by Janis Schoetterl-Glausch. Link: https://lore.kernel.org/all/20220930210751.225873-2-scgl@linux.ibm.com Cc: Janis Schoetterl-Glausch Link: https://lore.kernel.org/r/Y2J8axs+bcQ2dO/l@osiris Signed-off-by: Heiko Carstens --- arch/s390/include/asm/uaccess.h | 183 ++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h index f7038b800cc3..a125e60a1521 100644 --- a/arch/s390/include/asm/uaccess.h +++ b/arch/s390/include/asm/uaccess.h @@ -390,4 +390,187 @@ do { \ goto err_label; \ } while (0) +void __cmpxchg_user_key_called_with_bad_pointer(void); + +static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, + __uint128_t old, __uint128_t new, + unsigned long key, int size) +{ + int rc = 0; + + switch (size) { + case 1: { + unsigned int prev, tmp, shift; + + shift = (3 ^ (address & 3)) << 3; + address ^= address & 3; + asm volatile( + " spka 0(%[key])\n" + " sacf 256\n" + "0: l %[prev],%[address]\n" + "1: nr %[prev],%[mask]\n" + " lr %[tmp],%[prev]\n" + " or %[prev],%[old]\n" + " or %[tmp],%[new]\n" + "2: cs %[prev],%[tmp],%[address]\n" + "3: jnl 4f\n" + " xr %[tmp],%[prev]\n" + " nr %[tmp],%[mask]\n" + " jnz 1b\n" + "4: sacf 768\n" + " spka %[default_key]\n" + EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev]) + : [rc] "+&d" (rc), + [prev] "=&d" (prev), + [tmp] "=&d" (tmp), + [address] "+Q" (*(int *)address) + : [old] "d" (((unsigned int)old & 0xff) << shift), + [new] "d" (((unsigned int)new & 0xff) << shift), + [mask] "d" (~(0xff << shift)), + [key] "a" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY) + : "memory", "cc"); + *(unsigned char *)uval = prev >> shift; + return rc; + } + case 2: { + unsigned int prev, tmp, shift; + + shift = (2 ^ (address & 2)) << 3; + address ^= address & 2; + asm volatile( + " spka 0(%[key])\n" + " sacf 256\n" + "0: l %[prev],%[address]\n" + "1: nr %[prev],%[mask]\n" + " lr %[tmp],%[prev]\n" + " or %[prev],%[old]\n" + " or %[tmp],%[new]\n" + "2: cs %[prev],%[tmp],%[address]\n" + "3: jnl 4f\n" + " xr %[tmp],%[prev]\n" + " nr %[tmp],%[mask]\n" + " jnz 1b\n" + "4: sacf 768\n" + " spka %[default_key]\n" + EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev]) + : [rc] "+&d" (rc), + [prev] "=&d" (prev), + [tmp] "=&d" (tmp), + [address] "+Q" (*(int *)address) + : [old] "d" (((unsigned int)old & 0xffff) << shift), + [new] "d" (((unsigned int)new & 0xffff) << shift), + [mask] "d" (~(0xffff << shift)), + [key] "a" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY) + : "memory", "cc"); + *(unsigned short *)uval = prev >> shift; + return rc; + } + case 4: { + unsigned int prev = old; + + asm volatile( + " spka 0(%[key])\n" + " sacf 256\n" + "0: cs %[prev],%[new],%[address]\n" + "1: sacf 768\n" + " spka %[default_key]\n" + EX_TABLE_UA_LOAD_REG(0b, 1b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 1b, %[rc], %[prev]) + : [rc] "+&d" (rc), + [prev] "+&d" (prev), + [address] "+Q" (*(int *)address) + : [new] "d" ((unsigned int)new), + [key] "a" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY) + : "memory", "cc"); + *(unsigned int *)uval = prev; + return rc; + } + case 8: { + unsigned long prev = old; + + asm volatile( + " spka 0(%[key])\n" + " sacf 256\n" + "0: csg %[prev],%[new],%[address]\n" + "1: sacf 768\n" + " spka %[default_key]\n" + EX_TABLE_UA_LOAD_REG(0b, 1b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 1b, %[rc], %[prev]) + : [rc] "+&d" (rc), + [prev] "+&d" (prev), + [address] "+QS" (*(long *)address) + : [new] "d" ((unsigned long)new), + [key] "a" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY) + : "memory", "cc"); + *(unsigned long *)uval = prev; + return rc; + } + case 16: { + __uint128_t prev = old; + + asm volatile( + " spka 0(%[key])\n" + " sacf 256\n" + "0: cdsg %[prev],%[new],%[address]\n" + "1: sacf 768\n" + " spka %[default_key]\n" + EX_TABLE_UA_LOAD_REGPAIR(0b, 1b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REGPAIR(1b, 1b, %[rc], %[prev]) + : [rc] "+&d" (rc), + [prev] "+&d" (prev), + [address] "+QS" (*(__int128_t *)address) + : [new] "d" (new), + [key] "a" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY) + : "memory", "cc"); + *(__uint128_t *)uval = prev; + return rc; + } + } + __cmpxchg_user_key_called_with_bad_pointer(); + return rc; +} + +/** + * cmpxchg_user_key() - cmpxchg with user space target, honoring storage keys + * @ptr: User space address of value to compare to @old and exchange with + * @new. Must be aligned to sizeof(*@ptr). + * @uval: Address where the old value of *@ptr is written to. + * @old: Old value. Compared to the content pointed to by @ptr in order to + * determine if the exchange occurs. The old value read from *@ptr is + * written to *@uval. + * @new: New value to place at *@ptr. + * @key: Access key to use for checking storage key protection. + * + * Perform a cmpxchg on a user space target, honoring storage key protection. + * @key alone determines how key checking is performed, neither + * storage-protection-override nor fetch-protection-override apply. + * The caller must compare *@uval and @old to determine if values have been + * exchanged. In case of an exception *@uval is set to zero. + * + * Return: 0: cmpxchg executed + * -EFAULT: an exception happened when trying to access *@ptr + */ +#define cmpxchg_user_key(ptr, uval, old, new, key) \ +({ \ + __typeof__(ptr) __ptr = (ptr); \ + __typeof__(uval) __uval = (uval); \ + \ + BUILD_BUG_ON(sizeof(*(__ptr)) != sizeof(*(__uval))); \ + might_fault(); \ + __chk_user_ptr(__ptr); \ + __cmpxchg_user_key((unsigned long)(__ptr), (void *)(__uval), \ + (old), (new), (key), sizeof(*(__ptr))); \ +}) + #endif /* __S390_UACCESS_H */ From 51098f0eb22e2f54055d75dd25bc84eff07d6d8a Mon Sep 17 00:00:00 2001 From: Janis Schoetterl-Glausch Date: Wed, 16 Nov 2022 15:47:11 +0100 Subject: [PATCH 6/8] s390/cmpxchg: make loop condition for 1,2 byte cases precise The cmpxchg implementation for 1 and 2 bytes consists of a 4 byte cmpxchg loop. Currently, the decision to retry is imprecise, looping if bits outside the target byte(s) change instead of retrying until the target byte(s) differ from the old value. E.g. if an attempt to exchange (prev_left_0 old_bytes prev_right_0) is made and it fails because the word at the address is (prev_left_1 x prev_right_1) where both x != old_bytes and one of the prev_*_1 values differs from the respective prev_*_0 value, the cmpxchg is retried, even if by a semantic equivalent to a normal cmpxchg, the exchange would fail. Instead exit the loop if x != old_bytes and retry otherwise. Signed-off-by: Janis Schoetterl-Glausch Link: https://lore.kernel.org/r/20221116144711.3811011-1-scgl@linux.ibm.com Signed-off-by: Heiko Carstens --- arch/s390/include/asm/cmpxchg.h | 60 ++++++++++++++----------- arch/s390/include/asm/uaccess.h | 80 ++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 62 deletions(-) diff --git a/arch/s390/include/asm/cmpxchg.h b/arch/s390/include/asm/cmpxchg.h index 1c5785b851ec..3f26416c2ad8 100644 --- a/arch/s390/include/asm/cmpxchg.h +++ b/arch/s390/include/asm/cmpxchg.h @@ -90,55 +90,63 @@ static __always_inline unsigned long __cmpxchg(unsigned long address, { switch (size) { case 1: { - unsigned int prev, tmp, shift; + unsigned int prev, shift, mask; shift = (3 ^ (address & 3)) << 3; address ^= address & 3; + old = (old & 0xff) << shift; + new = (new & 0xff) << shift; + mask = ~(0xff << shift); asm volatile( " l %[prev],%[address]\n" - "0: nr %[prev],%[mask]\n" - " lr %[tmp],%[prev]\n" - " or %[prev],%[old]\n" - " or %[tmp],%[new]\n" - " cs %[prev],%[tmp],%[address]\n" + " nr %[prev],%[mask]\n" + " xilf %[mask],0xffffffff\n" + " or %[new],%[prev]\n" + " or %[prev],%[tmp]\n" + "0: lr %[tmp],%[prev]\n" + " cs %[prev],%[new],%[address]\n" " jnl 1f\n" " xr %[tmp],%[prev]\n" + " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jnz 0b\n" + " jz 0b\n" "1:" : [prev] "=&d" (prev), - [tmp] "=&d" (tmp), - [address] "+Q" (*(int *)address) - : [old] "d" ((old & 0xff) << shift), - [new] "d" ((new & 0xff) << shift), - [mask] "d" (~(0xff << shift)) - : "memory", "cc"); + [address] "+Q" (*(int *)address), + [tmp] "+&d" (old), + [new] "+&d" (new), + [mask] "+&d" (mask) + :: "memory", "cc"); return prev >> shift; } case 2: { - unsigned int prev, tmp, shift; + unsigned int prev, shift, mask; shift = (2 ^ (address & 2)) << 3; address ^= address & 2; + old = (old & 0xffff) << shift; + new = (new & 0xffff) << shift; + mask = ~(0xffff << shift); asm volatile( " l %[prev],%[address]\n" - "0: nr %[prev],%[mask]\n" - " lr %[tmp],%[prev]\n" - " or %[prev],%[old]\n" - " or %[tmp],%[new]\n" - " cs %[prev],%[tmp],%[address]\n" + " nr %[prev],%[mask]\n" + " xilf %[mask],0xffffffff\n" + " or %[new],%[prev]\n" + " or %[prev],%[tmp]\n" + "0: lr %[tmp],%[prev]\n" + " cs %[prev],%[new],%[address]\n" " jnl 1f\n" " xr %[tmp],%[prev]\n" + " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jnz 0b\n" + " jz 0b\n" "1:" : [prev] "=&d" (prev), - [tmp] "=&d" (tmp), - [address] "+Q" (*(int *)address) - : [old] "d" ((old & 0xffff) << shift), - [new] "d" ((new & 0xffff) << shift), - [mask] "d" (~(0xffff << shift)) - : "memory", "cc"); + [address] "+Q" (*(int *)address), + [tmp] "+&d" (old), + [new] "+&d" (new), + [mask] "+&d" (mask) + :: "memory", "cc"); return prev >> shift; } case 4: { diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h index a125e60a1521..d028ee59e941 100644 --- a/arch/s390/include/asm/uaccess.h +++ b/arch/s390/include/asm/uaccess.h @@ -400,74 +400,82 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, switch (size) { case 1: { - unsigned int prev, tmp, shift; + unsigned int prev, shift, mask, _old, _new; shift = (3 ^ (address & 3)) << 3; address ^= address & 3; + _old = (old & 0xff) << shift; + _new = (new & 0xff) << shift; + mask = ~(0xff << shift); asm volatile( " spka 0(%[key])\n" " sacf 256\n" "0: l %[prev],%[address]\n" "1: nr %[prev],%[mask]\n" - " lr %[tmp],%[prev]\n" - " or %[prev],%[old]\n" - " or %[tmp],%[new]\n" - "2: cs %[prev],%[tmp],%[address]\n" - "3: jnl 4f\n" + " xilf %[mask],0xffffffff\n" + " or %[new],%[prev]\n" + " or %[prev],%[tmp]\n" + "2: lr %[tmp],%[prev]\n" + "3: cs %[prev],%[new],%[address]\n" + "4: jnl 5f\n" " xr %[tmp],%[prev]\n" + " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jnz 1b\n" - "4: sacf 768\n" + " jz 2b\n" + "5: sacf 768\n" " spka %[default_key]\n" - EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev]) : [rc] "+&d" (rc), [prev] "=&d" (prev), - [tmp] "=&d" (tmp), - [address] "+Q" (*(int *)address) - : [old] "d" (((unsigned int)old & 0xff) << shift), - [new] "d" (((unsigned int)new & 0xff) << shift), - [mask] "d" (~(0xff << shift)), - [key] "a" (key << 4), + [address] "+Q" (*(int *)address), + [tmp] "+&d" (_old), + [new] "+&d" (_new), + [mask] "+&d" (mask) + : [key] "a" (key << 4), [default_key] "J" (PAGE_DEFAULT_KEY) : "memory", "cc"); *(unsigned char *)uval = prev >> shift; return rc; } case 2: { - unsigned int prev, tmp, shift; + unsigned int prev, shift, mask, _old, _new; shift = (2 ^ (address & 2)) << 3; address ^= address & 2; + _old = (old & 0xffff) << shift; + _new = (new & 0xffff) << shift; + mask = ~(0xffff << shift); asm volatile( " spka 0(%[key])\n" " sacf 256\n" "0: l %[prev],%[address]\n" "1: nr %[prev],%[mask]\n" - " lr %[tmp],%[prev]\n" - " or %[prev],%[old]\n" - " or %[tmp],%[new]\n" - "2: cs %[prev],%[tmp],%[address]\n" - "3: jnl 4f\n" + " xilf %[mask],0xffffffff\n" + " or %[new],%[prev]\n" + " or %[prev],%[tmp]\n" + "2: lr %[tmp],%[prev]\n" + "3: cs %[prev],%[new],%[address]\n" + "4: jnl 5f\n" " xr %[tmp],%[prev]\n" + " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jnz 1b\n" - "4: sacf 768\n" + " jz 2b\n" + "5: sacf 768\n" " spka %[default_key]\n" - EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev]) - EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev]) + EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev]) : [rc] "+&d" (rc), [prev] "=&d" (prev), - [tmp] "=&d" (tmp), - [address] "+Q" (*(int *)address) - : [old] "d" (((unsigned int)old & 0xffff) << shift), - [new] "d" (((unsigned int)new & 0xffff) << shift), - [mask] "d" (~(0xffff << shift)), - [key] "a" (key << 4), + [address] "+Q" (*(int *)address), + [tmp] "+&d" (_old), + [new] "+&d" (_new), + [mask] "+&d" (mask) + : [key] "a" (key << 4), [default_key] "J" (PAGE_DEFAULT_KEY) : "memory", "cc"); *(unsigned short *)uval = prev >> shift; From 739ad2e4e15b585a0eaf98b7bdee62b2dd9588c9 Mon Sep 17 00:00:00 2001 From: Janis Schoetterl-Glausch Date: Thu, 17 Nov 2022 11:07:45 +0100 Subject: [PATCH 7/8] s390/uaccess: limit number of retries for cmpxchg_user_key() cmpxchg_user_key() for byte and short values is implemented via a one word cmpxchg loop. Give up trying to perform the cmpxchg if it fails too often because of contention on the cache line. This ensures that the thread cannot become stuck in the kernel. Signed-off-by: Janis Schoetterl-Glausch Link: https://lore.kernel.org/r/20221117100745.3253896-1-scgl@linux.ibm.com Signed-off-by: Heiko Carstens --- arch/s390/include/asm/uaccess.h | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h index d028ee59e941..7c10f594e747 100644 --- a/arch/s390/include/asm/uaccess.h +++ b/arch/s390/include/asm/uaccess.h @@ -392,6 +392,8 @@ do { \ void __cmpxchg_user_key_called_with_bad_pointer(void); +#define CMPXCHG_USER_KEY_MAX_LOOPS 128 + static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, __uint128_t old, __uint128_t new, unsigned long key, int size) @@ -401,6 +403,7 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, switch (size) { case 1: { unsigned int prev, shift, mask, _old, _new; + unsigned long count; shift = (3 ^ (address & 3)) << 3; address ^= address & 3; @@ -410,6 +413,7 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, asm volatile( " spka 0(%[key])\n" " sacf 256\n" + " llill %[count],%[max_loops]\n" "0: l %[prev],%[address]\n" "1: nr %[prev],%[mask]\n" " xilf %[mask],0xffffffff\n" @@ -421,7 +425,8 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, " xr %[tmp],%[prev]\n" " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jz 2b\n" + " jnz 5f\n" + " brct %[count],2b\n" "5: sacf 768\n" " spka %[default_key]\n" EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev]) @@ -433,15 +438,20 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, [address] "+Q" (*(int *)address), [tmp] "+&d" (_old), [new] "+&d" (_new), - [mask] "+&d" (mask) - : [key] "a" (key << 4), - [default_key] "J" (PAGE_DEFAULT_KEY) + [mask] "+&d" (mask), + [count] "=a" (count) + : [key] "%[count]" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY), + [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS) : "memory", "cc"); *(unsigned char *)uval = prev >> shift; + if (!count) + rc = -EAGAIN; return rc; } case 2: { unsigned int prev, shift, mask, _old, _new; + unsigned long count; shift = (2 ^ (address & 2)) << 3; address ^= address & 2; @@ -451,6 +461,7 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, asm volatile( " spka 0(%[key])\n" " sacf 256\n" + " llill %[count],%[max_loops]\n" "0: l %[prev],%[address]\n" "1: nr %[prev],%[mask]\n" " xilf %[mask],0xffffffff\n" @@ -462,7 +473,8 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, " xr %[tmp],%[prev]\n" " xr %[new],%[tmp]\n" " nr %[tmp],%[mask]\n" - " jz 2b\n" + " jnz 5f\n" + " brct %[count],2b\n" "5: sacf 768\n" " spka %[default_key]\n" EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev]) @@ -474,11 +486,15 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, [address] "+Q" (*(int *)address), [tmp] "+&d" (_old), [new] "+&d" (_new), - [mask] "+&d" (mask) - : [key] "a" (key << 4), - [default_key] "J" (PAGE_DEFAULT_KEY) + [mask] "+&d" (mask), + [count] "=a" (count) + : [key] "%[count]" (key << 4), + [default_key] "J" (PAGE_DEFAULT_KEY), + [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS) : "memory", "cc"); *(unsigned short *)uval = prev >> shift; + if (!count) + rc = -EAGAIN; return rc; } case 4: { @@ -568,6 +584,7 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, * * Return: 0: cmpxchg executed * -EFAULT: an exception happened when trying to access *@ptr + * -EAGAIN: maxed out number of retries (byte and short only) */ #define cmpxchg_user_key(ptr, uval, old, new, key) \ ({ \ From b33d59fb37ddcb6ee65d4fa23cc3d58793d13c5b Mon Sep 17 00:00:00 2001 From: Heiko Carstens Date: Wed, 4 Jan 2023 16:55:53 +0100 Subject: [PATCH 8/8] s390/uaccess: avoid __ashlti3() call __cmpxchg_user_key() uses 128 bit types which, depending on compiler and config options, may lead to an __ashlti3() library call. Get rid of that by simply casting the 128 bit values to 32 bit values. Reported-by: kernel test robot Suggested-by: Janis Schoetterl-Glausch Fixes: 51098f0eb22e ("s390/cmpxchg: make loop condition for 1,2 byte cases precise") Link: https://lore.kernel.org/all/4b96b112d5415d08a81d30657feec2c8c3000f7c.camel@linux.ibm.com/ Signed-off-by: Heiko Carstens --- arch/s390/include/asm/uaccess.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h index 7c10f594e747..8a8c64a678c4 100644 --- a/arch/s390/include/asm/uaccess.h +++ b/arch/s390/include/asm/uaccess.h @@ -407,8 +407,8 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, shift = (3 ^ (address & 3)) << 3; address ^= address & 3; - _old = (old & 0xff) << shift; - _new = (new & 0xff) << shift; + _old = ((unsigned int)old & 0xff) << shift; + _new = ((unsigned int)new & 0xff) << shift; mask = ~(0xff << shift); asm volatile( " spka 0(%[key])\n" @@ -455,8 +455,8 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval, shift = (2 ^ (address & 2)) << 3; address ^= address & 2; - _old = (old & 0xffff) << shift; - _new = (new & 0xffff) << shift; + _old = ((unsigned int)old & 0xffff) << shift; + _new = ((unsigned int)new & 0xffff) << shift; mask = ~(0xffff << shift); asm volatile( " spka 0(%[key])\n"