Re: [PATCH v3 2/2] rseq/selftests: Add support for RISC-V

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Vincent Chen <vincent.chen@sifive.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	 Paul Walmsley <paul.walmsley@sifive.com>
Subject: Re: [PATCH v3 2/2] rseq/selftests: Add support for RISC-V
Date: Mon, 7 Mar 2022 08:40:26 -0500 (EST)	[thread overview]
Message-ID: <1732504666.125429.1646660426220.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CABvJ_xjStvTRF3LDraLADaBY01aUgsjEv6aArFgDv3uh9V9wXQ@mail.gmail.com>

----- Vincent Chen <vincent.chen@sifive.com> wrote:
> On Fri, Mar 4, 2022 at 2:50 PM Vincent Chen <vincent.chen@sifive.com> wrote:
> >
> > On Fri, Mar 4, 2022 at 5:50 AM Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> > >
> > > ----- On Mar 3, 2022, at 2:16 AM, Vincent Chen vincent.chen@sifive.com wrote:
> > >
> > > > On Thu, Mar 3, 2022 at 12:38 AM Mathieu Desnoyers
> > > > <mathieu.desnoyers@efficios.com> wrote:
> > > >>
> > > [...]
> > >
> > > >> > +#if __ORDER_LITTLE_ENDIAN__ == 1234
> > > >>
> > > >> I think we'll want to standardize on this for endianness checking (same as
> > > >> the updated uapi rseq.h):
> > > >>
> > > >> #if defined(__BYTE_ORDER) ? (__BYTE_ORDER == __LITTLE_ENDIAN) :
> > > >> defined(__LITTLE_ENDIAN)
> > > >>
> > > >> We may have to change rseq-mips.h in the rseq selftests to do the same as well
> > > >> rather than
> > > >> using "# ifdef __BIG_ENDIAN".
> > > >>
> > > >
> > > > OK, I can follow it. However, I found the endianness checking in
> > > > include/uapi/linux/rseq.h is
> > > > #if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) ||
> > > > defined(__BIG_ENDIAN)
> > > >
> > > > It is a little different than what you mentioned early. Should I
> > > > follow the format in include/uapi/linux/rseq.h? or both formats are
> > > > OK?
> > > >
> > >
> > > The form "#if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || defined(__BIG_ENDIAN)
> > > is completely buggy. Don't use that.
> > >
> > > I've sent the fix to Peter to remove the offending #if here:
> > >
> > > https://lore.kernel.org/lkml/1445357149.71067.1643137248305.JavaMail.zimbra@efficios.com/T/#mbbad3961494feefb98cb6d092879e3ea41b33df8
> > >
> > > Please use the correct non-bogus form instead:
> > >
> > > #if defined(__BYTE_ORDER) ? (__BYTE_ORDER == __LITTLE_ENDIAN) : defined(__LITTLE_ENDIAN)
> > >
> > > Thanks,
> > >
> > > Mathieu
> > >
> >
> > I understood. By the way, IIUC, the __BYTE_ORDER and __LITTLE_ENDIAN
> > are defined in the glibc endian.h. However, I find that no file in the
> > rseq folder includes the endian.h. Does it means I should include
> > <endian.h> in the rseq-riscv.h?
> 
> After tracing the hierarchy of include files, I found that endian.h is
> included by pthread.h, and pthread.h is included by param_test.c.
> Therefore, rseq-riscv.h can exclude endian.h. I will follow your
> suggestions to modify my next version patch. Thank you.

You should include what you use. Please include endian.h from the riscv header as well.

Thanks,

Mathieu

> 
> >
> > Thanks,
> > Vincent
> > >
> > > --
> > > Mathieu Desnoyers
> > > EfficiOS Inc.
> > > http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv