| Message ID | 20240528085915.1955987-2-tongtiangen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: add ARCH_HAS_COPY_MC support | expand |
Em Tue, 28 May 2024 16:59:10 +0800 Tong Tiangen <tongtiangen@huawei.com> escreveu: > x86/powerpc has it's implementation of copy_mc_to_user(), we add generic > fallback in include/linux/uaccess.h prepare for other architechures to > enable CONFIG_ARCH_HAS_COPY_MC. > > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > Acked-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/include/asm/uaccess.h | 1 + > arch/x86/include/asm/uaccess.h | 1 + > include/linux/uaccess.h | 8 ++++++++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index de10437fd206..df42e6ad647f 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -381,6 +381,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) > > return n; > } > +#define copy_mc_to_user copy_mc_to_user Such define looks weird on my eyes. What to do you want to do here? > #endif > > extern long __copy_from_user_flushcache(void *dst, const void __user *src, > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > index 0f9bab92a43d..309f2439327e 100644 > --- a/arch/x86/include/asm/uaccess.h > +++ b/arch/x86/include/asm/uaccess.h > @@ -497,6 +497,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); > > unsigned long __must_check > copy_mc_to_user(void __user *to, const void *from, unsigned len); > +#define copy_mc_to_user copy_mc_to_user Same here. > #endif > > /* > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index 3064314f4832..0dfa9241b6ee 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -205,6 +205,14 @@ copy_mc_to_kernel(void *dst, const void *src, size_t cnt) > } > #endif > > +#ifndef copy_mc_to_user > +static inline unsigned long __must_check > +copy_mc_to_user(void *dst, const void *src, size_t cnt) > +{ > + return copy_to_user(dst, src, cnt); > +} > +#endif > + > static __always_inline void pagefault_disabled_inc(void) > { > current->pagefault_disabled++; Thanks, Mauro
Em Thu, 11 Jul 2024 15:53:43 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > Em Tue, 28 May 2024 16:59:10 +0800 > Tong Tiangen <tongtiangen@huawei.com> escreveu: > > > x86/powerpc has it's implementation of copy_mc_to_user(), we add generic > > fallback in include/linux/uaccess.h prepare for other architechures to > > enable CONFIG_ARCH_HAS_COPY_MC. > > > > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> > > --- > > arch/powerpc/include/asm/uaccess.h | 1 + > > arch/x86/include/asm/uaccess.h | 1 + > > include/linux/uaccess.h | 8 ++++++++ > > 3 files changed, 10 insertions(+) > > > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > > index de10437fd206..df42e6ad647f 100644 > > --- a/arch/powerpc/include/asm/uaccess.h > > +++ b/arch/powerpc/include/asm/uaccess.h > > @@ -381,6 +381,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) > > > > return n; > > } > > +#define copy_mc_to_user copy_mc_to_user > > Such define looks weird on my eyes. What to do you want to do here? Ok, other places at uaccess.h have the same pattern. After sleeping over it, it is now clear to me the rationale: you're using an inline to enforce typecast check, as using just a macro won't be doing cast checks. The define will let to use gcc preprocessor #if/#ifdef logic to check if the symbol was defined or not, which makes sense as not all architectures have it. Clever. Patch LGTM. Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > #endif > > > > extern long __copy_from_user_flushcache(void *dst, const void __user *src, > > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > > index 0f9bab92a43d..309f2439327e 100644 > > --- a/arch/x86/include/asm/uaccess.h > > +++ b/arch/x86/include/asm/uaccess.h > > @@ -497,6 +497,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); > > > > unsigned long __must_check > > copy_mc_to_user(void __user *to, const void *from, unsigned len); > > +#define copy_mc_to_user copy_mc_to_user > > Same here. > > > #endif > > > > /* > > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > > index 3064314f4832..0dfa9241b6ee 100644 > > --- a/include/linux/uaccess.h > > +++ b/include/linux/uaccess.h > > @@ -205,6 +205,14 @@ copy_mc_to_kernel(void *dst, const void *src, size_t cnt) > > } > > #endif > > > > +#ifndef copy_mc_to_user > > +static inline unsigned long __must_check > > +copy_mc_to_user(void *dst, const void *src, size_t cnt) > > +{ > > + return copy_to_user(dst, src, cnt); > > +} > > +#endif > > + > > static __always_inline void pagefault_disabled_inc(void) > > { > > current->pagefault_disabled++; > > > > Thanks, > Mauro Thanks, Mauro
On Tue, 28 May 2024 16:59:10 +0800 Tong Tiangen <tongtiangen@huawei.com> wrote: > x86/powerpc has it's implementation of copy_mc_to_user(), we add generic > fallback in include/linux/uaccess.h prepare for other architechures to > enable CONFIG_ARCH_HAS_COPY_MC. > > Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> > Acked-by: Michael Ellerman <mpe@ellerman.id.au> Seems like a sensible approach to me given existing fallbacks in x86 if the relevant features are disabled. It may be worth exploring at some point if some of the special casing in the callers of this function can also be remove now there is a default version. There are some small differences but I've not analyzed if they matter or not. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > arch/powerpc/include/asm/uaccess.h | 1 + > arch/x86/include/asm/uaccess.h | 1 + > include/linux/uaccess.h | 8 ++++++++ > 3 files changed, 10 insertions(+) > > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index de10437fd206..df42e6ad647f 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -381,6 +381,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) > > return n; > } > +#define copy_mc_to_user copy_mc_to_user > #endif > > extern long __copy_from_user_flushcache(void *dst, const void __user *src, > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > index 0f9bab92a43d..309f2439327e 100644 > --- a/arch/x86/include/asm/uaccess.h > +++ b/arch/x86/include/asm/uaccess.h > @@ -497,6 +497,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); > > unsigned long __must_check > copy_mc_to_user(void __user *to, const void *from, unsigned len); > +#define copy_mc_to_user copy_mc_to_user > #endif > > /* > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index 3064314f4832..0dfa9241b6ee 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -205,6 +205,14 @@ copy_mc_to_kernel(void *dst, const void *src, size_t cnt) > } > #endif > > +#ifndef copy_mc_to_user > +static inline unsigned long __must_check > +copy_mc_to_user(void *dst, const void *src, size_t cnt) > +{ > + return copy_to_user(dst, src, cnt); > +} > +#endif > + > static __always_inline void pagefault_disabled_inc(void) > { > current->pagefault_disabled++;
在 2024/8/19 17:57, Jonathan Cameron 写道: > On Tue, 28 May 2024 16:59:10 +0800 > Tong Tiangen <tongtiangen@huawei.com> wrote: > >> x86/powerpc has it's implementation of copy_mc_to_user(), we add generic >> fallback in include/linux/uaccess.h prepare for other architechures to >> enable CONFIG_ARCH_HAS_COPY_MC. >> >> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com> >> Acked-by: Michael Ellerman <mpe@ellerman.id.au> > Seems like a sensible approach to me given existing fallbacks in x86 > if the relevant features are disabled. > > It may be worth exploring at some point if some of the special casing > in the callers of this function can also be remove now there > is a default version. There are some small differences but I've > not analyzed if they matter or not. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> copy_mc_to_user() and copy_to_user() have the same logic of copying the memory. The main difference is that the MC version can handle the hardware error. The implementation of MC version is related to the architecture. Therefore, when the architecture does not implement the MC version, it is logically correct to roll back to the no MC version. Thanks :) > >> --- >> arch/powerpc/include/asm/uaccess.h | 1 + >> arch/x86/include/asm/uaccess.h | 1 + >> include/linux/uaccess.h | 8 ++++++++ >> 3 files changed, 10 insertions(+) >> >> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h >> index de10437fd206..df42e6ad647f 100644 >> --- a/arch/powerpc/include/asm/uaccess.h >> +++ b/arch/powerpc/include/asm/uaccess.h >> @@ -381,6 +381,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) >> >> return n; >> } >> +#define copy_mc_to_user copy_mc_to_user >> #endif >> >> extern long __copy_from_user_flushcache(void *dst, const void __user *src, >> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h >> index 0f9bab92a43d..309f2439327e 100644 >> --- a/arch/x86/include/asm/uaccess.h >> +++ b/arch/x86/include/asm/uaccess.h >> @@ -497,6 +497,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); >> >> unsigned long __must_check >> copy_mc_to_user(void __user *to, const void *from, unsigned len); >> +#define copy_mc_to_user copy_mc_to_user >> #endif >> >> /* >> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h >> index 3064314f4832..0dfa9241b6ee 100644 >> --- a/include/linux/uaccess.h >> +++ b/include/linux/uaccess.h >> @@ -205,6 +205,14 @@ copy_mc_to_kernel(void *dst, const void *src, size_t cnt) >> } >> #endif >> >> +#ifndef copy_mc_to_user >> +static inline unsigned long __must_check >> +copy_mc_to_user(void *dst, const void *src, size_t cnt) >> +{ >> + return copy_to_user(dst, src, cnt); >> +} >> +#endif >> + >> static __always_inline void pagefault_disabled_inc(void) >> { >> current->pagefault_disabled++; > > .
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index de10437fd206..df42e6ad647f 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -381,6 +381,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n) return n; } +#define copy_mc_to_user copy_mc_to_user #endif extern long __copy_from_user_flushcache(void *dst, const void __user *src, diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 0f9bab92a43d..309f2439327e 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -497,6 +497,7 @@ copy_mc_to_kernel(void *to, const void *from, unsigned len); unsigned long __must_check copy_mc_to_user(void __user *to, const void *from, unsigned len); +#define copy_mc_to_user copy_mc_to_user #endif /* diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 3064314f4832..0dfa9241b6ee 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -205,6 +205,14 @@ copy_mc_to_kernel(void *dst, const void *src, size_t cnt) } #endif +#ifndef copy_mc_to_user +static inline unsigned long __must_check +copy_mc_to_user(void *dst, const void *src, size_t cnt) +{ + return copy_to_user(dst, src, cnt); +} +#endif + static __always_inline void pagefault_disabled_inc(void) { current->pagefault_disabled++;