xen/arm: silence ambiguous integer casting warning error

[PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Paran Lee 2 years, 7 months ago

GCC with "-g -Wall -Wextra" option throws warning message as below:

error: comparison of integer expressions of different signedness:
 ‘int’ and ‘unsigned int’ [-Werror=sign-compare]

Silence the warning by correcting the integer type.

Signed-off-by: Paran Lee <p4ranlee@gmail.com>
---
 xen/arch/arm/gic-v3.c | 5 +++--
 xen/arch/arm/setup.c  | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 3c472ed768..81ac25f528 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
     isb();
 }
 
-static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask *mask,
+static u16 gicv3_compute_target_list(unsigned int *base_cpu,
+                                     const struct cpumask *mask,
                                      uint64_t cluster_id)
 {
     int cpu = *base_cpu;
@@ -953,7 +954,7 @@ out:
 
 static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t *cpumask)
 {
-    int cpu = 0;
+    unsigned int cpu = 0;
     uint64_t val;
 
     for_each_cpu(cpu, cpumask)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index d5d0792ed4..5ab2aaecaf 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -862,7 +862,7 @@ void __init start_xen(unsigned long boot_phys_offset,
                       unsigned long fdt_paddr)
 {
     size_t fdt_size;
-    int cpus, i;
+    unsigned int cpus, i;
     const char *cmdline;
     struct bootmodule *xen_bootmodule;
     struct domain *d;
-- 
2.25.1

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Stefano Stabellini 2 years, 7 months ago

On Wed, 20 Apr 2022, Paran Lee wrote:

> GCC with "-g -Wall -Wextra" option throws warning message as below:
> 
> error: comparison of integer expressions of different signedness:
>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
> 
> Silence the warning by correcting the integer type.
> 
> Signed-off-by: Paran Lee <p4ranlee@gmail.com>

Thanks for the cleanup!


> ---
>  xen/arch/arm/gic-v3.c | 5 +++--
>  xen/arch/arm/setup.c  | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 3c472ed768..81ac25f528 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
>      isb();
>  }
>  
> -static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask *mask,
> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
> +                                     const struct cpumask *mask,
>                                       uint64_t cluster_id)
>  {
>      int cpu = *base_cpu;

I think we need to change cpu to unsigned int too to make it consistent


> @@ -953,7 +954,7 @@ out:
>  
>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t *cpumask)
>  {
> -    int cpu = 0;
> +    unsigned int cpu = 0;
>      uint64_t val;
>  
>      for_each_cpu(cpu, cpumask)
>
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed4..5ab2aaecaf 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>                        unsigned long fdt_paddr)
>  {
>      size_t fdt_size;
> -    int cpus, i;
> +    unsigned int cpus, i;
>      const char *cmdline;
>      struct bootmodule *xen_bootmodule;
>      struct domain *d;

I can see that we should change i to unsigned int.

cpus could cause a comparison between signed and unsigned int here:

  if ( (num_online_cpus() < cpus) && !cpu_online(i) )

num_online_cpus returns an int

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Julien Grall 2 years, 7 months ago

Hi,

On Tue, 19 Apr 2022, 15:41 Paran Lee, <p4ranlee@gmail.com> wrote:

> GCC with "-g -Wall -Wextra" option throws warning message as below:


Which version of the compiler? Also you specify the exact cflags, did you
tweak Xen?


> error: comparison of integer expressions of different signedness:
>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>

GCC should give you a line/file. Can you provide it?

Cheers,


> Silence the warning by correcting the integer type.
>
> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
> ---
>  xen/arch/arm/gic-v3.c | 5 +++--
>  xen/arch/arm/setup.c  | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 3c472ed768..81ac25f528 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
>      isb();
>  }
>
> -static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask
> *mask,
> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
> +                                     const struct cpumask *mask,
>                                       uint64_t cluster_id)
>  {
>      int cpu = *base_cpu;
> @@ -953,7 +954,7 @@ out:
>
>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t
> *cpumask)
>  {
> -    int cpu = 0;
> +    unsigned int cpu = 0;
>      uint64_t val;
>
>      for_each_cpu(cpu, cpumask)
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d5d0792ed4..5ab2aaecaf 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>                        unsigned long fdt_paddr)
>  {
>      size_t fdt_size;
> -    int cpus, i;
> +    unsigned int cpus, i;
>      const char *cmdline;
>      struct bootmodule *xen_bootmodule;
>      struct domain *d;
> --
> 2.25.1
>
>

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Paran Lee 2 years, 7 months ago

Hi, Julien Grall.

Thank you for checking it out. I'm sorry I forgot to attach the make log
as well.

My build configuration (include CFLGAS)

export ARCH=arm64
export XEN_TARGET_ARCH=arm64
export $(dpkg-architecture -aarm64);
export CROSS_COMPILE=aarch64-linux-gnu-
export CFLAGS="-g -Wall -Wextra -Wno-unused-parameter"

And i did     make dist-xen

my arm64 compiler information are here.
gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)

~/xen$ aarch64-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=aarch64-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/aarch64-linux-gnu/9/lto-wrapper
Target: aarch64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
9.4.0-1ubuntu1~20.04.1'
--with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,gm2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-9 --enable-shared
--enable-linker-build-id --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-libquadmath --disable-libquadmath-support --enable-plugin
--enable-default-pie --with-system-zlib --without-target-system-zlib
--enable-libpth-m2 --enable-multiarch --enable-fix-cortex-a53-843419
--disable-werror --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=aarch64-linux-gnu
--program-prefix=aarch64-linux-gnu-
--includedir=/usr/aarch64-linux-gnu/include
Thread model: posix
gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)

In arch/arm/gic-v3.c files.

arch/arm/gic-v3.c: In function ‘gicv3_compute_target_list’:
arch/arm/gic-v3.c:926:17: error: comparison of integer expressions of
different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  926 |     while ( cpu < nr_cpu_ids )
      |                 ^
arch/arm/gic-v3.c:936:18: error: comparison of integer expressions of
different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  936 |         if ( cpu == nr_cpu_ids )
      |                  ^~                           ^

In arch/arm/setup.c files.

arch/arm/setup.c: In function ‘start_xen’:
./include/xen/cpumask.h:374:13: error: comparison of integer expressions
of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  374 |       (cpu) < nr_cpu_ids;  \
      |             ^
./include/xen/cpumask.h:459:36: note: in expansion of macro ‘for_each_cpu’
  459 | #define for_each_present_cpu(cpu)  for_each_cpu(cpu,
&cpu_present_map)
      |                                    ^~~~~~~~~~~~
arch/arm/setup.c:989:5: note: in expansion of macro ‘for_each_present_cpu’
  989 |     for_each_present_cpu ( i )
      |     ^~~~~~~~~~~~~~~~~~~~             ^

Thank you!

