Skip to content

Commit 51098f0

Browse files
Janis Schoetterl-Glauschhcahca
Janis Schoetterl-Glausch
authored and
hcahca
committed
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 <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Heiko Carstens <[email protected]>
1 parent 4148575 commit 51098f0

File tree

2 files changed

+78
-62
lines changed

2 files changed

+78
-62
lines changed

‎arch/s390/include/asm/cmpxchg.h‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -90,55 +90,63 @@ static __always_inline unsigned long __cmpxchg(unsigned long address,
9090
{
9191
switch (size) {
9292
case 1: {
93-
unsigned int prev, tmp, shift;
93+
unsigned int prev, shift, mask;
9494

9595
shift = (3 ^ (address & 3)) << 3;
9696
address ^= address & 3;
97+
old = (old & 0xff) << shift;
98+
new = (new & 0xff) << shift;
99+
mask = ~(0xff << shift);
97100
asm volatile(
98101
" l %[prev],%[address]\n"
99-
"0: nr %[prev],%[mask]\n"
100-
" lr %[tmp],%[prev]\n"
101-
" or %[prev],%[old]\n"
102-
" or %[tmp],%[new]\n"
103-
" cs %[prev],%[tmp],%[address]\n"
102+
" nr %[prev],%[mask]\n"
103+
" xilf %[mask],0xffffffff\n"
104+
" or %[new],%[prev]\n"
105+
" or %[prev],%[tmp]\n"
106+
"0: lr %[tmp],%[prev]\n"
107+
" cs %[prev],%[new],%[address]\n"
104108
" jnl 1f\n"
105109
" xr %[tmp],%[prev]\n"
110+
" xr %[new],%[tmp]\n"
106111
" nr %[tmp],%[mask]\n"
107-
" jnz 0b\n"
112+
" jz 0b\n"
108113
"1:"
109114
: [prev] "=&d" (prev),
110-
[tmp] "=&d" (tmp),
111-
[address] "+Q" (*(int *)address)
112-
: [old] "d" ((old & 0xff) << shift),
113-
[new] "d" ((new & 0xff) << shift),
114-
[mask] "d" (~(0xff << shift))
115-
: "memory", "cc");
115+
[address] "+Q" (*(int *)address),
116+
[tmp] "+&d" (old),
117+
[new] "+&d" (new),
118+
[mask] "+&d" (mask)
119+
:: "memory", "cc");
116120
return prev >> shift;
117121
}
118122
case 2: {
119-
unsigned int prev, tmp, shift;
123+
unsigned int prev, shift, mask;
120124

121125
shift = (2 ^ (address & 2)) << 3;
122126
address ^= address & 2;
127+
old = (old & 0xffff) << shift;
128+
new = (new & 0xffff) << shift;
129+
mask = ~(0xffff << shift);
123130
asm volatile(
124131
" l %[prev],%[address]\n"
125-
"0: nr %[prev],%[mask]\n"
126-
" lr %[tmp],%[prev]\n"
127-
" or %[prev],%[old]\n"
128-
" or %[tmp],%[new]\n"
129-
" cs %[prev],%[tmp],%[address]\n"
132+
" nr %[prev],%[mask]\n"
133+
" xilf %[mask],0xffffffff\n"
134+
" or %[new],%[prev]\n"
135+
" or %[prev],%[tmp]\n"
136+
"0: lr %[tmp],%[prev]\n"
137+
" cs %[prev],%[new],%[address]\n"
130138
" jnl 1f\n"
131139
" xr %[tmp],%[prev]\n"
140+
" xr %[new],%[tmp]\n"
132141
" nr %[tmp],%[mask]\n"
133-
" jnz 0b\n"
142+
" jz 0b\n"
134143
"1:"
135144
: [prev] "=&d" (prev),
136-
[tmp] "=&d" (tmp),
137-
[address] "+Q" (*(int *)address)
138-
: [old] "d" ((old & 0xffff) << shift),
139-
[new] "d" ((new & 0xffff) << shift),
140-
[mask] "d" (~(0xffff << shift))
141-
: "memory", "cc");
145+
[address] "+Q" (*(int *)address),
146+
[tmp] "+&d" (old),
147+
[new] "+&d" (new),
148+
[mask] "+&d" (mask)
149+
:: "memory", "cc");
142150
return prev >> shift;
143151
}
144152
case 4: {

‎arch/s390/include/asm/uaccess.h‎

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -400,74 +400,82 @@ static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
400400

401401
switch (size) {
402402
case 1: {
403-
unsigned int prev, tmp, shift;
403+
unsigned int prev, shift, mask, _old, _new;
404404

405405
shift = (3 ^ (address & 3)) << 3;
406406
address ^= address & 3;
407+
_old = (old & 0xff) << shift;
408+
_new = (new & 0xff) << shift;
409+
mask = ~(0xff << shift);
407410
asm volatile(
408411
" spka 0(%[key])\n"
409412
" sacf 256\n"
410413
"0: l %[prev],%[address]\n"
411414
"1: nr %[prev],%[mask]\n"
412-
" lr %[tmp],%[prev]\n"
413-
" or %[prev],%[old]\n"
414-
" or %[tmp],%[new]\n"
415-
"2: cs %[prev],%[tmp],%[address]\n"
416-
"3: jnl 4f\n"
415+
" xilf %[mask],0xffffffff\n"
416+
" or %[new],%[prev]\n"
417+
" or %[prev],%[tmp]\n"
418+
"2: lr %[tmp],%[prev]\n"
419+
"3: cs %[prev],%[new],%[address]\n"
420+
"4: jnl 5f\n"
417421
" xr %[tmp],%[prev]\n"
422+
" xr %[new],%[tmp]\n"
418423
" nr %[tmp],%[mask]\n"
419-
" jnz 1b\n"
420-
"4: sacf 768\n"
424+
" jz 2b\n"
425+
"5: sacf 768\n"
421426
" spka %[default_key]\n"
422-
EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev])
423-
EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev])
424-
EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev])
425-
EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev])
427+
EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
428+
EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
429+
EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
430+
EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
426431
: [rc] "+&d" (rc),
427432
[prev] "=&d" (prev),
428-
[tmp] "=&d" (tmp),
429-
[address] "+Q" (*(int *)address)
430-
: [old] "d" (((unsigned int)old & 0xff) << shift),
431-
[new] "d" (((unsigned int)new & 0xff) << shift),
432-
[mask] "d" (~(0xff << shift)),
433-
[key] "a" (key << 4),
433+
[address] "+Q" (*(int *)address),
434+
[tmp] "+&d" (_old),
435+
[new] "+&d" (_new),
436+
[mask] "+&d" (mask)
437+
: [key] "a" (key << 4),
434438
[default_key] "J" (PAGE_DEFAULT_KEY)
435439
: "memory", "cc");
436440
*(unsigned char *)uval = prev >> shift;
437441
return rc;
438442
}
439443
case 2: {
440-
unsigned int prev, tmp, shift;
444+
unsigned int prev, shift, mask, _old, _new;
441445

442446
shift = (2 ^ (address & 2)) << 3;
443447
address ^= address & 2;
448+
_old = (old & 0xffff) << shift;
449+
_new = (new & 0xffff) << shift;
450+
mask = ~(0xffff << shift);
444451
asm volatile(
445452
" spka 0(%[key])\n"
446453
" sacf 256\n"
447454
"0: l %[prev],%[address]\n"
448455
"1: nr %[prev],%[mask]\n"
449-
" lr %[tmp],%[prev]\n"
450-
" or %[prev],%[old]\n"
451-
" or %[tmp],%[new]\n"
452-
"2: cs %[prev],%[tmp],%[address]\n"
453-
"3: jnl 4f\n"
456+
" xilf %[mask],0xffffffff\n"
457+
" or %[new],%[prev]\n"
458+
" or %[prev],%[tmp]\n"
459+
"2: lr %[tmp],%[prev]\n"
460+
"3: cs %[prev],%[new],%[address]\n"
461+
"4: jnl 5f\n"
454462
" xr %[tmp],%[prev]\n"
463+
" xr %[new],%[tmp]\n"
455464
" nr %[tmp],%[mask]\n"
456-
" jnz 1b\n"
457-
"4: sacf 768\n"
465+
" jz 2b\n"
466+
"5: sacf 768\n"
458467
" spka %[default_key]\n"
459-
EX_TABLE_UA_LOAD_REG(0b, 4b, %[rc], %[prev])
460-
EX_TABLE_UA_LOAD_REG(1b, 4b, %[rc], %[prev])
461-
EX_TABLE_UA_LOAD_REG(2b, 4b, %[rc], %[prev])
462-
EX_TABLE_UA_LOAD_REG(3b, 4b, %[rc], %[prev])
468+
EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
469+
EX_TABLE_UA_LOAD_REG(1b, 5b, %[rc], %[prev])
470+
EX_TABLE_UA_LOAD_REG(3b, 5b, %[rc], %[prev])
471+
EX_TABLE_UA_LOAD_REG(4b, 5b, %[rc], %[prev])
463472
: [rc] "+&d" (rc),
464473
[prev] "=&d" (prev),
465-
[tmp] "=&d" (tmp),
466-
[address] "+Q" (*(int *)address)
467-
: [old] "d" (((unsigned int)old & 0xffff) << shift),
468-
[new] "d" (((unsigned int)new & 0xffff) << shift),
469-
[mask] "d" (~(0xffff << shift)),
470-
[key] "a" (key << 4),
474+
[address] "+Q" (*(int *)address),
475+
[tmp] "+&d" (_old),
476+
[new] "+&d" (_new),
477+
[mask] "+&d" (mask)
478+
: [key] "a" (key << 4),
471479
[default_key] "J" (PAGE_DEFAULT_KEY)
472480
: "memory", "cc");
473481
*(unsigned short *)uval = prev >> shift;

0 commit comments

Comments
 (0)