| Message ID | 20240801033725.28816-3-samuel.holland@sifive.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 77514915b72c51ebc1c30a67a54d4a90ca2a4a39 |
| Headers | show |
| Series | riscv: Improve KASAN coverage to fix unit tests | expand |
On 01/08/2024 05:37, Samuel Holland wrote: > Instead of implementing the bitops functions directly in assembly, > provide the arch_-prefixed versions and use the wrappers from > asm-generic to add instrumentation. This improves KASAN coverage and > fixes the kasan_bitops_generic() unit test. > > Signed-off-by: Samuel Holland <samuel.holland@sifive.com> > --- > > arch/riscv/include/asm/bitops.h | 43 ++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 20 deletions(-) > > diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h > index 71af9ecfcfcb..fae152ea0508 100644 > --- a/arch/riscv/include/asm/bitops.h > +++ b/arch/riscv/include/asm/bitops.h > @@ -222,44 +222,44 @@ static __always_inline int variable_fls(unsigned int x) > #define __NOT(x) (~(x)) > > /** > - * test_and_set_bit - Set a bit and return its old value > + * arch_test_and_set_bit - Set a bit and return its old value > * @nr: Bit to set > * @addr: Address to count from > * > * This operation may be reordered on other architectures than x86. > */ > -static inline int test_and_set_bit(int nr, volatile unsigned long *addr) > +static inline int arch_test_and_set_bit(int nr, volatile unsigned long *addr) > { > return __test_and_op_bit(or, __NOP, nr, addr); > } > > /** > - * test_and_clear_bit - Clear a bit and return its old value > + * arch_test_and_clear_bit - Clear a bit and return its old value > * @nr: Bit to clear > * @addr: Address to count from > * > * This operation can be reordered on other architectures other than x86. > */ > -static inline int test_and_clear_bit(int nr, volatile unsigned long *addr) > +static inline int arch_test_and_clear_bit(int nr, volatile unsigned long *addr) > { > return __test_and_op_bit(and, __NOT, nr, addr); > } > > /** > - * test_and_change_bit - Change a bit and return its old value > + * arch_test_and_change_bit - Change a bit and return its old value > * @nr: Bit to change > * @addr: Address to count from > * > * This operation is atomic and cannot be reordered. > * It also implies a memory barrier. > */ > -static inline int test_and_change_bit(int nr, volatile unsigned long *addr) > +static inline int arch_test_and_change_bit(int nr, volatile unsigned long *addr) > { > return __test_and_op_bit(xor, __NOP, nr, addr); > } > > /** > - * set_bit - Atomically set a bit in memory > + * arch_set_bit - Atomically set a bit in memory > * @nr: the bit to set > * @addr: the address to start counting from > * > @@ -270,13 +270,13 @@ static inline int test_and_change_bit(int nr, volatile unsigned long *addr) > * Note that @nr may be almost arbitrarily large; this function is not > * restricted to acting on a single-word quantity. > */ > -static inline void set_bit(int nr, volatile unsigned long *addr) > +static inline void arch_set_bit(int nr, volatile unsigned long *addr) > { > __op_bit(or, __NOP, nr, addr); > } > > /** > - * clear_bit - Clears a bit in memory > + * arch_clear_bit - Clears a bit in memory > * @nr: Bit to clear > * @addr: Address to start counting from > * > @@ -284,13 +284,13 @@ static inline void set_bit(int nr, volatile unsigned long *addr) > * on non x86 architectures, so if you are writing portable code, > * make sure not to rely on its reordering guarantees. > */ > -static inline void clear_bit(int nr, volatile unsigned long *addr) > +static inline void arch_clear_bit(int nr, volatile unsigned long *addr) > { > __op_bit(and, __NOT, nr, addr); > } > > /** > - * change_bit - Toggle a bit in memory > + * arch_change_bit - Toggle a bit in memory > * @nr: Bit to change > * @addr: Address to start counting from > * > @@ -298,40 +298,40 @@ static inline void clear_bit(int nr, volatile unsigned long *addr) > * Note that @nr may be almost arbitrarily large; this function is not > * restricted to acting on a single-word quantity. > */ > -static inline void change_bit(int nr, volatile unsigned long *addr) > +static inline void arch_change_bit(int nr, volatile unsigned long *addr) > { > __op_bit(xor, __NOP, nr, addr); > } > > /** > - * test_and_set_bit_lock - Set a bit and return its old value, for lock > + * arch_test_and_set_bit_lock - Set a bit and return its old value, for lock > * @nr: Bit to set > * @addr: Address to count from > * > * This operation is atomic and provides acquire barrier semantics. > * It can be used to implement bit locks. > */ > -static inline int test_and_set_bit_lock( > +static inline int arch_test_and_set_bit_lock( > unsigned long nr, volatile unsigned long *addr) > { > return __test_and_op_bit_ord(or, __NOP, nr, addr, .aq); > } > > /** > - * clear_bit_unlock - Clear a bit in memory, for unlock > + * arch_clear_bit_unlock - Clear a bit in memory, for unlock > * @nr: the bit to set > * @addr: the address to start counting from > * > * This operation is atomic and provides release barrier semantics. > */ > -static inline void clear_bit_unlock( > +static inline void arch_clear_bit_unlock( > unsigned long nr, volatile unsigned long *addr) > { > __op_bit_ord(and, __NOT, nr, addr, .rl); > } > > /** > - * __clear_bit_unlock - Clear a bit in memory, for unlock > + * arch___clear_bit_unlock - Clear a bit in memory, for unlock > * @nr: the bit to set > * @addr: the address to start counting from > * Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex > @@ -345,13 +345,13 @@ static inline void clear_bit_unlock( > * non-atomic property here: it's a lot more instructions and we still have to > * provide release semantics anyway. > */ > -static inline void __clear_bit_unlock( > +static inline void arch___clear_bit_unlock( > unsigned long nr, volatile unsigned long *addr) > { > - clear_bit_unlock(nr, addr); > + arch_clear_bit_unlock(nr, addr); > } > > -static inline bool xor_unlock_is_negative_byte(unsigned long mask, > +static inline bool arch_xor_unlock_is_negative_byte(unsigned long mask, > volatile unsigned long *addr) > { > unsigned long res; > @@ -369,6 +369,9 @@ static inline bool xor_unlock_is_negative_byte(unsigned long mask, > #undef __NOT > #undef __AMO > > +#include <asm-generic/bitops/instrumented-atomic.h> > +#include <asm-generic/bitops/instrumented-lock.h> > + > #include <asm-generic/bitops/non-atomic.h> > #include <asm-generic/bitops/le.h> > #include <asm-generic/bitops/ext2-atomic.h>
diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h index 71af9ecfcfcb..fae152ea0508 100644 --- a/arch/riscv/include/asm/bitops.h +++ b/arch/riscv/include/asm/bitops.h @@ -222,44 +222,44 @@ static __always_inline int variable_fls(unsigned int x) #define __NOT(x) (~(x)) /** - * test_and_set_bit - Set a bit and return its old value + * arch_test_and_set_bit - Set a bit and return its old value * @nr: Bit to set * @addr: Address to count from * * This operation may be reordered on other architectures than x86. */ -static inline int test_and_set_bit(int nr, volatile unsigned long *addr) +static inline int arch_test_and_set_bit(int nr, volatile unsigned long *addr) { return __test_and_op_bit(or, __NOP, nr, addr); } /** - * test_and_clear_bit - Clear a bit and return its old value + * arch_test_and_clear_bit - Clear a bit and return its old value * @nr: Bit to clear * @addr: Address to count from * * This operation can be reordered on other architectures other than x86. */ -static inline int test_and_clear_bit(int nr, volatile unsigned long *addr) +static inline int arch_test_and_clear_bit(int nr, volatile unsigned long *addr) { return __test_and_op_bit(and, __NOT, nr, addr); } /** - * test_and_change_bit - Change a bit and return its old value + * arch_test_and_change_bit - Change a bit and return its old value * @nr: Bit to change * @addr: Address to count from * * This operation is atomic and cannot be reordered. * It also implies a memory barrier. */ -static inline int test_and_change_bit(int nr, volatile unsigned long *addr) +static inline int arch_test_and_change_bit(int nr, volatile unsigned long *addr) { return __test_and_op_bit(xor, __NOP, nr, addr); } /** - * set_bit - Atomically set a bit in memory + * arch_set_bit - Atomically set a bit in memory * @nr: the bit to set * @addr: the address to start counting from * @@ -270,13 +270,13 @@ static inline int test_and_change_bit(int nr, volatile unsigned long *addr) * Note that @nr may be almost arbitrarily large; this function is not * restricted to acting on a single-word quantity. */ -static inline void set_bit(int nr, volatile unsigned long *addr) +static inline void arch_set_bit(int nr, volatile unsigned long *addr) { __op_bit(or, __NOP, nr, addr); } /** - * clear_bit - Clears a bit in memory + * arch_clear_bit - Clears a bit in memory * @nr: Bit to clear * @addr: Address to start counting from * @@ -284,13 +284,13 @@ static inline void set_bit(int nr, volatile unsigned long *addr) * on non x86 architectures, so if you are writing portable code, * make sure not to rely on its reordering guarantees. */ -static inline void clear_bit(int nr, volatile unsigned long *addr) +static inline void arch_clear_bit(int nr, volatile unsigned long *addr) { __op_bit(and, __NOT, nr, addr); } /** - * change_bit - Toggle a bit in memory + * arch_change_bit - Toggle a bit in memory * @nr: Bit to change * @addr: Address to start counting from * @@ -298,40 +298,40 @@ static inline void clear_bit(int nr, volatile unsigned long *addr) * Note that @nr may be almost arbitrarily large; this function is not * restricted to acting on a single-word quantity. */ -static inline void change_bit(int nr, volatile unsigned long *addr) +static inline void arch_change_bit(int nr, volatile unsigned long *addr) { __op_bit(xor, __NOP, nr, addr); } /** - * test_and_set_bit_lock - Set a bit and return its old value, for lock + * arch_test_and_set_bit_lock - Set a bit and return its old value, for lock * @nr: Bit to set * @addr: Address to count from * * This operation is atomic and provides acquire barrier semantics. * It can be used to implement bit locks. */ -static inline int test_and_set_bit_lock( +static inline int arch_test_and_set_bit_lock( unsigned long nr, volatile unsigned long *addr) { return __test_and_op_bit_ord(or, __NOP, nr, addr, .aq); } /** - * clear_bit_unlock - Clear a bit in memory, for unlock + * arch_clear_bit_unlock - Clear a bit in memory, for unlock * @nr: the bit to set * @addr: the address to start counting from * * This operation is atomic and provides release barrier semantics. */ -static inline void clear_bit_unlock( +static inline void arch_clear_bit_unlock( unsigned long nr, volatile unsigned long *addr) { __op_bit_ord(and, __NOT, nr, addr, .rl); } /** - * __clear_bit_unlock - Clear a bit in memory, for unlock + * arch___clear_bit_unlock - Clear a bit in memory, for unlock * @nr: the bit to set * @addr: the address to start counting from * @@ -345,13 +345,13 @@ static inline void clear_bit_unlock( * non-atomic property here: it's a lot more instructions and we still have to * provide release semantics anyway. */ -static inline void __clear_bit_unlock( +static inline void arch___clear_bit_unlock( unsigned long nr, volatile unsigned long *addr) { - clear_bit_unlock(nr, addr); + arch_clear_bit_unlock(nr, addr); } -static inline bool xor_unlock_is_negative_byte(unsigned long mask, +static inline bool arch_xor_unlock_is_negative_byte(unsigned long mask, volatile unsigned long *addr) { unsigned long res; @@ -369,6 +369,9 @@ static inline bool xor_unlock_is_negative_byte(unsigned long mask, #undef __NOT #undef __AMO +#include <asm-generic/bitops/instrumented-atomic.h> +#include <asm-generic/bitops/instrumented-lock.h> + #include <asm-generic/bitops/non-atomic.h> #include <asm-generic/bitops/le.h> #include <asm-generic/bitops/ext2-atomic.h>
Instead of implementing the bitops functions directly in assembly, provide the arch_-prefixed versions and use the wrappers from asm-generic to add instrumentation. This improves KASAN coverage and fixes the kasan_bitops_generic() unit test. Signed-off-by: Samuel Holland <samuel.holland@sifive.com> --- arch/riscv/include/asm/bitops.h | 43 ++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 20 deletions(-)