[PATCH 0/2] Add endianness conversion support

[PATCH 0/2] Add endianness conversion support

[Rahul Pathak]

This series adds a new header which defines the generic
endianness conversion functions.
These new functions replaces the existing byte-swapping
functions defined in libfdt_env.h

Rahul Pathak (2):
  include: Add support for byteorder/endianness conversion
  lib: utils/fdt: Use byteorder conversion functions in libfdt_env.h

 include/sbi/sbi_byteorder.h   | 57 +++++++++++++++++++++++++++++++++++
 lib/utils/libfdt/libfdt_env.h | 29 ++++++------------
 2 files changed, 67 insertions(+), 19 deletions(-)
 create mode 100644 include/sbi/sbi_byteorder.h

-- 
2.34.1

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Rahul Pathak]

Define macros for general byteorder conversion
Define functions for endianness conversion
using general byteorder conversion macros

Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
---
 include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 include/sbi/sbi_byteorder.h

diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
new file mode 100644
index 000000000000..680710fe6d91
--- /dev/null
+++ b/include/sbi/sbi_byteorder.h
@@ -0,0 +1,57 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2023 Ventana Micro Systems Inc.
+ */
+
+#ifndef __SBI_BYTEORDER_H__
+#define __SBI_BYTEORDER_H__
+
+#include <sbi/sbi_types.h>
+
+#define EXTRACT_BYTE(x, n)	((unsigned long long)((uint8_t *)&x)[n])
+
+#define BSWAP16(x)	((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
+#define BSWAP32(x)	((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
+			 (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
+#define BSWAP64(x)	((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
+			 (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
+			 (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
+			 (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
+
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
+#define cpu_to_be16(x)		((uint16_t)BSWAP16(x))
+#define cpu_to_be32(x)		((uint32_t)BSWAP32(x))
+#define cpu_to_be64(x)		((uint64_t)BSWAP64(x))
+
+#define be16_to_cpu(x)		((uint16_t)BSWAP16(x))
+#define be32_to_cpu(x)		((uint32_t)BSWAP32(x))
+#define be64_to_cpu(x)		((uint64_t)BSWAP64(x))
+
+#define cpu_to_le16(x)		((uint16_t)(x))
+#define cpu_to_le32(x)		((uint32_t)(x))
+#define cpu_to_le64(x)		((uint64_t)(x))
+
+#define le16_to_cpu(x)		((uint16_t)(x))
+#define le32_to_cpu(x)		((uint32_t)(x))
+#define le64_to_cpu(x)		((uint64_t)(x))
+#else /* CPU(big-endian) */
+#define cpu_to_be16(x)		((uint16_t)(x))
+#define cpu_to_be32(x)		((uint32_t)(x))
+#define cpu_to_be64(x)		((uint64_t)(x))
+
+#define be16_to_cpu(x)		((uint16_t)(x))
+#define be32_to_cpu(x)		((uint32_t)(x))
+#define be64_to_cpu(x)		((uint64_t)(x))
+
+#define cpu_to_le16(x)		((uint16_t)BSWAP16(x))
+#define cpu_to_le32(x)		((uint32_t)BSWAP32(x))
+#define cpu_to_le64(x)		((uint64_t)BSWAP64(x))
+
+#define le16_to_cpu(x)		((uint16_t)BSWAP16(x))
+#define le32_to_cpu(x)		((uint32_t)BSWAP32(x))
+#define le64_to_cpu(x)		((uint64_t)BSWAP64(x))
+#endif
+
+#endif /* __SBI_BYTEORDER_H__ */
-- 
2.34.1

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Xiang W]

? 2023-01-28???? 13:16 +0530?Rahul Pathak???
> Define macros for general byteorder conversion
> Define functions for endianness conversion
> using general byteorder conversion macros
> 
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
Look good to me

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
> ?1 file changed, 57 insertions(+)
> ?create mode 100644 include/sbi/sbi_byteorder.h
> 
> diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> new file mode 100644
> index 000000000000..680710fe6d91
> --- /dev/null
> +++ b/include/sbi/sbi_byteorder.h
> @@ -0,0 +1,57 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + */
> +
> +#ifndef __SBI_BYTEORDER_H__
> +#define __SBI_BYTEORDER_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +#define EXTRACT_BYTE(x, n)?????((unsigned long long)((uint8_t *)&x)[n])
> +
> +#define BSWAP16(x)?????((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> +#define BSWAP32(x)?????((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> +??????????????????????? (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> +#define BSWAP64(x)?????((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> +??????????????????????? (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> +??????????????????????? (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> +??????????????????????? (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> +
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__? /* CPU(little-endian) */
> +#define cpu_to_be16(x)?????????((uint16_t)BSWAP16(x))
> +#define cpu_to_be32(x)?????????((uint32_t)BSWAP32(x))
> +#define cpu_to_be64(x)?????????((uint64_t)BSWAP64(x))
> +
> +#define be16_to_cpu(x)?????????((uint16_t)BSWAP16(x))
> +#define be32_to_cpu(x)?????????((uint32_t)BSWAP32(x))
> +#define be64_to_cpu(x)?????????((uint64_t)BSWAP64(x))
> +
> +#define cpu_to_le16(x)?????????((uint16_t)(x))
> +#define cpu_to_le32(x)?????????((uint32_t)(x))
> +#define cpu_to_le64(x)?????????((uint64_t)(x))
> +
> +#define le16_to_cpu(x)?????????((uint16_t)(x))
> +#define le32_to_cpu(x)?????????((uint32_t)(x))
> +#define le64_to_cpu(x)?????????((uint64_t)(x))
> +#else /* CPU(big-endian) */
> +#define cpu_to_be16(x)?????????((uint16_t)(x))
> +#define cpu_to_be32(x)?????????((uint32_t)(x))
> +#define cpu_to_be64(x)?????????((uint64_t)(x))
> +
> +#define be16_to_cpu(x)?????????((uint16_t)(x))
> +#define be32_to_cpu(x)?????????((uint32_t)(x))
> +#define be64_to_cpu(x)?????????((uint64_t)(x))
> +
> +#define cpu_to_le16(x)?????????((uint16_t)BSWAP16(x))
> +#define cpu_to_le32(x)?????????((uint32_t)BSWAP32(x))
> +#define cpu_to_le64(x)?????????((uint64_t)BSWAP64(x))
> +
> +#define le16_to_cpu(x)?????????((uint16_t)BSWAP16(x))
> +#define le32_to_cpu(x)?????????((uint32_t)BSWAP32(x))
> +#define le64_to_cpu(x)?????????((uint64_t)BSWAP64(x))
> +#endif
> +
> +#endif /* __SBI_BYTEORDER_H__ */
> -- 
> 2.34.1
> 
> 

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Sergey Matyukevich]

On Sat, Jan 28, 2023 at 01:16:16PM +0530, Rahul Pathak wrote:
> Define macros for general byteorder conversion
> Define functions for endianness conversion
> using general byteorder conversion macros
> 
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> ---
>  include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 include/sbi/sbi_byteorder.h

Reviewed-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>

By the way, is there any advantage of using custom macros instead of
compiler builtins ? E.g. see my patch loosely based on FreeBSD code:
http://lists.infradead.org/pipermail/opensbi/2022-December/003766.html

Regards,
Sergey


> diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> new file mode 100644
> index 000000000000..680710fe6d91
> --- /dev/null
> +++ b/include/sbi/sbi_byteorder.h
> @@ -0,0 +1,57 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2023 Ventana Micro Systems Inc.
> + */
> +
> +#ifndef __SBI_BYTEORDER_H__
> +#define __SBI_BYTEORDER_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +#define EXTRACT_BYTE(x, n)	((unsigned long long)((uint8_t *)&x)[n])
> +
> +#define BSWAP16(x)	((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> +#define BSWAP32(x)	((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> +			 (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> +#define BSWAP64(x)	((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> +			 (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> +			 (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> +			 (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> +
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
> +#define cpu_to_be16(x)		((uint16_t)BSWAP16(x))
> +#define cpu_to_be32(x)		((uint32_t)BSWAP32(x))
> +#define cpu_to_be64(x)		((uint64_t)BSWAP64(x))
> +
> +#define be16_to_cpu(x)		((uint16_t)BSWAP16(x))
> +#define be32_to_cpu(x)		((uint32_t)BSWAP32(x))
> +#define be64_to_cpu(x)		((uint64_t)BSWAP64(x))
> +
> +#define cpu_to_le16(x)		((uint16_t)(x))
> +#define cpu_to_le32(x)		((uint32_t)(x))
> +#define cpu_to_le64(x)		((uint64_t)(x))
> +
> +#define le16_to_cpu(x)		((uint16_t)(x))
> +#define le32_to_cpu(x)		((uint32_t)(x))
> +#define le64_to_cpu(x)		((uint64_t)(x))
> +#else /* CPU(big-endian) */
> +#define cpu_to_be16(x)		((uint16_t)(x))
> +#define cpu_to_be32(x)		((uint32_t)(x))
> +#define cpu_to_be64(x)		((uint64_t)(x))
> +
> +#define be16_to_cpu(x)		((uint16_t)(x))
> +#define be32_to_cpu(x)		((uint32_t)(x))
> +#define be64_to_cpu(x)		((uint64_t)(x))
> +
> +#define cpu_to_le16(x)		((uint16_t)BSWAP16(x))
> +#define cpu_to_le32(x)		((uint32_t)BSWAP32(x))
> +#define cpu_to_le64(x)		((uint64_t)BSWAP64(x))
> +
> +#define le16_to_cpu(x)		((uint16_t)BSWAP16(x))
> +#define le32_to_cpu(x)		((uint32_t)BSWAP32(x))
> +#define le64_to_cpu(x)		((uint64_t)BSWAP64(x))
> +#endif
> +
> +#endif /* __SBI_BYTEORDER_H__ */
> -- 
> 2.34.1
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Rahul Pathak]

Hi Sergey,

Using __builtins_bswapX causes weird issues on gcc,
due to reasons unknown the functions in libfdt_env.h
like below emit the library call to __bswapdi2
which causes errors.

static inline fdt32_t cpu_to_fdt32(uint32_t x)
{
return (FDT_FORCE fdt32_t)cpu_to_be32(x);
}

Errors -
fdt_reset_gpio.c:135: undefined reference to `__bswapsi2'
/fdt_reset_thead.c:77: undefined reference to `__bswapsi2'

Clang works perfectly

Did you face such issues?

Thanks
Rahul


On Tue, Jan 31, 2023 at 1:41 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> On Sat, Jan 28, 2023 at 01:16:16PM +0530, Rahul Pathak wrote:
> > Define macros for general byteorder conversion
> > Define functions for endianness conversion
> > using general byteorder conversion macros
> >
> > Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> > ---
> >  include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644 include/sbi/sbi_byteorder.h
>
> Reviewed-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
>
> By the way, is there any advantage of using custom macros instead of
> compiler builtins ? E.g. see my patch loosely based on FreeBSD code:
> http://lists.infradead.org/pipermail/opensbi/2022-December/003766.html
>
> Regards,
> Sergey
>
>
> > diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> > new file mode 100644
> > index 000000000000..680710fe6d91
> > --- /dev/null
> > +++ b/include/sbi/sbi_byteorder.h
> > @@ -0,0 +1,57 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > + */
> > +
> > +#ifndef __SBI_BYTEORDER_H__
> > +#define __SBI_BYTEORDER_H__
> > +
> > +#include <sbi/sbi_types.h>
> > +
> > +#define EXTRACT_BYTE(x, n)   ((unsigned long long)((uint8_t *)&x)[n])
> > +
> > +#define BSWAP16(x)   ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> > +#define BSWAP32(x)   ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> > +                      (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> > +#define BSWAP64(x)   ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> > +                      (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> > +                      (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> > +                      (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> > +
> > +
> > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
> > +#define cpu_to_be16(x)               ((uint16_t)BSWAP16(x))
> > +#define cpu_to_be32(x)               ((uint32_t)BSWAP32(x))
> > +#define cpu_to_be64(x)               ((uint64_t)BSWAP64(x))
> > +
> > +#define be16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > +#define be32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > +#define be64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > +
> > +#define cpu_to_le16(x)               ((uint16_t)(x))
> > +#define cpu_to_le32(x)               ((uint32_t)(x))
> > +#define cpu_to_le64(x)               ((uint64_t)(x))
> > +
> > +#define le16_to_cpu(x)               ((uint16_t)(x))
> > +#define le32_to_cpu(x)               ((uint32_t)(x))
> > +#define le64_to_cpu(x)               ((uint64_t)(x))
> > +#else /* CPU(big-endian) */
> > +#define cpu_to_be16(x)               ((uint16_t)(x))
> > +#define cpu_to_be32(x)               ((uint32_t)(x))
> > +#define cpu_to_be64(x)               ((uint64_t)(x))
> > +
> > +#define be16_to_cpu(x)               ((uint16_t)(x))
> > +#define be32_to_cpu(x)               ((uint32_t)(x))
> > +#define be64_to_cpu(x)               ((uint64_t)(x))
> > +
> > +#define cpu_to_le16(x)               ((uint16_t)BSWAP16(x))
> > +#define cpu_to_le32(x)               ((uint32_t)BSWAP32(x))
> > +#define cpu_to_le64(x)               ((uint64_t)BSWAP64(x))
> > +
> > +#define le16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > +#define le32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > +#define le64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > +#endif
> > +
> > +#endif /* __SBI_BYTEORDER_H__ */
> > --
> > 2.34.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi



-- 

Thanks
Rahul Pathak

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Sergey Matyukevich]

Hi Rahul,

> Using __builtins_bswapX causes weird issues on gcc,
> due to reasons unknown the functions in libfdt_env.h
> like below emit the library call to __bswapdi2
> which causes errors.
> 
> static inline fdt32_t cpu_to_fdt32(uint32_t x)
> {
> return (FDT_FORCE fdt32_t)cpu_to_be32(x);
> }
> 
> Errors -
> fdt_reset_gpio.c:135: undefined reference to `__bswapsi2'
> /fdt_reset_thead.c:77: undefined reference to `__bswapsi2'
> 
> Clang works perfectly
> 
> Did you face such issues?

Right. I see such issues for gcc revisions v10.2, v11.2, v12.1.
Missing references could be be pulled from libgcc, e.g.: 

diff --git a/Makefile b/Makefile
index 8db6eab..8cae625 100644
--- a/Makefile
+++ b/Makefile
@@ -369,7 +369,7 @@ ASFLAGS		+=	$(firmware-asflags-y)
 ARFLAGS		=	rcs
 
 ELFFLAGS	+=	$(USE_LD_FLAG)
-ELFFLAGS	+=	-Wl,--build-id=none -Wl,-N
+ELFFLAGS	+=	-Wl,--build-id=none -Wl,-N -Wl,-lgcc
 ELFFLAGS	+=	$(platform-ldflags-y)
 ELFFLAGS	+=	$(firmware-ldflags-y)


