2020-07-10 09:45:45

by Zhenyu Ye

[permalink] [raw]
Subject: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

NOTICE: this series are based on the arm64 for-next/tlbi branch:
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/tlbi

--
ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
range of input addresses. This series add support for this feature.

--
ChangeList:
v2:
- remove the __tlbi_last_level() macro.
- add check for parameters in __TLBI_VADDR_RANGE macro.

RFC patches:
- Link: https://lore.kernel.org/linux-arm-kernel/[email protected]/

Zhenyu Ye (2):
arm64: tlb: Detect the ARMv8.4 TLBI RANGE feature
arm64: tlb: Use the TLBI RANGE feature in arm64

arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/sysreg.h | 3 +
arch/arm64/include/asm/tlbflush.h | 138 +++++++++++++++++++++++-------
arch/arm64/kernel/cpufeature.c | 10 +++
4 files changed, 124 insertions(+), 30 deletions(-)

--
2.19.1



2020-07-10 19:11:50

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

On Fri, 10 Jul 2020 17:44:18 +0800, Zhenyu Ye wrote:
> NOTICE: this series are based on the arm64 for-next/tlbi branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/tlbi
>
> --
> ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
> range of input addresses. This series add support for this feature.
>
> [...]

Applied to arm64 (for-next/tlbi), thanks!

[1/2] arm64: tlb: Detect the ARMv8.4 TLBI RANGE feature
https://git.kernel.org/arm64/c/a2fd755f77ff
[2/2] arm64: tlb: Use the TLBI RANGE feature in arm64
https://git.kernel.org/arm64/c/db34a081d273

--
Catalin

2020-07-13 12:22:19

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

On Fri, Jul 10, 2020 at 08:11:19PM +0100, Catalin Marinas wrote:
> On Fri, 10 Jul 2020 17:44:18 +0800, Zhenyu Ye wrote:
> > NOTICE: this series are based on the arm64 for-next/tlbi branch:
> > git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/tlbi
> >
> > --
> > ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
> > range of input addresses. This series add support for this feature.
> >
> > [...]
>
> Applied to arm64 (for-next/tlbi), thanks!
>
> [1/2] arm64: tlb: Detect the ARMv8.4 TLBI RANGE feature
> https://git.kernel.org/arm64/c/a2fd755f77ff
> [2/2] arm64: tlb: Use the TLBI RANGE feature in arm64
> https://git.kernel.org/arm64/c/db34a081d273

I'm dropping these two patches from for-next/tlbi and for-next/core.
They need a check on whether binutils supports the new "tlbi rva*"
instructions, otherwise the build mail fail.

I kept the latest incarnation of these patches on devel/tlbi-range for
reference.

--
Catalin

2020-07-13 12:42:32

by Zhenyu Ye

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

Hi Catalin,

On 2020/7/13 20:21, Catalin Marinas wrote:
> On Fri, Jul 10, 2020 at 08:11:19PM +0100, Catalin Marinas wrote:
>> On Fri, 10 Jul 2020 17:44:18 +0800, Zhenyu Ye wrote:
>>> NOTICE: this series are based on the arm64 for-next/tlbi branch:
>>> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/tlbi
>>>
>>> --
>>> ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
>>> range of input addresses. This series add support for this feature.
>>>
>>> [...]
>>
>> Applied to arm64 (for-next/tlbi), thanks!
>>
>> [1/2] arm64: tlb: Detect the ARMv8.4 TLBI RANGE feature
>> https://git.kernel.org/arm64/c/a2fd755f77ff
>> [2/2] arm64: tlb: Use the TLBI RANGE feature in arm64
>> https://git.kernel.org/arm64/c/db34a081d273
>
> I'm dropping these two patches from for-next/tlbi and for-next/core.
> They need a check on whether binutils supports the new "tlbi rva*"
> instructions, otherwise the build mail fail.
>
> I kept the latest incarnation of these patches on devel/tlbi-range for
> reference.
>

Should we add a check for the binutils version? Just like:

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fad573883e89..d5fb6567e0d2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1300,6 +1300,20 @@ config ARM64_AMU_EXTN
correctly reflect reality. Most commonly, the value read will be 0,
indicating that the counter is not enabled.

+config ARM64_TLBI_RANGE
+ bool "Enable support for tlbi range feature"
+ default y
+ depends on AS_HAS_TLBI_RANGE
+ help
+ ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
+ range of input addresses.
+
+ The feature introduces new assembly instructions, and they were
+ support when binutils >= 2.30.
+
+config AS_HAS_TLBI_RANGE
+ def_bool $(as-option, -Wa$(comma)-march=armv8.4-a)
+
endmenu

menu "ARMv8.5 architectural features"

Then uses the check in the loop:

while (pages > 0) {
if (!IS_ENABLED(CONFIG_ARM64_TLBI_RANGE) ||
!cpus_have_const_cap(ARM64_HAS_TLBI_RANGE) ||


If this is ok, I could send a new series soon.

Thanks,
Zhenyu


2020-07-13 17:00:13

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

On Mon, Jul 13, 2020 at 08:41:31PM +0800, Zhenyu Ye wrote:
> On 2020/7/13 20:21, Catalin Marinas wrote:
> > On Fri, Jul 10, 2020 at 08:11:19PM +0100, Catalin Marinas wrote:
> >> On Fri, 10 Jul 2020 17:44:18 +0800, Zhenyu Ye wrote:
> >>> NOTICE: this series are based on the arm64 for-next/tlbi branch:
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/tlbi
> >>>
> >>> --
> >>> ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
> >>> range of input addresses. This series add support for this feature.
> >>>
> >>> [...]
> >>
> >> Applied to arm64 (for-next/tlbi), thanks!
> >>
> >> [1/2] arm64: tlb: Detect the ARMv8.4 TLBI RANGE feature
> >> https://git.kernel.org/arm64/c/a2fd755f77ff
> >> [2/2] arm64: tlb: Use the TLBI RANGE feature in arm64
> >> https://git.kernel.org/arm64/c/db34a081d273
> >
> > I'm dropping these two patches from for-next/tlbi and for-next/core.
> > They need a check on whether binutils supports the new "tlbi rva*"
> > instructions, otherwise the build mail fail.
> >
> > I kept the latest incarnation of these patches on devel/tlbi-range for
> > reference.
>
> Should we add a check for the binutils version? Just like:
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fad573883e89..d5fb6567e0d2 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1300,6 +1300,20 @@ config ARM64_AMU_EXTN
> correctly reflect reality. Most commonly, the value read will be 0,
> indicating that the counter is not enabled.
>
> +config ARM64_TLBI_RANGE
> + bool "Enable support for tlbi range feature"
> + default y
> + depends on AS_HAS_TLBI_RANGE
> + help
> + ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
> + range of input addresses.
> +
> + The feature introduces new assembly instructions, and they were
> + support when binutils >= 2.30.

It looks like 2.30. I tracked it down to this commit:

https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=793a194839bc8add71fdc7429c58b10f0667a6f6;hp=1a7ed57c840dcb0401f1a67c6763a89f7d2686d2

> +config AS_HAS_TLBI_RANGE
> + def_bool $(as-option, -Wa$(comma)-march=armv8.4-a)
> +
> endmenu

The problem is that we don't pass -Wa,-march=armv8.4-a to gas. AFAICT,
we only set an 8.3 for PAC but I'm not sure how passing two such options
goes.

I'm slightly surprised that my toolchains (and yours) did not complain
about these instructions. Looking at the binutils code, I think it
should have complained if -march=armv8.4-a wasn't passed but works fine.
I thought gas doesn't enable the maximum arch feature by default.

An alternative would be to check for a specific instruction (untested):

def_bool $(as-instr,tlbi rvae1is, x0)

but we need to figure out whether gas not requiring -march=armv8.4-a is
a bug (which may be fixed) or that gas accepts all TLBI instructions.

A safer bet may be to simply encode the instructions by hand:

#define SYS_TLBI_RVAE1IS(Rt) \
__emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 1) | ((Rt) & 0x1f))
#define SYS_TLBI_RVALE1IS(Rt) \
__emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 5) | ((Rt) & 0x1f))

(please check that they are correct)

--
Catalin

2020-07-14 15:20:28

by Zhenyu Ye

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

On 2020/7/14 0:59, Catalin Marinas wrote:
>> +config ARM64_TLBI_RANGE
>> + bool "Enable support for tlbi range feature"
>> + default y
>> + depends on AS_HAS_TLBI_RANGE
>> + help
>> + ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
>> + range of input addresses.
>> +
>> + The feature introduces new assembly instructions, and they were
>> + support when binutils >= 2.30.
>
> It looks like 2.30. I tracked it down to this commit:
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=793a194839bc8add71fdc7429c58b10f0667a6f6;hp=1a7ed57c840dcb0401f1a67c6763a89f7d2686d2
>
>> +config AS_HAS_TLBI_RANGE
>> + def_bool $(as-option, -Wa$(comma)-march=armv8.4-a)
>> +
>> endmenu
>
> The problem is that we don't pass -Wa,-march=armv8.4-a to gas. AFAICT,
> we only set an 8.3 for PAC but I'm not sure how passing two such options
> goes.
>

Pass the -march twice may not have bad impact. Test in my toolchains
and the newer one will be chosen. Anyway, we can add judgment to avoid
them be passed at the same time.

> I'm slightly surprised that my toolchains (and yours) did not complain
> about these instructions. Looking at the binutils code, I think it
> should have complained if -march=armv8.4-a wasn't passed but works fine.
> I thought gas doesn't enable the maximum arch feature by default.
>> An alternative would be to check for a specific instruction (untested):
>
> def_bool $(as-instr,tlbi rvae1is, x0)
>
> but we need to figure out whether gas not requiring -march=armv8.4-a is
> a bug (which may be fixed) or that gas accepts all TLBI instructions.
>

As you say in another email, this is a bug. So we should pass -march=
armv8.4-a to gas if we use toolchains to generate tlbi range instructions.

But this bug only affects the compilation (cause WARNING or ERROR if not
pass -march-armv8.4-a when compiling) but not the judgment.

> A safer bet may be to simply encode the instructions by hand:
>
> #define SYS_TLBI_RVAE1IS(Rt) \
> __emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 1) | ((Rt) & 0x1f))
> #define SYS_TLBI_RVALE1IS(Rt) \
> __emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 5) | ((Rt) & 0x1f))
>
> (please check that they are correct)
>

Currently in kernel, all tlbi instructions are passed through __tlbi()
and __tlbi_user(). If we encode the range instructions by hand, we may
should have to add a new mechanism for this:

1. choose a register and save it;
2. put the operations for tlbi range to the register;
3. do tlbi range by asm(SYS_TLBI_RVAE1IS(x0));
4. restore the value of the register.

It's complicated and will only be used with tlbi range instructions.
(Am I understand something wrong? )

So I am prefer to pass -march=armv8.4-a to toolschains to support tlbi
range instruction, just like what PAC does.

Thanks,
Zhenyu




2020-07-14 15:59:23

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] arm64: tlb: add support for TLBI RANGE instructions

On Tue, Jul 14, 2020 at 11:17:01PM +0800, Zhenyu Ye wrote:
> On 2020/7/14 0:59, Catalin Marinas wrote:
> >> +config ARM64_TLBI_RANGE
> >> + bool "Enable support for tlbi range feature"
> >> + default y
> >> + depends on AS_HAS_TLBI_RANGE
> >> + help
> >> + ARMv8.4-TLBI provides TLBI invalidation instruction that apply to a
> >> + range of input addresses.
> >> +
> >> + The feature introduces new assembly instructions, and they were
> >> + support when binutils >= 2.30.
> >
> > It looks like 2.30. I tracked it down to this commit:
> >
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=793a194839bc8add71fdc7429c58b10f0667a6f6;hp=1a7ed57c840dcb0401f1a67c6763a89f7d2686d2
> >
> >> +config AS_HAS_TLBI_RANGE
> >> + def_bool $(as-option, -Wa$(comma)-march=armv8.4-a)

You could make this more generic like AS_HAS_ARMV8_4.

> > The problem is that we don't pass -Wa,-march=armv8.4-a to gas. AFAICT,
> > we only set an 8.3 for PAC but I'm not sure how passing two such options
> > goes.
>
> Pass the -march twice may not have bad impact. Test in my toolchains
> and the newer one will be chosen. Anyway, we can add judgment to avoid
> them be passed at the same time.

I think the last one always overrides the previous (same with the .arch
statements in asm files). For example:

echo "paciasp" | aarch64-none-linux-gnu-as -march=armv8.2-a -march=armv8.3-a

succeeds but the one below fails:

echo "paciasp" | aarch64-none-linux-gnu-as -march=armv8.3-a -march=armv8.2-a

> > A safer bet may be to simply encode the instructions by hand:
> >
> > #define SYS_TLBI_RVAE1IS(Rt) \
> > __emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 1) | ((Rt) & 0x1f))
> > #define SYS_TLBI_RVALE1IS(Rt) \
> > __emit_inst(0xd5000000 | sys_insn(1, 0, 8, 2, 5) | ((Rt) & 0x1f))
> >
> > (please check that they are correct)
>
> Currently in kernel, all tlbi instructions are passed through __tlbi()
> and __tlbi_user(). If we encode the range instructions by hand, we may
> should have to add a new mechanism for this:
>
> 1. choose a register and save it;
> 2. put the operations for tlbi range to the register;
> 3. do tlbi range by asm(SYS_TLBI_RVAE1IS(x0));
> 4. restore the value of the register.
>
> It's complicated and will only be used with tlbi range instructions.
> (Am I understand something wrong? )
>
> So I am prefer to pass -march=armv8.4-a to toolschains to support tlbi
> range instruction, just like what PAC does.

It will indeed get more complicated than necessary. So please go with
the -Wa,-march=armv8.4-a check in Kconfig and update the
arch/arm64/Makefile to pass this option (after the 8.3 one).

Thanks.

--
Catalin