Hi Julien,
On Sun, 2024-02-18 at 19:22 +0000, Julien Grall wrote:
> Hi,
>
> On 05/02/2024 15:32, Oleksii Kurochko wrote:
> > From: Bobby Eshleman <bobbyeshle...@gmail.com>
> >
> > Additionally, this patch introduces macros in fence.h,
> > which are utilized in atomic.h.
> >
> > atomic##prefix##_*xchg_*(atomic##prefix##_t *v, c_t n)
> > were updated to use __*xchg_generic().
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com>
>
> The author is Bobby, but I don't see a Signed-off-by. Did you forgot
> it?
I missed to add that as I thought that it would be enough to change a
commit author.
>
> > ---
> > Changes in V4:
> > - do changes related to the updates of [PATCH v3 13/34]
> > xen/riscv: introduce cmpxchg.h
> > - drop casts in read_atomic_size(), write_atomic(), add_sized()
> > - tabs -> spaces
> > - drop #ifdef CONFIG_SMP ... #endif in fence.ha as it is simpler
> > to handle NR_CPUS=1
> > the same as NR_CPUS>1 with accepting less than ideal
> > performance.
> > ---
> > Changes in V3:
> > - update the commit message
> > - add SPDX for fence.h
> > - code style fixes
> > - Remove /* TODO: ... */ for add_sized macros. It looks correct
> > to me.
> > - re-order the patch
> > - merge to this patch fence.h
> > ---
> > Changes in V2:
> > - Change an author of commit. I got this header from Bobby's old
> > repo.
> > ---
> > xen/arch/riscv/include/asm/atomic.h | 395
> > ++++++++++++++++++++++++++++
> > xen/arch/riscv/include/asm/fence.h | 8 +
> > 2 files changed, 403 insertions(+)
> > create mode 100644 xen/arch/riscv/include/asm/atomic.h
> > create mode 100644 xen/arch/riscv/include/asm/fence.h
> >
> > diff --git a/xen/arch/riscv/include/asm/atomic.h
> > b/xen/arch/riscv/include/asm/atomic.h
> > new file mode 100644
> > index 0000000000..267d3c0803
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/atomic.h
> > @@ -0,0 +1,395 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Taken and modified from Linux.
>
> Which version of Linux? Can you also spell out what are the big
> changes?
> This would be helpful if we need to re-sync.
Sure, I'll add the changes here.
>
> > + *
> > + * atomic##prefix##_*xchg_*(atomic##prefix##_t *v, c_t n) were
> > updated to use
> > + * __*xchg_generic()
> > + *
> > + * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
> > + * Copyright (C) 2012 Regents of the University of California
> > + * Copyright (C) 2017 SiFive
> > + * Copyright (C) 2021 Vates SAS
> > + */
> > +
> > +#ifndef _ASM_RISCV_ATOMIC_H
> > +#define _ASM_RISCV_ATOMIC_H
> > +
> > +#include <xen/atomic.h>
> > +#include <asm/cmpxchg.h>
> > +#include <asm/fence.h>
> > +#include <asm/io.h>
> > +#include <asm/system.h>
> > +
> > +void __bad_atomic_size(void);
> > +
> > +static always_inline void read_atomic_size(const volatile void *p,
> > + void *res,
> > + unsigned int size)
> > +{
> > + switch ( size )
> > + {
> > + case 1: *(uint8_t *)res = readb(p); break;
> > + case 2: *(uint16_t *)res = readw(p); break;
> > + case 4: *(uint32_t *)res = readl(p); break;
> > + case 8: *(uint32_t *)res = readq(p); break;
> > + default: __bad_atomic_size(); break;
> > + }
> > +}
> > +
> > +#define read_atomic(p) ({ \
> > + union { typeof(*p) val; char c[0]; } x_; \
> > + read_atomic_size(p, x_.c, sizeof(*p)); \
> > + x_.val; \
> > +})
> > +
> > +#define write_atomic(p, x) \
> > +({ \
> > + typeof(*p) x__ = (x); \
> > + switch ( sizeof(*p) ) \
> > + { \
> > + case 1: writeb((uint8_t)x__, p); break; \
> > + case 2: writew((uint16_t)x__, p); break; \
> > + case 4: writel((uint32_t)x__, p); break; \
> > + case 8: writeq((uint64_t)x__, p); break; \
> > + default: __bad_atomic_size(); break; \
> > + } \
> > + x__; \
> > +})
> > +
> > +#define add_sized(p, x) \
> > +({ \
> > + typeof(*(p)) x__ = (x); \
> > + switch ( sizeof(*(p)) ) \
> > + { \
> > + case 1: writeb(read_atomic(p) + x__, p); break; \
> > + case 2: writew(read_atomic(p) + x__, p); break; \
> > + case 4: writel(read_atomic(p) + x__, p); break; \
> > + default: __bad_atomic_size(); break; \
> > + } \
> > +})
> > +
> > +/*
> > + * __unqual_scalar_typeof(x) - Declare an unqualified scalar
> > type, leaving
> > + * non-scalar types unchanged.
> > + *
> > + * Prefer C11 _Generic for better compile-times and simpler code.
> > Note: 'char'
>
> Xen is technically built using c99/gnu99. So it is feels a bit odd to
> introduce a C11 feature. I see that _Generic is already used in
> PPC...
> However, if we decide to add more use of it, then I think this should
> at
> minimum be documented in docs/misra/C-language-toolchain.rst (the
> more
> if we plan the macro is moved to common as Jan suggested).
>
> Cheers,
>
