Re: [RESEND PATCH v5 0/1] ARM64: ACPI: Update documentation for latest specification version

[Catalin Marinas]

On Tue, Jun 14, 2016 at 08:46:26AM -0600, Al Stone wrote:
> On 06/14/2016 03:24 AM, Will Deacon wrote:
> > On Tue, Jun 14, 2016 at 10:13:31AM +0100, Will Deacon wrote:
> >> On Mon, Jun 13, 2016 at 03:41:54PM -0600, Al Stone wrote:
> >>> This is a resend only: Ping?  Last ping was 26 May; there has been zero
> >>> response since then.  Already have one ACK from Lorenzo; another from an
> >>> arm64 maintainer would be really helpful.
> >>
> >> I thought there were outstanding comments on v4 of this?
> >>
> >> http://lkml.kernel.org/r/571e699b.9090...@linaro.org
> > 
> > Hmm, that's weird. You sent a v5 on 25 April:
> > 
> > http://archive.arm.linux.org.uk/lurker/message/20160425.212126.fe36116c.en.html
> > 
> > but I can't find that in either my ARM inbox or my gmail inbox that's
> > subscribed to the lists. I guess Catalin can pick this up for 4.8.
> 
> Huh.  I have no idea why that would have happened.  I used the same
> script to send the emails for all the revisions.
> 
> Thanks for checking, Will.  If this would get picked up for 4.8, that would be
> most excellent.

I'll queue it for 4.8 (should appear in -next towards the end of this
week).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/23] all: syscall wrappers: add documentation

[Catalin Marinas]

On Thu, May 26, 2016 at 03:50:01PM +0100, Szabolcs Nagy wrote:
> On 26/05/16 15:20, Catalin Marinas wrote:
> > While writing the above, I realised the current ILP32 patches still miss
> > on converting pointers passed from user space (unless I got myself
> > confused in macros). The new __SC_WRAP() and COMPAT_SYSCALL_WRAPx()
> > macros take care of zero or sign extension via __SC_COMPAT_CAST().
> > However, we have two more existing cases which I don't see covered:
> > 
> > a) Native syscalls taking a pointer argument and invoked directly from
> >ILP32. For example, sys_read() takes a pointer but I don't see any
> >__SC_WRAP added by patch 5
> > 
> > b) Current compat syscalls taking a pointer argument. For example,
> >compat_sys_vmsplice() gets the iov32 pointer and the compiler assumes
> >it is a 64-bit variable. I don't see where the upper half is zeroed
> 
> on x32 sign/zero extension is currently left to userspace,
> which is difficult to deal with, (long long)arg does the
> wrong thing for pointer args.

I agree, I don't think we should leave sign/zero extension to user. We
should do it in the kernel either in a way similar to s390 (specific
__SC_COMPAT_CAST, __SC_DELOUSE) or by always zeroing the arguments upper
half on kernel entry with a few additional wrappers (where we have
64-bit arguments or they require sign extension). The latter has the
disadvantage of having to split 64-bit arguments in user space while the
former adds more maintenance burden to the kernel.

I can't comment on performance aspects without some real numbers.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/23] all: syscall wrappers: add documentation

[Catalin Marinas]

On Fri, May 27, 2016 at 08:03:57AM +0200, Heiko Carstens wrote:
> > > > The cost is pretty trivial though. See kernel/compat_wrapper.o:
> > > > COMPAT_SYSCALL_WRAP2(creat, const char __user *, pathname, umode_t, 
> > > > mode);
> > > > 0:   a9bf7bfdstp x29, x30, [sp,#-16]!
> > > > 4:   910003fdmov x29, sp
> > > > 8:   2a0003e0mov w0, w0
> > > > c:   9400bl  0 
> > > > 10:  a8c17bfdldp x29, x30, [sp],#16
> > > > 14:  d65f03c0ret
> > > 
> > > I would say the above could be more expensive than 8 movs (16 bytes to
> > > write, read, a branch and a ret). You can also add the I-cache locality,
> > > having wrappers for each syscalls instead of a single place for zeroing
> > > the upper half (where no other wrapper is necessary).
> > > 
> > > Can we trick the compiler into doing a tail call optimisation. This
> > > could have simply been:
> > > 
> > > COMPAT_SYSCALL_WRAP2(creat, ...):
> > >   mov w0, w0
> > >   b   
> > 
> > What you talk about was in my initial version. But Heiko insisted on having 
> > all
> > wrappers together.
> > http://www.spinics.net/lists/linux-s390/msg11593.html
> > 
> > Grep your email for discussion.
> 
> I think Catalin's question was more about why there is even a stack frame
> generated. It looks like it is not necessary. I did ask this too a couple
> of months ago, when we discussed this.

Indeed, I was questioning the need for prologue/epilogue, not the use of
COMPAT_SYSCALL_WRAPx(). Maybe something like __naked would do.

> > > > > Cost wise, this seems like it all cancels out in the end, but what
> > > > > do I know?
> > > > 
> > > > I think you know something, and I also think Heiko and other s390 guys
> > > > know something as well. So I'd like to listen their arguments here.
> 
> If it comes to 64 bit arguments for compat system calls: s390 also has an
> x32-like ABI extension which allows user space to use full 64 bit
> registers. As far as I know hardly anybody ever made use of that.
> 
> However even if that would be widely used, to me it wouldn't make sense to
> add new compat system calls which allow 64 bit arguments, simply because
> something like
> 
> c = (u32)a | (u64)b << 32;
> 
> can be done with a single 1-cycle instruction. It's just not worth the
> extra effort to maintain additional system call variants.

If we split 64-bit arguments in two, we can go a step further and avoid
most of the COMPAT_SYSCALL_WRAPx annotations in favour of a common
upper-half zeroing of the argument registers on ILP32 syscall entry.
There would be a few exceptions where we need to re-build 64-bit
arguments on sign-extend.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/23] all: syscall wrappers: add documentation

[Catalin Marinas]

On Fri, May 27, 2016 at 12:49:11PM +0200, Arnd Bergmann wrote:
> On Friday, May 27, 2016 10:30:52 AM CEST Catalin Marinas wrote:
> > On Fri, May 27, 2016 at 10:42:59AM +0200, Arnd Bergmann wrote:
> > > On Friday, May 27, 2016 8:03:57 AM CEST Heiko Carstens wrote:
> > > > > > > > Cost wise, this seems like it all cancels out in the end, but 
> > > > > > > > what
> > > > > > > > do I know?
> > > > > > > 
> > > > > > > I think you know something, and I also think Heiko and other s390 
> > > > > > > guys
> > > > > > > know something as well. So I'd like to listen their arguments 
> > > > > > > here.
> > > > 
> > > > If it comes to 64 bit arguments for compat system calls: s390 also has 
> > > > an
> > > > x32-like ABI extension which allows user space to use full 64 bit
> > > > registers. As far as I know hardly anybody ever made use of that.
> > > > 
> > > > However even if that would be widely used, to me it wouldn't make sense 
> > > > to
> > > > add new compat system calls which allow 64 bit arguments, simply because
> > > > something like
> > > > 
> > > > c = (u32)a | (u64)b << 32;
> > > > 
> > > > can be done with a single 1-cycle instruction. It's just not worth the
> > > > extra effort to maintain additional system call variants.
> > > 
> > > For reference, both tile and mips also have separate 32-bit ABIs that are
> > > only used on 64-bit kernels (aside from the normal 32-bit ABI). Tile
> > > does it like s390 and passes 64-bit arguments as pairs, while MIPS
> > > and x86 and pass them as single registers.
> > 
> > AFAIK, x32 also requires that the upper half of a 64-bit reg is zeroed
> > by the user when a 32-bit value is passed. We could require the same on
> > AArch64/ILP32 but I'm a bit uneasy on trusting a multitude of C
> > libraries on this.
> 
> It's not about trusting a C library, it's about ensuring malicious code
> cannot pass argumentst that the kernel code assumes will never happen.

At least for pointers and sizes, we have additional checks in place
already, like __access_ok(). Most of the syscalls should be safe since
they either go through some compat functions taking 32-bit arguments or
are routed to native functions which already need to cope with a full
random 64-bit value.

On arm64, I think the only risk comes from syscall handlers expecting
32-bit arguments but using 64-bit types. Apart from pointer types, I
don't expect this to happen but we could enforce it via a
BUILD_BUG_ON(sizeof(t) > 4 && !__TYPE_IS_PTR(t)) in __SC_DELOUSE as per
the s390 implementation. With ILP32 if we go for 64-bit off_t, those
syscalls would be routed directly to the native layer.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/23] all: syscall wrappers: add documentation

[Catalin Marinas]

On Fri, May 27, 2016 at 10:42:59AM +0200, Arnd Bergmann wrote:
> On Friday, May 27, 2016 8:03:57 AM CEST Heiko Carstens wrote:
> > > > > > Cost wise, this seems like it all cancels out in the end, but what
> > > > > > do I know?
> > > > > 
> > > > > I think you know something, and I also think Heiko and other s390 guys
> > > > > know something as well. So I'd like to listen their arguments here.
> > 
> > If it comes to 64 bit arguments for compat system calls: s390 also has an
> > x32-like ABI extension which allows user space to use full 64 bit
> > registers. As far as I know hardly anybody ever made use of that.
> > 
> > However even if that would be widely used, to me it wouldn't make sense to
> > add new compat system calls which allow 64 bit arguments, simply because
> > something like
> > 
> > c = (u32)a | (u64)b << 32;
> > 
> > can be done with a single 1-cycle instruction. It's just not worth the
> > extra effort to maintain additional system call variants.
> 
> For reference, both tile and mips also have separate 32-bit ABIs that are
> only used on 64-bit kernels (aside from the normal 32-bit ABI). Tile
> does it like s390 and passes 64-bit arguments as pairs, while MIPS
> and x86 and pass them as single registers.

AFAIK, x32 also requires that the upper half of a 64-bit reg is zeroed
by the user when a 32-bit value is passed. We could require the same on
AArch64/ILP32 but I'm a bit uneasy on trusting a multitude of C
libraries on this.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/23] all: syscall wrappers: add documentation

[Catalin Marinas]

On Thu, May 26, 2016 at 11:48:19PM +0300, Yury Norov wrote:
> On Wed, May 25, 2016 at 02:28:21PM -0700, David Miller wrote:
> > From: Arnd Bergmann 
> > Date: Wed, 25 May 2016 23:01:06 +0200
> > 
> > > On Wednesday, May 25, 2016 1:50:39 PM CEST David Miller wrote:
> > >> From: Arnd Bergmann 
> > >> Date: Wed, 25 May 2016 22:47:33 +0200
> > >> 
> > >> > If we use the normal calling conventions, we could remove these 
> > >> > overrides
> > >> > along with the respective special-case handling in glibc. None of them
> > >> > look particularly performance-sensitive, but I could be wrong there.
> > >> 
> > >> You could set the lowest bit in the system call entry pointer to indicate
> > >> the upper-half clears should be elided.
> > > 
> > > Right, but that would introduce an extra conditional branch in the syscall
> > > hotpath, and likely eliminate the gains from passing the loff_t arguments
> > > in a single register instead of a pair.
> > 
> > Ok, then, how much are you really gaining from avoiding a 'shift' and
> > an 'or' to build the full 64-bit value?  3 cycles?  Maybe 4?
> 
> 4 cycles in kernel and ~same cost in glibc to create a pair.

It would take a single instruction per argument in the kernel to do
shift+or and maybe 1-2 more instructions to move the remaining arguments
in place (we do this for a few wrappers in arch/arm64/kernel/entry32.S).
And the glibc counterpart.

> And 8 'mov's that exist for every syscall, even yield().
> 
> > And the executing the wrappers, those have a non-trivial cost too.
> 
> The cost is pretty trivial though. See kernel/compat_wrapper.o:
> COMPAT_SYSCALL_WRAP2(creat, const char __user *, pathname, umode_t, mode);
> 0:   a9bf7bfdstp x29, x30, [sp,#-16]!
> 4:   910003fdmov x29, sp
> 8:   2a0003e0mov w0, w0
> c:   9400bl  0 
> 10:  a8c17bfdldp x29, x30, [sp],#16
> 14:  d65f03c0ret

I would say the above could be more expensive than 8 movs (16 bytes to
write, read, a branch and a ret). You can also add the I-cache locality,
having wrappers for each syscalls instead of a single place for zeroing
the upper half (where no other wrapper is necessary).

Can we trick the compiler into doing a tail call optimisation. This
could have simply been:

COMPAT_SYSCALL_WRAP2(creat, ...):
mov w0, w0
b   

> > Cost wise, this seems like it all cancels out in the end, but what
> > do I know?
> 
> I think you know something, and I also think Heiko and other s390 guys
> know something as well. So I'd like to listen their arguments here.
> 
> For me spark64 way is looking reasonable only because it's really simple
> and takes less coding. I'll try it on some branch and share here what 
> happened.

The kernel code will definitely look simpler ;). It would be good to see
if there actually is any performance impact. Even with 16 more cycles on
syscall entry, would they be lost in the noise? You don't need a full
implementation, just some dummy mov x0, x0 on the entry path.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arm64:swiotlb:Enable only when Input size through command line

[Catalin Marinas]

On Thu, Jun 23, 2016 at 05:43:40PM +0530, Manjeet Pawar wrote:
> From: Rohit Thapliyal 
> 
> swiotlb default size of 64M is too big as
> default value therefore it is made configurable
> through command line through swiotlb_size parameter.
> swiotlb allocation shall be done only when the
> swiotlb size is given through command line.
> Otherwise no swiotlb is allocated.

I already queued this patch:

http://lkml.kernel.org/g/1465372426-4077-1-git-send-email-jszh...@marvell.com

If you have any objections to it, please reply there.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH v2] ARM64: ACPI: Update documentation for latest specification version

[Catalin Marinas]

On Wed, Mar 16, 2016 at 10:20:23AM +0530, Vikas Sajjan wrote:
> On Wed, Mar 16, 2016 at 1:58 AM, Al Stone <al.st...@linaro.org> wrote:
> > The ACPI 6.1 specification was recently released at the end of January 2016,
> > but the arm64 kernel documentation for the use of ACPI was written for the
> > 5.1 version of the spec.  There were significant additions to the spec that
> > had not yet been mentioned -- for example, the 6.0 mechanisms added to make
> > it easier to define processors and low power idle states, as well as the
> > 6.1 addition allowing regular interrupts (not just from GPIO) be used to
> > signal ACPI general purpose events.
> >
> > This patch reflects going back through and examining the specs in detail
> > and updating content appropriately.  Whilst there, a few odds and ends of
> > typos were caught as well.  This brings the documentation up to date with
> > ACPI 6.1 for arm64.
> >
> > RESEND:
> >-- Corrected From: header and added missing Cc's
> >
> > Changes for v2:
> >-- Clean up white space (Harb Abdulhahmid)
> >-- Clarification on _CCA usage (Harb Abdulhamid)
> >-- IORT moved to required from recommended (Hanjun Guo)
> >-- Clarify IORT description (Hanjun Guo)
> >
> > Signed-off-by: Al Stone <al.st...@linaro.org>
> > Cc: Catalin Marinas <catalin.mari...@arm.com>
> > Cc: Will Deacon <will.dea...@arm.com>
> > Cc: Jonathan Corbet <cor...@lwn.net>
> > ---
> >  Documentation/arm64/acpi_object_usage.txt | 445 
> > ++
> >  Documentation/arm64/arm-acpi.txt  |  28 +-
> >  2 files changed, 356 insertions(+), 117 deletions(-)
> >
> > diff --git a/Documentation/arm64/acpi_object_usage.txt 
> > b/Documentation/arm64/acpi_object_usage.txt
> > index a6e1a18..29bc1a1 100644
> > --- a/Documentation/arm64/acpi_object_usage.txt
> > +++ b/Documentation/arm64/acpi_object_usage.txt
> > @@ -11,15 +11,16 @@ outside of the UEFI Forum (see Section 5.2.6 of the 
> > specification).
> >
> >  For ACPI on arm64, tables also fall into the following categories:
> >
> > -   -- Required: DSDT, FADT, GTDT, MADT, MCFG, RSDP, SPCR, XSDT
> > +   -- Required: DSDT, FADT, GTDT, IORT, MADT, MCFG, RSDP, SPCR, XSDT
> >
> > -   -- Recommended: BERT, EINJ, ERST, HEST, SSDT
> > +   -- Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT
> >
> > -   -- Optional: BGRT, CPEP, CSRT, DRTM, ECDT, FACS, FPDT, MCHI, MPST,
> > -  MSCT, RASF, SBST, SLIT, SPMI, SRAT, TCPA, TPM2, UEFI
> > +   -- Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, MCHI,
> > +  MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, SPMI, SRAT, STAO, TCPA,
> > +  TPM2, UEFI, XENV
> >
> > -   -- Not supported: BOOT, DBG2, DBGP, DMAR, ETDT, HPET, IBFT, IVRS,
> > -  LPIT, MSDM, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
> > +   -- Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IBFT, IVRS, LPIT,
> > +  MSDM, OEMx, PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
> >
> >
> >  Table  Usage for ARMv8 Linux
> > @@ -50,7 +51,8 @@ CSRT   Signature Reserved (signature == "CSRT")
> >
> >  DBG2   Signature Reserved (signature == "DBG2")
> > == DeBuG port table 2 ==
> > -   Microsoft only table, will not be supported.
> > +   License has changed and should be usable.  Optional if used instead
> > +   of earlycon= on the command line.
> >
> >  DBGP   Signature Reserved (signature == "DBGP")
> > == DeBuG Port table ==
> > @@ -133,10 +135,11 @@ GTDT   Section 5.2.24 (signature == "GTDT")
> >
> >  HEST   Section 18.3.2 (signature == "HEST")
> > == Hardware Error Source Table ==
> > -   Until further error source types are defined, use only types 6 (AER
> > -   Root Port), 7 (AER Endpoint), 8 (AER Bridge), or 9 (Generic Hardware
> > -   Error Source).  Firmware first error handling is possible if and 
> > only
> > -   if Trusted Firmware is being used on arm64.
> > +   ARM-specific error sources have been defined; please use those or 
> > the
> > +   PCI types such as type 6 (AER Root Port), 7 (AER Endpoint), or 8 
> > (AER
> > +   Bridge), or use type 9 (Generic Hardware Error Source).  Firmware 
> > first
> > +   error handling is possible if and only if Trusted Firmware is being
> > +   used on arm64.
> >
> > Must be supplied if RAS support is provided by the platform.  It
&g

Re: [PATCH 19/25] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Sat, Apr 23, 2016 at 12:40:13AM +0300, Yury Norov wrote:
> On Fri, Apr 22, 2016 at 06:10:09PM +0100, Catalin Marinas wrote:
> > On Wed, Apr 06, 2016 at 01:08:41AM +0300, Yury Norov wrote:
> > > Here new aarch32 ptrace syscall handler is introsuced to avoid run-time
> > > detection of the task type.
> > 
> > The reason for this patch isn't clear to me. What's wrong with the
> > run-time detection? It's not some performance critical code.
> 
> It was requested by Arnd, It's not 'new' syscall basically, just an
> attempt to avoid run-time detection of things that may be detected an
> compile-time.

OK. I noticed that it touches core files and wondering whether it was
necessary but I'm fine with this approach.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 19/25] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Wed, Apr 06, 2016 at 01:08:41AM +0300, Yury Norov wrote:
> Here new aarch32 ptrace syscall handler is introsuced to avoid run-time
> detection of the task type.

The reason for this patch isn't clear to me. What's wrong with the
run-time detection? It's not some performance critical code.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 12/25] arm64: compat: change config dependences to aarch32

[Catalin Marinas]

On Wed, Apr 06, 2016 at 01:08:34AM +0300, Yury Norov wrote:
> From: Bamvor Jian Zhang 
> 
> With the patches of ILP32, COMPAT is not equivalent to AARCH32 in EL0.
> This patch fix this by updating the dependency from COMPAT to
> AARCH32_EL0 for ARMV8_DEPRECATED and ARM64_ERRATUM_845719.
> 
> Signed-off-by: Bamvor Jian Zhang 
> Signed-off-by: Yury Norov 

Can you not merge patches 10-12 into a single one? They all deal with
the s/COMPAT/AARCH32_EL0/ replacement. I'm not even sure the series is
bisectable after patch 10.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/25] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Thu, Apr 14, 2016 at 11:20:29AM +0800, Zhangjian (Bamvor) wrote:
> I suggest we enable AARCH32_EL0 by default explicitly. I am not sure
> if it should be a dedicated commit. I am ok if you merge my commit, add

We currently enable COMPAT in defconfig. I think we keep the same
approach and change CONFIG_COMPAT to CONFIG_AARCH32_EL0 in defconfig.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 25/25] arm64:ilp32: add ARM64_ILP32 to Kconfig

[Catalin Marinas]

On Fri, Apr 29, 2016 at 07:08:55PM +0300, Yury Norov wrote:
> On Fri, Apr 29, 2016 at 05:03:34PM +0100, Catalin Marinas wrote:
> > On Wed, Apr 06, 2016 at 01:08:47AM +0300, Yury Norov wrote:
> > > +config ARM64_ILP32
> > > + bool "Kernel support for ILP32"
> > > + select COMPAT_WRAPPER
> > > + help
> > > +   This option enables support for AArch64 ILP32 user space.  ILP32
> > > +   is an ABI where long and pointers are 32bits but it uses the AARCH64
> > > +   instruction set.
> > 
> > Is there any penalty for AArch32 tasks when selecting COMPAT_WRAPPER?
> 
> No. AARCH32 doesn't define __SC_WRAP and so __SYSCALL macro is used,
> which fills syscall table with sys_xxx versions, not compat_sys_xxx.

Ah, I forgot it has its own unistd32.h.

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] arm64:ilp32: add vdso-ilp32 and use for signal return

[Catalin Marinas]

On Wed, Apr 06, 2016 at 01:08:46AM +0300, Yury Norov wrote:
> ILP32 VDSO exports next symbols:
>  __kernel_rt_sigreturn;
>  __kernel_gettimeofday;
>  __kernel_clock_gettime;
>  __kernel_clock_getres;

[...]

> +$(obj)/gettimeofday-ilp32.o: $(src)/../vdso/gettimeofday.S
> + $(call if_changed_dep,vdso-ilp32as)

Are struct timeval and timespec the same between ILP32 and LP64? For
example, __kernel_gettimeofday() assumes TVAL_TV_SEC offset defined in
asm-offsets.c based on the LP64 timeval.

> +$(obj)/sigreturn-ilp32.o: $(src)/../vdso/sigreturn.S
> + $(call if_changed_dep,vdso-ilp32as)

