Re: [PATCH v2 06/39] xen/riscv: introduce fence.h

2023-12-07 Thread Jan Beulich
On 07.12.2023 10:42, Oleksii wrote:
> On Tue, 2023-12-05 at 16:56 +0100, Jan Beulich wrote:
>> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/fence.h
>>> @@ -0,0 +1,12 @@
>>> +#ifndef _ASM_RISCV_FENCE_H
>>> +#define _ASM_RISCV_FENCE_H
>>> +
>>> +#ifdef CONFIG_SMP
>>> +#define RISCV_ACQUIRE_BARRIER  "\tfence r , rw\n"
>>> +#define RISCV_RELEASE_BARRIER  "\tfence rw,  w\n"
>>> +#else
>>> +#define RISCV_ACQUIRE_BARRIER
>>> +#define RISCV_RELEASE_BARRIER
>>> +#endif
>>> +
>>> +#endif /* _ASM_RISCV_FENCE_H */
>>
>> Imo such a header would be better to introduce once a use for the
>> constructs appears. Otherwise at the very least it wants explaining
>> in the description what this is going to be needed for. I can't
>> find items of these names in other architectures so far, so this
>> must be something RISC-V-specific.
> It is going to be used only in RISC-V. The things that use these
> definitions are introduced in the patches of this patch series:
> * [PATCH v2 18/39] xen/riscv: introduce cmpxchg.h
> * [PATCH v2 17/39] xen/riscv: introduce asm/atomic.h

Then perhaps fold this patch into patch 17?

Jan



Re: [PATCH v2 06/39] xen/riscv: introduce fence.h

2023-12-07 Thread Oleksii
On Tue, 2023-12-05 at 16:56 +0100, Jan Beulich wrote:
> On 24.11.2023 11:30, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/fence.h
> > @@ -0,0 +1,12 @@
> > +#ifndef _ASM_RISCV_FENCE_H
> > +#define _ASM_RISCV_FENCE_H
> > +
> > +#ifdef CONFIG_SMP
> > +#define RISCV_ACQUIRE_BARRIER  "\tfence r , rw\n"
> > +#define RISCV_RELEASE_BARRIER  "\tfence rw,  w\n"
> > +#else
> > +#define RISCV_ACQUIRE_BARRIER
> > +#define RISCV_RELEASE_BARRIER
> > +#endif
> > +
> > +#endif /* _ASM_RISCV_FENCE_H */
> 
> Imo such a header would be better to introduce once a use for the
> constructs appears. Otherwise at the very least it wants explaining
> in the description what this is going to be needed for. I can't
> find items of these names in other architectures so far, so this
> must be something RISC-V-specific.
It is going to be used only in RISC-V. The things that use these
definitions are introduced in the patches of this patch series:
* [PATCH v2 18/39] xen/riscv: introduce cmpxchg.h
* [PATCH v2 17/39] xen/riscv: introduce asm/atomic.h

~ Oleksii





Re: [PATCH v2 06/39] xen/riscv: introduce fence.h

2023-12-05 Thread Jan Beulich
On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/fence.h
> @@ -0,0 +1,12 @@
> +#ifndef _ASM_RISCV_FENCE_H
> +#define _ASM_RISCV_FENCE_H
> +
> +#ifdef CONFIG_SMP
> +#define RISCV_ACQUIRE_BARRIER"\tfence r , rw\n"
> +#define RISCV_RELEASE_BARRIER"\tfence rw,  w\n"
> +#else
> +#define RISCV_ACQUIRE_BARRIER
> +#define RISCV_RELEASE_BARRIER
> +#endif
> +
> +#endif   /* _ASM_RISCV_FENCE_H */

Imo such a header would be better to introduce once a use for the
constructs appears. Otherwise at the very least it wants explaining
in the description what this is going to be needed for. I can't
find items of these names in other architectures so far, so this
must be something RISC-V-specific.

Jan