From mboxrd@z Thu Jan  1 00:00:00 1970
From: mark.rutland@arm.com (Mark Rutland)
Date: Tue, 19 Jun 2018 14:15:24 +0100
Subject: [PATCHv3 10/19] arm64: convert native/compat syscall entry to C
In-Reply-To: <20180619121814.GR22983@e103592.cambridge.arm.com>
References: <20180618120310.39527-1-mark.rutland@arm.com>
 <20180618120310.39527-11-mark.rutland@arm.com>
 <20180619121814.GR22983@e103592.cambridge.arm.com>
Message-ID: <20180619130932.ww53s5rnragcodha@lakrids.cambridge.arm.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Tue, Jun 19, 2018 at 01:18:17PM +0100, Dave Martin wrote:
> On Mon, Jun 18, 2018 at 01:03:01PM +0100, Mark Rutland wrote:
> > +static inline void sve_user_reset(void)
> > +{
> 
> Can we call this "sve_user_discard" please?
> 
> "Reset" is a reasonable name for the concept, but the "discard"
> terminology has been used elsewhere.

Sure; done.

> > +	if (!system_supports_sve())
> > +		return;
> > +
> > +	/*
> > +	 * task_fpsimd_load() won't be called to update CPACR_EL1 in
> > +	 * ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
> > +	 * happens if a context switch or kernel_neon_begin() or context
> > +	 * modification (sigreturn, ptrace) intervenes.
> > +	 * So, ensure that CPACR_EL1 is already correct for the fast-path case.
> > +	 */
> 
> This comment should go after clear_thead_flag(), since it describes not
> the purpose of this function but the presence of sve_user_disable().
> 
> > +	clear_thread_flag(TIF_SVE);
> > +	sve_user_disable();
> > +}

Good point. I've moved the clear_thread_flag(TIF_SVE) above the comment
(with a blank line before the comment).

Thanks,
Mark.

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:50044 "EHLO
        foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1757333AbeFSNPb (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
        Tue, 19 Jun 2018 09:15:31 -0400
Date: Tue, 19 Jun 2018 14:15:24 +0100
From: Mark Rutland <mark.rutland@arm.com>
To: Dave Martin <Dave.Martin@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
        will.deacon@arm.com, marc.zyngier@arm.com,
        linux@dominikbrodowski.net, hch@infradead.org, james.morse@arm.com,
        viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCHv3 10/19] arm64: convert native/compat syscall entry to C
Message-ID: <20180619130932.ww53s5rnragcodha@lakrids.cambridge.arm.com>
References: <20180618120310.39527-1-mark.rutland@arm.com>
 <20180618120310.39527-11-mark.rutland@arm.com>
 <20180619121814.GR22983@e103592.cambridge.arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180619121814.GR22983@e103592.cambridge.arm.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Tue, Jun 19, 2018 at 01:18:17PM +0100, Dave Martin wrote:
> On Mon, Jun 18, 2018 at 01:03:01PM +0100, Mark Rutland wrote:
> > +static inline void sve_user_reset(void)
> > +{
> 
> Can we call this "sve_user_discard" please?
> 
> "Reset" is a reasonable name for the concept, but the "discard"
> terminology has been used elsewhere.

Sure; done.

> > +	if (!system_supports_sve())
> > +		return;
> > +
> > +	/*
> > +	 * task_fpsimd_load() won't be called to update CPACR_EL1 in
> > +	 * ret_to_user unless TIF_FOREIGN_FPSTATE is still set, which only
> > +	 * happens if a context switch or kernel_neon_begin() or context
> > +	 * modification (sigreturn, ptrace) intervenes.
> > +	 * So, ensure that CPACR_EL1 is already correct for the fast-path case.
> > +	 */
> 
> This comment should go after clear_thead_flag(), since it describes not
> the purpose of this function but the presence of sve_user_disable().
> 
> > +	clear_thread_flag(TIF_SVE);
> > +	sve_user_disable();
> > +}

Good point. I've moved the clear_thread_flag(TIF_SVE) above the comment
(with a blank line before the comment).

Thanks,
Mark.