This one should be fine because ILP32 uses the same generic
__NR_rt_sigreturn syscall number.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Mon, Apr 25, 2016 at 09:47:40PM +0300, Yury Norov wrote:
> On Mon, Apr 25, 2016 at 09:19:13PM +0300, Yury Norov wrote:
> > On Mon, Apr 25, 2016 at 06:26:56PM +0100, Catalin Marinas wrote:
> > > On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> > > > --- a/arch/arm64/kernel/entry.S
> > > > +++ b/arch/arm64/kernel/entry.S
> > > > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork)
> > > >   */
> > > > .align  6
> > > >  el0_svc:
> > > > -   adrpstbl, sys_call_table// load syscall table 
> > > > pointer
> > > > uxtwscno, w8// syscall number in w8
> > > > mov sc_nr, #__NR_syscalls
> > > > +#ifdef CONFIG_ARM64_ILP32
> > > > +   ldr x16, [tsk, #TI_FLAGS]
> > > > +   tbnzx16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using 
> > > > ILP32
> > > > +#endif
> > > 
> > > There is another ldr x16, [tsk, #TI_FLAGS] load further down in the
> > > el0_svc_naked block. We should rework these a bit to avoid loading the
> > > same location twice unnecessarily. E.g. move the ldr x16 just before
> > > el0_svc_naked and branch one line after in case of the ILP32 syscall.
> > > 
> > 
> > Yes, I thiks we can refactor it. Thanks for a catch.
> 
> Now it's better, I think
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index cf4d1ae..21312bb 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -715,16 +715,22 @@ ENDPROC(ret_from_fork)
>   */
>   .align  6
>  el0_svc:
> - adrpstbl, sys_call_table// load syscall table pointer
>   uxtwscno, w8// syscall number in w8
>   mov sc_nr, #__NR_syscalls
> + ldr x16, [tsk, #TI_FLAGS]

You can move this higher up for interlocking reasons (though these days
CPUs do a lot of speculative loads).

> +#ifdef CONFIG_ARM64_ILP32
> + tbz x16, #TIF_32BIT_AARCH64, el0_lp64_svc // We are using ILP32

// We are *not* using ILP32

> + adrpstbl, sys_call_ilp32_table  // load ilp32 syscall table 
> pointer
> + b el0_svc_naked
> +el0_lp64_svc:
> +#endif
> + adrpstbl, sys_call_table// load syscall table pointer

You can avoid the branches by using csel, something like this:

ldr x16, [tsk, #TI_FLAGS]
adrpstbl, sys_call_table
...
#ifdef CONFIG_ARM64_ILP32
adrpx17, sys_call_ilp32_table
tst x16, #_TIF_32BIT_AARCH64
cselstbl, stbl, x17, eq
#endif
el0_svc_naked:
...

>  el0_svc_naked:   // compat entry point
>   stp x0, scno, [sp, #S_ORIG_X0]  // save the original x0 and 
> syscall number
>   enable_dbg_and_irq
>   ct_user_exit 1
>  
> - ldr x16, [tsk, #TI_FLAGS]   // check for syscall hooks
> - tst x16, #_TIF_SYSCALL_WORK
> + tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks
>   b.ne__sys_trace
>   cmp scno, sc_nr // check upper syscall limit
>   b.hsni_sys

There is el0_svc_compat branching to el0_svc_naked and it won't have x16
set anymore. So you need to add an ldr x16 to el0_svc_compat as well.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote:
> +/* Using non-compat syscalls where necessary */
> +#define compat_sys_fadvise64_64sys_fadvise64_64
> +#define compat_sys_fallocate   sys_fallocate
> +#define compat_sys_ftruncate64 sys_ftruncate
> +#define compat_sys_lookup_dcookie  sys_lookup_dcookie
> +#define compat_sys_pread64 sys_pread64
> +#define compat_sys_pwrite64sys_pwrite64
> +#define compat_sys_readahead   sys_readahead
> +#define compat_sys_shmat   sys_shmat

Why don't we use compat_sys_shmat? Is it because of COMPAT_SHMLBA?

> +#define compat_sys_sync_file_range sys_sync_file_range
> +#define compat_sys_truncate64  sys_truncate
> +#define sys_llseek sys_lseek
> +#define sys_mmap2   sys_mmap

Nitpick: there are some whitespace inconsistencies above (just convert
all spaces to tabs).

I think you should also update Documentation/arm64/ilp32.txt to include
the list above.

> +
> +#include 
> +
> +#undef __SYSCALL
> +#undef __SC_COMP
> +#undef __SC_WRAP
> +#undef __SC_3264
> +#undef __SC_COMP_3264

Minor detail: do we actually need to undef all these? Maybe we can get
away with just defining __SYSCALL_COMPAT at the top of the file.

> +
> +#define __SYSCALL_COMPAT
> +#define __SYSCALL(nr, sym)   [nr] = sym,
> +#define __SC_WRAP(nr, sym)   [nr] = compat_##sym,
> +
> +/*
> + * The sys_call_ilp32_table array must be 4K aligned to be accessible from
> + * kernel/entry.S.
> + */
> +void *sys_call_ilp32_table[__NR_syscalls] __aligned(4096) = {
> + [0 ... __NR_syscalls - 1] = sys_ni_syscall,
> +#include 
> +};

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote:
> On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> > [...]
> > 
> > > >>Ok, I will test the ltp syscall test.
> > > >>With this changes, the issue I mentioned should be fixed. But we still
> > > >>use mmap2 syscall for ILP32 application when we pass the offset instead
> > > >>of page offset. Is it correct?
> > > >
> > > >I don't remember. It's probably not important whether we have the shift
> > > >in there, as long as it's independent of the actual kernel page size and
> > > >user space and kernel agree on the calling conventions.
> > > Well. I am ok with where to shift the pages size because we get the same
> > > result. I was just thinking if we should get rid of the name of mmap2 in 
> > > our
> > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may 
> > > confused
> > > if they do not know the implementations.
> > > 
> > 
> > This is what generic unistd.h does. If you want to change it, you'd
> > change each arch that uses generic unistd.h.
> 
> Generic unistd.h has this:
> 
> #ifdef __SYSCALL_COMPAT
> #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
> #else
> #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
> #endif
> 
> #define __NR3264_mmap 222
> __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> 
> 
> #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
> #define __NR_mmap __NR3264_mmap
> #else
> #define __NR_mmap2 __NR3264_mmap
> #endif
> 
> So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
> __NR_mmap and sys_mmap on 64-bit ABIs, as it should be.
> 
> The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
> this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
> we should either override both the implementation and the number,
> or neither of them.

I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2)
to keep it close to new 32-bit architectures, even though we would have
some shifts by 12 in both glibc and kernel.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > Test passes {iovec_base = 0x, iovec_len = 64} as one element
> > > of vector, and kernel reports successful read/write.
> > > 
> > > There are 2 problems:
> > > 1. How kernel allows such address to be passed to fs subsystem;
> > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > address.
> > > 
> > > I don't know the answer on 2'nd question, and it might be something
> > > generic. But I investigated first problem.
> > > 
> > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > validate user address, and on arm64 it ends up with checking buffer
> > > end against current_thread_info()->addr_limit.
> > > 
> > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > It happens because on thread creation we call flush_old_exec() to set 
> > > addr_limit, and completely ignore compat mode there.

[...]

> > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > @@ -12,6 +12,7 @@
> > >  do { \
> > >   clear_thread_flag(TIF_32BIT_AARCH64);   \
> > >   set_thread_flag(TIF_32BIT); \
> > > + set_fs(TASK_SIZE_32);   \
> > >  } while (0)
> > >  
> > >  #define COMPAT_ARCH_DLINFO
> > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c 
> > > b/arch/arm64/kernel/binfmt_ilp32.c
> > > index a934fd4..a8599c6 100644
> > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t 
> > > cputime,
> > >  do { 
> > > \
> > >   set_thread_flag(TIF_32BIT_AARCH64); \
> > >   clear_thread_flag(TIF_32BIT);   \
> > > + set_fs(TASK_SIZE_32);   \
> > >  } while (0)
> > 
> > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > setting the USER_DS for the new thread.
> 
> That's true, but USER_DS depends on personality which is not set yet
> for new thread, as I wrote above. In fact, I tried correct USER_DS
> only, and it doesn't work

Ah, it looks like load_elf_binary() sets the personality after
flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
maximum 64-bit task value, so they should have a similar issue with
native 32-bit vs compat behaviour.

So what exactly is LTP complaining about? Is different error (like
EFAULT vs EINVAL) or not getting an error at all.

(I need to update my LTP, the preadv tests appeared in December last
year)

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > Test passes {iovec_base = 0x, iovec_len = 64} as one element
> > > > of vector, and kernel reports successful read/write.
> > > > 
> > > > There are 2 problems:
> > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > address.
> > > > 
> > > > I don't know the answer on 2'nd question, and it might be something
> > > > generic. But I investigated first problem.
> > > > 
> > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > validate user address, and on arm64 it ends up with checking buffer
> > > > end against current_thread_info()->addr_limit.
> > > > 
> > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > addr_limit, and completely ignore compat mode there.
> 
> [...]
> 
> > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > @@ -12,6 +12,7 @@
> > > >  do {   \
> > > > clear_thread_flag(TIF_32BIT_AARCH64);   \
> > > > set_thread_flag(TIF_32BIT); \
> > > > +   set_fs(TASK_SIZE_32);   \
> > > >  } while (0)
> > > >  
> > > >  #define COMPAT_ARCH_DLINFO
> > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c 
> > > > b/arch/arm64/kernel/binfmt_ilp32.c
> > > > index a934fd4..a8599c6 100644
> > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const cputime_t 
> > > > cputime,
> > > >  do {   
> > > > \
> > > > set_thread_flag(TIF_32BIT_AARCH64); 
> > > > \
> > > > clear_thread_flag(TIF_32BIT);   
> > > > \
> > > > +   set_fs(TASK_SIZE_32);   
> > > > \
> > > >  } while (0)
> > > 
> > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > setting the USER_DS for the new thread.
> > 
> > That's true, but USER_DS depends on personality which is not set yet
> > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > only, and it doesn't work
> 
> Ah, it looks like load_elf_binary() sets the personality after
> flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> maximum 64-bit task value, so they should have a similar issue with
> native 32-bit vs compat behaviour.

I think we have another problem. flush_old_exec() calls the arm64
flush_thread() where tls_thread_flush() checks for is_compat_task(). So
starting a 32-bit application from a 64-bit one not go on the correct
path.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
> On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> > On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> > > On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> > > > On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> > > > > I debugged preadv02 and pwritev02 failures and found very weird bug.
> > > > > Test passes {iovec_base = 0x, iovec_len = 64} as one element
> > > > > of vector, and kernel reports successful read/write.
> > > > > 
> > > > > There are 2 problems:
> > > > > 1. How kernel allows such address to be passed to fs subsystem;
> > > > > 2. How fs successes to read/write at non-mapped, and in fact non-user
> > > > > address.
> > > > > 
> > > > > I don't know the answer on 2'nd question, and it might be something
> > > > > generic. But I investigated first problem.
> > > > > 
> > > > > The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> > > > > validate user address, and on arm64 it ends up with checking buffer
> > > > > end against current_thread_info()->addr_limit.
> > > > > 
> > > > > current_thread_info()->addr_limit for ilp32, and most probably for
> > > > > aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> > > > > It happens because on thread creation we call flush_old_exec() to set 
> > > > > addr_limit, and completely ignore compat mode there.
> > 
> > [...]
> > 
> > > > > --- a/arch/arm64/kernel/binfmt_elf32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_elf32.c
> > > > > @@ -12,6 +12,7 @@
> > > > >  do { \
> > > > >   clear_thread_flag(TIF_32BIT_AARCH64);   \
> > > > >   set_thread_flag(TIF_32BIT); \
> > > > > + set_fs(TASK_SIZE_32);   \
> > > > >  } while (0)
> > > > >  
> > > > >  #define COMPAT_ARCH_DLINFO
> > > > > diff --git a/arch/arm64/kernel/binfmt_ilp32.c 
> > > > > b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > index a934fd4..a8599c6 100644
> > > > > --- a/arch/arm64/kernel/binfmt_ilp32.c
> > > > > +++ b/arch/arm64/kernel/binfmt_ilp32.c
> > > > > @@ -59,6 +59,7 @@ static void cputime_to_compat_timeval(const 
> > > > > cputime_t cputime,
> > > > >  do { 
> > > > > \
> > > > >   set_thread_flag(TIF_32BIT_AARCH64); 
> > > > > \
> > > > >   clear_thread_flag(TIF_32BIT);   
> > > > > \
> > > > > + set_fs(TASK_SIZE_32);   
> > > > > \
> > > > >  } while (0)
> > > > 
> > > > I don't think we need these two. AFAICT, flush_old_exec() takes care of
> > > > setting the USER_DS for the new thread.
> > > 
> > > That's true, but USER_DS depends on personality which is not set yet
> > > for new thread, as I wrote above. In fact, I tried correct USER_DS
> > > only, and it doesn't work
> > 
> > Ah, it looks like load_elf_binary() sets the personality after
> > flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> > maximum 64-bit task value, so they should have a similar issue with
> > native 32-bit vs compat behaviour.
> 
> Hmmm. If so, it means we'd introduce generic fix. It would be removing 
> set_fs() from flush_old_exec() and appending it to load_elf_binary()
> after SET_PERSONALITY(). But I think it should be agreed with other
> arches developers.

The set_fs() in flush_old_exec() is probably fine, it may be meant to
re-set the USER_DS for the old thread.

It appears that at least powerpc and x86 don't have different USER_DS
setting for native and compat, so moving the set_fs() call further down
would not make any difference for them, nor will it fix the preadv02 LTP
test (if it fails for them, I haven't checked).

> I've sent standalone patch for aarch64 (you in CC) so let's move
> discussion there.

I've seen the patch but we would lose some discussion history here. I
think we should continue this thread and just summarise the conclusion
in reply to the other patch. This thread is also available on
l

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 04:11:23PM +0800, Zhangjian (Bamvor) wrote:
> On 2016/5/12 23:28, Catalin Marinas wrote:
> >On Thu, May 12, 2016 at 05:24:57PM +0300, Yury Norov wrote:
> >>On Thu, May 12, 2016 at 03:07:35PM +0100, Catalin Marinas wrote:
> >>>On Thu, May 12, 2016 at 04:44:31PM +0300, Yury Norov wrote:
> >>>>On Thu, May 12, 2016 at 02:35:34PM +0100, Catalin Marinas wrote:
> >>>>>On Thu, May 12, 2016 at 03:20:00AM +0300, Yury Norov wrote:
> >>>>>>I debugged preadv02 and pwritev02 failures and found very weird bug.
> >>>>>>Test passes {iovec_base = 0x, iovec_len = 64} as one element
> >>>>>>of vector, and kernel reports successful read/write.
> >>>>>>
> >>>>>>There are 2 problems:
> >>>>>>1. How kernel allows such address to be passed to fs subsystem;
> >>>>>>2. How fs successes to read/write at non-mapped, and in fact non-user
> >>>>>>address.
> >>>>>>
> >>>>>>I don't know the answer on 2'nd question, and it might be something
> >>>>>>generic. But I investigated first problem.
> >>>>>>
> >>>>>>The problem is that compat_rw_copy_check_uvector() uses access_ok() to
> >>>>>>validate user address, and on arm64 it ends up with checking buffer
> >>>>>>end against current_thread_info()->addr_limit.
> >>>>>>
> >>>>>>current_thread_info()->addr_limit for ilp32, and most probably for
> >>>>>>aarch32 is equal to aarch64 one, and so adress_ok() doesn't fail.
> >>>>>>It happens because on thread creation we call flush_old_exec() to set
> >>>>>>addr_limit, and completely ignore compat mode there.
[...]
> >>>>That's true, but USER_DS depends on personality which is not set yet
> >>>>for new thread, as I wrote above. In fact, I tried correct USER_DS
> >>>>only, and it doesn't work
> >>>
> >>>Ah, it looks like load_elf_binary() sets the personality after
> >>>flush_old_exec(). Looking at powerpc and x86, they set USER_DS to the
> >>>maximum 64-bit task value, so they should have a similar issue with
> >>>native 32-bit vs compat behaviour.
[...]
> >>>So what exactly is LTP complaining about? Is different error (like
> >>>EFAULT vs EINVAL) or not getting an error at all.
> >>
> >>It should be EINVAL, but it succeed. The other problem is that
> >>following fs routines does not complain on wrong address.
> >
> >I see. The test asks the kernel to write a single byte (out of maximum
> >64) to the user address 0x.
> 
> What address We should set for this limitation, TASK_SIZE or STACK_TOP?
> It is same for 64bit application. But STACK_TOP(0x) is below
> TASK_SIZE in 32bit application. The address above STACK_TOP is preserved
> for 32bit application.

The discussion is mainly around whether USER_DS for 32-bit compat apps
should be the same as USER_DS for native 32-bit apps. Even for native
32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
0x would fail in both cases anyway. I think the LTP test doesn't
even try to access such memory but only to probe the range validity (I
haven't managed to build the latest LTP yet).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:
> +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> +   unsigned long, prot, unsigned long, flags, unsigned long, fd,
> +   unsigned long, pgoff)

To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).

> +{
> +   if (pgoff & (~PAGE_MASK >> 12))
> +   return -EINVAL;
> +
> +   return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
> +(int) prot, (int) flags, (int) fd,
> +pgoff >> (PAGE_SHIFT-12));

Then we wouldn't need the explicit casting here.

> +}
> +
> +COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, 
> ubuf,
> +compat_size_t, count, off_t, offset)
> +{
> + return sys_pread64(fd, (char *) ubuf, count, offset);
> +}
> +
> +COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, 
> ubuf,
> +compat_size_t, count, off_t, offset)
> +{
> + return sys_pwrite64(fd, (char *) ubuf, count, offset);

Nitpick: no space between cast type and variable name: (char *)ubuf, ...

We can also make these functions static as they are not used outside
this file.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC6 PATCH v6 00/21] ILP32 for ARM64

[Catalin Marinas]

On Fri, May 13, 2016 at 01:51:15PM +0300, Yury Norov wrote:
> On Fri, May 13, 2016 at 09:28:03AM +0000, Catalin Marinas wrote:
> > The discussion is mainly around whether USER_DS for 32-bit compat apps
> > should be the same as USER_DS for native 32-bit apps. Even for native
> > 32-bit kernels, we don't use STACK_TOP as addr_limit. A read/write from
> > 0x would fail in both cases anyway. I think the LTP test doesn't
> > even try to access such memory but only to probe the range validity (I
> > haven't managed to build the latest LTP yet).
> 
> This fix lets me build it (on top of 7b3ef3b0b)
> Of course, it's not 'official'. :)
[...]
> --- a/testcases/kernel/syscalls/fstatat/fstatat01.c
> +++ b/testcases/kernel/syscalls/fstatat/fstatat01.c
> @@ -59,6 +59,7 @@ static const char *filenames[TEST_CASES];
>  static const int expected_errno[] = { 0, 0, ENOTDIR, EBADF, EINVAL, 0 };
>  static const int flags[] = { 0, 0, 0, 0, , 0 };
>  
> +#define HAVE_FSTATAT
>  #if !defined(HAVE_FSTATAT)
>  #if (__NR_fstatat64 > 0)
>  int fstatat(int dirfd, const char *filename, struct stat64 *statbuf, int 
> flags)
> diff --git a/testcases/kernel/syscalls/preadv/preadv.h 
> b/testcases/kernel/syscalls/preadv/preadv.h
> index f3ac30d..b001389 100644
> --- a/testcases/kernel/syscalls/preadv/preadv.h
> +++ b/testcases/kernel/syscalls/preadv/preadv.h
> @@ -21,6 +21,7 @@
>  #include "config.h"
>  #include "linux_syscall_numbers.h"
>  
> +#define HAVE_PREADV
>  #if !defined(HAVE_PREADV)
>  int preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
>  {
> diff --git a/testcases/kernel/syscalls/pwritev/pwritev.h 
> b/testcases/kernel/syscalls/pwritev/pwritev.h
> index ae9d999..2a4d188 100644
> --- a/testcases/kernel/syscalls/pwritev/pwritev.h
> +++ b/testcases/kernel/syscalls/pwritev/pwritev.h
> @@ -21,6 +21,7 @@
>  #include "config.h"
>  #include "linux_syscall_numbers.h"
>  
> +#define HAVE_PWRITEV
>  #if !defined(HAVE_PWRITEV)
>  int pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
>  {

I didn't need any of these defines as they are automatically generated
in include/config.h for AArch32. However, I need to run autoreconf to
generate the configure script prior to building (archlinuxarm
filesystem)

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

[Catalin Marinas]

On Wed, May 11, 2016 at 09:30:07PM +0200, Arnd Bergmann wrote:
> On Wednesday 11 May 2016 17:59:01 Catalin Marinas wrote:
> > On Wed, May 11, 2016 at 12:55:01PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 11 May 2016 11:04:38 Yury Norov wrote:
> > > > On Wed, May 11, 2016 at 10:04:16AM +0800, Zhangjian (Bamvor) wrote:
> > > > [...]
> > > > 
> > > > > >>Ok, I will test the ltp syscall test.
> > > > > >>With this changes, the issue I mentioned should be fixed. But we 
> > > > > >>still
> > > > > >>use mmap2 syscall for ILP32 application when we pass the offset 
> > > > > >>instead
> > > > > >>of page offset. Is it correct?
> > > > > >
> > > > > >I don't remember. It's probably not important whether we have the 
> > > > > >shift
> > > > > >in there, as long as it's independent of the actual kernel page size 
> > > > > >and
> > > > > >user space and kernel agree on the calling conventions.
> > > > > Well. I am ok with where to shift the pages size because we get the 
> > > > > same
> > > > > result. I was just thinking if we should get rid of the name of mmap2 
> > > > > in our
> > > > > ILP32 porting. Actually, it is mmap but we name it as mmap2. User may 
> > > > > confused
> > > > > if they do not know the implementations.
> > > > > 
> > > > 
> > > > This is what generic unistd.h does. If you want to change it, you'd
> > > > change each arch that uses generic unistd.h.
> > > 
> > > Generic unistd.h has this:
> > > 
> > > #ifdef __SYSCALL_COMPAT
> > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SYSCALL(_nr, _comp)
> > > #else
> > > #define __SC_COMP_3264(_nr, _32, _64, _comp) __SC_3264(_nr, _32, _64)
> > > #endif
> > > 
> > > #define __NR3264_mmap 222
> > > __SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)
> > > 
> > > 
> > > #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT)
> > > #define __NR_mmap __NR3264_mmap
> > > #else
> > > #define __NR_mmap2 __NR3264_mmap
> > > #endif
> > > 
> > > So by default we get __NR_mmap2 and sys_mmap2 on 32-bit ABIs, but
> > > __NR_mmap and sys_mmap on 64-bit ABIs, as it should be.
> > > 
> > > The problem is that arch/arm64/kernel/sys_ilp32.c now overrides
> > > this to use __NR_mmap2 with sys_mmap, so we have a mismatch. I think
> > > we should either override both the implementation and the number,
> > > or neither of them.
> > 
> > I would vote for "neither of them" (so we use __NR_mmap2 and sys_mmap2)
> > to keep it close to new 32-bit architectures, even though we would have
> > some shifts by 12 in both glibc and kernel.
> 
> I don't think the shifts are a problem, the main downside would be
> the limit to 44 bits of file offsets (16TB files), but it's also
> unclear if that is a practical problem at all. If it is, we run
> into the same problem on all other 32-bit architectures too.

I hope people are seriously thinking of moving to an LP64 ABI if they
have such large file offset needs.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] arm64:ilp32: add vdso-ilp32 and use for signal return

[Catalin Marinas]

On Fri, Apr 29, 2016 at 07:30:19PM +0200, Arnd Bergmann wrote:
> On Friday 29 April 2016 17:01:55 Catalin Marinas wrote:
> > On Wed, Apr 06, 2016 at 01:08:46AM +0300, Yury Norov wrote:
> > > ILP32 VDSO exports next symbols:
> > >  __kernel_rt_sigreturn;
> > >  __kernel_gettimeofday;
> > >  __kernel_clock_gettime;
> > >  __kernel_clock_getres;
> > 
> > [...]
> > 
> > > +$(obj)/gettimeofday-ilp32.o: $(src)/../vdso/gettimeofday.S
> > > + $(call if_changed_dep,vdso-ilp32as)
> > 
> > Are struct timeval and timespec the same between ILP32 and LP64? For
> > example, __kernel_gettimeofday() assumes TVAL_TV_SEC offset defined in
> > asm-offsets.c based on the LP64 timeval.
> 
> No, ilp32 uses the generic 32-bit data structures, which have a 32-bit
> time_t. I guess that means it can work for little-endian but not
> big-endian, right?

I don't think it works for little-endian either. The LP64 struct timeval
is 16 bytes while the ILP32 one is 8 bytes. The VDSO gettimeofday is
storing 16 bytes (stp x10, x11, [x0, #TVAL_TV_SEC])

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Thu, Aug 11, 2016 at 10:53:01AM +0200, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> > On 2016/6/18 7:54, Yury Norov wrote:
> > > From: Andrew Pinski 
> > >
> > > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> > >
> > >  From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > > and COMPAT indicates that one of them, or both, is enabled.
> > >
> > > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 
> > > instead
> > >
> > > Reviewed-by: David Daney 
> > > Signed-off-by: Andrew Pinski 
> > > Signed-off-by: Philipp Tomsich 
> > > Signed-off-by: Christoph Muellner 
> > > 
> > > Signed-off-by: Bamvor Jian Zhang 
> > > Signed-off-by: Yury Norov 
> > ...
> > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > > index c173d32..af200a8 100644
> > > --- a/arch/arm64/kernel/cpuinfo.c
> > > +++ b/arch/arm64/kernel/cpuinfo.c
> > > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> > >*/
> > >   seq_puts(m, "Features\t:");
> > >   if (compat) {
> > > -#ifdef CONFIG_COMPAT
> > > - for (j = 0; compat_hwcap_str[j]; j++)
> > > - if (compat_elf_hwcap & (1 << j))
> > > - seq_printf(m, " %s", 
> > > compat_hwcap_str[j]);
> > > -
> > > - for (j = 0; compat_hwcap2_str[j]; j++)
> > > - if (compat_elf_hwcap2 & (1 << j))
> > > - seq_printf(m, " %s", 
> > > compat_hwcap2_str[j]);
> > > -#endif /* CONFIG_COMPAT */
> > > +#ifdef CONFIG_AARCH32_EL0
> > I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> > "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> > in c show()?
> > > + if (personality(current->personality) == PER_LINUX32) {
> > And "compat" is "personality(current->personality) == PER_LINUX32;",
> > it seems that there is no need to add this twice.
> 
> I think it would be best to remove the #ifdef here completely,
> the PER_LINUX32 concept is not strictly tied to the emulation
> of ARM binaries, it literally just changes the output of
> /proc/cpuinfo and 'uname',

It's not strictly related to ARM binaries, however it is related to
AArch32 CPU features being supported and detected by the kernel.
Currently, with CONFIG_COMPAT disabled, we won't have access to a
(meaningful) compat_elf_hwcap.

> and you can have ARM binaries with
> PER_LINUX (using the arm64 uname) just like you can have
> arm64 binaries running with PER_LINUX32.

I was actually looking to enforce the 32-bit binaries to only see
PER_LINUX32, though with a risk of breaking the ABI. OTOH, people are
abusing this and write 32-bit apps relying on the 64-bit /proc/cpuinfo:

http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.mari...@arm.com

(you were summoned on that discussion couple of times ;))

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Thu, Aug 11, 2016 at 05:16:45PM +0200, Arnd Bergmann wrote:
> On Thursday, August 11, 2016 3:50:00 PM CEST Catalin Marinas wrote:
> > On Thu, Aug 11, 2016 at 10:53:01AM +0200, Arnd Bergmann wrote:
> > > On Thursday, August 11, 2016 3:35:01 PM CEST Zhangjian (Bamvor) wrote:
> > > > On 2016/6/18 7:54, Yury Norov wrote:
> > > > > From: Andrew Pinski <apin...@cavium.com>
> > > > >
> > > > > In this patchset  ILP32 ABI support is added. Additionally to AARCH32,
> > > > > which is binary-compatible with ARM, ILP32 is (mostly) ABI-compatible.
> > > > >
> > > > >  From now, AARCH32_EL0 (former COMPAT) config option means the 
> > > > > support of
> > > > > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next 
> > > > > patches),
> > > > > and COMPAT indicates that one of them, or both, is enabled.
> > > > >
> > > > > Where needed, CONFIG_COMPAT is changed over to use CONFIG_AARCH32_EL0 
> > > > > instead
> > > > >
> > > > > Reviewed-by: David Daney <dda...@caviumnetworks.com>
> > > > > Signed-off-by: Andrew Pinski <andrew.pin...@caviumnetworks.com>
> > > > > Signed-off-by: Philipp Tomsich <philipp.toms...@theobroma-systems.com>
> > > > > Signed-off-by: Christoph Muellner 
> > > > > <christoph.muell...@theobroma-systems.com>
> > > > > Signed-off-by: Bamvor Jian Zhang <bamvor.zhangj...@linaro.org>
> > > > > Signed-off-by: Yury Norov <yno...@caviumnetworks.com>
> > > > ...
> > > > > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > > > > index c173d32..af200a8 100644
> > > > > --- a/arch/arm64/kernel/cpuinfo.c
> > > > > +++ b/arch/arm64/kernel/cpuinfo.c
> > > > > @@ -134,15 +134,17 @@ static int c_show(struct seq_file *m, void *v)
> > > > >*/
> > > > >   seq_puts(m, "Features\t:");
> > > > >   if (compat) {
> > > > > -#ifdef CONFIG_COMPAT
> > > > > - for (j = 0; compat_hwcap_str[j]; j++)
> > > > > - if (compat_elf_hwcap & (1 << j))
> > > > > - seq_printf(m, " %s", 
> > > > > compat_hwcap_str[j]);
> > > > > -
> > > > > - for (j = 0; compat_hwcap2_str[j]; j++)
> > > > > - if (compat_elf_hwcap2 & (1 << j))
> > > > > - seq_printf(m, " %s", 
> > > > > compat_hwcap2_str[j]);
> > > > > -#endif /* CONFIG_COMPAT */
> > > > > +#ifdef CONFIG_AARCH32_EL0
> > > > I saw that compat_hwcap_str and compat_hwcap2_str is defined when
> > > > "CONFIG_COMPAT" is true. Why we only change it to CONFIG_AARCH32_EL0
> > > > in c show()?
> > > > > + if (personality(current->personality) == 
> > > > > PER_LINUX32) {
> > > > And "compat" is "personality(current->personality) == PER_LINUX32;",
> > > > it seems that there is no need to add this twice.
> > > 
> > > I think it would be best to remove the #ifdef here completely,
> > > the PER_LINUX32 concept is not strictly tied to the emulation
> > > of ARM binaries, it literally just changes the output of
> > > /proc/cpuinfo and 'uname',
> > 
> > It's not strictly related to ARM binaries, however it is related to
> > AArch32 CPU features being supported and detected by the kernel.
> > Currently, with CONFIG_COMPAT disabled, we won't have access to a
> > (meaningful) compat_elf_hwcap.
> 
> Ah, makes sense. In that case, using CONFIG_AARCH32_EL0 sounds like
> the right thing to do here, though I guess we can just drop the
> "if (compat)" check, as we specifically want to print the supported
> features of the CPU, and they are still present even if a
> process with PER_LINUX reads them.

Do you mean always printing both compat and native hwcaps in
/proc/cpuinfo? We discussed this in the past and it's not something we
can easily fix at this stage without breaking the ABI. If we noticed
this before, we could have used distinct feature strings for AArch32 and
AArch64 (e.g. crypto stuff like aes32 and aes64 but we only have aes for
both).

> > > and you can have ARM bi

Re: [PATCH 08/10] docs: sphinxify kmemleak.txt and move it to dev-tools

[Catalin Marinas]

On Mon, Aug 08, 2016 at 05:35:00PM -0600, Jonathan Corbet wrote:
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Signed-off-by: Jonathan Corbet <cor...@lwn.net>
> ---
>  .../{kmemleak.txt => dev-tools/kmemleak.rst}   | 93 
> --
>  Documentation/dev-tools/tools.rst  |  1 +
>  MAINTAINERS|  2 +-
>  3 files changed, 52 insertions(+), 44 deletions(-)
>  rename Documentation/{kmemleak.txt => dev-tools/kmemleak.rst} (73%)

FWIW:

Acked-by: Catalin Marinas <catalin.mari...@arm.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/19] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Sat, Aug 13, 2016 at 06:17:03PM +0300, Yury Norov wrote:
> On Fri, Aug 12, 2016 at 03:36:12PM +0100, Catalin Marinas wrote:
> > On Thu, Aug 11, 2016 at 10:29:03PM +0200, Arnd Bergmann wrote:
> > > On Thursday, August 11, 2016 5:30:03 PM CEST Catalin Marinas wrote:
> > > > > > > and you can have ARM binaries with
> > > > > > > PER_LINUX (using the arm64 uname) just like you can have
> > > > > > > arm64 binaries running with PER_LINUX32.
> > > > > > 
> > > > > > I was actually looking to enforce the 32-bit binaries to only see
> > > > > > PER_LINUX32, though with a risk of breaking the ABI. OTOH, people 
> > > > > > are
> > > > > > abusing this and write 32-bit apps relying on the 64-bit 
> > > > > > /proc/cpuinfo:
> > > > > > 
> > > > > > http://lkml.kernel.org/g/1464706504-25224-3-git-send-email-catalin.mari...@arm.com
> > > > > > 
> > > > > > (you were summoned on that discussion couple of times ;))
> > > > > 
> > > > > Hmm, I thought I saw the thread and didn't have any good idea for
> > > > > the uname information, but didn't notice it was for /proc/cpuinfo.
> > > > > 
> > > > > What's wrong with always showing both the 32-bit and the 64-bit
> > > > > hwcap strings here (minus the duplicates, which hopefully have
> > > > > the same meaning here)?
> > > > 
> > > > As I said above, some of them have the same name (which may be a good
> > > > thing at a first look) but we don't have an architecture guarantee that
> > > > the feature is present in both AArch32 and AArch64 modes (e.g. AES may
> > > > only be available in AArch64).
> > > 
> > > Is this the case on actual implementations that exist today? If they
> > > are actually always both present, we might be able to get away with it.
> > 
> > It may be fine on current implementations but what would we do when/if
> > we actually find such discrepancy? It's not just ARM Ltd designing the
> > chips, so as long as the architecture doesn't mandate it you may find
> > strange implementations.
> > 
> > Imposing such restriction in the architecture doesn't make sense if the
> > only reason is the /proc/cpuinfo file (and I can't think of any other
> > reason why this should be enforced).
> > 
> > What I'm worried about is 32-bit apps running on an arm64 kernel and
> > making use of the 64-bit /proc/cpuinfo without any guarantee that the
> > AArch32 state has such features. In my patch proposal linked above I
> > wanted to always force the compat /proc/cpuinfo for 32-bit tasks.
> 
> The link doesn't work for me. Is there other link, or what's the
> maillist there?

With lkml.kernel.org, just change the 'g' with an 'r':

http://lkml.kernel.org/r/1464706504-25224-3-git-send-email-catalin.mari...@arm.com

It was on linux-arm-kernel.

> So, what we decided finally? Is my understanding correct that we leave
> everything as is in ilp32 series, and it will be resolved separately?

ILP32 is not affected by this since the hwcap for ILP32 should match
native. It was more a question about whether AArch32 tasks should ever
have access to AArch64 hwcaps (and potential misuse).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: kprobes: Document jprobes stack copying limitations

[Catalin Marinas]

On Mon, Aug 15, 2016 at 09:32:43AM -0600, Jonathan Corbet wrote:
> On Mon, 15 Aug 2016 10:49:36 -0400
> David Long <dave.l...@linaro.org> wrote:
> 
> > On 08/15/2016 10:25 AM, Jonathan Corbet wrote:
> > > On Fri, 12 Aug 2016 16:24:44 -0400
> > > David Long <dave.l...@linaro.org> wrote:
> > >  
> > >> Some architectures (i.e.: sparc64 and arm64) make reasonable partial 
> > >> stack
> > >> duplication for jprobes problematic. Document this.  
> > >
> > > Applied to the docs tree, thanks.
> > >
> > > jon
> > 
> > Was kind of hoping to see an ack (or critique) from a sparc maintainer.
> 
> So are you saying you don't want the patch applied at this point?

To avoid any doubt, for arm64:

Acked-by: Catalin Marinas <catalin.mari...@arm.com>

(and I want the patch applied)
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC2 nowrap: PATCH v7 00/18] ILP32 for ARM64

[Catalin Marinas]

On Wed, Aug 17, 2016 at 02:54:59PM +0200, Dr. Philipp Tomsich wrote:
> On 17 Aug 2016, at 14:48, Yury Norov  wrote:
> > On Wed, Aug 17, 2016 at 02:28:50PM +0200, Alexander Graf wrote:
> >> On 17 Aug 2016, at 13:46, Yury Norov  wrote:
> >>> This series enables aarch64 with ilp32 mode, and as supporting work,
> >>> introduces ARCH_32BIT_OFF_T configuration option that is enabled for
> >>> existing 32-bit architectures but disabled for new arches (so 64-bit
> >>> off_t is is used by new userspace).
> >>> 
> >>> This version is based on kernel v4.8-rc2.
> >>> It works with glibc-2.23, and tested with LTP.
> >>> 
> >>> This is RFC because there is still no solid understanding what type of 
> >>> registers
> >>> top-halves delousing we prefer. In this patchset, w0-w7 are cleared for 
> >>> each
> >>> syscall in assembler entry. The alternative approach is in introducing 
> >>> compat
> >>> wrappers which is little faster for natively routed syscalls (~2.6% for 
> >>> syscall
> >>> with no payload) but much more complicated.
> >> 
> >> So you’re saying there are 2 options:
> >> 
> >>  1) easy to get right, slightly slower, same ABI to user space as 2
> >>  2) harder to get right, minor performance benefit
> > 
> > No, ABI is little different. If 1) we pass off_t in a pair to syscalls,
> > if 2) - in a single register. So if 1, we 'd take some wrappers from 
> > aarch32.
> > See patch 12 here.
> 
> From our experience with ILP32, I’d prefer to have off_t (and similar)
> in a single register whenever possible (i.e. option #2).  It feels
> more natural to use the full 64bit registers whenever possible, as
> ILP32 on ARMv8 should really be understood as a 64bit ABI with a 32bit
> memory model.

I think we are well past the point where we considered ILP32 a 64-bit
ABI. It would have been nice but we decided that breaking POSIX
compatibility is a bad idea, so we went back (again) to a 32-bit ABI for
ILP32. While there are 64-bit arguments that, at a first look, would
make sense to be passed in 64-bit registers, the kernel maintenance cost
is significant with changes to generic files.

Allowing 64-bit wide registers at the ILP32 syscall interface means that
the kernel would have to zero/sign-extend the upper half of the 32-bit
arguments for the cases where they are passed directly to a native
syscall that expects a 64-bit argument. This (a) adds a significant
number of wrappers to the generic code together additional annotations
to the generic unistd.h and (b) it adds a small overhead to the AArch32
(compat) ABI since it doesn't need such generic wrapping (the upper half
of 64-bit registers is guaranteed to be zero/preserved by the
architecture when coming from the AArch32 mode).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 01, 2017 at 04:33:58PM +, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 11:29:22AM -0500, Christopher Covington wrote:
> > On 01/31/2017 12:56 PM, Marc Zyngier wrote:
> > > Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be
> > > enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in
> > > that case too, and hope that people do enable the HW version.
> > 
> > Okay, I'll do my best to add support for the SW PAN case. I rebased and
> > submitted v6 of the E1009 patch [1] so that it no longer depends on this
> > patch landing first, if you all are inclined to pick it up while work on
> > this E1003 patch continues.
> 
> The alternative is not enabling SW_PAN (at runtime) if this errata is
> present, along with a warning stating that hardware-PAN should be
> enabled in kconfig instead. Not sure what distributions will make of that
> though.

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled
and in the absence of hardware PAN (or ARM64_PAN disabled),
cpu_do_switch_mm is no longer called for user process switching, so the
workaround is pretty much useless.

I'm ok with adding the Kconfig dependency below to
QCOM_FALKOR_ERRATUM_1003:

depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

I'm not keen on adding the workaround to the uaccess macros. They are
complex enough already and people who do want SW PAN (and single Image)
would end up with 6-8 extra unnecessary nops on each side of a uaccess
(and nops are not entirely free).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 01, 2017 at 05:49:34PM +, Catalin Marinas wrote:
> On Wed, Feb 01, 2017 at 05:41:05PM +, Will Deacon wrote:
> > On Wed, Feb 01, 2017 at 05:36:09PM +, Catalin Marinas wrote:
> > > On Wed, Feb 01, 2017 at 04:33:58PM +, Will Deacon wrote:
> > > > On Wed, Feb 01, 2017 at 11:29:22AM -0500, Christopher Covington wrote:
> > > > > On 01/31/2017 12:56 PM, Marc Zyngier wrote:
> > > > > > Given that all ARMv8 CPUs can support SW_PAN, it is more likely to 
> > > > > > be
> > > > > > enabled than the ARMv8.1 PAN. I'd vote for supporting the 
> > > > > > workaround in
> > > > > > that case too, and hope that people do enable the HW version.
> > > > > 
> > > > > Okay, I'll do my best to add support for the SW PAN case. I rebased 
> > > > > and
> > > > > submitted v6 of the E1009 patch [1] so that it no longer depends on 
> > > > > this
> > > > > patch landing first, if you all are inclined to pick it up while work 
> > > > > on
> > > > > this E1003 patch continues.
> > > > 
> > > > The alternative is not enabling SW_PAN (at runtime) if this errata is
> > > > present, along with a warning stating that hardware-PAN should be
> > > > enabled in kconfig instead. Not sure what distributions will make of 
> > > > that
> > > > though.
> > > 
> > > The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled
> > > and in the absence of hardware PAN (or ARM64_PAN disabled),
> > > cpu_do_switch_mm is no longer called for user process switching, so the
> > > workaround is pretty much useless.
> > 
> > Oh, I see what you mean now.
> > 
> > > I'm ok with adding the Kconfig dependency below to
> > > QCOM_FALKOR_ERRATUM_1003:
> > > 
> > >   depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN
> > > 
> > > together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.
> > 
> > That makes it look like hardware-PAN is the cause of the erratum.
> 
> With the right Kconfig comment we could make this clearer.
> 
> > Maybe
> > just select ARM64_PAN if the erratum workaround is selected, then
> > runtime warning if we find that the h/w doesn't have PAN but does have
> > the erratum (which should never fire)?
> 
> You still need this workaround even if you don't want any PAN (both sw
> and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not
> a dependency. It's more like if you do need a PAN, make sure you only
> use the hw one.

Alternatively, your select idea could be refined to:

select ARM64_PAN if ARM64_SW_TTBR0_PAN

but we still need a proper comment as people would wonder what this is
for.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 01, 2017 at 05:41:05PM +, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 05:36:09PM +0000, Catalin Marinas wrote:
> > On Wed, Feb 01, 2017 at 04:33:58PM +, Will Deacon wrote:
> > > On Wed, Feb 01, 2017 at 11:29:22AM -0500, Christopher Covington wrote:
> > > > On 01/31/2017 12:56 PM, Marc Zyngier wrote:
> > > > > Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be
> > > > > enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround 
> > > > > in
> > > > > that case too, and hope that people do enable the HW version.
> > > > 
> > > > Okay, I'll do my best to add support for the SW PAN case. I rebased and
> > > > submitted v6 of the E1009 patch [1] so that it no longer depends on this
> > > > patch landing first, if you all are inclined to pick it up while work on
> > > > this E1003 patch continues.
> > > 
> > > The alternative is not enabling SW_PAN (at runtime) if this errata is
> > > present, along with a warning stating that hardware-PAN should be
> > > enabled in kconfig instead. Not sure what distributions will make of that
> > > though.
> > 
> > The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled
> > and in the absence of hardware PAN (or ARM64_PAN disabled),
> > cpu_do_switch_mm is no longer called for user process switching, so the
> > workaround is pretty much useless.
> 
> Oh, I see what you mean now.
> 
> > I'm ok with adding the Kconfig dependency below to
> > QCOM_FALKOR_ERRATUM_1003:
> > 
> > depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN
> > 
> > together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.
> 
> That makes it look like hardware-PAN is the cause of the erratum.

With the right Kconfig comment we could make this clearer.

> Maybe
> just select ARM64_PAN if the erratum workaround is selected, then
> runtime warning if we find that the h/w doesn't have PAN but does have
> the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw
and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not
a dependency. It's more like if you do need a PAN, make sure you only
use the hw one.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Tue, Feb 07, 2017 at 07:35:16PM -0500, Christopher Covington wrote:
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -480,6 +480,18 @@ config CAVIUM_ERRATUM_27456
>  
> If unsure, say Y.
>  
> +config QCOM_FALKOR_ERRATUM_1003
> + bool "Falkor E1003: Incorrect translation due to ASID change"
> + default y
> + select ARM64_PAN if ARM64_SW_TTBR0_PAN
> + help
> +   On Falkor v1, an incorrect ASID may be cached in the TLB when ASID
> +   and BADDR are changed together in TTBRx_EL1. The workaround for this
> +   issue is to use a reserved ASID in cpu_do_switch_mm() before
> +   switching to the new ASID.
> +
> +   If unsure, say Y.

It would be good to have a comment here on why PAN is selected.

> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -79,6 +79,13 @@ void verify_cpu_asid_bits(void)
>   }
>  }
>  
> +static void set_reserved_asid_bits(void)
> +{
> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
> + cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
> + __set_bit(FALKOR_RESERVED_ASID, asid_map);
> +}

