mbox series

[v1,0/3] Import Arm Optimized Routines str{n}cmp functions

Message ID 20220215170723.21266-1-joey.gouly@arm.com (mailing list archive)
Headers show
Series Import Arm Optimized Routines str{n}cmp functions | expand

Message

Joey Gouly Feb. 15, 2022, 5:07 p.m. UTC
Hi all,

The previous str{n}cmp routines were not MTE safe, so were disabled in:
  59a68d413808 ("arm64: Mitigate MTE issues with str{n}cmp()")

The Arm Optimized Routines repository recently merged [1] their strcmp.S and
strcmp-mte.S files into a single file that is MTE safe.

Therefore we can import these new MTE safe functions and remove the workaround.

I did some light boot tests using QEMU.

Thanks,
Joey

[1] https://github.com/ARM-software/optimized-routines/commit/7b91c3cdb12b023004cb4dda30a1aa3424329ce6

Joey Gouly (3):
  arm64: lib:  Import latest version of Arm Optimized Routines' strcmp
  arm64: lib:  Import latest version of Arm Optimized Routines' strncmp
  Revert "arm64: Mitigate MTE issues with str{n}cmp()"

 arch/arm64/include/asm/assembler.h |   5 -
 arch/arm64/include/asm/string.h    |   2 -
 arch/arm64/lib/strcmp.S            | 240 +++++++++++++++--------------
 arch/arm64/lib/strncmp.S           | 236 +++++++++++++++++-----------
 4 files changed, 269 insertions(+), 214 deletions(-)

Comments

Mark Rutland Feb. 16, 2022, 4:30 p.m. UTC | #1
On Tue, Feb 15, 2022 at 05:07:20PM +0000, Joey Gouly wrote:
> Hi all,

Hi,

> The previous str{n}cmp routines were not MTE safe, so were disabled in:
>   59a68d413808 ("arm64: Mitigate MTE issues with str{n}cmp()")
> 
> The Arm Optimized Routines repository recently merged [1] their strcmp.S and
> strcmp-mte.S files into a single file that is MTE safe.
> 
> Therefore we can import these new MTE safe functions and remove the workaround.
> 
> I did some light boot tests using QEMU.

Nice!

As as minor thing, on the two import patches, I think we should be more
explicit about what's going on with licensing, so that it's clear the
license change relative to upstream is intended and legitimate.

For example, in commit:

  758602c04409d8c5 ("arm64: Import latest version of Cortex Strings' strcmp")

We had a note in the commit message:

| Note that for simplicity Arm have chosen to contribute this code
| to Linux under GPLv2 rather than the original MIT license.

... and I reckon it's worth being slightly more explicit, e.g.

| Note that for simplicity Arm have chosen to contribute this code
| to Linux under GPLv2 rather than the original MIT license. Arm is the
| sole copyright holder for this code.

That was previously confirmed at: 

  https://lore.kernel.org/linux-arm-kernel/20210526101723.GA3806@C02TD0UTHF1T.local/

So with that latter wording added to the import patches:

Acked-by: Mark Rutland <mark.rutland@arm.com>

As a heads-up, I believe this will conflict with changes I'm making to
the way symbol aliasing works:

  https://lore.kernel.org/lkml/20220216162229.1076788-1-mark.rutland@arm.com/

... and I reckon we need to fix that conflict in the arm64 tree, either
as a merge resolution or rebasing this atop my series.

Thanks,
Mark.

> 
> Thanks,
> Joey
> 
> [1] https://github.com/ARM-software/optimized-routines/commit/7b91c3cdb12b023004cb4dda30a1aa3424329ce6
> 
> Joey Gouly (3):
>   arm64: lib:  Import latest version of Arm Optimized Routines' strcmp
>   arm64: lib:  Import latest version of Arm Optimized Routines' strncmp
>   Revert "arm64: Mitigate MTE issues with str{n}cmp()"
> 
>  arch/arm64/include/asm/assembler.h |   5 -
>  arch/arm64/include/asm/string.h    |   2 -
>  arch/arm64/lib/strcmp.S            | 240 +++++++++++++++--------------
>  arch/arm64/lib/strncmp.S           | 236 +++++++++++++++++-----------
>  4 files changed, 269 insertions(+), 214 deletions(-)
> 
> -- 
> 2.17.1
>
Russell King (Oracle) Feb. 16, 2022, 4:52 p.m. UTC | #2
On Wed, Feb 16, 2022 at 04:30:26PM +0000, Mark Rutland wrote:
> On Tue, Feb 15, 2022 at 05:07:20PM +0000, Joey Gouly wrote:
> > Hi all,
> 
> Hi,
> 
> > The previous str{n}cmp routines were not MTE safe, so were disabled in:
> >   59a68d413808 ("arm64: Mitigate MTE issues with str{n}cmp()")
> > 
> > The Arm Optimized Routines repository recently merged [1] their strcmp.S and
> > strcmp-mte.S files into a single file that is MTE safe.
> > 
> > Therefore we can import these new MTE safe functions and remove the workaround.
> > 
> > I did some light boot tests using QEMU.
> 
> Nice!
> 
> As as minor thing, on the two import patches, I think we should be more
> explicit about what's going on with licensing, so that it's clear the
> license change relative to upstream is intended and legitimate.
> 
> For example, in commit:
> 
>   758602c04409d8c5 ("arm64: Import latest version of Cortex Strings' strcmp")
> 
> We had a note in the commit message:
> 
> | Note that for simplicity Arm have chosen to contribute this code
> | to Linux under GPLv2 rather than the original MIT license.
> 
> ... and I reckon it's worth being slightly more explicit, e.g.
> 
> | Note that for simplicity Arm have chosen to contribute this code
> | to Linux under GPLv2 rather than the original MIT license. Arm is the
> | sole copyright holder for this code.
> 
> That was previously confirmed at: 
> 
>   https://lore.kernel.org/linux-arm-kernel/20210526101723.GA3806@C02TD0UTHF1T.local/
> 
> So with that latter wording added to the import patches:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

As MIT is regarded as compatible with GPL v2 (MIT code can be integrated
into GPL v2 projects) it seems rather strange.

In any case, it's confusing to see the SPDX license identifiers saying
it's GPLv2 only code, but when you look at the LICENSE file in the
source repository, it's quite different. This probably needs some
explanation in the files, or the SPDX saying that it's GPLv2 only or
MIT or ...