2022-04-20 오전 12:50에 Julien Grall 이(가) 쓴 글:
> Hi,
> 
> On Tue, 19 Apr 2022, 15:41 Paran Lee, <p4ranlee@gmail.com> wrote:
> 
>> GCC with "-g -Wall -Wextra" option throws warning message as below:
> 
> 
> Which version of the compiler? Also you specify the exact cflags, did you
> tweak Xen?
> 
> 
>> error: comparison of integer expressions of different signedness:
>>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>
> 
> GCC should give you a line/file. Can you provide it?
> 
> Cheers,
> 
> 
>> Silence the warning by correcting the integer type.
>>
>> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
>> ---
>>  xen/arch/arm/gic-v3.c | 5 +++--
>>  xen/arch/arm/setup.c  | 2 +-
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 3c472ed768..81ac25f528 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
>>      isb();
>>  }
>>
>> -static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask
>> *mask,
>> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
>> +                                     const struct cpumask *mask,
>>                                       uint64_t cluster_id)
>>  {
>>      int cpu = *base_cpu;
>> @@ -953,7 +954,7 @@ out:
>>
>>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t
>> *cpumask)
>>  {
>> -    int cpu = 0;
>> +    unsigned int cpu = 0;
>>      uint64_t val;
>>
>>      for_each_cpu(cpu, cpumask)
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index d5d0792ed4..5ab2aaecaf 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>>                        unsigned long fdt_paddr)
>>  {
>>      size_t fdt_size;
>> -    int cpus, i;
>> +    unsigned int cpus, i;
>>      const char *cmdline;
>>      struct bootmodule *xen_bootmodule;
>>      struct domain *d;
>> --
>> 2.25.1
>>
>>
>

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Paran Lee 2 years, 7 months ago

Debugging with GDB from head.S with QEMU runtime was very convenient for
analysis(linux). so I have trying it in Xen. As I built it.

Wouldn't it be helpful if I fixed the code little by little?

2022-04-20 오전 1:31에 Paran Lee 이(가) 쓴 글:
> Hi, Julien Grall.
> 
> Thank you for checking it out. I'm sorry I forgot to attach the make log
> as well.
> 
> My build configuration (include CFLGAS)
> 
> export ARCH=arm64
> export XEN_TARGET_ARCH=arm64
> export $(dpkg-architecture -aarm64);
> export CROSS_COMPILE=aarch64-linux-gnu-
> export CFLAGS="-g -Wall -Wextra -Wno-unused-parameter"
> 
> And i did     make dist-xen
> 
> my arm64 compiler information are here.
> gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
> 
> ~/xen$ aarch64-linux-gnu-gcc -v
> Using built-in specs.
> COLLECT_GCC=aarch64-linux-gnu-gcc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/aarch64-linux-gnu/9/lto-wrapper
> Target: aarch64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
> 9.4.0-1ubuntu1~20.04.1'
> --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs
> --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,gm2 --prefix=/usr
> --with-gcc-major-version-only --program-suffix=-9 --enable-shared
> --enable-linker-build-id --libexecdir=/usr/lib
> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
> --enable-nls --with-sysroot=/ --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
> --disable-libquadmath --disable-libquadmath-support --enable-plugin
> --enable-default-pie --with-system-zlib --without-target-system-zlib
> --enable-libpth-m2 --enable-multiarch --enable-fix-cortex-a53-843419
> --disable-werror --enable-checking=release --build=x86_64-linux-gnu
> --host=x86_64-linux-gnu --target=aarch64-linux-gnu
> --program-prefix=aarch64-linux-gnu-
> --includedir=/usr/aarch64-linux-gnu/include
> Thread model: posix
> gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
> 
> In arch/arm/gic-v3.c files.
> 
> arch/arm/gic-v3.c: In function ‘gicv3_compute_target_list’:
> arch/arm/gic-v3.c:926:17: error: comparison of integer expressions of
> different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>   926 |     while ( cpu < nr_cpu_ids )
>       |                 ^
> arch/arm/gic-v3.c:936:18: error: comparison of integer expressions of
> different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>   936 |         if ( cpu == nr_cpu_ids )
>       |                  ^~                           ^
> 
> In arch/arm/setup.c files.
> 
> arch/arm/setup.c: In function ‘start_xen’:
> ./include/xen/cpumask.h:374:13: error: comparison of integer expressions
> of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>   374 |       (cpu) < nr_cpu_ids;  \
>       |             ^
> ./include/xen/cpumask.h:459:36: note: in expansion of macro ‘for_each_cpu’
>   459 | #define for_each_present_cpu(cpu)  for_each_cpu(cpu,
> &cpu_present_map)
>       |                                    ^~~~~~~~~~~~
> arch/arm/setup.c:989:5: note: in expansion of macro ‘for_each_present_cpu’
>   989 |     for_each_present_cpu ( i )
>       |     ^~~~~~~~~~~~~~~~~~~~             ^
> 
> Thank you!
> 
> 2022-04-20 오전 12:50에 Julien Grall 이(가) 쓴 글:
>> Hi,
>>
>> On Tue, 19 Apr 2022, 15:41 Paran Lee, <p4ranlee@gmail.com> wrote:
>>
>>> GCC with "-g -Wall -Wextra" option throws warning message as below:
>>
>>
>> Which version of the compiler? Also you specify the exact cflags, did you
>> tweak Xen?
>>
>>
>>> error: comparison of integer expressions of different signedness:
>>>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>>
>>
>> GCC should give you a line/file. Can you provide it?
>>
>> Cheers,
>>
>>
>>> Silence the warning by correcting the integer type.
>>>
>>> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
>>> ---
>>>  xen/arch/arm/gic-v3.c | 5 +++--
>>>  xen/arch/arm/setup.c  | 2 +-
>>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>>> index 3c472ed768..81ac25f528 100644
>>> --- a/xen/arch/arm/gic-v3.c
>>> +++ b/xen/arch/arm/gic-v3.c
>>> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
>>>      isb();
>>>  }
>>>
>>> -static u16 gicv3_compute_target_list(int *base_cpu, const struct cpumask
>>> *mask,
>>> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
>>> +                                     const struct cpumask *mask,
>>>                                       uint64_t cluster_id)
>>>  {
>>>      int cpu = *base_cpu;
>>> @@ -953,7 +954,7 @@ out:
>>>
>>>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t
>>> *cpumask)
>>>  {
>>> -    int cpu = 0;
>>> +    unsigned int cpu = 0;
>>>      uint64_t val;
>>>
>>>      for_each_cpu(cpu, cpumask)
>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>> index d5d0792ed4..5ab2aaecaf 100644
>>> --- a/xen/arch/arm/setup.c
>>> +++ b/xen/arch/arm/setup.c
>>> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>                        unsigned long fdt_paddr)
>>>  {
>>>      size_t fdt_size;
>>> -    int cpus, i;
>>> +    unsigned int cpus, i;
>>>      const char *cmdline;
>>>      struct bootmodule *xen_bootmodule;
>>>      struct domain *d;
>>> --
>>> 2.25.1
>>>
>>>
>>

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Julien Grall 2 years, 7 months ago

Hi,

On Tue, 19 Apr 2022, 17:09 Paran Lee, <p4ranlee@gmail.com> wrote:

> Debugging with GDB from head.S with QEMU runtime was very convenient for
> analysis(linux). so I have trying it in Xen. As I built it.
>

I don't understand how this is related to adding extra cflags. Can you
clarify it?


> Wouldn't it be helpful if I fixed the code little by little?


I am all to make Xen more robust. However, you are now implicitly casting a
signed int to unsigned int. I am not convinced this is a good move.

I think it would be better to fix the other side of the equation or
properly modify the function return.

Cheers,


> 2022-04-20 오전 1:31에 Paran Lee 이(가) 쓴 글:
> > Hi, Julien Grall.
> >
> > Thank you for checking it out. I'm sorry I forgot to attach the make log
> > as well.
> >
> > My build configuration (include CFLGAS)
> >
> > export ARCH=arm64
> > export XEN_TARGET_ARCH=arm64
> > export $(dpkg-architecture -aarm64);
> > export CROSS_COMPILE=aarch64-linux-gnu-
> > export CFLAGS="-g -Wall -Wextra -Wno-unused-parameter"
> >
> > And i did     make dist-xen
> >
> > my arm64 compiler information are here.
> > gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
> >
> > ~/xen$ aarch64-linux-gnu-gcc -v
> > Using built-in specs.
> > COLLECT_GCC=aarch64-linux-gnu-gcc
> > COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/aarch64-linux-gnu/9/lto-wrapper
> > Target: aarch64-linux-gnu
> > Configured with: ../src/configure -v --with-pkgversion='Ubuntu
> > 9.4.0-1ubuntu1~20.04.1'
> > --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs
> > --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,gm2 --prefix=/usr
> > --with-gcc-major-version-only --program-suffix=-9 --enable-shared
> > --enable-linker-build-id --libexecdir=/usr/lib
> > --without-included-gettext --enable-threads=posix --libdir=/usr/lib
> > --enable-nls --with-sysroot=/ --enable-clocale=gnu
> > --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> > --with-default-libstdcxx-abi=new --enable-gnu-unique-object
> > --disable-libquadmath --disable-libquadmath-support --enable-plugin
> > --enable-default-pie --with-system-zlib --without-target-system-zlib
> > --enable-libpth-m2 --enable-multiarch --enable-fix-cortex-a53-843419
> > --disable-werror --enable-checking=release --build=x86_64-linux-gnu
> > --host=x86_64-linux-gnu --target=aarch64-linux-gnu
> > --program-prefix=aarch64-linux-gnu-
> > --includedir=/usr/aarch64-linux-gnu/include
> > Thread model: posix
> > gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
> >
> > In arch/arm/gic-v3.c files.
> >
> > arch/arm/gic-v3.c: In function ‘gicv3_compute_target_list’:
> > arch/arm/gic-v3.c:926:17: error: comparison of integer expressions of
> > different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
> >   926 |     while ( cpu < nr_cpu_ids )
> >       |                 ^
> > arch/arm/gic-v3.c:936:18: error: comparison of integer expressions of
> > different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
> >   936 |         if ( cpu == nr_cpu_ids )
> >       |                  ^~                           ^
> >
> > In arch/arm/setup.c files.
> >
> > arch/arm/setup.c: In function ‘start_xen’:
> > ./include/xen/cpumask.h:374:13: error: comparison of integer expressions
> > of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
> >   374 |       (cpu) < nr_cpu_ids;  \
> >       |             ^
> > ./include/xen/cpumask.h:459:36: note: in expansion of macro
> ‘for_each_cpu’
> >   459 | #define for_each_present_cpu(cpu)  for_each_cpu(cpu,
> > &cpu_present_map)
> >       |                                    ^~~~~~~~~~~~
> > arch/arm/setup.c:989:5: note: in expansion of macro
> ‘for_each_present_cpu’
> >   989 |     for_each_present_cpu ( i )
> >       |     ^~~~~~~~~~~~~~~~~~~~             ^
> >
> > Thank you!
> >
> > 2022-04-20 오전 12:50에 Julien Grall 이(가) 쓴 글:
> >> Hi,
> >>
> >> On Tue, 19 Apr 2022, 15:41 Paran Lee, <p4ranlee@gmail.com> wrote:
> >>
> >>> GCC with "-g -Wall -Wextra" option throws warning message as below:
> >>
> >>
> >> Which version of the compiler? Also you specify the exact cflags, did
> you
> >> tweak Xen?
> >>
> >>
> >>> error: comparison of integer expressions of different signedness:
> >>>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
> >>>
> >>
> >> GCC should give you a line/file. Can you provide it?
> >>
> >> Cheers,
> >>
> >>
> >>> Silence the warning by correcting the integer type.
> >>>
> >>> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
> >>> ---
> >>>  xen/arch/arm/gic-v3.c | 5 +++--
> >>>  xen/arch/arm/setup.c  | 2 +-
> >>>  2 files changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> >>> index 3c472ed768..81ac25f528 100644
> >>> --- a/xen/arch/arm/gic-v3.c
> >>> +++ b/xen/arch/arm/gic-v3.c
> >>> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
> >>>      isb();
> >>>  }
> >>>
> >>> -static u16 gicv3_compute_target_list(int *base_cpu, const struct
> cpumask
> >>> *mask,
> >>> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
> >>> +                                     const struct cpumask *mask,
> >>>                                       uint64_t cluster_id)
> >>>  {
> >>>      int cpu = *base_cpu;
> >>> @@ -953,7 +954,7 @@ out:
> >>>
> >>>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t
> >>> *cpumask)
> >>>  {
> >>> -    int cpu = 0;
> >>> +    unsigned int cpu = 0;
> >>>      uint64_t val;
> >>>
> >>>      for_each_cpu(cpu, cpumask)
> >>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> >>> index d5d0792ed4..5ab2aaecaf 100644
> >>> --- a/xen/arch/arm/setup.c
> >>> +++ b/xen/arch/arm/setup.c
> >>> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long
> boot_phys_offset,
> >>>                        unsigned long fdt_paddr)
> >>>  {
> >>>      size_t fdt_size;
> >>> -    int cpus, i;
> >>> +    unsigned int cpus, i;
> >>>      const char *cmdline;
> >>>      struct bootmodule *xen_bootmodule;
> >>>      struct domain *d;
> >>> --
> >>> 2.25.1
> >>>
> >>>
> >>
>

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Paran Lee 2 years, 7 months ago

Hi, Julien Grall.

Thank you! After thinking about it, I agree that the patch I suggested
is not a good way to go.

> I don't understand how this is related to adding extra cflags. Can you
> clarify it?

https://www.youtube.com/watch?v=RPgYinVQUgw

I took a short video of debugging through qemu and gdb.
I would like to try doing this with Xen in the same way.

I'm sorry I couldn't explain the debugging process well in writing. It's
not easy. :)