You should use cpus_have_const_cap() as it would be optimised using jump
labels.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 01, 2017 at 05:59:48PM +, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 05:49:34PM +0000, Catalin Marinas wrote:
> > On Wed, Feb 01, 2017 at 05:41:05PM +, Will Deacon wrote:
> > > On Wed, Feb 01, 2017 at 05:36:09PM +0000, Catalin Marinas wrote:
> > > > On Wed, Feb 01, 2017 at 04:33:58PM +, Will Deacon wrote:
> > > > > On Wed, Feb 01, 2017 at 11:29:22AM -0500, Christopher Covington wrote:
> > > > > > On 01/31/2017 12:56 PM, Marc Zyngier wrote:
> > > > > > > Given that all ARMv8 CPUs can support SW_PAN, it is more likely 
> > > > > > > to be
> > > > > > > enabled than the ARMv8.1 PAN. I'd vote for supporting the 
> > > > > > > workaround in
> > > > > > > that case too, and hope that people do enable the HW version.
> > > > > > 
> > > > > > Okay, I'll do my best to add support for the SW PAN case. I rebased 
> > > > > > and
> > > > > > submitted v6 of the E1009 patch [1] so that it no longer depends on 
> > > > > > this
> > > > > > patch landing first, if you all are inclined to pick it up while 
> > > > > > work on
> > > > > > this E1003 patch continues.
> > > > > 
> > > > > The alternative is not enabling SW_PAN (at runtime) if this errata is
> > > > > present, along with a warning stating that hardware-PAN should be
> > > > > enabled in kconfig instead. Not sure what distributions will make of 
> > > > > that
> > > > > though.
> > > > 
> > > > The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled
> > > > and in the absence of hardware PAN (or ARM64_PAN disabled),
> > > > cpu_do_switch_mm is no longer called for user process switching, so the
> > > > workaround is pretty much useless.
> > > 
> > > Oh, I see what you mean now.
> > > 
> > > > I'm ok with adding the Kconfig dependency below to
> > > > QCOM_FALKOR_ERRATUM_1003:
> > > > 
> > > > depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN
> > > > 
> > > > together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.
> > > 
> > > That makes it look like hardware-PAN is the cause of the erratum.
> > 
> > With the right Kconfig comment we could make this clearer.
> 
> It's not just a comment though, the kconfig option for the workaround
> will disappear from menuconfig as long as the dependencies aren't met.
> The dependency is really that SW_PAN depends on !ERRATUM_1003, but that
> doesn't work for the distributions.