Regards,
Sergey

> On Tue, Jan 31, 2023 at 1:41 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
> >
> > On Sat, Jan 28, 2023 at 01:16:16PM +0530, Rahul Pathak wrote:
> > > Define macros for general byteorder conversion
> > > Define functions for endianness conversion
> > > using general byteorder conversion macros
> > >
> > > Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> > > ---
> > >  include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 57 insertions(+)
> > >  create mode 100644 include/sbi/sbi_byteorder.h
> >
> > Reviewed-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
> >
> > By the way, is there any advantage of using custom macros instead of
> > compiler builtins ? E.g. see my patch loosely based on FreeBSD code:
> > http://lists.infradead.org/pipermail/opensbi/2022-December/003766.html
> >
> > Regards,
> > Sergey
> >
> >
> > > diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> > > new file mode 100644
> > > index 000000000000..680710fe6d91
> > > --- /dev/null
> > > +++ b/include/sbi/sbi_byteorder.h
> > > @@ -0,0 +1,57 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > > + */
> > > +
> > > +#ifndef __SBI_BYTEORDER_H__
> > > +#define __SBI_BYTEORDER_H__
> > > +
> > > +#include <sbi/sbi_types.h>
> > > +
> > > +#define EXTRACT_BYTE(x, n)   ((unsigned long long)((uint8_t *)&x)[n])
> > > +
> > > +#define BSWAP16(x)   ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> > > +#define BSWAP32(x)   ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> > > +                      (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> > > +#define BSWAP64(x)   ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> > > +                      (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> > > +                      (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> > > +                      (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> > > +
> > > +
> > > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
> > > +#define cpu_to_be16(x)               ((uint16_t)BSWAP16(x))
> > > +#define cpu_to_be32(x)               ((uint32_t)BSWAP32(x))
> > > +#define cpu_to_be64(x)               ((uint64_t)BSWAP64(x))
> > > +
> > > +#define be16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > > +#define be32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > > +#define be64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > > +
> > > +#define cpu_to_le16(x)               ((uint16_t)(x))
> > > +#define cpu_to_le32(x)               ((uint32_t)(x))
> > > +#define cpu_to_le64(x)               ((uint64_t)(x))
> > > +
> > > +#define le16_to_cpu(x)               ((uint16_t)(x))
> > > +#define le32_to_cpu(x)               ((uint32_t)(x))
> > > +#define le64_to_cpu(x)               ((uint64_t)(x))
> > > +#else /* CPU(big-endian) */
> > > +#define cpu_to_be16(x)               ((uint16_t)(x))
> > > +#define cpu_to_be32(x)               ((uint32_t)(x))
> > > +#define cpu_to_be64(x)               ((uint64_t)(x))
> > > +
> > > +#define be16_to_cpu(x)               ((uint16_t)(x))
> > > +#define be32_to_cpu(x)               ((uint32_t)(x))
> > > +#define be64_to_cpu(x)               ((uint64_t)(x))
> > > +
> > > +#define cpu_to_le16(x)               ((uint16_t)BSWAP16(x))
> > > +#define cpu_to_le32(x)               ((uint32_t)BSWAP32(x))
> > > +#define cpu_to_le64(x)               ((uint64_t)BSWAP64(x))
> > > +
> > > +#define le16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > > +#define le32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > > +#define le64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > > +#endif
> > > +
> > > +#endif /* __SBI_BYTEORDER_H__ */
> > > --
> > > 2.34.1
> > >
> > >
> > > --
> > > opensbi mailing list
> > > opensbi at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/opensbi
> 
> 
> 
> -- 
> 
> Thanks
> Rahul Pathak

