Re: [PATCH v3 15/34] xen/riscv: introduce atomic.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.01.2024 13:24, Oleksii wrote:
> On Tue, 2024-01-23 at 11:30 +0100, Jan Beulich wrote:
>> On 23.01.2024 11:21, Oleksii wrote:
>>> On Mon, 2024-01-22 at 17:56 +0100, Jan Beulich wrote:
>>>> On 22.12.2023 16:12, Oleksii Kurochko wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/riscv/include/asm/fence.h
>>>>> @@ -0,0 +1,13 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>>> +#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
>>>>
>>>> Do you really care about the !SMP case? On x86 at least we
>>>> stopped
>>>> special-
>>>> casing that configuration many years ago (the few cases where for
>>>> typically
>>>> build reasons it matters, using CONFIG_NR_CPUS is sufficient). If
>>>> you
>>>> care
>>>> about it, there needs to be somewhere you actually #define
>>>> CONFIG_SMP.
>>> Can't we use instead of CONFIG_SMP - CONFIG_NR_CPUS?
>>
>> You can. Question is whether there's a point in doing so. Do you
>> expect people to actually want to run Xen on single-CPU systems?
>> They're generally not overly well suited for virtualization ...
> Just to clarify.
> 
> Do you mean physically single based CPU?
> Then I don't expect to run Xen on such systems and it is not nesessary
> to define *_BARRIER in this case. Should we have to add build error
> notification that we don't support single-CPU systems in this header?
> 
> If you are speaking about we have ,let it be, 4 CPUs and only 1 CPU is
> currently supported by Xen then it still makes sense.

No, that's still not what I mean. The question is: Is it useful for you
to _special case_ the NR_CPUS=1 case? Or is it instead simpler to handle
NR_CPUS=1 the same as NR_CPUS>1 (accepting less than ideal performance,
on the basis that in reality nobody's expected to use such in production
anyway)?

Jan