I agree.

> > > Maybe
> > > just select ARM64_PAN if the erratum workaround is selected, then
> > > runtime warning if we find that the h/w doesn't have PAN but does have
> > > the erratum (which should never fire)?
> > 
> > You still need this workaround even if you don't want any PAN (both sw
> > and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not
> > a dependency. It's more like if you do need a PAN, make sure you only
> > use the hw one.
> 
> True, in the case that all PAN options are disabled we still want this
> to work. How about:
> 
>   select ARM64_PAN if ARM64_SW_TTBR0_PAN

As I replied to myself, the above would work for me as well, so let's go
for this.

> In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a
> config combination? Why not just have "PAN" that enables them both and
> uses the hardware feature if it's there?

Because SW PAN has a non-trivial performance hit. You would enable SW
PAN only if you are paranoid about security. HW PAN, OTOH, is very cheap
and I wouldn't want to miss enabling it in a single Image supporting
ARMv8.0 and ARMv8.1 just because SW PAN is slow on ARMv8.0.

IOW, ARM64_PAN is default y while ARM64_SW_TTBR0_PAN is default n.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 01, 2017 at 06:34:01PM +, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 06:22:44PM +0000, Catalin Marinas wrote:
> > On Wed, Feb 01, 2017 at 05:59:48PM +, Will Deacon wrote:
> > > On Wed, Feb 01, 2017 at 05:49:34PM +0000, Catalin Marinas wrote:
> > > > On Wed, Feb 01, 2017 at 05:41:05PM +, Will Deacon wrote:
> > > > > Maybe
> > > > > just select ARM64_PAN if the erratum workaround is selected, then
> > > > > runtime warning if we find that the h/w doesn't have PAN but does have
> > > > > the erratum (which should never fire)?
> > > > 
> > > > You still need this workaround even if you don't want any PAN (both sw
> > > > and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not
> > > > a dependency. It's more like if you do need a PAN, make sure you only
> > > > use the hw one.
> > > 
> > > True, in the case that all PAN options are disabled we still want this
> > > to work. How about:
> > > 
> > >   select ARM64_PAN if ARM64_SW_TTBR0_PAN
> > 
> > As I replied to myself, the above would work for me as well, so let's go
> > for this.
> > 
> > > In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a
> > > config combination? Why not just have "PAN" that enables them both and
> > > uses the hardware feature if it's there?
> > 
> > Because SW PAN has a non-trivial performance hit. You would enable SW
> > PAN only if you are paranoid about security. HW PAN, OTOH, is very cheap
> > and I wouldn't want to miss enabling it in a single Image supporting
> > ARMv8.0 and ARMv8.1 just because SW PAN is slow on ARMv8.0.
> > 
> > IOW, ARM64_PAN is default y while ARM64_SW_TTBR0_PAN is default n.
> 
> Ok, in that case, then how about another permutation: we make
> ARM64_SW_TTBR0_PAN depend on ARM64_PAN? Then when you select "PAN Support"
> you get a new menu option underneath it for the emulation? I think that
> solves the erratum case and the use-case above.

The problem is that ARM64_PAN is an ARMv8.1 feature and currently
grouped accordingly in Kconfig. ARM64_SW_TTBR0_PAN is complementary (and
even not recommended on ARMv8.1). We can do this if we break the ARMv8.x
feature grouping but just for this erratum, I don't think it's worth.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Feb 08, 2017 at 03:08:37PM -0500, Christopher Covington wrote:
> The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries
> using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum
> is triggered, page table entries using the new translation table base
> address (BADDR) will be allocated into the TLB using the old ASID. All
> circumstances leading to the incorrect ASID being cached in the TLB arise
> when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory
> operation is in the process of performing a translation using the specific
> TTBRx_EL1 being written, and the memory operation uses a translation table
> descriptor designated as non-global. EL2 and EL3 code changing the EL1&0
> ASID is not subject to this erratum because hardware is prohibited from
> performing translations from an out-of-context translation regime.
> 
> Consider the following pseudo code.
> 
>   write new BADDR and ASID values to TTBRx_EL1
> 
> Replacing the above sequence with the one below will ensure that no TLB
> entries with an incorrect ASID are used by software.
> 
>   write reserved value to TTBRx_EL1[ASID]
>   ISB
>   write new value to TTBRx_EL1[BADDR]
>   ISB
>   write new value to TTBRx_EL1[ASID]
>   ISB
> 
> When the above sequence is used, page table entries using the new BADDR
> value may still be incorrectly allocated into the TLB using the reserved
> ASID. Yet this will not reduce functionality, since TLB entries incorrectly
> tagged with the reserved ASID will never be hit by a later instruction.
> 
> Based on work by Shanker Donthineni <shank...@codeaurora.org>
> 
> Signed-off-by: Christopher Covington <c...@codeaurora.org>

Reviewed-by: Catalin Marinas <catalin.mari...@arm.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ILP32 for ARM64: testing with glibc testsuite

[Catalin Marinas]

On Wed, Nov 16, 2016 at 03:22:26PM +0400, Maxim Kuvyrkov wrote:
> Regarding ILP32 runtime, my opinion is that it is acceptable for ILP32
> to have extra failures compared to LP64, since these are not
> regressions, but, rather, failures of a new configuration.

I disagree with this. We definitely need to understand why they fail,
otherwise we run the risk of potential glibc or kernel implementation
bugs becoming ABI.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/18] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Tue, Dec 06, 2016 at 11:55:08AM +0530, Yury Norov wrote:
> On Mon, Dec 05, 2016 at 04:34:23PM +0000, Catalin Marinas wrote:
> > On Fri, Oct 21, 2016 at 11:33:15PM +0300, Yury Norov wrote:
> > > New aarch32 ptrace syscall handler is introduced to avoid run-time
> > > detection of the task type.
> > 
> > What's wrong with the run-time detection? If it's just to avoid a
> > negligible overhead, I would rather keep the code simpler by avoiding
> > duplicating the generic compat_sys_ptrace().
> 
> Nothing wrong. This is how Arnd asked me to do. You already asked this
> question: http://lkml.iu.edu/hypermail/linux/kernel/1604.3/00930.html

Hmm, I completely forgot about this ;). There is still an advantage to
doing run-time checking if we avoid touching core code (less acks to
gather and less code duplication).

Let's see what Arnd says but the initial patch looked simpler.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/18] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Wed, Dec 07, 2016 at 09:40:13PM +0100, Arnd Bergmann wrote:
> On Wednesday, December 7, 2016 4:59:13 PM CET Catalin Marinas wrote:
> > On Tue, Dec 06, 2016 at 11:55:08AM +0530, Yury Norov wrote:
> > > On Mon, Dec 05, 2016 at 04:34:23PM +0000, Catalin Marinas wrote:
> > > > On Fri, Oct 21, 2016 at 11:33:15PM +0300, Yury Norov wrote:
> > > > > New aarch32 ptrace syscall handler is introduced to avoid run-time
> > > > > detection of the task type.
> > > > 
> > > > What's wrong with the run-time detection? If it's just to avoid a
> > > > negligible overhead, I would rather keep the code simpler by avoiding
> > > > duplicating the generic compat_sys_ptrace().
> > > 
> > > Nothing wrong. This is how Arnd asked me to do. You already asked this
> > > question: http://lkml.iu.edu/hypermail/linux/kernel/1604.3/00930.html
> > 
> > Hmm, I completely forgot about this ;). There is still an advantage to
> > doing run-time checking if we avoid touching core code (less acks to
> > gather and less code duplication).
> > 
> > Let's see what Arnd says but the initial patch looked simpler.
> 
> I don't currently have either version of the patch in my inbox
> (the archive is on a different machine), but in general I'd still
> think it's best to avoid the runtime check for aarch64-ilp32
> altogether. I'd have to look at the overall kernel source to
> see if it's worth avoiding one or two instances though, or
> if there are an overwhelming number of other checks that we
> can't avoid at all.

Just in case you haven't found them already, current version:

https://marc.info/?l=linux-arm-kernel=147708276818318=2

Original version:

https://patchwork.kernel.org/patch/7980521/

The old one looks more readable and given that ptrace is not really a
fast path, I'm not two worried about run-time checks

> Regarding ptrace, I notice that arch/tile doesn't even use
> the compat entry point for its ilp32 user space on 64-bit
> kernels, it just calls the regular 64-bit one. Would that
> help here?

I don't know whether it would work, we have incompatible siginfo_t on
AArch64/ILP32.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

Some minor comments below, nothing fundamental (as long as you say the
new sequence doesn't have the speculative TLB load problem I mentioned
on a previous version).

On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> diff --git a/Documentation/arm64/silicon-errata.txt 
> b/Documentation/arm64/silicon-errata.txt
> index 405da11..7151aed 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the 
> Linux Kernel and
>  will be updated when new workarounds are committed and backported to
>  stable kernels.
>  
> -| Implementor| Component   | Erratum ID  | Kconfig   
>   |
> -++-+-+-+
> -| ARM| Cortex-A53  | #826319 | ARM64_ERRATUM_826319  
>   |
> -| ARM| Cortex-A53  | #827319 | ARM64_ERRATUM_827319  
>   |
> -| ARM| Cortex-A53  | #824069 | ARM64_ERRATUM_824069  
>   |
> -| ARM| Cortex-A53  | #819472 | ARM64_ERRATUM_819472  
>   |
> -| ARM| Cortex-A53  | #845719 | ARM64_ERRATUM_845719  
>   |
> -| ARM| Cortex-A53  | #843419 | ARM64_ERRATUM_843419  
>   |
> -| ARM| Cortex-A57  | #832075 | ARM64_ERRATUM_832075  
>   |
> -| ARM| Cortex-A57  | #852523 | N/A   
>   |
> -| ARM| Cortex-A57  | #834220 | ARM64_ERRATUM_834220  
>   |
> -| ARM| Cortex-A72  | #853709 | N/A   
>   |
> -| ARM| MMU-500 | #841119,#826419 | N/A   
>   |
> -|| | |   
>   |
> -| Cavium | ThunderX ITS| #22375, #24313  | CAVIUM_ERRATUM_22375  
>   |
> -| Cavium | ThunderX ITS| #23144  | CAVIUM_ERRATUM_23144  
>   |
> -| Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154  
>   |
> -| Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456  
>   |
> -| Cavium | ThunderX SMMUv2 | #27704  | N/A  |
> -|| | |   
>   |
> -| Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585   
>   |
> +| Implementor   | Component   | Erratum ID  | Kconfig
>   |
> ++---+-+-+--+
> +| ARM   | Cortex-A53  | #826319 | ARM64_ERRATUM_826319   
>   |
> +| ARM   | Cortex-A53  | #827319 | ARM64_ERRATUM_827319   
>   |
> +| ARM   | Cortex-A53  | #824069 | ARM64_ERRATUM_824069   
>   |
> +| ARM   | Cortex-A53  | #819472 | ARM64_ERRATUM_819472   
>   |
> +| ARM   | Cortex-A53  | #845719 | ARM64_ERRATUM_845719   
>   |
> +| ARM   | Cortex-A53  | #843419 | ARM64_ERRATUM_843419   
>   |
> +| ARM   | Cortex-A57  | #832075 | ARM64_ERRATUM_832075   
>   |
> +| ARM   | Cortex-A57  | #852523 | N/A
>   |
> +| ARM   | Cortex-A57  | #834220 | ARM64_ERRATUM_834220   
>   |
> +| ARM   | Cortex-A72  | #853709 | N/A
>   |
> +| ARM   | MMU-500 | #841119,#826419 | N/A
>   |
> +|   | | |
>   |
> +| Cavium| ThunderX ITS| #22375, #24313  | CAVIUM_ERRATUM_22375   
>   |
> +| Cavium| ThunderX ITS| #23144  | CAVIUM_ERRATUM_23144   
>   |
> +| Cavium| ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154   
>   |
> +| Cavium| ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456   
>   |
> +| Cavium| ThunderX SMMUv2 | #27704  | N/A
>   |
> +|   | | |
>   |
> +| Freescale/NXP | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585
>   |
> +| Qualcomm  | Falkor v1   | E1003   | 
> QCOM_FALKOR_ERRATUM_1003 |

Please don't change the "Implementor" column width, there is no point
and it makes the patch harder to read (i.e. this hunk should only have
one line).

> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 4c63cb1..5a0a82a 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
>   /* Update the list of reserved ASIDs and the ASID bitmap. */
>   bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
>  
> + /* Reserve ASID for Falkor erratum 1003 */
> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
> 

Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Jan 11, 2017 at 06:37:39PM +, Mark Rutland wrote:
> On Wed, Jan 11, 2017 at 12:35:55PM -0600, Timur Tabi wrote:
> > On 01/11/2017 12:33 PM, Mark Rutland wrote:
> > >It'll need to affect all lines since the kconfig column needs to expand
> > >by at least one character to fit QCOM_FALKOR_ERRATUM_1003.
> > 
> > Or we can make the macro shorter.
> 
> The name, as it is, is perfectly descriptive.
> 
> Let's not sacrifice legibility over a non-issue.

