Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Julien Grall <julien@xxxxxxx>]

Hi,

On 30/01/2023 11:40, Oleksii wrote:
> On Fri, 2023-01-27 at 14:54 +0000, Julien Grall wrote:
> > Hi Oleksii,
> >
> > On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > > +static inline void wfi(void)
> > > +{
> > > +    __asm__ __volatile__ ("wfi");
> > I have starred at this line for a while and I am not quite too sure
> > to
> > understand why we don't need to clobber the memory like we do on Arm.
> I don't have an answer. The code was based on Linux so...
Hmmm ok. It would probably wise to understand how code imported from 
Linux work so we don't end up introducing bug when calling such function.
From your current use in this patch, I don't expect any issue. That may 
chance for the others use.
> > FWIW, Linux is doing the same, so I guess this is correct. For Arm we
> > also follow the Linux implementation.
> >
> > But I am wondering whether we are just too strict on Arm, RISCv
> > compiler
> > offer a different guarantee, or you expect the user to be responsible
> > to
> > prevent the compiler to do harmful optimization.
> >
> > > +/*
> > > + * panic() isn't available at the moment so an infinite loop will
> > > be
> > > + * used temporarily.
> > > + * TODO: change it to panic()
> > > + */
> > > +static inline void die(void)
> > > +{
> > > +    for( ;; ) wfi();
> > Please move wfi() to a newline.
> Thanks.
>
> I thought that it is fine to put into one line in this case but I'll
> move it to a newline. It's fine.
I am not aware of any place in Xen where we would combine the lines.
Also, you want a space after 'for'.

Cheers,

--
Julien Grall