[PATCH 1/2] include: Add support for byteorder/endianness conversion

[Rahul Pathak]

But should libgcc be pulled in the opensbi, I doubt that.

Also just for trying i did tried linking with libgcc, it was giving me
build errors
libgcc.a(_bswapdi2.o): can't link double-float modules with soft-float modules

On Thu, Feb 2, 2023 at 2:54 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
>
> Hi Rahul,
>
> > Using __builtins_bswapX causes weird issues on gcc,
> > due to reasons unknown the functions in libfdt_env.h
> > like below emit the library call to __bswapdi2
> > which causes errors.
> >
> > static inline fdt32_t cpu_to_fdt32(uint32_t x)
> > {
> > return (FDT_FORCE fdt32_t)cpu_to_be32(x);
> > }
> >
> > Errors -
> > fdt_reset_gpio.c:135: undefined reference to `__bswapsi2'
> > /fdt_reset_thead.c:77: undefined reference to `__bswapsi2'
> >
> > Clang works perfectly
> >
> > Did you face such issues?
>
> Right. I see such issues for gcc revisions v10.2, v11.2, v12.1.
> Missing references could be be pulled from libgcc, e.g.:
>
> diff --git a/Makefile b/Makefile
> index 8db6eab..8cae625 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -369,7 +369,7 @@ ASFLAGS             +=      $(firmware-asflags-y)
>  ARFLAGS                =       rcs
>
>  ELFFLAGS       +=      $(USE_LD_FLAG)
> -ELFFLAGS       +=      -Wl,--build-id=none -Wl,-N
> +ELFFLAGS       +=      -Wl,--build-id=none -Wl,-N -Wl,-lgcc
>  ELFFLAGS       +=      $(platform-ldflags-y)
>  ELFFLAGS       +=      $(firmware-ldflags-y)
>
>
> Regards,
> Sergey
>
> > On Tue, Jan 31, 2023 at 1:41 PM Sergey Matyukevich <geomatsi@gmail.com> wrote:
> > >
> > > On Sat, Jan 28, 2023 at 01:16:16PM +0530, Rahul Pathak wrote:
> > > > Define macros for general byteorder conversion
> > > > Define functions for endianness conversion
> > > > using general byteorder conversion macros
> > > >
> > > > Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
> > > > ---
> > > >  include/sbi/sbi_byteorder.h | 57 +++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 57 insertions(+)
> > > >  create mode 100644 include/sbi/sbi_byteorder.h
> > >
> > > Reviewed-by: Sergey Matyukevich <sergey.matyukevich@syntacore.com>
> > >
> > > By the way, is there any advantage of using custom macros instead of
> > > compiler builtins ? E.g. see my patch loosely based on FreeBSD code:
> > > http://lists.infradead.org/pipermail/opensbi/2022-December/003766.html
> > >
> > > Regards,
> > > Sergey
> > >
> > >
> > > > diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> > > > new file mode 100644
> > > > index 000000000000..680710fe6d91
> > > > --- /dev/null
> > > > +++ b/include/sbi/sbi_byteorder.h
> > > > @@ -0,0 +1,57 @@
> > > > +/*
> > > > + * SPDX-License-Identifier: BSD-2-Clause
> > > > + *
> > > > + * Copyright (c) 2023 Ventana Micro Systems Inc.
> > > > + */
> > > > +
> > > > +#ifndef __SBI_BYTEORDER_H__
> > > > +#define __SBI_BYTEORDER_H__
> > > > +
> > > > +#include <sbi/sbi_types.h>
> > > > +
> > > > +#define EXTRACT_BYTE(x, n)   ((unsigned long long)((uint8_t *)&x)[n])
> > > > +
> > > > +#define BSWAP16(x)   ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> > > > +#define BSWAP32(x)   ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> > > > +                      (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> > > > +#define BSWAP64(x)   ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> > > > +                      (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> > > > +                      (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> > > > +                      (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> > > > +
> > > > +
> > > > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
> > > > +#define cpu_to_be16(x)               ((uint16_t)BSWAP16(x))
> > > > +#define cpu_to_be32(x)               ((uint32_t)BSWAP32(x))
> > > > +#define cpu_to_be64(x)               ((uint64_t)BSWAP64(x))
> > > > +
> > > > +#define be16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > > > +#define be32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > > > +#define be64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > > > +
> > > > +#define cpu_to_le16(x)               ((uint16_t)(x))
> > > > +#define cpu_to_le32(x)               ((uint32_t)(x))
> > > > +#define cpu_to_le64(x)               ((uint64_t)(x))
> > > > +
> > > > +#define le16_to_cpu(x)               ((uint16_t)(x))
> > > > +#define le32_to_cpu(x)               ((uint32_t)(x))
> > > > +#define le64_to_cpu(x)               ((uint64_t)(x))
> > > > +#else /* CPU(big-endian) */
> > > > +#define cpu_to_be16(x)               ((uint16_t)(x))
> > > > +#define cpu_to_be32(x)               ((uint32_t)(x))
> > > > +#define cpu_to_be64(x)               ((uint64_t)(x))
> > > > +
> > > > +#define be16_to_cpu(x)               ((uint16_t)(x))
> > > > +#define be32_to_cpu(x)               ((uint32_t)(x))
> > > > +#define be64_to_cpu(x)               ((uint64_t)(x))
> > > > +
> > > > +#define cpu_to_le16(x)               ((uint16_t)BSWAP16(x))
> > > > +#define cpu_to_le32(x)               ((uint32_t)BSWAP32(x))
> > > > +#define cpu_to_le64(x)               ((uint64_t)BSWAP64(x))
> > > > +
> > > > +#define le16_to_cpu(x)               ((uint16_t)BSWAP16(x))
> > > > +#define le32_to_cpu(x)               ((uint32_t)BSWAP32(x))
> > > > +#define le64_to_cpu(x)               ((uint64_t)BSWAP64(x))
> > > > +#endif
> > > > +
> > > > +#endif /* __SBI_BYTEORDER_H__ */
> > > > --
> > > > 2.34.1
> > > >
> > > >
> > > > --
> > > > opensbi mailing list
> > > > opensbi at lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/opensbi
> >
> >
> >
> > --
> >
> > Thanks
> > Rahul Pathak



