Hi,
On 14/07/2023 12:49, Nicola Vetrini wrote:
The macro 'testop' expands to a function that declares the local
variable 'oldbit', which is written before being set, but is such a
way that is not amenable to automatic checking.
Therefore, a deviation comment, is introduced to document this situation.
A similar reasoning applies to macro 'guest_testop'.
Would you be able to check if the code below (only compile tested so
far) would silence Eclair?
diff --git a/xen/arch/arm/arm64/lib/bitops.c
b/xen/arch/arm/arm64/lib/bitops.c
index 20e3f3d6ceaf..f7a10b790f2a 100644
--- a/xen/arch/arm/arm64/lib/bitops.c
+++ b/xen/arch/arm/arm64/lib/bitops.c
@@ -64,13 +64,15 @@ bool name##_timeout(int nr, volatile void *p,
unsigned int max_try) \
}
#define testop(name, instr)
\
-static always_inline bool int_##name(int nr, volatile void *p, int
*oldbit, \
- bool timeout, unsigned int
max_try) \
+static always_inline int int_##name(int nr, volatile void *p,
\
+ bool allow_timeout, bool *timeout,
\
+ unsigned int max_try)
\
{
\
volatile uint32_t *ptr = (uint32_t *)p + BITOP_WORD((unsigned
int)nr); \
unsigned int bit = (unsigned int)nr % BITOP_BITS_PER_WORD;
\
const uint32_t mask = BITOP_MASK(bit);
\
unsigned long res, tmp;
\
+ int oldbit;
\
\
do
\
{
\
@@ -79,27 +81,30 @@ static always_inline bool int_##name(int nr,
volatile void *p, int *oldbit, \
" lsr %w1, %w3, %w5 // Save old value of bit\n"
\
" " __stringify(instr) " %w3, %w3, %w4 // Toggle bit\n"
\
" stlxr %w0, %w3, %2\n"
\
- : "=&r" (res), "=&r" (*oldbit), "+Q" (*ptr), "=&r" (tmp)
\
+ : "=&r" (res), "=&r" (oldbit), "+Q" (*ptr), "=&r" (tmp)
\
: "r" (mask), "r" (bit)
\
: "memory");
\
\
if ( !res )
\
break;
\
- } while ( !timeout || ((--max_try) > 0) );
\
+ } while ( !allow_timeout || ((--max_try) > 0) );
\
\
dmb(ish);
\
\
- *oldbit &= 1;
\
+ ASSERT(!allow_timeout || timeout);
\
+ if ( timeout )
\
+ *timeout = !res;
\
+ else if ( !res )
\
+ ASSERT_UNREACHABLE();
\
\
- return !res;
\
+ return (oldbit & 1);
\
}
\
\
int name(int nr, volatile void *p)
\
{
\
int oldbit;
\
\
- if ( !int_##name(nr, p, &oldbit, false, 0) )
\
- ASSERT_UNREACHABLE();
\
+ oldbit = int_##name(nr, p, false, NULL, 0);
\
\
Cheers,
--
Julien Grall
