[PATCH] xen/riscv: Fix build with GCC 10

[PATCH] xen/riscv: Fix build with GCC 10

[Andrew Cooper]

  riscv64-linux-gnu-gcc -MMD -MP -MF arch/riscv/.early_printk.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include ./include/xen/config.h -Wa,--strip-local-absolute -g -mabi=lp64  -I./include -I./arch/riscv/include -march=rv64gc -mstrict-align -mcmodel=medany   -c arch/riscv/early_printk.c -o arch/riscv/early_printk.o
  arch/riscv/early_printk.c:18:2: error: #error "early_*() can be called from head.S with MMU-off"
     18 | #error "early_*() can be called from head.S with MMU-off"
        |  ^~~~~

  $ riscv64-linux-gnu-gcc --version
  riscv64-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110

The binary is otherwise correct, so remove the incorrect check.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Bob Eshleman <bobbyeshleman@gmail.com>
CC: Alistair Francis <alistair.francis@wdc.com>
CC: Connor Davis <connojdavis@gmail.com>

I was honestly hoping to leave this to some poor sole in the future.

But the irony of this check, after all the argument it caused, breaking the
very case it was trying to enforce, speaks volumes.
---
 xen/arch/riscv/early_printk.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
index dfe4ad77e25d..b66a11f1bc1a 100644
--- a/xen/arch/riscv/early_printk.c
+++ b/xen/arch/riscv/early_printk.c
@@ -7,17 +7,6 @@
 #include <asm/early_printk.h>
 #include <asm/sbi.h>
 