-- 

Thanks
Rahul Pathak

[PATCH 2/2] lib: utils/fdt: Use byteorder conversion functions in libfdt_env.h

[Rahul Pathak]

FDT follows big-endian and CPU can be little or big
endian as per the implementation.
libfdt_env.h defines function for conversion between
fdt and cpu byteorder according to the endianness.

Currently, libfdt_env.h defines custom byte swapping
macros and then undefines them. Instead, use the generic
endianness conversion functions

Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
---
 lib/utils/libfdt/libfdt_env.h | 29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/lib/utils/libfdt/libfdt_env.h b/lib/utils/libfdt/libfdt_env.h
index f9d9c6783c52..e5ad7698efca 100644
--- a/lib/utils/libfdt/libfdt_env.h
+++ b/lib/utils/libfdt/libfdt_env.h
@@ -9,6 +9,7 @@
 
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_types.h>
+#include <sbi/sbi_byteorder.h>
 
 #define INT_MAX		((int)(~0U >> 1))
 #define UINT_MAX	((unsigned int)~0U)
@@ -41,45 +42,35 @@ typedef uint16_t FDT_BITWISE fdt16_t;
 typedef uint32_t FDT_BITWISE fdt32_t;
 typedef uint64_t FDT_BITWISE fdt64_t;
 
