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

[Paran Lee <p4ranlee@xxxxxxxxx>]

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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx>
>>>>> ---
>>>>>  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
>>>>>
>>>>>
>>>>
>>
>