[3/3] arm64/sve: Disentangle <uapi/asm/ptrace.h> from <uapi/asm/sigcontext.h>

Currently, <uapi/asm/sigcontext.h> provides common definitions for
describing SVE context structures that are also used by the ptrace
definitions in <uapi/asm/ptrace.h>.

For this reason, a #include of <asm/sigcontext.h> was added in
ptrace.h, but it this turns out that this can interact badly with
userspace code that tries to include ptrace.h on top of the libc
headers (which may provide their own shadow definitions for
sigcontext.h).

To make the headers easier for userspace to consume, this patch
bounces the common definitions into an __SVE_* namespace and moves
them to a backend header <uapi/asm/sve_context.h> that can be
included by the other headers as appropriate.  This should allow
ptrace.h to be used alongside libc's sigcontext.h (if any) without
ill effects.

This should make the situation unambiguous: <asm/sigcontext.h> is
the header to include for the sigframe-specific definitions, while
<asm/ptrace.h> is the header to include for ptrace-specific
definitions.

To avoid conflicting with existing usage, <asm/sigcontext.h>
remains the canonical way to get the common definitions for
SVE_VQ_MIN, sve_vq_from_vl() etc., both in userspace and in the
kernel: relying on these being defined as a side effect of
including just <asm/ptrace.h> was never intended to be safe.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>

---
 arch/arm64/include/uapi/asm/ptrace.h      | 39 ++++++++++-----------
 arch/arm64/include/uapi/asm/sigcontext.h  | 56 +++++++++++++++----------------
 arch/arm64/include/uapi/asm/sve_context.h | 50 +++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 49 deletions(-)
 create mode 100644 arch/arm64/include/uapi/asm/sve_context.h

-- 
2.1.4

Hi Dave,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on v4.20-rc6 next-20181214]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Dave-Martin/arm64-sve-UAPI-Disentangle-ptrace-h-from-sigcontext-h/20181214-225154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

>> ./usr/include/asm/sve_context.h:30: found __[us]{8,16,32,64} type without #include <linux/types.h>


---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

On Sat, Dec 15, 2018 at 05:20:29PM +0800, kbuild test robot wrote:
> Hi Dave,

> 

> Thank you for the patch! Perhaps something to improve:

> 

> [auto build test WARNING on arm64/for-next/core]

> [also build test WARNING on v4.20-rc6 next-20181214]

> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

> 

> url:    https://github.com/0day-ci/linux/commits/Dave-Martin/arm64-sve-UAPI-Disentangle-ptrace-h-from-sigcontext-h/20181214-225154

> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core

> config: arm64-allmodconfig (attached as .config)

> compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0

> reproduce:

>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross

>         chmod +x ~/bin/make.cross

>         # save the attached .config to linux build tree

>         GCC_VERSION=7.2.0 make.cross ARCH=arm64 

> 

> All warnings (new ones prefixed by >>):

> 

> >> ./usr/include/asm/sve_context.h:30: found __[us]{8,16,32,64} type without #include <linux/types.h>


Since the new header is not meant to be included directly (and has a
guard to that effect), we don't strictly need to do anything here.

The way to include <asm/sve_context.h> in userspace is via
<asm/sigcontext.h> or <asm/ptrace.h>, both of which include
<linux/types.h> first.


Ironically, the type casts in sve_context.h that necessitate this
#include are probably not needed either.  I misunderstood the C
standard as indicating that if vq is a 16-bit type (which is the case
in some places) then the result of multiplying it by something may be
truncated to 16 bits.  This is not the case: the operands of * are
promoted at least to int first, and int is large enough to hold the
result of these multiplication for all valid vq values.

Removing the cast causes some otherwise __u32 expressions to become
ints (albeit with the same numeric value).

This affects semantics and code generation, so it's probably safest
not to touch these casts now given that they have already been exposed
as UAPI.

We should probably add the #include for cleanliness, but nothing should
break if we don't.

Cheers
---Dave

On 18/12/2018 12:14, Dave Martin wrote:
> On Sat, Dec 15, 2018 at 05:20:29PM +0800, kbuild test robot wrote:

