[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 13/39] xen/riscv: introduce asm/system.h
On 24.11.2023 11:30, Oleksii Kurochko wrote: > --- /dev/null > +++ b/xen/arch/riscv/include/asm/system.h > @@ -0,0 +1,79 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef _ASM_RISCV_BARRIER_H > +#define _ASM_RISCV_BARRIER_H > + > +#include <asm/csr.h> > + > +#ifndef __ASSEMBLY__ > + > +#define RISCV_FENCE(p, s) \ > + __asm__ __volatile__ ("fence " #p "," #s : : : "memory") Nit (style): Missing blanks immediately inside the parentheses. > +/* These barriers need to enforce ordering on both devices or memory. */ > +#define mb() RISCV_FENCE(iorw,iorw) > +#define rmb() RISCV_FENCE(ir,ir) > +#define wmb() RISCV_FENCE(ow,ow) Nit (style): Missing blanks after the commas (also again below). > +/* These barriers do not need to enforce ordering on devices, just memory. */ > +#define smp_mb() RISCV_FENCE(rw,rw) > +#define smp_rmb() RISCV_FENCE(r,r) > +#define smp_wmb() RISCV_FENCE(w,w) > +#define smp_mb__before_atomic() smp_mb() > +#define smp_mb__after_atomic() smp_mb() > + > +/* > +#define __smp_store_release(p, v) \ Is there a need for the double underscores here? We try to not introduce new instances of undue leading underscores, but there might be e.g. a strong desire to stay in sync with, say, Linux. > +do { \ > + compiletime_assert_atomic_type(*p); \ > + RISCV_FENCE(rw,w); \ > + WRITE_ONCE(*p, v); \ Nit: Can the trailing backslashes be aligned, please? > +} while (0) > + > +#define __smp_load_acquire(p) \ > +({ \ > + typeof(*p) ___p1 = READ_ONCE(*p); \ Hmm, yet more leading underscores, and here surely not needed. > + compiletime_assert_atomic_type(*p); \ > + RISCV_FENCE(r,rw); \ > + ___p1; \ > +}) > +*/ > + > +static inline unsigned long local_save_flags(void) > +{ > + return csr_read(sstatus); > +} > + > +static inline void local_irq_enable(void) > +{ > + csr_set(sstatus, SSTATUS_SIE); > +} > + > +static inline void local_irq_disable(void) > +{ > + csr_clear(sstatus, SSTATUS_SIE); > +} > + > +#define local_irq_save(x) \ > +({ \ > + x = csr_read_clear(CSR_SSTATUS, SSTATUS_SIE); \ > + local_irq_disable(); \ > +}) > + > +static inline void local_irq_restore(unsigned long flags) > +{ > + csr_set(CSR_SSTATUS, flags & SSTATUS_SIE); > +} > + > +static inline int local_irq_is_enabled(void) > +{ > + unsigned long flags = local_save_flags(); > + > + return flags & SSTATUS_SIE; SSTATUS_SIE doesn't even happen to be 1, so I think you're better off adding != 0, unless you would do as I think I had suggested before and have the function return bool right away. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |