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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2024-01-24 at 12:27 +0100, Jan Beulich wrote:
> 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.
I understand that it will be required update "-march=..._zihintpause"
and it should be a check that this extension is supported by a
toolchain.

But I am not sure that I know how can I enforce that a system should
have this extension, and considering Linux kernel implementation which
uses always pause instruction, it looks like all available systems
support this extension.

But I agree what I wrote above isn't fully correct.

~ Oleksii