All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 16:58 ` Mark Salyzyn
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 16:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Salyzyn, Catalin Marinas, Will Deacon, Christoffer Dall,
	Mark Rutland, Marc Zyngier, Suzuki K Poulose, Stefan Traby,
	Dave Martin, linux-arm-kernel

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(+)

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
-- 
2.15.0.403.gc27cc4dac6-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 16:58 ` Mark Salyzyn
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 16:58 UTC (permalink / raw)
  To: linux-arm-kernel

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 at lists.infradead.org
Cc: linux-kernel at 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
-- 
2.15.0.403.gc27cc4dac6-goog

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 16:58 ` Mark Salyzyn
@ 2017-11-01 17:14   ` Robin Murphy
  -1 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2017-11-01 17:14 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel
  Cc: Mark Rutland, Christoffer Dall, Stefan Traby, Suzuki K Poulose,
	Marc Zyngier, Catalin Marinas, Will Deacon, Dave Martin,
	linux-arm-kernel

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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 17:14   ` Robin Murphy
  0 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2017-11-01 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

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 at lists.infradead.org
> Cc: linux-kernel at 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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 17:14   ` Robin Murphy
@ 2017-11-01 17:49     ` Mark Salyzyn
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 17:49 UTC (permalink / raw)
  To: Robin Murphy, linux-kernel
  Cc: Mark Rutland, Christoffer Dall, Stefan Traby, Suzuki K Poulose,
	Marc Zyngier, Catalin Marinas, Will Deacon, Dave Martin,
	linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 17:49     ` Mark Salyzyn
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 17:49 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 17:49     ` Mark Salyzyn
@ 2017-11-01 17:56       ` Mark Rutland
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2017-11-01 17:56 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Robin Murphy, linux-kernel, Christoffer Dall, Stefan Traby,
	Suzuki K Poulose, Marc Zyngier, Catalin Marinas, Will Deacon,
	Dave Martin, linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 17:56       ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2017-11-01 17:56 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 17:56       ` Mark Rutland
@ 2017-11-01 18:16         ` Mark Salyzyn
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 18:16 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Robin Murphy, linux-kernel, Christoffer Dall, Stefan Traby,
	Suzuki K Poulose, Marc Zyngier, Catalin Marinas, Will Deacon,
	Dave Martin, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 18:16         ` Mark Salyzyn
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 18:16 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 18:16         ` Mark Salyzyn
@ 2017-11-01 20:32           ` Mark Salyzyn
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 20:32 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Robin Murphy, linux-kernel, Christoffer Dall, Stefan Traby,
	Suzuki K Poulose, Marc Zyngier, Catalin Marinas, Will Deacon,
	Dave Martin, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-01 20:32           ` Mark Salyzyn
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Salyzyn @ 2017-11-01 20:32 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 16:58 ` Mark Salyzyn
@ 2017-11-02 10:05   ` Marc Zyngier
  -1 siblings, 0 replies; 16+ messages in thread
From: Marc Zyngier @ 2017-11-02 10:05 UTC (permalink / raw)
  To: Mark Salyzyn, linux-kernel
  Cc: Catalin Marinas, Will Deacon, Christoffer Dall, Mark Rutland,
	Suzuki K Poulose, Stefan Traby, Dave Martin, linux-arm-kernel

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.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-02 10:05   ` Marc Zyngier
  0 siblings, 0 replies; 16+ messages in thread
From: Marc Zyngier @ 2017-11-02 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

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.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] arm64: write_sysreg asm illegal for aarch32
  2017-11-01 20:32           ` Mark Salyzyn
@ 2017-11-02 10:08             ` Mark Rutland
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2017-11-02 10:08 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: Robin Murphy, linux-kernel, Christoffer Dall, Stefan Traby,
	Suzuki K Poulose, Marc Zyngier, Catalin Marinas, Will Deacon,
	Dave Martin, linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] arm64: write_sysreg asm illegal for aarch32
@ 2017-11-02 10:08             ` Mark Rutland
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2017-11-02 10:08 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-11-02 10:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 16:58 [PATCH] arm64: write_sysreg asm illegal for aarch32 Mark Salyzyn
2017-11-01 16:58 ` Mark Salyzyn
2017-11-01 17:14 ` Robin Murphy
2017-11-01 17:14   ` Robin Murphy
2017-11-01 17:49   ` Mark Salyzyn
2017-11-01 17:49     ` Mark Salyzyn
2017-11-01 17:56     ` Mark Rutland
2017-11-01 17:56       ` Mark Rutland
2017-11-01 18:16       ` Mark Salyzyn
2017-11-01 18:16         ` Mark Salyzyn
2017-11-01 20:32         ` Mark Salyzyn
2017-11-01 20:32           ` Mark Salyzyn
2017-11-02 10:08           ` Mark Rutland
2017-11-02 10:08             ` Mark Rutland
2017-11-02 10:05 ` Marc Zyngier
2017-11-02 10:05   ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.