| Message ID | 20171101170014.20931-1-salyzyn@android.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/11/17 16:58, Mark Salyzyn wrote: > Cross compiling to aarch32 (for vdso32) using clang correctly > identifies that (the unused) write_sysreg inline asm directive is > illegal in that architectural context: > > arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm > write_sysreg(cntkctl, cntkctl_el1); > ^ > arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' > : : "rZ" (__val)); > ^ > > GCC normally checks for correctness everywhere. But uniquely for > unused asm, will optimize out and suppress the error report. It sounds more like some paths are wrong in the compat vDSO build if it's pulling in this header in the first place - nothing in this file is relevant to AArch32. Robin. > Signed-off-by: Mark Salyzyn <salyzyn@android.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Christoffer Dall <cdall@linaro.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Stefan Traby <stefan@hello-penguin.com> > Cc: Dave Martin <Dave.Martin@arm.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > --- > arch/arm64/include/asm/sysreg.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index f707fed5886f..a7b61c9327db 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -492,11 +492,15 @@ asm( > * The "Z" constraint normally means a zero immediate, but when combined with > * the "%x0" template means XZR. > */ > +#if defined(__aarch64__) > #define write_sysreg(v, r) do { \ > u64 __val = (u64)(v); \ > asm volatile("msr " __stringify(r) ", %x0" \ > : : "rZ" (__val)); \ > } while (0) > +#else > +#define write_sysreg(v, r) BUG() > +#endif > > /* > * For registers without architectural names, or simply unsupported by >
On 11/01/2017 10:14 AM, Robin Murphy wrote: > On 01/11/17 16:58, Mark Salyzyn wrote: >> Cross compiling to aarch32 (for vdso32) using clang correctly >> identifies that (the unused) write_sysreg inline asm directive is >> illegal in that architectural context: >> >> arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm >> write_sysreg(cntkctl, cntkctl_el1); >> ^ >> arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' >> : : "rZ" (__val)); >> ^ >> >> GCC normally checks for correctness everywhere. But uniquely for >> unused asm, will optimize out and suppress the error report. > It sounds more like some paths are wrong in the compat vDSO build if > it's pulling in this header in the first place - nothing in this file is > relevant to AArch32. > > Robin. > And yet, when you CROSS_COMPILE_ARM32 a vdso32, you have no choice but to utilize the arm64 headers since they contain all the relevant kernel structures and environment. asm/arch_timer.h (remember we are using arm instructions to access arch64 timers) linux/time.h (really only for struct timespec()) asm/processor.h (eg: cpu_relax()) pull in a _lot_ of architectural related cruft that always somehow picks up asm/sysreg.h somewhere in the multitude of includes to fulfill some unused inline's needs. -- Mark
On Wed, Nov 01, 2017 at 10:49:00AM -0700, Mark Salyzyn wrote: > On 11/01/2017 10:14 AM, Robin Murphy wrote: > > On 01/11/17 16:58, Mark Salyzyn wrote: > > > Cross compiling to aarch32 (for vdso32) using clang correctly > > > identifies that (the unused) write_sysreg inline asm directive is > > > illegal in that architectural context: > > > > > > arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm > > > write_sysreg(cntkctl, cntkctl_el1); > > > ^ > > > arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' > > > : : "rZ" (__val)); > > > ^ > > > > > > GCC normally checks for correctness everywhere. But uniquely for > > > unused asm, will optimize out and suppress the error report. > > It sounds more like some paths are wrong in the compat vDSO build if > > it's pulling in this header in the first place - nothing in this file is > > relevant to AArch32. > > > > Robin. > > > And yet, when you CROSS_COMPILE_ARM32 a vdso32, you have no choice but to > utilize the arm64 headers since they contain all the relevant kernel > structures and environment. This itself is the underlying issue. When building the compat VDSO, we must ensure that we only include headers that make sense for 32-bit arm. If the build system can't do that today, we should rework it so that it can. Anything else cannot be a complete fix. > asm/arch_timer.h (remember we are using arm instructions to access arch64 > timers) > > linux/time.h (really only for struct timespec()) > > asm/processor.h (eg: cpu_relax()) > > pull in a _lot_ of architectural related cruft that always somehow picks up > asm/sysreg.h somewhere in the multitude of includes to fulfill some unused > inline's needs. ... these are just the particular symptoms this problem results in today. Thanks, Mark.
On 11/01/2017 10:56 AM, Mark Rutland wrote: > On Wed, Nov 01, 2017 at 10:49:00AM -0700, Mark Salyzyn wrote: >> On 11/01/2017 10:14 AM, Robin Murphy wrote: >>> On 01/11/17 16:58, Mark Salyzyn wrote: >>>> Cross compiling to aarch32 (for vdso32) using clang correctly >>>> identifies that (the unused) write_sysreg inline asm directive is >>>> illegal in that architectural context: >>>> >>>> arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm >>>> write_sysreg(cntkctl, cntkctl_el1); >>>> ^ >>>> arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' >>>> : : "rZ" (__val)); >>>> ^ >>>> >>>> GCC normally checks for correctness everywhere. But uniquely for >>>> unused asm, will optimize out and suppress the error report. >>> It sounds more like some paths are wrong in the compat vDSO build if >>> it's pulling in this header in the first place - nothing in this file is >>> relevant to AArch32. >>> >>> Robin. >>> >> And yet, when you CROSS_COMPILE_ARM32 a vdso32, you have no choice but to >> utilize the arm64 headers since they contain all the relevant kernel >> structures and environment. > This itself is the underlying issue. > > When building the compat VDSO, we must ensure that we only include > headers that make sense for 32-bit arm. > > If the build system can't do that today, we should rework it so that it > can. Anything else cannot be a complete fix. > >> asm/arch_timer.h (remember we are using arm instructions to access arch64 >> timers) >> >> linux/time.h (really only for struct timespec()) >> >> asm/processor.h (eg: cpu_relax()) >> >> pull in a _lot_ of architectural related cruft that always somehow picks up >> asm/sysreg.h somewhere in the multitude of includes to fulfill some unused >> inline's needs. > ... these are just the particular symptoms this problem results in > today. > > Thanks, > Mark. Ok, will attack it and see just how bad the scale is... These ones trouble me ... there was no intent to pick up the wrong pieces. In file included from ./include/linux/time.h:4: In file included from ./include/linux/cache.h:5: In file included from ./arch/arm64/include/asm/cache.h:19: In file included from ./arch/arm64/include/asm/cachetype.h:19: In file included from ./arch/arm64/include/asm/cputype.h:94: ./arch/arm64/include/asm/sysreg.h:303:2: error: invalid input constraint 'rZ' in asm In file included from ./include/linux/hrtimer.h:18: In file included from ./include/linux/rbtree.h:34: In file included from ./include/linux/rcupdate.h:47: In file included from ./include/linux/ktime.h:25: In file included from ./include/linux/jiffies.h:8: In file included from ./include/linux/timex.h:65: In file included from ./arch/arm64/include/asm/timex.h:19: ./arch/arm64/include/asm/arch_timer.h:83:4: error: invalid input constraint 'rZ' in asm write_sysreg(cntkctl, cntkctl_el1); ^ ./arch/arm64/include/asm/sysreg.h:278:12: note: expanded from macro 'write_sysreg' : : "rZ" (__val)); -- Mark
On 11/01/2017 11:16 AM, Mark Salyzyn wrote: > On 11/01/2017 10:56 AM, Mark Rutland wrote: >> On Wed, Nov 01, 2017 at 10:49:00AM -0700, Mark Salyzyn wrote: >>> On 11/01/2017 10:14 AM, Robin Murphy wrote: >>>> On 01/11/17 16:58, Mark Salyzyn wrote: >>>>> Cross compiling to aarch32 (for vdso32) using clang correctly >>>>> identifies that (the unused) write_sysreg inline asm directive is >>>>> illegal in that architectural context: >>>>> >>>>> arch/arm64/include/asm/arch_timer.h: error: invalid input >>>>> constraint 'rZ' in asm >>>>> write_sysreg(cntkctl, cntkctl_el1); >>>>> ^ >>>>> arch/arm64/include/asm/sysreg.h: note: expanded from macro >>>>> 'write_sysreg' >>>>> : : "rZ" (__val)); >>>>> ^ >>>>> >>>>> GCC normally checks for correctness everywhere. But uniquely for >>>>> unused asm, will optimize out and suppress the error report. >>>> It sounds more like some paths are wrong in the compat vDSO build if >>>> it's pulling in this header in the first place - nothing in this >>>> file is >>>> relevant to AArch32. >>>> >>>> Robin. >>>> >>> And yet, when you CROSS_COMPILE_ARM32 a vdso32, you have no choice >>> but to >>> utilize the arm64 headers since they contain all the relevant kernel >>> structures and environment. >> This itself is the underlying issue. >> >> When building the compat VDSO, we must ensure that we only include >> headers that make sense for 32-bit arm. >> >> If the build system can't do that today, we should rework it so that it >> can. Anything else cannot be a complete fix. >> >>> asm/arch_timer.h (remember we are using arm instructions to access >>> arch64 >>> timers) >>> >>> linux/time.h (really only for struct timespec()) >>> >>> asm/processor.h (eg: cpu_relax()) >>> >>> pull in a _lot_ of architectural related cruft that always somehow >>> picks up >>> asm/sysreg.h somewhere in the multitude of includes to fulfill some >>> unused >>> inline's needs. >> ... these are just the particular symptoms this problem results in >> today. >> >> Thanks, >> Mark. > > Ok, will attack it and see just how bad the scale is... > > . . . > > -- Mark > > Scoped, not as bad as I thought, but there is some open-coded evilness to fix: 1) linux/jiffies.h can not be included, replace with open coding: #include <asm/param.h> #define TICK_NSEC TICK_NSEC ((NSEC_PER_SEC+HZ/2)/HZ) 2) linux/hrtimer.h can not be included, replace with open coding (must have above to work): #define LOW_RES_NSEC TICK_NSEC #ifdef CONFIG_HIGH_RES_TIMERS # define HIGH_RES_NSEC 1 # define MONOTONIC_RES_NSEC HIGH_RES_NSEC #else # define MONOTONIC_RES_NSEC LOW_RES_NSEC #endif 3) asm/processor.h can not be included, replace with open coding: static inline void cpu_relax(void) { asm volatile("yield" ::: "memory"); } 4) linux/time.h can not be included, replace with open coding: #include <linux/compiler.h> #include <linux/math64.h> #include <uapi/linux/time.h> #define NSEC_PER_SEC 1000000000L static __always_inline void timespec_add_ns(struct timespec *a, u64 ns) { a->tv_sec += __iter_div_u64_rem(a->tv_nsec + ns, NSEC_PER_SEC, &ns); a->tv_nsec = ns; } I am at a loss to determine if there is an acceptable way to split off the open-coding. For instance asm-generic/processor.h (for cpu_relax()), uapi/linux/hrtimer.h and uapi/linux/jiffies.h for the #defines (uapi is a bad choice, flipping coin?). The time open-coding is probably OK given that they have not changed since near the limits of git history. -- Mark
On 01/11/17 16:58, Mark Salyzyn wrote: > Cross compiling to aarch32 (for vdso32) using clang correctly > identifies that (the unused) write_sysreg inline asm directive is > illegal in that architectural context: > > arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm > write_sysreg(cntkctl, cntkctl_el1); > ^ > arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' > : : "rZ" (__val)); > ^ > > GCC normally checks for correctness everywhere. But uniquely for > unused asm, will optimize out and suppress the error report. -ENOTABUG. Please fix the build system instead (why on Earth are you using arm64 headers for an Aarch32 binary?), and don't introduce stuff that is making understanding the kernel more difficult to understand and maintain. Thanks, M.
On Wed, Nov 01, 2017 at 01:32:50PM -0700, Mark Salyzyn wrote: > On 11/01/2017 11:16 AM, Mark Salyzyn wrote: > > On 11/01/2017 10:56 AM, Mark Rutland wrote: > > > On Wed, Nov 01, 2017 at 10:49:00AM -0700, Mark Salyzyn wrote: > > > > On 11/01/2017 10:14 AM, Robin Murphy wrote: > > > > > On 01/11/17 16:58, Mark Salyzyn wrote: > > > > > > Cross compiling to aarch32 (for vdso32) using clang correctly > > > > > > identifies that (the unused) write_sysreg inline asm directive is > > > > > > illegal in that architectural context: > > > > > > > > > > > > arch/arm64/include/asm/arch_timer.h: error: invalid > > > > > > input constraint 'rZ' in asm > > > > > > write_sysreg(cntkctl, cntkctl_el1); > > > > > > ^ > > > > > > arch/arm64/include/asm/sysreg.h: note: expanded from > > > > > > macro 'write_sysreg' > > > > > > : : "rZ" (__val)); > > > > > > ^ > > > > > > > > > > > > GCC normally checks for correctness everywhere. But uniquely for > > > > > > unused asm, will optimize out and suppress the error report. > > > > > It sounds more like some paths are wrong in the compat vDSO build if > > > > > it's pulling in this header in the first place - nothing in > > > > > this file is > > > > > relevant to AArch32. > > > > > > > > > > Robin. > > > > > > > > > And yet, when you CROSS_COMPILE_ARM32 a vdso32, you have no > > > > choice but to > > > > utilize the arm64 headers since they contain all the relevant kernel > > > > structures and environment. > > > This itself is the underlying issue. > > > > > > When building the compat VDSO, we must ensure that we only include > > > headers that make sense for 32-bit arm. > > > > > > If the build system can't do that today, we should rework it so that it > > > can. Anything else cannot be a complete fix. > > Ok, will attack it and see just how bad the scale is... > Scoped, not as bad as I thought, but there is some open-coded evilness to > fix: > > 1) linux/jiffies.h can not be included, replace with open coding: [...] > 2) linux/hrtimer.h can not be included, replace with open coding (must have > above to work): [...] > 3) asm/processor.h can not be included, replace with open coding: [...] > 4) linux/time.h can not be included, replace with open coding: [...] Evidently I was not sufficiently clear. I don't think that we should try to bodge the #include directives to try to avoid including 64-bit headers. The above changes might work today, but they're *extremely* fragile. When I said that we should rework the build system above, I mean we should rework the Makefile logic to pass the 32-bit include paths into the compat vdso. That way, we can include the usual set of headers, as we'll pick up the 32-bit headers. We might have to have separate native/compat vdso data pages to make sure that the types align, but that's not the end of the world... Thanks, Mark.
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index f707fed5886f..a7b61c9327db 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -492,11 +492,15 @@ asm( * The "Z" constraint normally means a zero immediate, but when combined with * the "%x0" template means XZR. */ +#if defined(__aarch64__) #define write_sysreg(v, r) do { \ u64 __val = (u64)(v); \ asm volatile("msr " __stringify(r) ", %x0" \ : : "rZ" (__val)); \ } while (0) +#else +#define write_sysreg(v, r) BUG() +#endif /* * For registers without architectural names, or simply unsupported by
Cross compiling to aarch32 (for vdso32) using clang correctly identifies that (the unused) write_sysreg inline asm directive is illegal in that architectural context: arch/arm64/include/asm/arch_timer.h: error: invalid input constraint 'rZ' in asm write_sysreg(cntkctl, cntkctl_el1); ^ arch/arm64/include/asm/sysreg.h: note: expanded from macro 'write_sysreg' : : "rZ" (__val)); ^ GCC normally checks for correctness everywhere. But uniquely for unused asm, will optimize out and suppress the error report. Signed-off-by: Mark Salyzyn <salyzyn@android.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Christoffer Dall <cdall@linaro.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Stefan Traby <stefan@hello-penguin.com> Cc: Dave Martin <Dave.Martin@arm.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- arch/arm64/include/asm/sysreg.h | 4 ++++ 1 file changed, 4 insertions(+)