* [PATCH v2 0/2] OpenSBI LLVM related fixes
@ 2024-12-10 5:23 Anup Patel
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
2024-12-10 5:23 ` [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option Anup Patel
0 siblings, 2 replies; 9+ messages in thread
From: Anup Patel @ 2024-12-10 5:23 UTC (permalink / raw)
To: opensbi
This series does few LLVM related fixes encountered using
"Ubuntu clang version 18.1.3 (1ubuntu1)".
These patches can also be found in the riscv_llvm_compile_fixes_v2
branch at: https://github.com/avpatel/opensbi.git
Changes since v1:
- Removed -fno-tree-vectorize from PATCH2
Anup Patel (2):
lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
Makefile: Don't enable V-extension using -march option
Makefile | 8 ++++----
lib/sbi/sbi_trap_v_ldst.c | 5 +++--
lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
3 files changed, 12 insertions(+), 8 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
2024-12-10 5:23 [PATCH v2 0/2] OpenSBI LLVM related fixes Anup Patel
@ 2024-12-10 5:23 ` Anup Patel
2024-12-11 0:05 ` Samuel Holland
` (2 more replies)
2024-12-10 5:23 ` [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option Anup Patel
1 sibling, 3 replies; 9+ messages in thread
From: Anup Patel @ 2024-12-10 5:23 UTC (permalink / raw)
To: opensbi
The following error is observed when compiling fdt_cppc_rpmi
driver using LLVM:
lib/utils/cppc/fdt_cppc_rpmi.c:87:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
87 | u64 db_val_u64 = 0;
To fix the above issue, move the variable declaration at the
start of function.
Fixes: 591a98bdd549 ("lib: utils/cppc: Add RPMI CPPC driver")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
index 26e2d4f6..b6789901 100644
--- a/lib/utils/cppc/fdt_cppc_rpmi.c
+++ b/lib/utils/cppc/fdt_cppc_rpmi.c
@@ -59,6 +59,11 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
u8 db_val_u8 = 0;
u16 db_val_u16 = 0;
u32 db_val_u32 = 0;
+#if __riscv_xlen != 32
+ u64 db_val_u64 = 0;
+#else
+ u32 db_val_u32_hi = 0;
+#endif
switch (cppc->fc_db_width) {
case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
@@ -84,14 +89,12 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
break;
case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
#if __riscv_xlen != 32
- u64 db_val_u64 = 0;
db_val_u64 = readq((void *)cppc->fc_db_addr);
db_val_u64 = cppc->fc_db_setmask |
(db_val_u64 & cppc->fc_db_preservemask);
writeq(db_val_u64, (void *)cppc->fc_db_addr);
#else
- u32 db_val_u32_hi = 0;
db_val_u32 = readl((void *)cppc->fc_db_addr);
db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option
2024-12-10 5:23 [PATCH v2 0/2] OpenSBI LLVM related fixes Anup Patel
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
@ 2024-12-10 5:23 ` Anup Patel
2024-12-11 0:11 ` Samuel Holland
1 sibling, 1 reply; 9+ messages in thread
From: Anup Patel @ 2024-12-10 5:23 UTC (permalink / raw)
To: opensbi
Enabling V-extension using -march option causes OpenSBI boot-time
hang with LLVM compiler.
As a work-around, don't enable V-extension using -march option and
instead use a custom OpenSBI specific define inform availability of
V-extension to lib/sbi/sbi_trap_v_ldst.c.
Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
Makefile | 8 ++++----
lib/sbi/sbi_trap_v_ldst.c | 5 +++--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 5ac95a0f..1b949a9f 100644
--- a/Makefile
+++ b/Makefile
@@ -190,7 +190,7 @@ CC_SUPPORT_STRICT_ALIGN := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib
CC_SUPPORT_ZICSR_ZIFENCEI := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)imafd_zicsr_zifencei -x c /dev/null -o /dev/null 2>&1 | grep -e "zicsr" -e "zifencei" > /dev/null && echo n || echo y)
# Check whether the assembler and the compiler support the Vector extension
-CC_SUPPORT_VECT := $(shell echo | $(CC) -dM -E -march=rv$(OPENSBI_CC_XLEN)gv - | grep -q riscv.*vector && echo y || echo n)
+CC_SUPPORT_VECT := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)gv -dM -E -x c /dev/null 2>&1 | grep -q riscv.*vector && echo y || echo n)
ifneq ($(OPENSBI_LD_PIE),y)
$(error Your linker does not support creating PIEs, opensbi requires this.)
@@ -298,9 +298,6 @@ endif
ifndef PLATFORM_RISCV_ISA
ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1)
PLATFORM_RISCV_ISA := rv$(PLATFORM_RISCV_XLEN)imafdc
- ifeq ($(CC_SUPPORT_VECT), y)
- PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)v
- endif
ifeq ($(CC_SUPPORT_ZICSR_ZIFENCEI), y)
PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)_zicsr_zifencei
endif
@@ -363,6 +360,9 @@ GENFLAGS += $(firmware-genflags-y)
CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib -fno-stack-protector -fno-strict-aliasing -ffunction-sections -fdata-sections
CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
# Optionally supported flags
+ifeq ($(CC_SUPPORT_VECT),y)
+CFLAGS += -DOPENSBI_CC_SUPPORT_VECT
+endif
ifeq ($(CC_SUPPORT_SAVE_RESTORE),y)
CFLAGS += -mno-save-restore
endif
diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
index 9929215c..75b79baa 100644
--- a/lib/sbi/sbi_trap_v_ldst.c
+++ b/lib/sbi/sbi_trap_v_ldst.c
@@ -17,7 +17,8 @@
#include <sbi/sbi_unpriv.h>
#include <sbi/sbi_trap.h>
-#ifdef __riscv_vector
+#ifdef OPENSBI_CC_SUPPORT_VECT
+
#define VLEN_MAX 65536
static inline void set_vreg(ulong vlenb, ulong which,
@@ -340,4 +341,4 @@ int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
{
return 0;
}
-#endif /* __riscv_vector */
+#endif /* OPENSBI_CC_SUPPORT_VECT */
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
@ 2024-12-11 0:05 ` Samuel Holland
2024-12-11 9:59 ` Xiang W
2024-12-15 6:03 ` Anup Patel
2 siblings, 0 replies; 9+ messages in thread
From: Samuel Holland @ 2024-12-11 0:05 UTC (permalink / raw)
To: opensbi
On 2024-12-09 11:23 PM, Anup Patel wrote:
> The following error is observed when compiling fdt_cppc_rpmi
> driver using LLVM:
>
> lib/utils/cppc/fdt_cppc_rpmi.c:87:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
> 87 | u64 db_val_u64 = 0;
>
> To fix the above issue, move the variable declaration at the
> start of function.
>
> Fixes: 591a98bdd549 ("lib: utils/cppc: Add RPMI CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option
2024-12-10 5:23 ` [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option Anup Patel
@ 2024-12-11 0:11 ` Samuel Holland
2024-12-15 6:04 ` Anup Patel
0 siblings, 1 reply; 9+ messages in thread
From: Samuel Holland @ 2024-12-11 0:11 UTC (permalink / raw)
To: opensbi
On 2024-12-09 11:23 PM, Anup Patel wrote:
> Enabling V-extension using -march option causes OpenSBI boot-time
> hang with LLVM compiler.
>
> As a work-around, don't enable V-extension using -march option and
> instead use a custom OpenSBI specific define inform availability of
> V-extension to lib/sbi/sbi_trap_v_ldst.c.
>
> Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> Makefile | 8 ++++----
> lib/sbi/sbi_trap_v_ldst.c | 5 +++--
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 5ac95a0f..1b949a9f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -190,7 +190,7 @@ CC_SUPPORT_STRICT_ALIGN := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib
> CC_SUPPORT_ZICSR_ZIFENCEI := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)imafd_zicsr_zifencei -x c /dev/null -o /dev/null 2>&1 | grep -e "zicsr" -e "zifencei" > /dev/null && echo n || echo y)
>
> # Check whether the assembler and the compiler support the Vector extension
> -CC_SUPPORT_VECT := $(shell echo | $(CC) -dM -E -march=rv$(OPENSBI_CC_XLEN)gv - | grep -q riscv.*vector && echo y || echo n)
> +CC_SUPPORT_VECT := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)gv -dM -E -x c /dev/null 2>&1 | grep -q riscv.*vector && echo y || echo n)
>
> ifneq ($(OPENSBI_LD_PIE),y)
> $(error Your linker does not support creating PIEs, opensbi requires this.)
> @@ -298,9 +298,6 @@ endif
> ifndef PLATFORM_RISCV_ISA
> ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1)
> PLATFORM_RISCV_ISA := rv$(PLATFORM_RISCV_XLEN)imafdc
> - ifeq ($(CC_SUPPORT_VECT), y)
> - PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)v
> - endif
> ifeq ($(CC_SUPPORT_ZICSR_ZIFENCEI), y)
> PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)_zicsr_zifencei
> endif
> @@ -363,6 +360,9 @@ GENFLAGS += $(firmware-genflags-y)
> CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib -fno-stack-protector -fno-strict-aliasing -ffunction-sections -fdata-sections
> CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> # Optionally supported flags
> +ifeq ($(CC_SUPPORT_VECT),y)
> +CFLAGS += -DOPENSBI_CC_SUPPORT_VECT
nit: Since you're changing this, could you please spell out "VECTOR"? The
abbreviation isn't great for greppability.
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> +endif
> ifeq ($(CC_SUPPORT_SAVE_RESTORE),y)
> CFLAGS += -mno-save-restore
> endif
> diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> index 9929215c..75b79baa 100644
> --- a/lib/sbi/sbi_trap_v_ldst.c
> +++ b/lib/sbi/sbi_trap_v_ldst.c
> @@ -17,7 +17,8 @@
> #include <sbi/sbi_unpriv.h>
> #include <sbi/sbi_trap.h>
>
> -#ifdef __riscv_vector
> +#ifdef OPENSBI_CC_SUPPORT_VECT
> +
> #define VLEN_MAX 65536
>
> static inline void set_vreg(ulong vlenb, ulong which,
> @@ -340,4 +341,4 @@ int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> {
> return 0;
> }
> -#endif /* __riscv_vector */
> +#endif /* OPENSBI_CC_SUPPORT_VECT */
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
2024-12-11 0:05 ` Samuel Holland
@ 2024-12-11 9:59 ` Xiang W
2024-12-15 6:02 ` Anup Patel
2024-12-15 6:03 ` Anup Patel
2 siblings, 1 reply; 9+ messages in thread
From: Xiang W @ 2024-12-11 9:59 UTC (permalink / raw)
To: opensbi
? 2024-12-10?? 10:53 +0530?Anup Patel???
> The following error is observed when compiling fdt_cppc_rpmi
> driver using LLVM:
>
> lib/utils/cppc/fdt_cppc_rpmi.c:87:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
> ?? 87 |???????????????? u64 db_val_u64 = 0;
>
> To fix the above issue, move the variable declaration at the
> start of function.
>
> Fixes: 591a98bdd549 ("lib: utils/cppc: Add RPMI CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> ?lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
> ?1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
> index 26e2d4f6..b6789901 100644
> --- a/lib/utils/cppc/fdt_cppc_rpmi.c
> +++ b/lib/utils/cppc/fdt_cppc_rpmi.c
> @@ -59,6 +59,11 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> ? u8 db_val_u8 = 0;
> ? u16 db_val_u16 = 0;
> ? u32 db_val_u32 = 0;
> +#if __riscv_xlen != 32
> + u64 db_val_u64 = 0;
> +#else
> + u32 db_val_u32_hi = 0;
> +#endif
> ?
> ? switch (cppc->fc_db_width) {
> ? case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
> @@ -84,14 +89,12 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> ? break;
> ? case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
> ?#if __riscv_xlen != 32
> - u64 db_val_u64 = 0;
> ? db_val_u64 = readq((void *)cppc->fc_db_addr);
> ? db_val_u64 = cppc->fc_db_setmask |
> ? (db_val_u64 & cppc->fc_db_preservemask);
> ?
> ? writeq(db_val_u64, (void *)cppc->fc_db_addr);
> ?#else
> - u32 db_val_u32_hi = 0;
> ? db_val_u32 = readl((void *)cppc->fc_db_addr);
> ? db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
> ?
> --
> 2.43.0
>
>
We may be able to remove db_val_u32_hi. as follows
Regards,
Xiang W
diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
index 26e2d4f6..f1f757ff 100644
--- a/lib/utils/cppc/fdt_cppc_rpmi.c
+++ b/lib/utils/cppc/fdt_cppc_rpmi.c
@@ -59,6 +59,7 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
u8 db_val_u8 = 0;
u16 db_val_u16 = 0;
u32 db_val_u32 = 0;
+ u64 db_val_u64 = 0;
switch (cppc->fc_db_width) {
case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
@@ -84,24 +85,20 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
break;
case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
#if __riscv_xlen != 32
- u64 db_val_u64 = 0;
db_val_u64 = readq((void *)cppc->fc_db_addr);
db_val_u64 = cppc->fc_db_setmask |
(db_val_u64 & cppc->fc_db_preservemask);
writeq(db_val_u64, (void *)cppc->fc_db_addr);
#else
- u32 db_val_u32_hi = 0;
- db_val_u32 = readl((void *)cppc->fc_db_addr);
- db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
-
- db_val_u32 = (u32)cppc->fc_db_setmask |
- (db_val_u32 & (u32)cppc->fc_db_preservemask);
- db_val_u32_hi = (u32)(cppc->fc_db_setmask >> 32) |
- (db_val_u32 & (u32)(cppc->fc_db_preservemask >> 32));
+ db_val_u64 = readl((void *)(cppc->fc_db_addr + 4));
+ db_val_u64 <<= 32;
+ db_val_u64 |= readl((void *)cppc->fc_db_addr);
+ db_val_u64 = cppc->fc_db_setmask |
+ (db_val_u64 & cppc->fc_db_preservemask);
- writel(db_val_u32, (void *)cppc->fc_db_addr);
- writel(db_val_u32_hi, (void *)(cppc->fc_db_addr + 4));
+ writel(db_val_u64, (void *)cppc->fc_db_addr);
+ writel(db_val_u64 >> 32, (void *)(cppc->fc_db_addr + 4));
#endif
break;
default:
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
2024-12-11 9:59 ` Xiang W
@ 2024-12-15 6:02 ` Anup Patel
0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-12-15 6:02 UTC (permalink / raw)
To: opensbi
On Wed, Dec 11, 2024 at 3:31?PM Xiang W <wxjstz@126.com> wrote:
>
> ? 2024-12-10?? 10:53 +0530?Anup Patel???
> > The following error is observed when compiling fdt_cppc_rpmi
> > driver using LLVM:
> >
> > lib/utils/cppc/fdt_cppc_rpmi.c:87:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
> > 87 | u64 db_val_u64 = 0;
> >
> > To fix the above issue, move the variable declaration at the
> > start of function.
> >
> > Fixes: 591a98bdd549 ("lib: utils/cppc: Add RPMI CPPC driver")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
> > index 26e2d4f6..b6789901 100644
> > --- a/lib/utils/cppc/fdt_cppc_rpmi.c
> > +++ b/lib/utils/cppc/fdt_cppc_rpmi.c
> > @@ -59,6 +59,11 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> > u8 db_val_u8 = 0;
> > u16 db_val_u16 = 0;
> > u32 db_val_u32 = 0;
> > +#if __riscv_xlen != 32
> > + u64 db_val_u64 = 0;
> > +#else
> > + u32 db_val_u32_hi = 0;
> > +#endif
> >
> > switch (cppc->fc_db_width) {
> > case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
> > @@ -84,14 +89,12 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> > break;
> > case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
> > #if __riscv_xlen != 32
> > - u64 db_val_u64 = 0;
> > db_val_u64 = readq((void *)cppc->fc_db_addr);
> > db_val_u64 = cppc->fc_db_setmask |
> > (db_val_u64 & cppc->fc_db_preservemask);
> >
> > writeq(db_val_u64, (void *)cppc->fc_db_addr);
> > #else
> > - u32 db_val_u32_hi = 0;
> > db_val_u32 = readl((void *)cppc->fc_db_addr);
> > db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
> >
> > --
> > 2.43.0
> >
> >
>
> We may be able to remove db_val_u32_hi. as follows
>
> Regards,
> Xiang W
>
> diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
> index 26e2d4f6..f1f757ff 100644
> --- a/lib/utils/cppc/fdt_cppc_rpmi.c
> +++ b/lib/utils/cppc/fdt_cppc_rpmi.c
> @@ -59,6 +59,7 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> u8 db_val_u8 = 0;
> u16 db_val_u16 = 0;
> u32 db_val_u32 = 0;
> + u64 db_val_u64 = 0;
>
> switch (cppc->fc_db_width) {
> case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
> @@ -84,24 +85,20 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> break;
> case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
> #if __riscv_xlen != 32
> - u64 db_val_u64 = 0;
> db_val_u64 = readq((void *)cppc->fc_db_addr);
> db_val_u64 = cppc->fc_db_setmask |
> (db_val_u64 & cppc->fc_db_preservemask);
>
> writeq(db_val_u64, (void *)cppc->fc_db_addr);
> #else
> - u32 db_val_u32_hi = 0;
> - db_val_u32 = readl((void *)cppc->fc_db_addr);
> - db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
> -
> - db_val_u32 = (u32)cppc->fc_db_setmask |
> - (db_val_u32 & (u32)cppc->fc_db_preservemask);
> - db_val_u32_hi = (u32)(cppc->fc_db_setmask >> 32) |
> - (db_val_u32 & (u32)(cppc->fc_db_preservemask >> 32));
> + db_val_u64 = readl((void *)(cppc->fc_db_addr + 4));
> + db_val_u64 <<= 32;
> + db_val_u64 |= readl((void *)cppc->fc_db_addr);
> + db_val_u64 = cppc->fc_db_setmask |
> + (db_val_u64 & cppc->fc_db_preservemask);
>
> - writel(db_val_u32, (void *)cppc->fc_db_addr);
> - writel(db_val_u32_hi, (void *)(cppc->fc_db_addr + 4));
> + writel(db_val_u64, (void *)cppc->fc_db_addr);
> + writel(db_val_u64 >> 32, (void *)(cppc->fc_db_addr + 4));
> #endif
Your suggestion looks good to me but it is more of an improvement.
Feel free to send this as a separate improvement patch.
Regards,
Anup
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
2024-12-11 0:05 ` Samuel Holland
2024-12-11 9:59 ` Xiang W
@ 2024-12-15 6:03 ` Anup Patel
2 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-12-15 6:03 UTC (permalink / raw)
To: opensbi
On Tue, Dec 10, 2024 at 10:53?AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The following error is observed when compiling fdt_cppc_rpmi
> driver using LLVM:
>
> lib/utils/cppc/fdt_cppc_rpmi.c:87:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions]
> 87 | u64 db_val_u64 = 0;
>
> To fix the above issue, move the variable declaration at the
> start of function.
>
> Fixes: 591a98bdd549 ("lib: utils/cppc: Add RPMI CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Applied this patch to the riscv/opensbi repo.
Regards,
Anup
> ---
> lib/utils/cppc/fdt_cppc_rpmi.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/utils/cppc/fdt_cppc_rpmi.c b/lib/utils/cppc/fdt_cppc_rpmi.c
> index 26e2d4f6..b6789901 100644
> --- a/lib/utils/cppc/fdt_cppc_rpmi.c
> +++ b/lib/utils/cppc/fdt_cppc_rpmi.c
> @@ -59,6 +59,11 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> u8 db_val_u8 = 0;
> u16 db_val_u16 = 0;
> u32 db_val_u32 = 0;
> +#if __riscv_xlen != 32
> + u64 db_val_u64 = 0;
> +#else
> + u32 db_val_u32_hi = 0;
> +#endif
>
> switch (cppc->fc_db_width) {
> case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_8:
> @@ -84,14 +89,12 @@ static void rpmi_cppc_fc_db_trigger(struct rpmi_cppc *cppc)
> break;
> case RPMI_CPPC_FAST_CHANNEL_DB_WIDTH_64:
> #if __riscv_xlen != 32
> - u64 db_val_u64 = 0;
> db_val_u64 = readq((void *)cppc->fc_db_addr);
> db_val_u64 = cppc->fc_db_setmask |
> (db_val_u64 & cppc->fc_db_preservemask);
>
> writeq(db_val_u64, (void *)cppc->fc_db_addr);
> #else
> - u32 db_val_u32_hi = 0;
> db_val_u32 = readl((void *)cppc->fc_db_addr);
> db_val_u32_hi = readl((void *)(cppc->fc_db_addr + 4));
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option
2024-12-11 0:11 ` Samuel Holland
@ 2024-12-15 6:04 ` Anup Patel
0 siblings, 0 replies; 9+ messages in thread
From: Anup Patel @ 2024-12-15 6:04 UTC (permalink / raw)
To: opensbi
On Wed, Dec 11, 2024 at 5:41?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-12-09 11:23 PM, Anup Patel wrote:
> > Enabling V-extension using -march option causes OpenSBI boot-time
> > hang with LLVM compiler.
> >
> > As a work-around, don't enable V-extension using -march option and
> > instead use a custom OpenSBI specific define inform availability of
> > V-extension to lib/sbi/sbi_trap_v_ldst.c.
> >
> > Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > Makefile | 8 ++++----
> > lib/sbi/sbi_trap_v_ldst.c | 5 +++--
> > 2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 5ac95a0f..1b949a9f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -190,7 +190,7 @@ CC_SUPPORT_STRICT_ALIGN := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib
> > CC_SUPPORT_ZICSR_ZIFENCEI := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)imafd_zicsr_zifencei -x c /dev/null -o /dev/null 2>&1 | grep -e "zicsr" -e "zifencei" > /dev/null && echo n || echo y)
> >
> > # Check whether the assembler and the compiler support the Vector extension
> > -CC_SUPPORT_VECT := $(shell echo | $(CC) -dM -E -march=rv$(OPENSBI_CC_XLEN)gv - | grep -q riscv.*vector && echo y || echo n)
> > +CC_SUPPORT_VECT := $(shell $(CC) $(CLANG_TARGET) $(RELAX_FLAG) -nostdlib -march=rv$(OPENSBI_CC_XLEN)gv -dM -E -x c /dev/null 2>&1 | grep -q riscv.*vector && echo y || echo n)
> >
> > ifneq ($(OPENSBI_LD_PIE),y)
> > $(error Your linker does not support creating PIEs, opensbi requires this.)
> > @@ -298,9 +298,6 @@ endif
> > ifndef PLATFORM_RISCV_ISA
> > ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1)
> > PLATFORM_RISCV_ISA := rv$(PLATFORM_RISCV_XLEN)imafdc
> > - ifeq ($(CC_SUPPORT_VECT), y)
> > - PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)v
> > - endif
> > ifeq ($(CC_SUPPORT_ZICSR_ZIFENCEI), y)
> > PLATFORM_RISCV_ISA := $(PLATFORM_RISCV_ISA)_zicsr_zifencei
> > endif
> > @@ -363,6 +360,9 @@ GENFLAGS += $(firmware-genflags-y)
> > CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib -fno-stack-protector -fno-strict-aliasing -ffunction-sections -fdata-sections
> > CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> > # Optionally supported flags
> > +ifeq ($(CC_SUPPORT_VECT),y)
> > +CFLAGS += -DOPENSBI_CC_SUPPORT_VECT
>
> nit: Since you're changing this, could you please spell out "VECTOR"? The
> abbreviation isn't great for greppability.
I have taken care of this at the time of merging this patch.
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Applied this patch to the riscv/opensbi repo.
Regards,
Anup
>
> > +endif
> > ifeq ($(CC_SUPPORT_SAVE_RESTORE),y)
> > CFLAGS += -mno-save-restore
> > endif
> > diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> > index 9929215c..75b79baa 100644
> > --- a/lib/sbi/sbi_trap_v_ldst.c
> > +++ b/lib/sbi/sbi_trap_v_ldst.c
> > @@ -17,7 +17,8 @@
> > #include <sbi/sbi_unpriv.h>
> > #include <sbi/sbi_trap.h>
> >
> > -#ifdef __riscv_vector
> > +#ifdef OPENSBI_CC_SUPPORT_VECT
> > +
> > #define VLEN_MAX 65536
> >
> > static inline void set_vreg(ulong vlenb, ulong which,
> > @@ -340,4 +341,4 @@ int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> > {
> > return 0;
> > }
> > -#endif /* __riscv_vector */
> > +#endif /* OPENSBI_CC_SUPPORT_VECT */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-12-15 6:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10 5:23 [PATCH v2 0/2] OpenSBI LLVM related fixes Anup Patel
2024-12-10 5:23 ` [PATCH v2 1/2] lib: utils/fdt_cppc_rpmi: Fix compile error with LLVM Anup Patel
2024-12-11 0:05 ` Samuel Holland
2024-12-11 9:59 ` Xiang W
2024-12-15 6:02 ` Anup Patel
2024-12-15 6:03 ` Anup Patel
2024-12-10 5:23 ` [PATCH v2 2/2] Makefile: Don't enable V-extension using -march option Anup Patel
2024-12-11 0:11 ` Samuel Holland
2024-12-15 6:04 ` Anup Patel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox