On 18/03/2019 14:11, Andrew Cooper wrote:
>
>>> @@ -63,36 +65,38 @@ static always_inline __uint128_t cmpxchg16b_local_(
>>> * If no fault occurs then _o is updated to the value we saw at _p. If this
>>> * is the same as the initial value of _o then _n is written to location
>>> _p.
>>> */
>>> -#define __cmpxchg_user(_p,_o,_n,_isuff,_oppre,_regtype) \
>>> +#define __cmpxchg_user(_p, _o, _n, _oppre) \
>>> stac(); \
>>> asm volatile ( \
>>> - "1: lock; cmpxchg"_isuff" %"_oppre"2,%3\n" \
>>> + "1: lock cmpxchg %"_oppre"[new], %[ptr]\n" \
>>> "2:\n" \
>>> ".section .fixup,\"ax\"\n" \
>>> - "3: movl $1,%1\n" \
>>> + "3: movl $1, %[rc]\n" \
>>> " jmp 2b\n" \
>>> ".previous\n" \
>>> _ASM_EXTABLE(1b, 3b) \
>>> - : "=a" (_o), "=r" (_rc) \
>>> - : _regtype (_n), "m" (*__xg((volatile void *)_p)), "0" (_o), "1"
>>> (0) \
>>> + : "+a" (_o), [rc] "=r" (_rc), \
>>> + [ptr] "+m" (*(volatile typeof(*(_p)) *)(_p)) \
>>> + : [new] "r" (_n), "[rc]" (0) \
>> Wouldn't it further help readability a little if _rc was initialized to zero
>> right when getting declared, eliminating the last input arg here (the
>> output then would need to be "+r" of course)?
> I can do.
I've got the following incremental fix which I intend to fold in.
diff --git a/xen/include/asm-x86/x86_64/system.h
b/xen/include/asm-x86/x86_64/system.h
index 5b6e964..d65562b 100644
--- a/xen/include/asm-x86/x86_64/system.h
+++ b/xen/include/asm-x86/x86_64/system.h
@@ -65,7 +65,7 @@ static always_inline __uint128_t cmpxchg16b_local_(
* If no fault occurs then _o is updated to the value we saw at _p. If this
* is the same as the initial value of _o then _n is written to location _p.
*/
-#define __cmpxchg_user(_p, _o, _n, _oppre) \
+#define __cmpxchg_user(_p, _o, _n, _rc, _oppre) \
stac(); \
asm volatile ( \
"1: lock cmpxchg %"_oppre"[new], %[ptr]\n" \
@@ -75,28 +75,29 @@ static always_inline __uint128_t cmpxchg16b_local_(
" jmp 2b\n" \
".previous\n" \
_ASM_EXTABLE(1b, 3b) \
- : "+a" (_o), [rc] "=r" (_rc), \
+ : "+a" (_o), [rc] "+r" (_rc), \
[ptr] "+m" (*(volatile typeof(*(_p)) *)(_p)) \
- : [new] "r" (_n), "[rc]" (0) \
+ : [new] "r" (_n) \
: "memory"); \
clac()
#define cmpxchg_user(_p, _o, _n) \
({ \
- int _rc; \
+ int _rc = 0; \
+ \
switch ( sizeof(*(_p)) ) \
{ \
case 1: \
- __cmpxchg_user(_p, _o, _n, "b"); \
+ __cmpxchg_user(_p, _o, _n, _rc, "b"); \
break; \
case 2: \
- __cmpxchg_user(_p, _o, _n, "w"); \
+ __cmpxchg_user(_p, _o, _n, _rc, "w"); \
break; \
case 4: \
- __cmpxchg_user(_p, _o, _n, "k"); \
+ __cmpxchg_user(_p, _o, _n, _rc, "k"); \
break; \
case 8: \
- __cmpxchg_user(_p, _o, _n, "q"); \
+ __cmpxchg_user(_p, _o, _n, _rc, "q"); \
break; \
} \
_rc; \
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
