On Mon, Dec 4, 2023 at 2:05 PM Luc Van Oostenryck
<luc.vanoostenryck@xxxxxxxxx> wrote:
>
> On Fri, Dec 01, 2023 at 03:10:35PM +0100, Uros Bizjak wrote:
> > > sparse warnings: (new ones prefixed by >>)
> > > kernel/bpf/percpu_freelist.c: note: in included file (through arch/x86/include/asm/preempt.h, include/linux/preempt.h, include/linux/spinlock.h, ...):
> > > arch/x86/include/asm/percpu.h:550:49: sparse: sparse: Expected ) at end of cast operator
> > > arch/x86/include/asm/percpu.h:550:49: sparse: sparse: got __seg_gs
> > > arch/x86/include/asm/percpu.h:564:33: sparse: sparse: Expected ) at end of cast operator
> > > arch/x86/include/asm/percpu.h:564:33: sparse: sparse: got __seg_gs
> >
> > sparse is too strict here. The following code is perfectly legal:
> >
>
> It's not that sparse is too strict, it's because it knows nothing about __seg_gs.
>
> What is missing is:
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 547ea1ff806e..9214f6d54e63 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -23,6 +23,7 @@
> # define __iomem __attribute__((noderef, address_space(__iomem)))
> # define __percpu __attribute__((noderef, address_space(__percpu)))
> # define __rcu __attribute__((noderef, address_space(__rcu)))
> +# define __seg_gs __attribute__((noderef, address_space(__seg_gs)))
> static inline void __chk_user_ptr(const volatile void __user *ptr) { }
> static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
> /* context/locking */
>
>
> But with this patch sparse will now complain with 'warning: dereference of noderef expression'
> which are quite legit given the noderef and the way __seg_gs is used.
> You have two choices quiet these:
> 1) If variables qualified with __seg_gs should only be dereferenced via some
> special accessor like __raw_cpu_{read,write}() the patch here above is
> correct and you need to fix the typing inside these accessors (using __force or
> casting to unsigned long like already done anyway in __my_cpu_ptr()).
> I think this is the correct solution.
> 2) If it's OK to dereference __seg_gs qualified variables anywhere in the code
> (I don't think we should), then the 'noderef' in the patch should be removed.
__seg_gs qualified variables can be referenced anywhere in the code, e.g.:
--cut here--
__seg_gs int m;
int foo (void)
{
return m + m;
}
--cut here--
so option 2) should be the correct option here.
These __seg_gs and __seg_fs qualifiers are specific to x86, so I'd put
the definitions in the x86 specific percpu header, as in the attached
patch. The patch solves the reported issue.
Thanks,
Uros.
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 0f12b2004b94..30e823f5ab55 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -95,6 +95,11 @@
#endif /* CONFIG_SMP */
+#ifdef __CHECKER__
+#define __seg_gs __attribute__((address_space(__seg_gs)))
+#define __seg_fs __attribute__((address_space(__seg_fs)))
+#endif
+
#define __my_cpu_type(var) typeof(var) __percpu_seg_override
#define __my_cpu_ptr(ptr) (__my_cpu_type(*ptr) *)(uintptr_t)(ptr)
#define __my_cpu_var(var) (*__my_cpu_ptr(&var))