>> Hi Dave,

>>

>> Thank you for the patch! Perhaps something to improve:

>>

>> [auto build test WARNING on arm64/for-next/core]

>> [also build test WARNING on v4.20-rc6 next-20181214]

>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

>>

>> url:    https://github.com/0day-ci/linux/commits/Dave-Martin/arm64-sve-UAPI-Disentangle-ptrace-h-from-sigcontext-h/20181214-225154

>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core

>> config: arm64-allmodconfig (attached as .config)

>> compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0

>> reproduce:

>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross

>>         chmod +x ~/bin/make.cross

>>         # save the attached .config to linux build tree

>>         GCC_VERSION=7.2.0 make.cross ARCH=arm64 

>>

>> All warnings (new ones prefixed by >>):

>>

>>>> ./usr/include/asm/sve_context.h:30: found __[us]{8,16,32,64} type without #include <linux/types.h>

> 

> Since the new header is not meant to be included directly (and has a

> guard to that effect), we don't strictly need to do anything here.

> 

> The way to include <asm/sve_context.h> in userspace is via

> <asm/sigcontext.h> or <asm/ptrace.h>, both of which include

> <linux/types.h> first.

> 


i think there is no need to explicitly prevent the inclusion of
the header.

it is enough to have a comment that it's not supposed to be
included by user code (so the header can be later refactored).

and then such automated header checks (or whatever other hacks
ppl do temporarily) can continue to work.

On Wed, Dec 19, 2018 at 03:11:52PM +0000, Szabolcs Nagy wrote:
> On 18/12/2018 12:14, Dave Martin wrote:

> > On Sat, Dec 15, 2018 at 05:20:29PM +0800, kbuild test robot wrote:


[...]

> >>>> ./usr/include/asm/sve_context.h:30: found __[us]{8,16,32,64} type without #include <linux/types.h>

> > 

> > Since the new header is not meant to be included directly (and has a

> > guard to that effect), we don't strictly need to do anything here.

> > 

> > The way to include <asm/sve_context.h> in userspace is via

> > <asm/sigcontext.h> or <asm/ptrace.h>, both of which include

> > <linux/types.h> first.

> > 

> 

> i think there is no need to explicitly prevent the inclusion of

> the header.

> 

> it is enough to have a comment that it's not supposed to be

> included by user code (so the header can be later refactored).

> 

> and then such automated header checks (or whatever other hacks

> ppl do temporarily) can continue to work.


The guard is in linux-next now AFAIK.

Are you saying that it's likely to break something and needs to be
removed, or it is unnecessary but harmless?

Cheers
---Dave

On 19/12/2018 15:23, Dave Martin wrote:
> On Wed, Dec 19, 2018 at 03:11:52PM +0000, Szabolcs Nagy wrote:

>> On 18/12/2018 12:14, Dave Martin wrote:

>>> On Sat, Dec 15, 2018 at 05:20:29PM +0800, kbuild test robot wrote:

> 

> [...]

> 

>>>>>> ./usr/include/asm/sve_context.h:30: found __[us]{8,16,32,64} type without #include <linux/types.h>

>>>

>>> Since the new header is not meant to be included directly (and has a

>>> guard to that effect), we don't strictly need to do anything here.

>>>

>>> The way to include <asm/sve_context.h> in userspace is via

>>> <asm/sigcontext.h> or <asm/ptrace.h>, both of which include

>>> <linux/types.h> first.

>>>

>>

>> i think there is no need to explicitly prevent the inclusion of

>> the header.

>>

>> it is enough to have a comment that it's not supposed to be

>> included by user code (so the header can be later refactored).

>>

>> and then such automated header checks (or whatever other hacks

>> ppl do temporarily) can continue to work.

> 

> The guard is in linux-next now AFAIK.

> 

> Are you saying that it's likely to break something and needs to be

> removed, or it is unnecessary but harmless?


unnecessary but harmless.

(i assumed the header check bot would want to include every
header on its own and see if there are undefined symbols, but
if it works with the guard then fine)