Re: [PATCH v3 30/34] xen/riscv: add minimal stuff to processor.h to build full Xen

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.01.2024 11:12, Oleksii wrote:
> On Wed, 2024-01-24 at 09:19 +0100, Jan Beulich wrote:
>> On 23.01.2024 18:08, Oleksii wrote:
>>> On Tue, 2024-01-23 at 12:39 +0100, Jan Beulich wrote:
>>>> On 22.12.2023 16:13, Oleksii Kurochko wrote:
>>>>> @@ -53,6 +56,18 @@ struct cpu_user_regs
>>>>>      unsigned long pregs;
>>>>>  };
>>>>>  
>>>>> +/* TODO: need to implement */
>>>>> +#define cpu_to_core(cpu)   (0)
>>>>> +#define cpu_to_socket(cpu) (0)
>>>>> +
>>>>> +static inline void cpu_relax(void)
>>>>> +{
>>>>> +    /* Encoding of the pause instruction */
>>>>> +    __asm__ __volatile__ ( ".insn 0x100000F" );
>>>>
>>>> binutils 2.40 knows "pause" - why use .insn then?
>>> I thought that for this instruction it is needed to have extension
>>> ZIHINTPAUSE ( according to Linux Kernel source code [1] ) and to
>>> cover
>>> older version.
>>
>> Well, of course you'll need to enable the extension then for gas. But
>> as long as you use the insn unconditionally, that's all fine and
>> natural. Another thing would be if you meant to also run on systems
>> not supporting the extension: Then the above use of .insn would need
>> to become conditional anyway.
> Then it makes sense to use "pause". 
> Let's assume that for now we are running only on systems which support
> the extension until we won't face compilation issue for some system.

Gives me the impression that you still don't properly separate the two
aspects: One is what systems Xen is to run on, and other is what's
needed to make Xen build properly. The first needs documenting (and
ideally at some point actually enforcing), while the latter may require
e.g. compiler command line option adjustments.

Jan