* [PATCH-next] arm: fix clang build warning in include/asm/memory.h
@ 2024-03-14 7:54 Yipeng Zou
2024-03-15 0:43 ` Nathan Chancellor
0 siblings, 1 reply; 10+ messages in thread
From: Yipeng Zou @ 2024-03-14 7:54 UTC (permalink / raw)
To: linux, nathan, ndesaulniers, morbo, justinstitt, linus.walleij,
ssantosh, linux-arm-kernel
Cc: zouyipeng
There is a build error has been founded with build in clang-15.0.4:
./arch/arm/include/asm/memory.h:358:12: error: result of comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false [-Werror, -Wtautological-type-limit-compare]
if (addr > (u32)~0)
~~~~ ^ ~~~~~~~
It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT.
Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT.
Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() functionality")
Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
---
arch/arm/include/asm/memory.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index ef2aa79ece5a..64ced9eb42ff 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void)
return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0;
}
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
#define IDMAP_INVALID_ADDR ((u32)~0)
+#endif
static inline unsigned long phys_to_idmap(phys_addr_t addr)
{
if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) {
addr += arch_phys_to_idmap_offset;
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
if (addr > (u32)~0)
addr = IDMAP_INVALID_ADDR;
+#endif
}
return addr;
}
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-14 7:54 [PATCH-next] arm: fix clang build warning in include/asm/memory.h Yipeng Zou @ 2024-03-15 0:43 ` Nathan Chancellor 2024-03-15 7:40 ` Linus Walleij 2024-03-19 3:16 ` Yipeng Zou 0 siblings, 2 replies; 10+ messages in thread From: Nathan Chancellor @ 2024-03-15 0:43 UTC (permalink / raw) To: Yipeng Zou Cc: linux, ndesaulniers, morbo, justinstitt, linus.walleij, ssantosh, linux-arm-kernel On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: > There is a build error has been founded with build in clang-15.0.4: > > ./arch/arm/include/asm/memory.h:358:12: error: result of comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false [-Werror, -Wtautological-type-limit-compare] > if (addr > (u32)~0) > ~~~~ ^ ~~~~~~~ > > It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. > > Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. > > Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() functionality") > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> > --- > arch/arm/include/asm/memory.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > index ef2aa79ece5a..64ced9eb42ff 100644 > --- a/arch/arm/include/asm/memory.h > +++ b/arch/arm/include/asm/memory.h > @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) > return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; > } > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > #define IDMAP_INVALID_ADDR ((u32)~0) > +#endif > > static inline unsigned long phys_to_idmap(phys_addr_t addr) > { > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > addr += arch_phys_to_idmap_offset; > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > if (addr > (u32)~0) > addr = IDMAP_INVALID_ADDR; > +#endif > } > return addr; > } > -- > 2.34.1 > Would it be better to avoid the ifdef's here and just use IS_ENABLED()? I can't see why this would not work for avoiding that warning. diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h index ef2aa79ece5a..fbff07bc3b28 100644 --- a/arch/arm/include/asm/memory.h +++ b/arch/arm/include/asm/memory.h @@ -353,7 +353,7 @@ static inline unsigned long phys_to_idmap(phys_addr_t addr) { if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { addr += arch_phys_to_idmap_offset; - if (addr > (u32)~0) + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) addr = IDMAP_INVALID_ADDR; } return addr; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 0:43 ` Nathan Chancellor @ 2024-03-15 7:40 ` Linus Walleij 2024-03-15 10:08 ` Russell King (Oracle) 2024-03-19 3:13 ` Yipeng Zou 2024-03-19 3:16 ` Yipeng Zou 1 sibling, 2 replies; 10+ messages in thread From: Linus Walleij @ 2024-03-15 7:40 UTC (permalink / raw) To: Nathan Chancellor Cc: Yipeng Zou, linux, ndesaulniers, morbo, justinstitt, ssantosh, linux-arm-kernel On Fri, Mar 15, 2024 at 1:43 AM Nathan Chancellor <nathan@kernel.org> wrote: > On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: > > There is a build error has been founded with build in clang-15.0.4: > > > > ./arch/arm/include/asm/memory.h:358:12: error: result of comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false [-Werror, -Wtautological-type-limit-compare] > > if (addr > (u32)~0) > > ~~~~ ^ ~~~~~~~ > > > > It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. > > > > Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. > > > > Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() functionality") > > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> > > --- > > arch/arm/include/asm/memory.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > > index ef2aa79ece5a..64ced9eb42ff 100644 > > --- a/arch/arm/include/asm/memory.h > > +++ b/arch/arm/include/asm/memory.h > > @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) > > return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; > > } > > > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > > #define IDMAP_INVALID_ADDR ((u32)~0) > > +#endif > > > > static inline unsigned long phys_to_idmap(phys_addr_t addr) > > { > > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > > addr += arch_phys_to_idmap_offset; > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > > if (addr > (u32)~0) > > addr = IDMAP_INVALID_ADDR; > > +#endif > > } > > return addr; > > } > > -- > > 2.34.1 > > > > Would it be better to avoid the ifdef's here and just use IS_ENABLED()? > I can't see why this would not work for avoiding that warning. > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > index ef2aa79ece5a..fbff07bc3b28 100644 > --- a/arch/arm/include/asm/memory.h > +++ b/arch/arm/include/asm/memory.h > @@ -353,7 +353,7 @@ static inline unsigned long phys_to_idmap(phys_addr_t addr) > { > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > addr += arch_phys_to_idmap_offset; > - if (addr > (u32)~0) > + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) > addr = IDMAP_INVALID_ADDR; +1 and I would probably use this as well: <linux/limits.h> #define IDMAP_INVALID_ADDR PHYS_ADDR_MAX if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > PHYS_ADDR_MAX) addr = IDMAP_INVALID_ADDR; Because then it is clear what is going on: we are capping to the max physical address. PHYS_ADDR_MAX is defined to (~(phys_addr_t)0) which on ARM is (~(u32)0). Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 7:40 ` Linus Walleij @ 2024-03-15 10:08 ` Russell King (Oracle) 2024-03-15 13:16 ` Linus Walleij 2024-03-19 3:13 ` Yipeng Zou 1 sibling, 1 reply; 10+ messages in thread From: Russell King (Oracle) @ 2024-03-15 10:08 UTC (permalink / raw) To: Linus Walleij Cc: Nathan Chancellor, Yipeng Zou, ndesaulniers, morbo, justinstitt, ssantosh, linux-arm-kernel On Fri, Mar 15, 2024 at 08:40:28AM +0100, Linus Walleij wrote: > +1 and I would probably use this as well: > > <linux/limits.h> > > #define IDMAP_INVALID_ADDR PHYS_ADDR_MAX > > if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > PHYS_ADDR_MAX) > addr = IDMAP_INVALID_ADDR; > > Because then it is clear what is going on: we are capping to the max physical > address. > > PHYS_ADDR_MAX is defined to (~(phys_addr_t)0) which on > ARM is (~(u32)0). ... which is _not_ the same as (u32)~0, so using PHYS_ADDR_MAX is not appropriate. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 10:08 ` Russell King (Oracle) @ 2024-03-15 13:16 ` Linus Walleij 2024-03-15 14:46 ` Russell King (Oracle) 0 siblings, 1 reply; 10+ messages in thread From: Linus Walleij @ 2024-03-15 13:16 UTC (permalink / raw) To: Russell King (Oracle) Cc: Nathan Chancellor, Yipeng Zou, ndesaulniers, morbo, justinstitt, ssantosh, linux-arm-kernel On Fri, Mar 15, 2024 at 11:08 AM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > On Fri, Mar 15, 2024 at 08:40:28AM +0100, Linus Walleij wrote: > > +1 and I would probably use this as well: > > > > <linux/limits.h> > > > > #define IDMAP_INVALID_ADDR PHYS_ADDR_MAX > > > > if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > PHYS_ADDR_MAX) > > addr = IDMAP_INVALID_ADDR; > > > > Because then it is clear what is going on: we are capping to the max physical > > address. > > > > PHYS_ADDR_MAX is defined to (~(phys_addr_t)0) which on > > ARM is (~(u32)0). > > ... which is _not_ the same as (u32)~0, so using PHYS_ADDR_MAX is not > appropriate. Attention to detail! Thanks Russell. However U32_MAX is #define U32_MAX ((u32)~0U) So that should work and also convey the attention I think? Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 13:16 ` Linus Walleij @ 2024-03-15 14:46 ` Russell King (Oracle) 0 siblings, 0 replies; 10+ messages in thread From: Russell King (Oracle) @ 2024-03-15 14:46 UTC (permalink / raw) To: Linus Walleij Cc: Nathan Chancellor, Yipeng Zou, ndesaulniers, morbo, justinstitt, ssantosh, linux-arm-kernel On Fri, Mar 15, 2024 at 02:16:23PM +0100, Linus Walleij wrote: > On Fri, Mar 15, 2024 at 11:08 AM Russell King (Oracle) > <linux@armlinux.org.uk> wrote: > > On Fri, Mar 15, 2024 at 08:40:28AM +0100, Linus Walleij wrote: > > > +1 and I would probably use this as well: > > > > > > <linux/limits.h> > > > > > > #define IDMAP_INVALID_ADDR PHYS_ADDR_MAX > > > > > > if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > PHYS_ADDR_MAX) > > > addr = IDMAP_INVALID_ADDR; > > > > > > Because then it is clear what is going on: we are capping to the max physical > > > address. > > > > > > PHYS_ADDR_MAX is defined to (~(phys_addr_t)0) which on > > > ARM is (~(u32)0). > > > > ... which is _not_ the same as (u32)~0, so using PHYS_ADDR_MAX is not > > appropriate. > > Attention to detail! Thanks Russell. > > However U32_MAX is > #define U32_MAX ((u32)~0U) > > So that should work and also convey the attention I think? If you want to use that to define IDMAP_INVALID_ADDR then yes. If you're thinking about getting rid of IDMAP_INVALID_ADDR, then I think that would reduce code readability, because the idea here is that addresses above the 32-bit virtual space are invalid for IDMAP, and just using U32_MAX is a rather opaque way to convey that. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 7:40 ` Linus Walleij 2024-03-15 10:08 ` Russell King (Oracle) @ 2024-03-19 3:13 ` Yipeng Zou 1 sibling, 0 replies; 10+ messages in thread From: Yipeng Zou @ 2024-03-19 3:13 UTC (permalink / raw) To: Linus Walleij, Nathan Chancellor Cc: linux, ndesaulniers, morbo, justinstitt, ssantosh, linux-arm-kernel 在 2024/3/15 15:40, Linus Walleij 写道: > On Fri, Mar 15, 2024 at 1:43 AM Nathan Chancellor <nathan@kernel.org> wrote: >> On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: >>> There is a build error has been founded with build in clang-15.0.4: >>> >>> ./arch/arm/include/asm/memory.h:358:12: error: result of comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false [-Werror, -Wtautological-type-limit-compare] >>> if (addr > (u32)~0) >>> ~~~~ ^ ~~~~~~~ >>> >>> It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. >>> >>> Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. >>> >>> Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() functionality") >>> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> >>> --- >>> arch/arm/include/asm/memory.h | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h >>> index ef2aa79ece5a..64ced9eb42ff 100644 >>> --- a/arch/arm/include/asm/memory.h >>> +++ b/arch/arm/include/asm/memory.h >>> @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) >>> return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; >>> } >>> >>> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >>> #define IDMAP_INVALID_ADDR ((u32)~0) >>> +#endif >>> >>> static inline unsigned long phys_to_idmap(phys_addr_t addr) >>> { >>> if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { >>> addr += arch_phys_to_idmap_offset; >>> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >>> if (addr > (u32)~0) >>> addr = IDMAP_INVALID_ADDR; >>> +#endif >>> } >>> return addr; >>> } >>> -- >>> 2.34.1 >>> >> Would it be better to avoid the ifdef's here and just use IS_ENABLED()? >> I can't see why this would not work for avoiding that warning. >> >> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h >> index ef2aa79ece5a..fbff07bc3b28 100644 >> --- a/arch/arm/include/asm/memory.h >> +++ b/arch/arm/include/asm/memory.h >> @@ -353,7 +353,7 @@ static inline unsigned long phys_to_idmap(phys_addr_t addr) >> { >> if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { >> addr += arch_phys_to_idmap_offset; >> - if (addr > (u32)~0) >> + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) >> addr = IDMAP_INVALID_ADDR; > +1 and I would probably use this as well: > > <linux/limits.h> > > #define IDMAP_INVALID_ADDR PHYS_ADDR_MAX > > if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > PHYS_ADDR_MAX) > addr = IDMAP_INVALID_ADDR; > > Because then it is clear what is going on: we are capping to the max physical > address. Yes, indeed, It's clearer to indicate what is going on. Will fix it in that way. > PHYS_ADDR_MAX is defined to (~(phys_addr_t)0) which on > ARM is (~(u32)0). > > Yours, > Linus Walleij > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Regards, Yipeng Zou _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-15 0:43 ` Nathan Chancellor 2024-03-15 7:40 ` Linus Walleij @ 2024-03-19 3:16 ` Yipeng Zou 2024-03-19 3:38 ` Yipeng Zou 1 sibling, 1 reply; 10+ messages in thread From: Yipeng Zou @ 2024-03-19 3:16 UTC (permalink / raw) To: Nathan Chancellor Cc: linux, ndesaulniers, morbo, justinstitt, linus.walleij, ssantosh, linux-arm-kernel 在 2024/3/15 8:43, Nathan Chancellor 写道: > On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: >> There is a build error has been founded with build in clang-15.0.4: >> >> ./arch/arm/include/asm/memory.h:358:12: error: result of comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false [-Werror, -Wtautological-type-limit-compare] >> if (addr > (u32)~0) >> ~~~~ ^ ~~~~~~~ >> >> It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. >> >> Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. >> >> Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() functionality") >> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> >> --- >> arch/arm/include/asm/memory.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h >> index ef2aa79ece5a..64ced9eb42ff 100644 >> --- a/arch/arm/include/asm/memory.h >> +++ b/arch/arm/include/asm/memory.h >> @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) >> return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; >> } >> >> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >> #define IDMAP_INVALID_ADDR ((u32)~0) >> +#endif >> >> static inline unsigned long phys_to_idmap(phys_addr_t addr) >> { >> if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { >> addr += arch_phys_to_idmap_offset; >> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >> if (addr > (u32)~0) >> addr = IDMAP_INVALID_ADDR; >> +#endif >> } >> return addr; >> } >> -- >> 2.34.1 >> > Would it be better to avoid the ifdef's here and just use IS_ENABLED()? > I can't see why this would not work for avoiding that warning. > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > index ef2aa79ece5a..fbff07bc3b28 100644 > --- a/arch/arm/include/asm/memory.h > +++ b/arch/arm/include/asm/memory.h > @@ -353,7 +353,7 @@ static inline unsigned long phys_to_idmap(phys_addr_t addr) > { > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > addr += arch_phys_to_idmap_offset; > - if (addr > (u32)~0) > + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) > addr = IDMAP_INVALID_ADDR; > } > return addr; It absolutely works for it and yes, It's a better way to avoiding that. Will fix it in that way. > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Regards, Yipeng Zou _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-19 3:16 ` Yipeng Zou @ 2024-03-19 3:38 ` Yipeng Zou 2024-03-19 14:46 ` Nathan Chancellor 0 siblings, 1 reply; 10+ messages in thread From: Yipeng Zou @ 2024-03-19 3:38 UTC (permalink / raw) To: Nathan Chancellor Cc: linux, ndesaulniers, morbo, justinstitt, linus.walleij, ssantosh, linux-arm-kernel 在 2024/3/19 11:16, Yipeng Zou 写道: > > 在 2024/3/15 8:43, Nathan Chancellor 写道: >> On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: >>> There is a build error has been founded with build in clang-15.0.4: >>> >>> ./arch/arm/include/asm/memory.h:358:12: error: result of comparison >>> "phys addr_t’ (aka 'unsigned int’) > 4294967295 is always false >>> [-Werror, -Wtautological-type-limit-compare] >>> if (addr > (u32)~0) >>> ~~~~ ^ ~~~~~~~ >>> >>> It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. >>> >>> Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. >>> >>> Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() >>> functionality") >>> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> >>> --- >>> arch/arm/include/asm/memory.h | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/arch/arm/include/asm/memory.h >>> b/arch/arm/include/asm/memory.h >>> index ef2aa79ece5a..64ced9eb42ff 100644 >>> --- a/arch/arm/include/asm/memory.h >>> +++ b/arch/arm/include/asm/memory.h >>> @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) >>> return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; >>> } >>> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >>> #define IDMAP_INVALID_ADDR ((u32)~0) >>> +#endif >>> static inline unsigned long phys_to_idmap(phys_addr_t addr) >>> { >>> if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { >>> addr += arch_phys_to_idmap_offset; >>> +#ifdef CONFIG_PHYS_ADDR_T_64BIT >>> if (addr > (u32)~0) >>> addr = IDMAP_INVALID_ADDR; >>> +#endif >>> } >>> return addr; >>> } >>> -- >>> 2.34.1 >>> >> Would it be better to avoid the ifdef's here and just use IS_ENABLED()? >> I can't see why this would not work for avoiding that warning. >> >> diff --git a/arch/arm/include/asm/memory.h >> b/arch/arm/include/asm/memory.h >> index ef2aa79ece5a..fbff07bc3b28 100644 >> --- a/arch/arm/include/asm/memory.h >> +++ b/arch/arm/include/asm/memory.h >> @@ -353,7 +353,7 @@ static inline unsigned long >> phys_to_idmap(phys_addr_t addr) >> { >> if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { >> addr += arch_phys_to_idmap_offset; >> - if (addr > (u32)~0) >> + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) >> addr = IDMAP_INVALID_ADDR; >> } >> return addr; > > It absolutely works for it and yes, It's a better way to avoiding that. > > Will fix it in that way. > But I notice that, this way can not silence it when compile, it only can avoid it in running time. So, maybe we still need to silence it in ifdef's. Other than that, IDMAP_INVALID_ADDR was used in other place, need to keep it defined. diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h index 158ad13e9f6a..7143147cd7cc 100644 --- a/arch/arm/include/asm/memory.h +++ b/arch/arm/include/asm/memory.h @@ -351,8 +351,10 @@ static inline unsigned long phys_to_idmap(phys_addr_t addr) { if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { addr += arch_phys_to_idmap_offset; +#ifdef CONFIG_PHYS_ADDR_T_64BIT if (addr > (u32)~0) addr = IDMAP_INVALID_ADDR; +#endif } return addr; } >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Regards, Yipeng Zou _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h 2024-03-19 3:38 ` Yipeng Zou @ 2024-03-19 14:46 ` Nathan Chancellor 0 siblings, 0 replies; 10+ messages in thread From: Nathan Chancellor @ 2024-03-19 14:46 UTC (permalink / raw) To: Yipeng Zou Cc: linux, ndesaulniers, morbo, justinstitt, linus.walleij, ssantosh, linux-arm-kernel On Tue, Mar 19, 2024 at 11:38:02AM +0800, Yipeng Zou wrote: > > 在 2024/3/19 11:16, Yipeng Zou 写道: > > > > 在 2024/3/15 8:43, Nathan Chancellor 写道: > > > On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote: > > > > There is a build error has been founded with build in clang-15.0.4: > > > > > > > > ./arch/arm/include/asm/memory.h:358:12: error: result of > > > > comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is > > > > always false [-Werror, -Wtautological-type-limit-compare] > > > > if (addr > (u32)~0) > > > > ~~~~ ^ ~~~~~~~ > > > > > > > > It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT. > > > > > > > > Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT. > > > > > > > > Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap() > > > > functionality") > > > > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com> > > > > --- > > > > arch/arm/include/asm/memory.h | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/arch/arm/include/asm/memory.h > > > > b/arch/arm/include/asm/memory.h > > > > index ef2aa79ece5a..64ced9eb42ff 100644 > > > > --- a/arch/arm/include/asm/memory.h > > > > +++ b/arch/arm/include/asm/memory.h > > > > @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void) > > > > return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0; > > > > } > > > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > > > > #define IDMAP_INVALID_ADDR ((u32)~0) > > > > +#endif > > > > static inline unsigned long phys_to_idmap(phys_addr_t addr) > > > > { > > > > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > > > > addr += arch_phys_to_idmap_offset; > > > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > > > > if (addr > (u32)~0) > > > > addr = IDMAP_INVALID_ADDR; > > > > +#endif > > > > } > > > > return addr; > > > > } > > > > -- > > > > 2.34.1 > > > > > > > Would it be better to avoid the ifdef's here and just use IS_ENABLED()? > > > I can't see why this would not work for avoiding that warning. > > > > > > diff --git a/arch/arm/include/asm/memory.h > > > b/arch/arm/include/asm/memory.h > > > index ef2aa79ece5a..fbff07bc3b28 100644 > > > --- a/arch/arm/include/asm/memory.h > > > +++ b/arch/arm/include/asm/memory.h > > > @@ -353,7 +353,7 @@ static inline unsigned long > > > phys_to_idmap(phys_addr_t addr) > > > { > > > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > > > addr += arch_phys_to_idmap_offset; > > > - if (addr > (u32)~0) > > > + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0) > > > addr = IDMAP_INVALID_ADDR; > > > } > > > return addr; > > > > It absolutely works for it and yes, It's a better way to avoiding that. > > > > Will fix it in that way. > > > But I notice that, this way can not silence it when compile, it only can > avoid it in running time. Yeah, I actually tested my suggestion and saw the same thing. Sometimes diagnostics will be hidden by '0 &&' but it seems like that is not the case here. It is not the end of the world, I think that diff looks fine for the issue at hand. > So, maybe we still need to silence it in ifdef's. > > Other than that, IDMAP_INVALID_ADDR was used in other place, need to keep it > defined. > > diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h > index 158ad13e9f6a..7143147cd7cc 100644 > --- a/arch/arm/include/asm/memory.h > +++ b/arch/arm/include/asm/memory.h > @@ -351,8 +351,10 @@ static inline unsigned long phys_to_idmap(phys_addr_t > addr) > { > if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) { > addr += arch_phys_to_idmap_offset; > +#ifdef CONFIG_PHYS_ADDR_T_64BIT > if (addr > (u32)~0) > addr = IDMAP_INVALID_ADDR; > +#endif > } > return addr; > } > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > -- > Regards, > Yipeng Zou > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-19 14:47 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-14 7:54 [PATCH-next] arm: fix clang build warning in include/asm/memory.h Yipeng Zou 2024-03-15 0:43 ` Nathan Chancellor 2024-03-15 7:40 ` Linus Walleij 2024-03-15 10:08 ` Russell King (Oracle) 2024-03-15 13:16 ` Linus Walleij 2024-03-15 14:46 ` Russell King (Oracle) 2024-03-19 3:13 ` Yipeng Zou 2024-03-19 3:16 ` Yipeng Zou 2024-03-19 3:38 ` Yipeng Zou 2024-03-19 14:46 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).