Re: [PATCH v2 33/39] xen/riscv: add minimal stuff to asm/processor.h to build full Xen

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/processor.h
> +++ b/xen/arch/riscv/include/asm/processor.h
> @@ -12,6 +12,9 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +/* TODO: need to be implemeted */
> +#define get_processor_id() 0

Please don't re-introduce this - it was just recently dropped from the
code base.

> @@ -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)

No need for leading underscores here.

> +static inline void cpu_relax(void)
> +{
> +     int dummy;
> +     /* In lieu of a halt instruction, induce a long-latency stall. */
> +     __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));

Any reason for this, when Arm's is just barrier(), and apparently they got
away with this quite fine? Also isn't this causing a division by zero,
which I'd expect to cause some kind of exception? (Terminology-wise I'm of
course biased by x86, where "halt instruction" wouldn't be suitable to use
here. But if that terminology is fine on RISC-V, then obviously no
objection.)

Jan