-/*
- * When the MMU is off during early boot, any C function called has to
- * use PC-relative rather than absolute address because the physical address
- * may not match the virtual address.
- *
- * To guarantee PC-relative address cmodel=medany should be used
- */
-#ifndef __riscv_cmodel_medany
-#error "early_*() can be called from head.S with MMU-off"
-#endif
-
 /*
  * TODO:
  *   sbi_console_putchar is already planned for deprecation
-- 
2.30.2

Re: [PATCH] xen/riscv: Fix build with GCC 10

[Julien Grall]

[-- Attachment #1: Type: text/plain, Size: 2843 bytes --]

On Wed, 15 Mar 2023 at 11:52, Andrew Cooper <andrew.cooper3@citrix.com>
wrote:

>   riscv64-linux-gnu-gcc -MMD -MP -MF arch/riscv/.early_printk.o.d
> -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
> -Wdeclaration-after-statement -Wno-unused-but-set-variable
> -Wno-unused-local-typedefs   -O1 -fno-omit-frame-pointer -nostdinc
> -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla
> -pipe -D__XEN__ -include ./include/xen/config.h -Wa,--strip-local-absolute
> -g -mabi=lp64  -I./include -I./arch/riscv/include -march=rv64gc
> -mstrict-align -mcmodel=medany   -c arch/riscv/early_printk.c -o
> arch/riscv/early_printk.o
>   arch/riscv/early_printk.c:18:2: error: #error "early_*() can be called
> from head.S with MMU-off"
>      18 | #error "early_*() can be called from head.S with MMU-off"
>         |  ^~~~~
>
>   $ riscv64-linux-gnu-gcc --version
>   riscv64-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
>
> The binary is otherwise correct, so remove the incorrect check.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Bob Eshleman <bobbyeshleman@gmail.com>
> CC: Alistair Francis <alistair.francis@wdc.com>
> CC: Connor Davis <connojdavis@gmail.com>
>
> I was honestly hoping to leave this to some poor sole in the future.
>
> But the irony of this check, after all the argument it caused, breaking the
> very case it was trying to enforce, speaks volumes.


I appreciate you dislike this check. But I think the fact it gets triggered
proved its effectiveness rather than the other way around.

From a brief look online, the define is meant to be present with medany. So
I think some digging would be worth here as this may indicate another
latent issue either on the command line or the compiler.

(I would like to point out that Linux has the exact same check. So why
would it work there but not in Xen?)

Cheers,




> ——
>  xen/arch/riscv/early_printk.c | 11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
> index dfe4ad77e25d..b66a11f1bc1a 100644
> --- a/xen/arch/riscv/early_printk.c
> +++ b/xen/arch/riscv/early_printk.c
> @@ -7,17 +7,6 @@
>  #include <asm/early_printk.h>
>  #include <asm/sbi.h>
>
> -/*
> - * When the MMU is off during early boot, any C function called has to
> - * use PC-relative rather than absolute address because the physical
> address
> - * may not match the virtual address.
> - *
> - * To guarantee PC-relative address cmodel=medany should be used
> - */
> -#ifndef __riscv_cmodel_medany
> -#error "early_*() can be called from head.S with MMU-off"
> -#endif
> -
>  /*
>   * TODO:
>   *   sbi_console_putchar is already planned for deprecation
> --
> 2.30.2
>
>
>

[-- Attachment #2: Type: text/html, Size: 4013 bytes --]

Re: [PATCH] xen/riscv: Fix build with GCC 10

[Jan Beulich]

On 15.03.2023 19:51, Andrew Cooper wrote:
>   riscv64-linux-gnu-gcc -MMD -MP -MF arch/riscv/.early_printk.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include ./include/xen/config.h -Wa,--strip-local-absolute -g -mabi=lp64  -I./include -I./arch/riscv/include -march=rv64gc -mstrict-align -mcmodel=medany   -c arch/riscv/early_printk.c -o arch/riscv/early_printk.o
>   arch/riscv/early_printk.c:18:2: error: #error "early_*() can be called from head.S with MMU-off"
>      18 | #error "early_*() can be called from head.S with MMU-off"
>         |  ^~~~~
> 
>   $ riscv64-linux-gnu-gcc --version
>   riscv64-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
> 
> The binary is otherwise correct, so remove the incorrect check.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I'm with Julien here - this needs further explaining: The compiler (even 8.2)
clearly provides this definition with the given set of command line options,
as supported by trying it out om godbolt. So there must be more to this -
could be a bad patch in Debian's build, could be some odd interaction of
command line options which for whatever reason only triggers with certain
builds, or about anything else.

Jan

Re: [PATCH] xen/riscv: Fix build with GCC 10

[Julien Grall]

Hi,

On 16/03/2023 07:21, Jan Beulich wrote:
> On 15.03.2023 19:51, Andrew Cooper wrote:
>>    riscv64-linux-gnu-gcc -MMD -MP -MF arch/riscv/.early_printk.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include ./include/xen/config.h -Wa,--strip-local-absolute -g -mabi=lp64  -I./include -I./arch/riscv/include -march=rv64gc -mstrict-align -mcmodel=medany   -c arch/riscv/early_printk.c -o arch/riscv/early_printk.o
>>    arch/riscv/early_printk.c:18:2: error: #error "early_*() can be called from head.S with MMU-off"
>>       18 | #error "early_*() can be called from head.S with MMU-off"
>>          |  ^~~~~
>>
>>    $ riscv64-linux-gnu-gcc --version
>>    riscv64-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
>>
>> The binary is otherwise correct, so remove the incorrect check.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> I'm with Julien here - this needs further explaining: The compiler (even 8.2)
> clearly provides this definition with the given set of command line options,
> as supported by trying it out om godbolt. So there must be more to this -
> could be a bad patch in Debian's build, could be some odd interaction of
> command line options which for whatever reason only triggers with certain
> builds, or about anything else.

I have tried to build RISC-v on my Debian Bulleyes machine today. The 
build failed with the same reason.

The Linux kernel (which has the exact same check) could be built. So I 
decided to dig why this happens.

I got a below code compiled when both -mcmodel=medany and -fno-pie are 
passed to the GCC command line:

#ifndef __riscv_cmodel_medany
#error "medany not enabled"
#endif

I am guessing that's because GCC on Debian has PIE enabled by default.
Now, Xen is meant to be built with -fno-pie, but this is not on the 
command line. So does any flags from EMBEDDED_EXTRA_CFLAGS.

Skimming through xen-devel, there is already a patch from Oleksii to add 
the cflags (see [1]). So with that in place, this patch becomes 
unnecessary to build Xen RISC-v on Debian.

Cheers,

[1] 
https://lore.kernel.org/all/2785518800dce64fafb3096480a5ae4c4e026bcb.1678970065.git.oleksii.kurochko@gmail.com/

-- 
Julien Grall