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