If I could explain the above my video in one sentence,
qemu booting arm64 kernel & gdb remote debugging with debug symbol
information.

BR,
Paran Lee


2022-04-20 오전 2:31에 Julien Grall 이(가) 쓴 글:
> Hi,
> 
> On Tue, 19 Apr 2022, 17:09 Paran Lee, <p4ranlee@gmail.com> wrote:
> 
>> Debugging with GDB from head.S with QEMU runtime was very convenient for
>> analysis(linux). so I have trying it in Xen. As I built it.
>>
> 
> I don't understand how this is related to adding extra cflags. Can you
> clarify it?
> 
> 
>> Wouldn't it be helpful if I fixed the code little by little?
> 
> 
> I am all to make Xen more robust. However, you are now implicitly casting a
> signed int to unsigned int. I am not convinced this is a good move.
> 
> I think it would be better to fix the other side of the equation or
> properly modify the function return.
> 
> Cheers,
> 
> 
>> 2022-04-20 오전 1:31에 Paran Lee 이(가) 쓴 글:
>>> Hi, Julien Grall.
>>>
>>> Thank you for checking it out. I'm sorry I forgot to attach the make log
>>> as well.
>>>
>>> My build configuration (include CFLGAS)
>>>
>>> export ARCH=arm64
>>> export XEN_TARGET_ARCH=arm64
>>> export $(dpkg-architecture -aarm64);
>>> export CROSS_COMPILE=aarch64-linux-gnu-
>>> export CFLAGS="-g -Wall -Wextra -Wno-unused-parameter"
>>>
>>> And i did     make dist-xen
>>>
>>> my arm64 compiler information are here.
>>> gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
>>>
>>> ~/xen$ aarch64-linux-gnu-gcc -v
>>> Using built-in specs.
>>> COLLECT_GCC=aarch64-linux-gnu-gcc
>>> COLLECT_LTO_WRAPPER=/usr/lib/gcc-cross/aarch64-linux-gnu/9/lto-wrapper
>>> Target: aarch64-linux-gnu
>>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>>> 9.4.0-1ubuntu1~20.04.1'
>>> --with-bugurl=file:///usr/share/doc/gcc-9/README.Bugs
>>> --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,gm2 --prefix=/usr
>>> --with-gcc-major-version-only --program-suffix=-9 --enable-shared
>>> --enable-linker-build-id --libexecdir=/usr/lib
>>> --without-included-gettext --enable-threads=posix --libdir=/usr/lib
>>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>>> --with-default-libstdcxx-abi=new --enable-gnu-unique-object
>>> --disable-libquadmath --disable-libquadmath-support --enable-plugin
>>> --enable-default-pie --with-system-zlib --without-target-system-zlib
>>> --enable-libpth-m2 --enable-multiarch --enable-fix-cortex-a53-843419
>>> --disable-werror --enable-checking=release --build=x86_64-linux-gnu
>>> --host=x86_64-linux-gnu --target=aarch64-linux-gnu
>>> --program-prefix=aarch64-linux-gnu-
>>> --includedir=/usr/aarch64-linux-gnu/include
>>> Thread model: posix
>>> gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
>>>
>>> In arch/arm/gic-v3.c files.
>>>
>>> arch/arm/gic-v3.c: In function ‘gicv3_compute_target_list’:
>>> arch/arm/gic-v3.c:926:17: error: comparison of integer expressions of
>>> different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>>   926 |     while ( cpu < nr_cpu_ids )
>>>       |                 ^
>>> arch/arm/gic-v3.c:936:18: error: comparison of integer expressions of
>>> different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>>   936 |         if ( cpu == nr_cpu_ids )
>>>       |                  ^~                           ^
>>>
>>> In arch/arm/setup.c files.
>>>
>>> arch/arm/setup.c: In function ‘start_xen’:
>>> ./include/xen/cpumask.h:374:13: error: comparison of integer expressions
>>> of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>>   374 |       (cpu) < nr_cpu_ids;  \
>>>       |             ^
>>> ./include/xen/cpumask.h:459:36: note: in expansion of macro
>> ‘for_each_cpu’
>>>   459 | #define for_each_present_cpu(cpu)  for_each_cpu(cpu,
>>> &cpu_present_map)
>>>       |                                    ^~~~~~~~~~~~
>>> arch/arm/setup.c:989:5: note: in expansion of macro
>> ‘for_each_present_cpu’
>>>   989 |     for_each_present_cpu ( i )
>>>       |     ^~~~~~~~~~~~~~~~~~~~             ^
>>>
>>> Thank you!
>>>
>>> 2022-04-20 오전 12:50에 Julien Grall 이(가) 쓴 글:
>>>> Hi,
>>>>
>>>> On Tue, 19 Apr 2022, 15:41 Paran Lee, <p4ranlee@gmail.com> wrote:
>>>>
>>>>> GCC with "-g -Wall -Wextra" option throws warning message as below:
>>>>
>>>>
>>>> Which version of the compiler? Also you specify the exact cflags, did
>> you
>>>> tweak Xen?
>>>>
>>>>
>>>>> error: comparison of integer expressions of different signedness:
>>>>>  ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
>>>>>
>>>>
>>>> GCC should give you a line/file. Can you provide it?
>>>>
>>>> Cheers,
>>>>
>>>>
>>>>> Silence the warning by correcting the integer type.
>>>>>
>>>>> Signed-off-by: Paran Lee <p4ranlee@gmail.com>
>>>>> ---
>>>>>  xen/arch/arm/gic-v3.c | 5 +++--
>>>>>  xen/arch/arm/setup.c  | 2 +-
>>>>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>>>>> index 3c472ed768..81ac25f528 100644
>>>>> --- a/xen/arch/arm/gic-v3.c
>>>>> +++ b/xen/arch/arm/gic-v3.c
>>>>> @@ -916,7 +916,8 @@ static void gicv3_hyp_disable(void)
>>>>>      isb();
>>>>>  }
>>>>>
>>>>> -static u16 gicv3_compute_target_list(int *base_cpu, const struct
>> cpumask
>>>>> *mask,
>>>>> +static u16 gicv3_compute_target_list(unsigned int *base_cpu,
>>>>> +                                     const struct cpumask *mask,
>>>>>                                       uint64_t cluster_id)
>>>>>  {
>>>>>      int cpu = *base_cpu;
>>>>> @@ -953,7 +954,7 @@ out:
>>>>>
>>>>>  static void gicv3_send_sgi_list(enum gic_sgi sgi, const cpumask_t
>>>>> *cpumask)
>>>>>  {
>>>>> -    int cpu = 0;
>>>>> +    unsigned int cpu = 0;
>>>>>      uint64_t val;
>>>>>
>>>>>      for_each_cpu(cpu, cpumask)
>>>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>>>> index d5d0792ed4..5ab2aaecaf 100644
>>>>> --- a/xen/arch/arm/setup.c
>>>>> +++ b/xen/arch/arm/setup.c
>>>>> @@ -862,7 +862,7 @@ void __init start_xen(unsigned long
>> boot_phys_offset,
>>>>>                        unsigned long fdt_paddr)
>>>>>  {
>>>>>      size_t fdt_size;
>>>>> -    int cpus, i;
>>>>> +    unsigned int cpus, i;
>>>>>      const char *cmdline;
>>>>>      struct bootmodule *xen_bootmodule;
>>>>>      struct domain *d;
>>>>> --
>>>>> 2.25.1
>>>>>
>>>>>
>>>>
>>
>

Re: [PATCH] xen/arm: silence ambiguous integer casting warning error

Posted by Julien Grall 2 years, 7 months ago

On Tue, 19 Apr 2022, 16:31 Paran Lee, <p4ranlee@gmail.com> wrote:

> Hi, Julien Grall.
>
> Thank you for checking it out. I'm sorry I forgot to attach the make log
> as well.
>

Thanks for the logs!

> My build configuration (include CFLGAS)
>
> export ARCH=arm64
> export XEN_TARGET_ARCH=arm64
> export $(dpkg-architecture -aarm64);
> export CROSS_COMPILE=aarch64-linux-gnu-
> export CFLAGS="-g -Wall -Wextra -Wno-unused-parameter"
>

May I ask why you are modifying the flags? What's your end goal?

There reason I am asking is a lot of the helper for cpumask are using
signed int. So switching only the local variables to unsigned sounds wrong
to me.

Cheers,