I agree, I didn't realise that the we expand the last column already.
It's a non-issue indeed.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Jan 11, 2017 at 06:40:52PM +, Mark Rutland wrote:
> On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> > On 11/01/17 18:06, Catalin Marinas wrote:
> > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > >> index 32682be..9ee46df 100644
> > >> --- a/arch/arm64/mm/proc.S
> > >> +++ b/arch/arm64/mm/proc.S
> > >> @@ -23,6 +23,7 @@
> > >>  #include 
> > >>  #include 
> > >>  #include 
> > >> +#include 
> > >>  #include 
> > >>  #include 
> > >>  #include 
> > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> > >>  ENTRY(cpu_do_switch_mm)
> > >>  mmidx1, x1  // get mm->context.id
> > >>  bfi x0, x1, #48, #16// set the ASID
> > >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
> > >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
> > >> +mrs x2, ttbr0_el1
> > >> +mov x3, #FALKOR_RESERVED_ASID
> > >> +bfi x2, x3, #48, #16// reserved ASID + old 
> > >> BADDR
> > >> +msr ttbr0_el1, x2
> > >> +isb
> > >> +bfi x2, x0, #0, #48 // reserved ASID + new 
> > >> BADDR
> > >> +msr ttbr0_el1, x2
> > >> +isb
> > >> +alternative_else_nop_endif
> > >> +#endif
> > >>  msr ttbr0_el1, x0   // set TTBR0
> > >>  isb
> > >>  post_ttbr0_update_workaround
> > > 
> > > Please move the above hunk to a pre_ttbr0_update_workaround macro for
> > > consistency with post_ttbr0_update_workaround.
> > 
> > In which case (and also for consistency), should we add that pre_ttbr0
> > macro to entry.S, just before __uaccess_ttbr0_enable? It may not be
> > needed in the SW pan case, but it is probably worth entertaining the
> > idea that there may be something to do there...
> 
> Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0().

This may be fine if my assumptions about this erratum are correct. In
the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without
any entries, so no new entries could be tagged with the old ASID.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

[Catalin Marinas]

On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> On 11/01/17 18:06, Catalin Marinas wrote:
> > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >> index 32682be..9ee46df 100644
> >> --- a/arch/arm64/mm/proc.S
> >> +++ b/arch/arm64/mm/proc.S
> >> @@ -23,6 +23,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> >>  ENTRY(cpu_do_switch_mm)
> >>mmidx1, x1  // get mm->context.id
> >>bfi x0, x1, #48, #16// set the ASID
> >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
> >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
> >> +  mrs x2, ttbr0_el1
> >> +  mov x3, #FALKOR_RESERVED_ASID
> >> +  bfi x2, x3, #48, #16// reserved ASID + old BADDR
> >> +  msr ttbr0_el1, x2
> >> +  isb
> >> +  bfi x2, x0, #0, #48 // reserved ASID + new BADDR
> >> +  msr ttbr0_el1, x2
> >> +  isb
> >> +alternative_else_nop_endif
> >> +#endif
> >>msr ttbr0_el1, x0   // set TTBR0
> >>isb
> >>post_ttbr0_update_workaround
> > 
> > Please move the above hunk to a pre_ttbr0_update_workaround macro for
> > consistency with post_ttbr0_update_workaround.
> 
> In which case (and also for consistency), should we add that pre_ttbr0
> macro to entry.S, just before __uaccess_ttbr0_enable? It may not be
> needed in the SW pan case, but it is probably worth entertaining the
> idea that there may be something to do there...

It may actually be needed in entry.S as well. With SW PAN, we move the
context switching from cpu_do_switch_mm to the kernel_exit macro when
returning to user. In this case we are switching from the reserved ASID
0 and reserved TTBR0_EL1 (pointing to a zeroed page) to the user's
TTBR0_EL1 and ASID. If the ASID switch isn't taken into account, we may
end up with new TLB entries being tagged with the reserved ASID. Apart
from a potential loss of protection with TTBR0 PAN, is there anything
else that could go wrong? Maybe a TLB conflict if we mix TLBs from
multiple address spaces tagged with the same reserved ASID.

If the above is an issue, we would need to patch
__uaccess_ttbr0_enable() as well, though I'm more inclined to make this
erratum not selectable when TTBR0 PAN is enabled.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ILP32 for ARM64 - testing with lmbench

[Catalin Marinas]

On Mon, Dec 05, 2016 at 06:16:09PM +0800, Zhangjian (Bamvor) wrote:
> Do you have suggestion of next move of upstreaming ILP32?

I mentioned the steps a few time before. I'm pasting them again here:

1. Complete the review of the Linux patches and ABI (no merge yet)
2. Review the corresponding glibc patches (no merge yet)
3. Ask (Linaro, Cavium) for toolchain + filesystem (pre-built and more
   than just busybox) to be able to reproduce the testing in ARM
4. More testing (LTP, trinity, performance regressions etc.)
5. Move the ILP32 PCS out of beta (based on the results from 4)
6. Check the market again to see if anyone still needs ILP32
7. Based on 6, decide whether to merge the kernel and glibc patches

What's not explicitly mentioned in step 4 is glibc testing. Point 5 is
ARM's responsibility (toolchain folk).

> There are already the test results of lmbench and specint. Do you they
> are ok or need more data to prove no regression?

I would need to reproduce the tests myself, see step 3.

> I have also noticed that there are ILP32 failures in glibc testsuite.
> Is it the only blocker for merge ILP32(in technology part)?

It's probably not the only blocker but I have to review the kernel
patches again to make sure. I'd also like to see whether the libc-alpha
community is ok with the glibc counterpart (but don't merge the patches
until the ABI is agreed on both sides).

On performance, I want to make sure there are no regressions on
AArch32/compat and AArch64/LP64.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/18] arm64: introduce binfmt_elf32.c

[Catalin Marinas]

On Fri, Oct 21, 2016 at 11:33:08PM +0300, Yury Norov wrote:
> As we support more than one compat formats, it looks more reasonable
> to not use fs/compat_binfmt.c. Custom binfmt_elf32.c allows to move aarch32
> specific definitions there and make code more maintainable and readable.

Can you remind me why we need this patch (rather than using the default
fs/compat_binfmt_elf.c which you include here anyway)?

> --- /dev/null
> +++ b/arch/arm64/kernel/binfmt_elf32.c
> @@ -0,0 +1,31 @@
> +/*
> + * Support for AArch32 Linux ELF binaries.
> + */
> +
> +/* AArch32 EABI. */
> +#define EF_ARM_EABI_MASK 0xff00
> +
> +#define compat_start_thread  compat_start_thread
> +#define COMPAT_SET_PERSONALITY(ex)   \
> +do { \
> + clear_thread_flag(TIF_32BIT_AARCH64);   \
> + set_thread_flag(TIF_32BIT); \
> +} while (0)

You introduce this here but it seems to still be present in asm/elf.h.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/18] arm64: ilp32: introduce binfmt_ilp32.c

[Catalin Marinas]

On Fri, Oct 21, 2016 at 11:33:09PM +0300, Yury Norov wrote:
> binfmt_ilp32.c is needed to handle ILP32 binaries
> 
> Signed-off-by: Yury Norov 
> Signed-off-by: Bamvor Zhang Jian 
> ---
>  arch/arm64/include/asm/elf.h |  6 +++
>  arch/arm64/kernel/Makefile   |  1 +
>  arch/arm64/kernel/binfmt_ilp32.c | 97 
> 
>  3 files changed, 104 insertions(+)
>  create mode 100644 arch/arm64/kernel/binfmt_ilp32.c
> 
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index f259fe8..be29dde 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -175,10 +175,16 @@ extern int arch_setup_additional_pages(struct 
> linux_binprm *bprm,
>  
>  #define COMPAT_ELF_ET_DYN_BASE   (2 * TASK_SIZE_32 / 3)
>  
> +#ifndef USE_AARCH64_GREG
>  /* AArch32 registers. */
>  #define COMPAT_ELF_NGREG 18
>  typedef unsigned int compat_elf_greg_t;
>  typedef compat_elf_greg_tcompat_elf_gregset_t[COMPAT_ELF_NGREG];
> +#else /* AArch64 registers for AARCH64/ILP32 */
> +#define COMPAT_ELF_NGREG ELF_NGREG
> +#define compat_elf_greg_telf_greg_t
> +#define compat_elf_gregset_t elf_gregset_t
> +#endif

I think you only need compat_elf_gregset_t definition here and leave the
other two undefined.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 14/18] arm64: signal32: move ilp32 and aarch32 common code to separated file

[Catalin Marinas]

On Fri, Oct 21, 2016 at 11:33:13PM +0300, Yury Norov wrote:
> Signed-off-by: Yury Norov 

Please add some description, even if it means copying the subject.

> ---
>  arch/arm64/include/asm/signal32.h|   3 +
>  arch/arm64/include/asm/signal32_common.h |  27 +++
>  arch/arm64/kernel/Makefile   |   2 +-
>  arch/arm64/kernel/signal32.c | 107 
>  arch/arm64/kernel/signal32_common.c  | 135 
> +++
>  5 files changed, 166 insertions(+), 108 deletions(-)
>  create mode 100644 arch/arm64/include/asm/signal32_common.h
>  create mode 100644 arch/arm64/kernel/signal32_common.c

I wonder whether you can make such patches more readable by setting
"diff.renames" to "copy" in your gitconfig (unless it's set already and
Git cannot detect partial file code moving/copying).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/18] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Fri, Oct 21, 2016 at 11:33:15PM +0300, Yury Norov wrote:
> New aarch32 ptrace syscall handler is introduced to avoid run-time
> detection of the task type.

What's wrong with the run-time detection? If it's just to avoid a
negligible overhead, I would rather keep the code simpler by avoiding
duplicating the generic compat_sys_ptrace().

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 16/18] arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32

[Catalin Marinas]

On Fri, Jan 06, 2017 at 02:10:03AM +0530, Yury Norov wrote:
> On Wed, Dec 07, 2016 at 09:40:13PM +0100, Arnd Bergmann wrote:
> > On Wednesday, December 7, 2016 4:59:13 PM CET Catalin Marinas wrote:
> > > On Tue, Dec 06, 2016 at 11:55:08AM +0530, Yury Norov wrote:
> > > > On Mon, Dec 05, 2016 at 04:34:23PM +, Catalin Marinas wrote:
> > > > > On Fri, Oct 21, 2016 at 11:33:15PM +0300, Yury Norov wrote:
> > > > > > New aarch32 ptrace syscall handler is introduced to avoid run-time
> > > > > > detection of the task type.
> > > > > 
> > > > > What's wrong with the run-time detection? If it's just to avoid a
> > > > > negligible overhead, I would rather keep the code simpler by avoiding
> > > > > duplicating the generic compat_sys_ptrace().
> > > > 
> > > > Nothing wrong. This is how Arnd asked me to do. You already asked this
> > > > question: http://lkml.iu.edu/hypermail/linux/kernel/1604.3/00930.html
> > > 
> > > Hmm, I completely forgot about this ;). There is still an advantage to
> > > doing run-time checking if we avoid touching core code (less acks to
> > > gather and less code duplication).
> > > 
> > > Let's see what Arnd says but the initial patch looked simpler.
> > 
> > I don't currently have either version of the patch in my inbox
> > (the archive is on a different machine), but in general I'd still
> > think it's best to avoid the runtime check for aarch64-ilp32
> > altogether. I'd have to look at the overall kernel source to
> > see if it's worth avoiding one or two instances though, or
> > if there are an overwhelming number of other checks that we
> > can't avoid at all.
> > 
> > Regarding ptrace, I notice that arch/tile doesn't even use
> > the compat entry point for its ilp32 user space on 64-bit
> > kernels, it just calls the regular 64-bit one. Would that
> > help here?
> 
> ILP32 tasks has unique context that is not like aarch64 or aarch32,
> so we have to have unique ptrace handler. I prepared the patch for
> ptrace with runtime ABI detection, as Catalin said, see there:
> https://github.com/norov/linux/commit/1f66dc22a4450b192e83458f2c3cc0e79f53e670
> 
> If it's OK, I'd like to update submission.

This looks better to me (and even better if you no longer need to touch
the generic ptrace code).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC3 nowrap: PATCH v7 00/18] ILP32 for ARM64

[Catalin Marinas]

On Sun, Dec 18, 2016 at 12:38:23PM +0530, Yury Norov wrote:
> On Fri, Oct 21, 2016 at 11:32:59PM +0300, Yury Norov wrote:
> > This series enables aarch64 with ilp32 mode, and as supporting work,
> > introduces ARCH_32BIT_OFF_T configuration option that is enabled for
> > existing 32-bit architectures but disabled for new arches (so 64-bit
> > off_t is is used by new userspace).
> > 
> > This version is based on kernel v4.9-rc1.  It works with glibc-2.24,
> > and tested with LTP.
>  
> Hi Arnd, Catalin
> 
> For last few days I'm trying to rebase this series on current master,
> and I see significant conflicts and regressions. In fact, every time
> I rebase on next rc1, I feel like I play a roulette.
> 
> This is not a significant problem now because it's almost for sure
> that this series will not get into 4.10, for reasons not related to
> kernel code. And I have time to deal with regressions. But in general,
> I'd like to try my patches on top of other candidates for next merge
> window. I cannot read all emails in LKML, but I can easily detect
> problems and join to the discussion at early stage if I see any problem.
> 
> This is probably a noob question, and there are well-known branches,
> like Andrew Morton's one. But at this stage it's very important to
> have this series prepared for merge, and I'd prefer to ask about it.

I'm not entirely sure what the question is. For development, you could
base your series on a final release, e.g. 4.9. For reviews and
especially if you are targeting a certain merging window, it's useful to
rebase your patches on a fairly recent -rc, e.g. 4.10-rc3. I would
entirely skip any non-tagged kernel states (like middle of the merging
window) or out of tree branches. There may be a case to rebase on some
other developer's branch but only if there is a dependency that can't be
avoided and usually with prior agreement from both the respective
developer (as not to rebase the branch) and the involved maintainers.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/18] arm64: ilp32: introduce binfmt_ilp32.c

[Catalin Marinas]

On Thu, Dec 22, 2016 at 12:26:40AM +0530, Yury Norov wrote:
> On Mon, Dec 05, 2016 at 03:38:01PM +0000, Catalin Marinas wrote:
> > On Fri, Oct 21, 2016 at 11:33:09PM +0300, Yury Norov wrote:
> > > binfmt_ilp32.c is needed to handle ILP32 binaries
> > > 
> > > Signed-off-by: Yury Norov <yno...@caviumnetworks.com>
> > > Signed-off-by: Bamvor Zhang Jian <bamvor.zhangj...@linaro.org>
> > > ---
> > >  arch/arm64/include/asm/elf.h |  6 +++
> > >  arch/arm64/kernel/Makefile   |  1 +
> > >  arch/arm64/kernel/binfmt_ilp32.c | 97 
> > > 
> > >  3 files changed, 104 insertions(+)
> > >  create mode 100644 arch/arm64/kernel/binfmt_ilp32.c
> > > 
> > > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> > > index f259fe8..be29dde 100644
> > > --- a/arch/arm64/include/asm/elf.h
> > > +++ b/arch/arm64/include/asm/elf.h
> > > @@ -175,10 +175,16 @@ extern int arch_setup_additional_pages(struct 
> > > linux_binprm *bprm,
> > >  
> > >  #define COMPAT_ELF_ET_DYN_BASE   (2 * TASK_SIZE_32 / 3)
> > >  
> > > +#ifndef USE_AARCH64_GREG
> > >  /* AArch32 registers. */
> > >  #define COMPAT_ELF_NGREG 18
> > >  typedef unsigned int compat_elf_greg_t;
> > >  typedef compat_elf_greg_t
> > > compat_elf_gregset_t[COMPAT_ELF_NGREG];
> > > +#else /* AArch64 registers for AARCH64/ILP32 */
> > > +#define COMPAT_ELF_NGREG ELF_NGREG
> > > +#define compat_elf_greg_telf_greg_t
> > > +#define compat_elf_gregset_t elf_gregset_t
> > > +#endif
> > 
> > I think you only need compat_elf_gregset_t definition here and leave the
> > other two undefined.
> 
> I checked everything here again, and found that almost all compat defines
> may be moved to corresponding binfmt files. If everything is OK, I'll
> incorporate next patch to the series

It seems fine at a quick look but I'll have to see the final patch.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

Hi Yury,

Just a quick reply as I'm about to go on holiday for the next two weeks.

On Fri, Jul 07, 2017 at 12:59:02AM +0300, Yury Norov wrote:
> On Thu, Jun 29, 2017 at 05:10:36PM +0100, Catalin Marinas wrote:
> > On Mon, Jun 19, 2017 at 06:49:43PM +0300, Yury Norov wrote:
> > > This series enables aarch64 with ilp32 mode.
> > 
> > What I'd like to propose is that Will and I (as arm64 maintainers, maybe
> > with with the help of others including this series' authors) take over
> > the series and push it to a staging branch under the arm64 kernel on
> > git.kernel.org. This is aimed as a commitment to keep the ABI *stable*
> > and will be rebased with every kernel release (starting with 4.13). The
> > decision to merge upstream will be revisited every 6 months, assessing
> > the progress on the points I mentioned above, with a time limit of 2
> > years when, if still not upstream, we will stop maintaining such branch.
> 
> Thanks for the email. I appreciate your concern about long-term
> support load for a new ABI. I also think that stabilizing the ABI
> is a good idea.
> 
> At this point, most people expect the ABI to not change unless
> critical issues are uncovered. IMO, if there is a good technical reason
> to change the ABI -- the change will happen even on the "staging" branch.

What I would like is that we change the ABI *only* if there is a serious
bug, otherwise we should try hard to keep it stable. The "good technical
reason" can be subjective (i.e. let's pass 64-bit arguments in a single
register because some benchmark is slightly faster; with a wider user
base, we may get more suggestions for ABI changes that we should
reject).