-#define EXTRACT_BYTE(x, n)	((unsigned long long)((uint8_t *)&x)[n])
-#define CPU_TO_FDT16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
-#define CPU_TO_FDT32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
-			 (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
-#define CPU_TO_FDT64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
-			 (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
-			 (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
-			 (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
-
 static inline uint16_t fdt16_to_cpu(fdt16_t x)
 {
-	return (FDT_FORCE uint16_t)CPU_TO_FDT16(x);
+	return (FDT_FORCE uint16_t)be16_to_cpu(x);
 }
+
 static inline fdt16_t cpu_to_fdt16(uint16_t x)
 {
-	return (FDT_FORCE fdt16_t)CPU_TO_FDT16(x);
+	return (FDT_FORCE fdt16_t)cpu_to_be16(x);
 }
 
 static inline uint32_t fdt32_to_cpu(fdt32_t x)
 {
-	return (FDT_FORCE uint32_t)CPU_TO_FDT32(x);
+	return (FDT_FORCE uint32_t)be32_to_cpu(x);
 }
+
 static inline fdt32_t cpu_to_fdt32(uint32_t x)
 {
-	return (FDT_FORCE fdt32_t)CPU_TO_FDT32(x);
+	return (FDT_FORCE fdt32_t)cpu_to_be32(x);
 }
 
 static inline uint64_t fdt64_to_cpu(fdt64_t x)
 {
-	return (FDT_FORCE uint64_t)CPU_TO_FDT64(x);
+	return (FDT_FORCE uint64_t)be64_to_cpu(x);
 }
+
 static inline fdt64_t cpu_to_fdt64(uint64_t x)
 {
-	return (FDT_FORCE fdt64_t)CPU_TO_FDT64(x);
+	return (FDT_FORCE fdt64_t)cpu_to_be64(x);
 }
-#undef CPU_TO_FDT64
-#undef CPU_TO_FDT32
-#undef CPU_TO_FDT16
-#undef EXTRACT_BYTE
 
 #ifdef __APPLE__
 #include <AvailabilityMacros.h>
-- 
2.34.1

[PATCH 2/2] lib: utils/fdt: Use byteorder conversion functions in libfdt_env.h

[Xiang W]

? 2023-01-28???? 13:16 +0530?Rahul Pathak???
> FDT follows big-endian and CPU can be little or big
> endian as per the implementation.
> libfdt_env.h defines function for conversion between
> fdt and cpu byteorder according to the endianness.
> 
> Currently, libfdt_env.h defines custom byte swapping
> macros and then undefines them. Instead, use the generic
> endianness conversion functions
> 
> Signed-off-by: Rahul Pathak <rpathak@ventanamicro.com>
Look good to me

Reviewed-by: Xiang W <wxjstz@126.com>
> ---
> ?lib/utils/libfdt/libfdt_env.h | 29 ++++++++++-------------------
> ?1 file changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/utils/libfdt/libfdt_env.h b/lib/utils/libfdt/libfdt_env.h
> index f9d9c6783c52..e5ad7698efca 100644
> --- a/lib/utils/libfdt/libfdt_env.h
> +++ b/lib/utils/libfdt/libfdt_env.h
> @@ -9,6 +9,7 @@
> ?
> ?#include <sbi/sbi_string.h>
> ?#include <sbi/sbi_types.h>
> +#include <sbi/sbi_byteorder.h>
> ?
> ?#define INT_MAX????????????????((int)(~0U >> 1))
> ?#define UINT_MAX???????((unsigned int)~0U)
> @@ -41,45 +42,35 @@ typedef uint16_t FDT_BITWISE fdt16_t;
> ?typedef uint32_t FDT_BITWISE fdt32_t;
> ?typedef uint64_t FDT_BITWISE fdt64_t;
> ?
> -#define EXTRACT_BYTE(x, n)?????((unsigned long long)((uint8_t *)&x)[n])
> -#define CPU_TO_FDT16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> -#define CPU_TO_FDT32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> -??????????????????????? (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> -#define CPU_TO_FDT64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> -??????????????????????? (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> -??????????????????????? (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> -??????????????????????? (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> -
> ?static inline uint16_t fdt16_to_cpu(fdt16_t x)
> ?{
> -???????return (FDT_FORCE uint16_t)CPU_TO_FDT16(x);
> +???????return (FDT_FORCE uint16_t)be16_to_cpu(x);
> ?}
> +
> ?static inline fdt16_t cpu_to_fdt16(uint16_t x)
> ?{
> -???????return (FDT_FORCE fdt16_t)CPU_TO_FDT16(x);
> +???????return (FDT_FORCE fdt16_t)cpu_to_be16(x);
> ?}
> ?
> ?static inline uint32_t fdt32_to_cpu(fdt32_t x)
> ?{
> -???????return (FDT_FORCE uint32_t)CPU_TO_FDT32(x);
> +???????return (FDT_FORCE uint32_t)be32_to_cpu(x);
> ?}
> +
> ?static inline fdt32_t cpu_to_fdt32(uint32_t x)
> ?{
> -???????return (FDT_FORCE fdt32_t)CPU_TO_FDT32(x);
> +???????return (FDT_FORCE fdt32_t)cpu_to_be32(x);
> ?}
> ?
> ?static inline uint64_t fdt64_to_cpu(fdt64_t x)
> ?{
> -???????return (FDT_FORCE uint64_t)CPU_TO_FDT64(x);
> +???????return (FDT_FORCE uint64_t)be64_to_cpu(x);
> ?}
> +
> ?static inline fdt64_t cpu_to_fdt64(uint64_t x)
> ?{
> -???????return (FDT_FORCE fdt64_t)CPU_TO_FDT64(x);
> +???????return (FDT_FORCE fdt64_t)cpu_to_be64(x);
> ?}
> -#undef CPU_TO_FDT64
> -#undef CPU_TO_FDT32
> -#undef CPU_TO_FDT16
> -#undef EXTRACT_BYTE
> ?
> ?#ifdef __APPLE__
> ?#include <AvailabilityMacros.h>
> -- 
> 2.34.1
> 
>