> And vise-versa, if there is no need for a change, the ABI will be stable
> on my local branch, just like on staging branch you propose. I think it
> will be that way until there will appear strong community of users who
> will resist the changing of ABI. From this point of view, I don't see
> major difference for ILP32 where to host the patchset.

If we don't treat the ABI as stable now, regardless of the number of
users, then it is not ready for upstream (we already have a user in the
openSUSE build).

The arm64 git.kernel.org suggestion was more of an endorsement from the
maintainers that the ABI is stable. If you want to maintain it in your
own tree, that's fine by me. If you want wider visibility, we could
mirror it to git.kernel.org (though given how many trees are there, it
may not mean much).

> Is my understanding correct that you, Will and me will be responsible
> for rebasing and maintainance of patches? To be clear, this it not an
> automatic task - sometimes simple rebase take the whole day of my time,
> and I rebase almost every week. The rebase in 2-month timeframe may become
> unpredictable task, by time and amount of work. I think you understand what
> I mean, as once before you already told that the series is too intrusive.

I am fully aware there is significant effort to rebase the series and if
you can help maintain such branch it would be great. I don't see the
point of rebasing weekly though and it's not just the git handling
process but doing the validation of such branch, regression testing etc.

If it's too time consuming, we could do it only for LTS releases.

> To make it more easy for maintainance, I would suggest to split the series
> to 3 parts:
>  - arch and generic patches that not related to ilp32 or arm64 (already
>done);

That's fine.

>  - arm64 patches that do cleanup and refactoring in aim to apply
>ilp32 patches smoothly, but not ilp32-specific;

If there are such changes, that's fine. However, I wouldn't merge the
AARCH32_EL0 #ifdef'ery since it's unnecessary if we never merge the
ILP32 patches.

> If we'll follow your suggestion, does it mean that you expect the 4.12-based
> branch from me soon to put on staging?

We can create one for 4.12 if you want (in about 2-3 weeks time when I'm
back). If there is no rush we could aim for 4.13 (there are some
non-trivial conflicts in the sigframe handling code as we are preparing
it for SVE support).

> > The decision to merge upstream will be revisited every 6 months,
> > assessing the progress on the points I mentioned above, with a time
> > limit of 2 years
> 
> IIUC, this is your personal decision based on responses and comments
> from community?

Yes, as arm64 kernel maintainer.

> If so, I would like to ask you to do the first ILP32 community poll
> now, not in 6 months. So we'll collect opinions and requests from
> people interested in ILP32, and in 6 month will be able to check the
> progress. I would like to see this thread public because if we are not
> taking ILP32 to official sources right now, this is the only way to
> inform people that the p

Re: [PATCH v2] kmemleak: add oom=<disable|ignore> runtime parameter

[Catalin Marinas]

On Mon, Jul 24, 2017 at 05:16:34PM +0800, shuw...@redhat.com wrote:
> When running memory stress tests, kmemleak could be easily disabled in
> function create_object as system is out of memory and kmemleak failed to
> alloc from object_cache. Since there's no way to enable kmemleak after
> it's off, simply ignore the object_cache alloc failure will just loses
> track of some memory objects, but could increase the usability of kmemleak
> under memory stress.

I wonder how usable kmemleak is when not recording all the allocated
objects. If some of these memory blocks contain references to others,
such referenced objects could be reported as leaks (basically increasing
the false positives rate).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

Hi Yury,

On Mon, Jul 24, 2017 at 02:26:24PM +0300, Yury Norov wrote:
> On Fri, Jul 07, 2017 at 06:11:36PM +0100, Catalin Marinas wrote:
> > On Fri, Jul 07, 2017 at 12:59:02AM +0300, Yury Norov wrote:
> > > If so, I would like to ask you to do the first ILP32 community poll
> > > now, not in 6 months. So we'll collect opinions and requests from
> > > people interested in ILP32, and in 6 month will be able to check the
> > > progress. I would like to see this thread public because if we are not
> > > taking ILP32 to official sources right now, this is the only way to
> > > inform people that the project exists and is ready to use.
> > 
> > That's an ongoing process, I'm not going to ask for people's opinion
> > every 6 months. It's just that I will revisit periodically the progress
> > on automated testing, public availability of a cross-toolchain,
> > Tested/Acked/Reviewed-by tags on these patches from interested parties.
> > Since I haven't seen any of these now, I don't see any point in asking.
> > 
> > To be clear, I'm not really interested in "we need this too" opinions, I
> > get lots of these via the marketing channels. I'm looking for actual
> > users with a long-term view of making/keeping ILP32 a first class ABI.
> 
> From my side, there are people who ask me for help with ilo32, and who
> write from big companies mailservers. But they don't want to ask
> their questions publicly for some reason. From my point of view, there
> is not so big but stable interest in ILP32.

It would be nice if they were more open about what they need/use.

> This is the 4.12 and linux-next - based kernel patches:
> https://github.com/norov/linux/tree/ilp32-4.12
> https://github.com/norov/linux/tree/ilp32-20170724

Thanks. I'll publish the 4.12-based branch sometime next week.

At this stage I don't see much value in a linux-next based ILP32. I
would rather like to see a 4.13-rc3 based one, in preparation for a 4.13
branch once released.

> Should I resend kernel patches to LKML, or links above are enough for
> you?

The 4.12 link is ok. However, could you please post a 4.13-rcX based
series, maybe split in two so that a few generic patches can be merged
in 4.14? Given the reworking of the sigcontext code in 4.13, it would be
good to review the ILP32 changes in this area again.

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

Hi Yury,

On Mon, Jun 19, 2017 at 06:49:43PM +0300, Yury Norov wrote:
> This series enables aarch64 with ilp32 mode.

Thanks for putting this series together, I do appreciate the effort.
There are still some review comments coming in but I'm happy with how
the ABI looks now. I did some LTP testing (AArch64/LP64, AArch64/ILP32,
AArch32) and benchmarking and didn't see any regressions (apart from an
LTP bug with sync_file_range2). James Morse is working on reproducing
similar testing in ARM. Szabolcs reported some glibc test-suite
regressions on the libc-alpha list which I assume will be followed up.
VDSO in C is another issue I'd like sorted but this is not strictly
specific to ILP32 and can be done as a follow up. Note that I didn't run
any big-endian tests, though this is something that needs doing.

Now, having agreed on the ABI and implementation very close to being
ready doesn't necessarily make the code suitable for upstream. With my
maintainer hat on, I'm trying to see where ILP32 will be in 2-5-10
years, whether anyone still cares about it in this time frame. The
difference from a driver or SoC support is that ABIs are very hard to
revert, though are as (or even more) likely to bit-rot when not in use
or regularly tested (we have the big-endian experience here).

There are two main aspects to make the code upstream-worthy:

1. Actual/real users (current, future). I don't mean just a few distros
   showing that it can be done but actual/planned real deployments

2. Long term testing/maintenance plan. This is not about kernel code
   maintenance but a healthy ILP32 ecosystem:
   a) readily available toolchains (x86-hosted and AArch64-hosted)
   b) filesystems (can be large distros like openSUSE or more
  embedded-oriented like Yocto or OpenEmbedded)
   c) suitable continuous regression testing (kernel + userland)
   d) commitment from all parties involved (including ARM Ltd) to treat
  the ILP32 ABI as a (nearly) first class citizen

It is pretty clear from private discussions that there are potential
users but at the moment I can't tell if those would turn into real
deployments of production systems. As for (2), the long term plans are
not convincing (or I haven't spotted them yet), so I'd like to see the
interested parties putting a plan together (something along the lines of
kernelci.org + LTP, glibc buildbot).

What I'd like to propose is that Will and I (as arm64 maintainers, maybe
with with the help of others including this series' authors) take over
the series and push it to a staging branch under the arm64 kernel on
git.kernel.org. This is aimed as a commitment to keep the ABI *stable*
and will be rebased with every kernel release (starting with 4.13). The
decision to merge upstream will be revisited every 6 months, assessing
the progress on the points I mentioned above, with a time limit of 2
years when, if still not upstream, we will stop maintaining such branch.

I am aware that the above proposal has an impact on the glibc patches
since they will not merge a new ABI upstream until officially supported
by the kernel. I cc'ed some of the glibc developers and they will follow
up on the libc-alpha list.

> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t userspace type is used by new userspace).
> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
[...]
> Patches 1, 2, 3 and 8 are general, and may be applied separately.

These 4 patches should be merged independently, I don't see a point in
carrying them with the ILP32 series. Arnd, are you ok to push them
upstream?

BTW, patch 3 seems to never make it to the linux-arm-kernel list, I
guess too many on cc.

--
Catalin

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

On Mon, Jul 24, 2017 at 02:26:24PM +0300, Yury Norov wrote:
> This is the 4.12 and linux-next - based kernel patches:
> https://github.com/norov/linux/tree/ilp32-4.12
> https://github.com/norov/linux/tree/ilp32-20170724

I published the 4.12 branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=staging/ilp32-4.12

(or git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git 
staging/ilp32-4.12)

There are two patches on top, one to fix SET_PERSONALITY and the other
to make ILP32 default y since you'd expect people using this branch to
need such option enabled.

I'll publish a 4.13-based branch when the corresponding kernel version
is released.

Thanks.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/20] arm64: ilp32: share aarch32 syscall handlers

[Catalin Marinas]

On Sun, Jun 04, 2017 at 03:00:02PM +0300, Yury Norov wrote:
> off_t is  passed in register pair just like in aarch32.
> In this patch corresponding aarch32 handlers are shared to
> ilp32 code.

Is the comment here relevant? IOW, do we have any AArch64/ILP32 syscall
where off_t is used as an argument? AFAICT, the *64 syscalls use loff_t
or loff_t *.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/20] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Sun, Jun 04, 2017 at 02:59:54PM +0300, Yury Norov wrote:
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -402,7 +402,7 @@ config ARM64_ERRATUM_834220
>  
>  config ARM64_ERRATUM_845719
>   bool "Cortex-A53: 845719: a load might read incorrect data"
> - depends on COMPAT
> + depends on AARCH32_EL0
>   default y
>   help
> This option adds an alternative code sequence to work around ARM
> @@ -784,7 +784,7 @@ config FORCE_MAX_ZONEORDER
>  
>  menuconfig ARMV8_DEPRECATED
>   bool "Emulate deprecated/obsolete ARMv8 instructions"
> - depends on COMPAT
> + depends on AARCH32_EL0
>   help
> Legacy software support may require certain instructions
> that have been deprecated or obsoleted in the architecture.
> @@ -1062,9 +1062,15 @@ menu "Userspace binary formats"
>  source "fs/Kconfig.binfmt"
>  
>  config COMPAT
> + bool
> + depends on AARCH32_EL0

You could just use "def_bool y" here

> +
> +config AARCH32_EL0
>   bool "Kernel support for 32-bit EL0"
> + def_bool y
>   depends on ARM64_4K_PAGES || EXPERT
>   select COMPAT_BINFMT_ELF if BINFMT_ELF
> + select COMPAT

and avoid the explicit select.

>   select HAVE_UID16
>   select OLD_SIGSUSPEND3
>   select COMPAT_OLD_SIGACTION
[...]
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -139,15 +139,17 @@ static int c_show(struct seq_file *m, void *v)
>*/
>   seq_puts(m, "Features\t:");
>   if (compat) {
> -#ifdef CONFIG_COMPAT
> - for (j = 0; compat_hwcap_str[j]; j++)
> - if (compat_elf_hwcap & (1 << j))
> - seq_printf(m, " %s", 
> compat_hwcap_str[j]);
> -
> - for (j = 0; compat_hwcap2_str[j]; j++)
> - if (compat_elf_hwcap2 & (1 << j))
> - seq_printf(m, " %s", 
> compat_hwcap2_str[j]);
> -#endif /* CONFIG_COMPAT */
> +#ifdef CONFIG_AARCH32_EL0
> + if (personality(current->personality) == PER_LINUX32) {
> + for (j = 0; compat_hwcap_str[j]; j++)
> + if (compat_elf_hwcap & (1 << j))
> + seq_printf(m, " %s", 
> compat_hwcap_str[j]);
> +
> + for (j = 0; compat_hwcap2_str[j]; j++)
> + if (compat_elf_hwcap2 & (1 << j))
> + seq_printf(m, " %s", 
> compat_hwcap2_str[j]);
> + }
> +#endif /* CONFIG_AARCH32_EL0 */

I don't understand this hunk. Why do you need another check on
personality? "compat" is already true if PER_LINUX32.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/20] 32-bit ABI: introduce ARCH_32BIT_OFF_T config option

[Catalin Marinas]

On Sun, Jun 04, 2017 at 02:59:51PM +0300, Yury Norov wrote:
> All new 32-bit architectures should have 64-bit off_t type, but existing
> architectures has 32-bit ones.
> 
> To handle it, new config option is added to arch/Kconfig that defaults
> ARCH_32BIT_OFF_T to be disabled for non-64 bit architectures. All existing
> 32-bit architectures enable it explicitly here.
> 
> New option affects force_o_largefile() behaviour. Namely, if off_t is
> 64-bits long, we have no reason to reject user to open big files.
> 
> Note that even if architectures has only 64-bit off_t in the kernel
> (arc, c6x, h8300, hexagon, metag, nios2, openrisc, tile32 and unicore32),
> a libc may use 32-bit off_t, and therefore want to limit the file size
> to 4GB unless specified differently in the open flags.
> 
> Signed-off-by: Yury Norov 
> Acked-by: Arnd Bergmann 
[...]
> diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> index 1b48d9c9a561..297993c92490 100644
> --- a/include/linux/fcntl.h
> +++ b/include/linux/fcntl.h
> @@ -11,7 +11,7 @@
>O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
>  
>  #ifndef force_o_largefile
> -#define force_o_largefile() (BITS_PER_LONG != 32)
> +#define force_o_largefile() (!IS_ENABLED(CONFIG_ARCH_32BIT_OFF_T))
>  #endif

I may have confused myself with which off_t is 64-bit here for new
32-bit architectures. Are we referring to the glibc definition, the
kernel one or simply that force_o_largefile() is true by default.
Because the type off_t for 32-bit kernel builds is still, well, 32-bit.

Otherwise it seems that the first paragraph in the description above
should read "all new 32-bit ABIs on a 64-bit kernel..." but then
AArch64/ILP32 is no longer the same as a new, pure 32-bit architecture.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 19/20] arm64:ilp32: add vdso-ilp32 and use for signal return

[Catalin Marinas]

On Sun, Jun 04, 2017 at 03:00:08PM +0300, Yury Norov wrote:
> From: Philipp Tomsich 
> 
> ILP32 VDSO exports following symbols:
>  __kernel_rt_sigreturn;
>  __kernel_gettimeofday;
>  __kernel_clock_gettime;
>  __kernel_clock_getres.
> 
> What shared object to use, kernel selects depending on result of
> is_ilp32_compat_task() in arch/arm64/kernel/vdso.c, so it substitutes
> correct pages and spec.
> 
> Adjusted to move the data page before code pages in sync with
> commit 601255ae3c98 ("arm64: vdso: move data page before code pages")
> 
> Signed-off-by: Philipp Tomsich 
> Signed-off-by: Christoph Muellner 
> Signed-off-by: Yury Norov 
> Signed-off-by: Bamvor Jian Zhang 
> ---
>  arch/arm64/Makefile   |  3 +
>  arch/arm64/include/asm/vdso.h |  6 ++
>  arch/arm64/kernel/Makefile|  1 +
>  arch/arm64/kernel/asm-offsets.c   |  7 ++
>  arch/arm64/kernel/signal.c|  2 +
>  arch/arm64/kernel/vdso-ilp32/.gitignore   |  2 +
>  arch/arm64/kernel/vdso-ilp32/Makefile | 80 ++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S | 33 ++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S | 95 
> +++
>  arch/arm64/kernel/vdso.c  | 65 +++---
>  arch/arm64/kernel/vdso/gettimeofday.S | 20 +-
>  arch/arm64/kernel/vdso/vdso.S |  6 +-
>  12 files changed, 304 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S

Does this patch get simpler with Andrew Pinski's vdso in C proposal?
I have to read the other thread in detail, Will followed up already.

> diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> index 839ce0031bd5..649a9a416500 100644
> --- a/arch/arm64/include/asm/vdso.h
> +++ b/arch/arm64/include/asm/vdso.h
> @@ -29,6 +29,12 @@
>  
>  #include 
>  
> +#ifdef CONFIG_ARM64_ILP32
> +#include 
> +#else
> +#define vdso_offset_sigtramp_ilp32
> +#endif

BTW, here you could do something like:

#define vdso_offset_sigtramp_ilp32  ({ BUILD_BUG(); 0; })

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/20] arm64: rename COMPAT to AARCH32_EL0 in Kconfig

[Catalin Marinas]

On Fri, Jun 09, 2017 at 01:40:59AM +0300, Yury Norov wrote:
> On Thu, Jun 08, 2017 at 03:09:12PM +0100, Catalin Marinas wrote:
> > On Sun, Jun 04, 2017 at 02:59:54PM +0300, Yury Norov wrote:
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -402,7 +402,7 @@ config ARM64_ERRATUM_834220
> > >  
> > >  config ARM64_ERRATUM_845719
> > >   bool "Cortex-A53: 845719: a load might read incorrect data"
> > > - depends on COMPAT
> > > + depends on AARCH32_EL0
> > >   default y
> > >   help
> > > This option adds an alternative code sequence to work around ARM
> > > @@ -784,7 +784,7 @@ config FORCE_MAX_ZONEORDER
> > >  
> > >  menuconfig ARMV8_DEPRECATED
> > >   bool "Emulate deprecated/obsolete ARMv8 instructions"
> > > - depends on COMPAT
> > > + depends on AARCH32_EL0
> > >   help
> > > Legacy software support may require certain instructions
> > > that have been deprecated or obsoleted in the architecture.
> > > @@ -1062,9 +1062,15 @@ menu "Userspace binary formats"
> > >  source "fs/Kconfig.binfmt"
> > >  
> > >  config COMPAT
> > > + bool
> > > + depends on AARCH32_EL0
> > 
> > You could just use "def_bool y" here
> > 
> > > +
> > > +config AARCH32_EL0
> > >   bool "Kernel support for 32-bit EL0"
> > > + def_bool y
> > >   depends on ARM64_4K_PAGES || EXPERT
> > >   select COMPAT_BINFMT_ELF if BINFMT_ELF
> > > + select COMPAT
> > 
> > and avoid the explicit select.
> 
> in patch 20 COMPAT becomes depending also on ARM64_ILP32, like this:
> -   depends on AARCH32_EL0
> +   depends on AARCH32_EL0 || ARM64_ILP32
> 
> So this is a preparation for it. If it looks confusing, I think it's
> better to underline it in the description to the patch in addition to
> this:
> 
> > From now, AARCH32_EL0 (former COMPAT) config option means the support of
> > AARCH32 userspace, ARM64_ILP32 - support of ILP32 ABI (see next patches),
> > and COMPAT indicates that one of them, or both, is enabled.
> 
> But if you prefer, I can do like you suggested here and make COMPAT
> depend on AARCH32_EL0 in the last patch.

What I meant is that if you define COMPAT as "def_bool y", you no longer
need the explicit "select COMPAT". When AARCH32_EL0 is disabled, COMPAT
would automatically be disabled because of the "depends on AARCH32_EL0"
line.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/20] 32-bit ABI: introduce ARCH_32BIT_OFF_T config option

[Catalin Marinas]

On Tue, Jun 13, 2017 at 02:04:11PM +0300, Yury Norov wrote:
> On Thu, Jun 08, 2017 at 04:09:50PM +0100, Catalin Marinas wrote:
> > On Sun, Jun 04, 2017 at 02:59:51PM +0300, Yury Norov wrote:
> > > All new 32-bit architectures should have 64-bit off_t type, but existing
> > > architectures has 32-bit ones.
> > > 
> > > To handle it, new config option is added to arch/Kconfig that defaults
> > > ARCH_32BIT_OFF_T to be disabled for non-64 bit architectures. All existing
> > > 32-bit architectures enable it explicitly here.
> > > 
> > > New option affects force_o_largefile() behaviour. Namely, if off_t is
> > > 64-bits long, we have no reason to reject user to open big files.
> > > 
> > > Note that even if architectures has only 64-bit off_t in the kernel
> > > (arc, c6x, h8300, hexagon, metag, nios2, openrisc, tile32 and unicore32),
> > > a libc may use 32-bit off_t, and therefore want to limit the file size
> > > to 4GB unless specified differently in the open flags.
> > > 
> > > Signed-off-by: Yury Norov <yno...@caviumnetworks.com>
> > > Acked-by: Arnd Bergmann <a...@arndb.de>
> > [...]
> > > diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
> > > index 1b48d9c9a561..297993c92490 100644
> > > --- a/include/linux/fcntl.h
> > > +++ b/include/linux/fcntl.h
> > > @@ -11,7 +11,7 @@
> > >O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
> > >  
> > >  #ifndef force_o_largefile
> > > -#define force_o_largefile() (BITS_PER_LONG != 32)
> > > +#define force_o_largefile() (!IS_ENABLED(CONFIG_ARCH_32BIT_OFF_T))
> > >  #endif
> > 
> > I may have confused myself with which off_t is 64-bit here for new
> > 32-bit architectures. Are we referring to the glibc definition, the
> > kernel one or simply that force_o_largefile() is true by default.
> > Because the type off_t for 32-bit kernel builds is still, well, 32-bit.
> > 
> > Otherwise it seems that the first paragraph in the description above
> > should read "all new 32-bit ABIs on a 64-bit kernel..." but then
> > AArch64/ILP32 is no longer the same as a new, pure 32-bit architecture.
>  
> This is all about userspace off_t types, like Arnd told in the comment
> to patch 13. I'll underline it in the comment to the patch. If it's
> not enough, I can also rename the config option to
> CONFIG_ARCH_32BIT_USER_OFF_T or similar. For me it's too much, but if
> you find it reasonable, I'll do it. Just let me know.

Thanks for clarification. I had the impression that it should match the
kernel's off_t (which is exported in the kernel headers as 32-bit) but
compiling with -mabi=ilp32 indeed shows sizeof(off_t) == 8. So that's
just a user decision to use loff_t instead and such port shouldn't use
any of the syscalls that pass the kernel's off_t.

I would rather see the comment in the arch/Kconfig help entry in this
patch for future reference.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

On Mon, Sep 04, 2017 at 02:54:50PM +0300, Yury Norov wrote:
> This is 4.13-based and next-20170901-based ilp32 patches. 
> https://github.com/norov/linux/tree/ilp32-4.13

Thanks. I'll mirror it on kernel.org sometime this week after doing some
tests (I've been mostly away for the past two weeks).

Regarding the generic patches in this series, could you please post them
to linux-arch (without the whole ILP32 series)? It would be good to get
them merged for 4.15.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 00/20] ILP32 for ARM64

[Catalin Marinas]

Hi Yury,

On Thu, Nov 16, 2017 at 02:11:30PM +0300, Yury Norov wrote:
> This is ILP32 patches on top of 4.14 kernel:
> https://github.com/norov/linux/commits/ilp32-4.14
> 
> I tested the series with LTP lite built by Linaro toolchain, and no
> regressions found.

Thanks. I gave it a try as well with LTP and pushed a staging/ilp32-4.14
branch to the arm64 tree on git.kernel.org. BTW, you've added two random
patches on top of this branch (which I removed).

> By the way, do you have plans to upstream arch subseries? 
> https://lkml.org/lkml/2017/9/25/574

I was hoping Arnd would pick them up since they are rather UAPI specific
than arm64.

BTW, I wonder whether the nds32 patches actually follow the proposed
defaults in your patches like force_o_largefile() and get/setrlimit:

https://lkml.org/lkml/2017/11/27/474

(I haven't reviewed the nds32 patches in detail though)

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] mm, arm64: untag user addresses in mm/gup.c

[Catalin Marinas]

On Wed, May 02, 2018 at 07:25:17PM +0200, Andrey Konovalov wrote:
> On Wed, May 2, 2018 at 5:36 PM, Kirill A. Shutemov
>  wrote:
> > On Wed, May 02, 2018 at 02:38:42PM +, Andrey Konovalov wrote:
> >> > Does having a tagged address here makes any difference? I couldn't hit a
> >> > failure with my simple tests (LD_PRELOAD a library that randomly adds
> >> > tags to pointers returned by malloc).
> >>
> >> I think you're right, follow_page_mask is only called from
> >> __get_user_pages, which already untagged the address. I'll remove
> >> untagging here.
> >
> > It also called from follow_page(). Have you covered all its callers?
> 
> Oh, missed that, will take a look.
> 
> Thinking about that, would it make sense to add untagging to find_vma
> (and others) instead of trying to cover all find_vma callers?

I don't think adding the untagging to find_vma() is sufficient. In many
cases the caller does a subsequent check like 'start < vma->vm_start'
(see sys_msync() as an example, there are a few others as well). What I
did in my tests was a WARN_ON_ONCE() in find_vma() if the address is
tagged.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] arm64: untag user pointers passed to the kernel

[Catalin Marinas]

On Wed, Apr 25, 2018 at 04:45:37PM +0200, Andrey Konovalov wrote:
> On Thu, Apr 19, 2018 at 11:33 AM, Kirill A. Shutemov
>  wrote:
> > On Wed, Apr 18, 2018 at 08:53:09PM +0200, Andrey Konovalov wrote:
> >> arm64 has a feature called Top Byte Ignore, which allows to embed pointer
> >> tags into the top byte of each pointer. Userspace programs (such as
> >> HWASan, a memory debugging tool [1]) might use this feature and pass
> >> tagged user pointers to the kernel through syscalls or other interfaces.
> >>
> >> This patch makes a few of the kernel interfaces accept tagged user
> >> pointers. The kernel is already able to handle user faults with tagged
> >> pointers and has the untagged_addr macro, which this patchset reuses.
> >>
> >> We're not trying to cover all possible ways the kernel accepts user
> >> pointers in one patchset, so this one should be considered as a start.
> >
> > How many changes do you anticipate?
> >
> > This patchset looks small and reasonable, but I see a potential to become a
> > boilerplate. Would we need to change every driver which implements ioctl()
> > to strip these bits?
> 
> I've replied to somewhat similar question in one of the previous
> versions of the patchset.
> 
> """
> There are two different approaches to untagging the user pointers that I see:
> 
> 1. Untag user pointers right after they are passed to the kernel.
> 
> While this might be possible for pointers that are passed to syscalls
> as arguments (Catalin's "hack"), this leaves user pointers, that are
> embedded into for example structs that are passed to the kernel. Since
> there's no specification of the interface between user space and the
> kernel, different kernel parts handle user pointers differently and I
> don't see an easy way to cover them all.
> 
> 2. Untag user pointers where they are used in the kernel.
> 
> Although there's no specification on the interface between the user
> space and the kernel, the kernel still has to use one of a few
> specific ways to access user data (copy_from_user, etc.). So the idea
> here is to add untagging into them. This patchset mostly takes this
> approach (with the exception of memory subsystem syscalls).
> 
> If there's a better approach, I'm open to suggestions.
> """
> 
> So if we go with the first way, we'll need to go through every syscall
> and ioctl handler, which doesn't seem feasible.

I agree with you that (1) isn't feasible. My hack is sufficient for the
pointer arguments but doesn't help with pointers in user structures
passed to the kernel.

Now, since the hardware allows access to user pointers with non-zero top
8-bit, the kernel uaccess routines can also use such pointers directly.
What's needed, as per your patches, is the access_ok() macro and
whatever ends up calling find_vma() (at a first look, there may be other
cases). I don't think drivers need changing as long as the in-kernel API
they use performs the untagging (e.g. get_user_pages()).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] mm, arm64: untag user addresses in mm/gup.c

[Catalin Marinas]

On Wed, Apr 18, 2018 at 08:53:13PM +0200, Andrey Konovalov wrote:
> diff --git a/mm/gup.c b/mm/gup.c
> index 76af4cfeaf68..fb375de7d40d 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -386,6 +386,8 @@ struct page *follow_page_mask(struct vm_area_struct *vma,
>   struct page *page;
>   struct mm_struct *mm = vma->vm_mm;
>  
> + address = untagged_addr(address);
> +
>   *page_mask = 0;
>  
>   /* make this handle hugepd */

Does having a tagged address here makes any difference? I couldn't hit a
failure with my simple tests (LD_PRELOAD a library that randomly adds
tags to pointers returned by malloc).

> @@ -647,6 +649,8 @@ static long __get_user_pages(struct task_struct *tsk, 
> struct mm_struct *mm,
>   if (!nr_pages)
>   return 0;
>  
> + start = untagged_addr(start);
> +
>   VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
>  
>   /*
> @@ -801,6 +805,8 @@ int fixup_user_fault(struct task_struct *tsk, struct 
> mm_struct *mm,
>   struct vm_area_struct *vma;
>   int ret, major = 0;
>  
> + address = untagged_addr(address);
> +
>   if (unlocked)
>   fault_flags |= FAULT_FLAG_ALLOW_RETRY;
>  
> @@ -854,6 +860,8 @@ static __always_inline long 
> __get_user_pages_locked(struct task_struct *tsk,
>   long ret, pages_done;
>   bool lock_dropped;
>  
> + start = untagged_addr(start);
> +
>   if (locked) {
>   /* if VM_FAULT_RETRY can be returned, vmas become invalid */
>   BUG_ON(vmas);

Isn't __get_user_pages() untagging enough to cover this case as well?
Can this function not cope with tagged pointers?

> @@ -1751,6 +1759,8 @@ int __get_user_pages_fast(unsigned long start, int 
> nr_pages, int write,
>   unsigned long flags;
>   int nr = 0;
>  
> + start = untagged_addr(start);
> +
>   start &= PAGE_MASK;
>   addr = start;
>   len = (unsigned long) nr_pages << PAGE_SHIFT;
> @@ -1803,6 +1813,8 @@ int get_user_pages_fast(unsigned long start, int 
> nr_pages, int write,
>   unsigned long addr, len, end;
>   int nr = 0, ret = 0;
>  
> + start = untagged_addr(start);
> +
>   start &= PAGE_MASK;
>   addr = start;
>   len = (unsigned long) nr_pages << PAGE_SHIFT;

Have you hit a problem with the fast gup functions and tagged pointers?
The page table walking macros (e.g. p*d_index()) should mask the tag out
already.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] arm64: untag user addresses in copy_from_user and others

[Catalin Marinas]

On Wed, Apr 18, 2018 at 08:53:12PM +0200, Andrey Konovalov wrote:
> @@ -238,12 +239,15 @@ static inline void uaccess_enable_not_uao(void)
>  /*
>   * Sanitise a uaccess pointer such that it becomes NULL if above the
>   * current addr_limit.
> + * Also untag user pointers that have the top byte tag set.
>   */
>  #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
>  static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
>  {
>   void __user *safe_ptr;
>  
> + ptr = untagged_addr(ptr);
> +
>   asm volatile(
>   "   bicsxzr, %1, %2\n"
>   "   csel%0, %1, xzr, eq\n"

First of all, passing a tagged user pointer throughout the kernel is
safe with uaccess routines but not suitable for find_vma() etc.

With this change, we may have an inconsistent behaviour on the tag
masking, depending on whether the entry code uses __uaccess_mask_ptr()
or not. We could preserve the tag with something like:

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index e66b0fca99c2..ed15bfcbd797 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -244,10 +244,11 @@ static inline void __user *__uaccess_mask_ptr(const void 
__user *ptr)
void __user *safe_ptr;
 
asm volatile(
-   "   bicsxzr, %1, %2\n"
+   "   bicsxzr, %3, %2\n"
"   csel%0, %1, xzr, eq\n"
: "=" (safe_ptr)
-   : "r" (ptr), "r" (current_thread_info()->addr_limit)
+   : "r" (ptr), "r" (current_thread_info()->addr_limit),
+ "r" (untagged_addr(ptr))
: "cc");
 
csdb();

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] arm64: v8.4: Support for new floating point multiplication instructions

[Catalin Marinas]

On Fri, Jan 05, 2018 at 04:22:24PM +0800, gengdongjiu wrote:
> On 2018/1/5 15:57, Greg KH wrote:
> > On Fri, Jan 05, 2018 at 09:22:54AM +0800, gengdongjiu wrote:
> >> Hi will/catalin
> >>
> >> On 2017/12/13 18:09, Suzuki K Poulose wrote:
> >>> On 13/12/17 10:13, Dongjiu Geng wrote:
>  ARM v8.4 extensions add new neon instructions for performing a
>  multiplication of each FP16 element of one vector with the corresponding
>  FP16 element of a second vector, and to add or subtract this without an
>  intermediate rounding to the corresponding FP32 element in a third 
>  vector.
> 
>  This patch detects this feature and let the userspace know about it via a
>  HWCAP bit and MRS emulation.
> 
>  Cc: Dave Martin 
>  Cc: Suzuki K Poulose 
>  Signed-off-by: Dongjiu Geng 
>  Reviewed-by: Dave Martin 
> >>>
> >>> Looks good to me.
> >>>
> >>> Reviewed-by: Suzuki K Poulose 
> >>
> >>  sorry to disturb you. Reminder, hope this patch can be applied to Linux 
> >> 4.15-rc7.
> > 
> > New features should not be going into 4.15-rc, that should be a 4.16-rc1
> > thing, right?
> 
> It is also great if it can be applied to 4.16-rc1. Thanks a lot!

I will queue it for 4.16-rc1.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html