This patchset implements the asynchronous mode support for ARMv8.5-A
Memory Tagging Extension (MTE), which is a debugging feature that allows
to detect with the help of the architecture the C and C++ programmatic
memory errors like buffer overflow, use-after-free, use-after-return, etc.
MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
(Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
subset of its address space that is multiple of a 16 bytes granule. MTE
is based on a lock-key mechanism where the lock is the tag associated to
the physical memory and the key is the tag associated to the virtual
address.
When MTE is enabled and tags are set for ranges of address space of a task,
the PE will compare the tag related to the physical memory with the tag
related to the virtual address (tag check operation). Access to the memory
is granted only if the two tags match. In case of mismatch the PE will raise
an exception.
The exception can be handled synchronously or asynchronously. When the
asynchronous mode is enabled:
- Upon fault the PE updates the TFSR_EL1 register.
- The kernel detects the change during one of the following:
- Context switching
- Return to user/EL0
- Kernel entry from EL1
- Kernel exit to EL1
- If the register has been updated by the PE the kernel clears it and
reports the error.
The series contains as well an optimization to mte_assign_mem_tag_range().
The series is based on linux 5.11-rc3.
To simplify the testing a tree with the new patches on top has been made
available at [1].
[1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async
Changes:
--------
v4:
- Added support for kasan.mode (sync/async) kernel
command line parameter.
- Addressed review comments.
v3:
- Exposed kasan_hw_tags_mode to convert the internal
KASAN represenetation.
- Added dsb() for kernel exit paths in arm64.
- Addressed review comments.
v2:
- Fixed a compilation issue reported by krobot.
- General cleanup.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Marco Elver <[email protected]>
Cc: Evgenii Stepanov <[email protected]>
Cc: Branislav Rankov <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
Vincenzo Frascino (5):
arm64: mte: Add asynchronous mode support
kasan: Add KASAN mode kernel parameter
kasan: Add report for async mode
arm64: mte: Enable async tag check fault
arm64: mte: Inline mte_assign_mem_tag_range()
Documentation/dev-tools/kasan.rst | 3 ++
arch/arm64/include/asm/memory.h | 3 +-
arch/arm64/include/asm/mte-kasan.h | 9 ++++-
arch/arm64/include/asm/mte.h | 58 ++++++++++++++++++++++++++-
arch/arm64/kernel/entry-common.c | 6 +++
arch/arm64/kernel/mte.c | 63 +++++++++++++++++++++++++++++-
arch/arm64/lib/mte.S | 15 -------
include/linux/kasan.h | 3 ++
mm/kasan/hw_tags.c | 31 ++++++++++++++-
mm/kasan/kasan.h | 3 +-
mm/kasan/report.c | 16 +++++++-
11 files changed, 185 insertions(+), 25 deletions(-)
--
2.30.0
mte_assign_mem_tag_range() is called on production KASAN HW hot
paths. It makes sense to inline it in an attempt to reduce the
overhead.
Inline mte_assign_mem_tag_range() based on the indications provided at
[1].
[1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
arch/arm64/lib/mte.S | 15 ---------------
2 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index 237bb2f7309d..1a6fd53f82c3 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
int mte_ptrace_copy_tags(struct task_struct *child, long request,
unsigned long addr, unsigned long data);
-void mte_assign_mem_tag_range(void *addr, size_t size);
+static inline void mte_assign_mem_tag_range(void *addr, size_t size)
+{
+ u64 _addr = (u64)addr;
+ u64 _end = _addr + size;
+
+ /*
+ * This function must be invoked from an MTE enabled context.
+ *
+ * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
+ * size must be non-zero and MTE_GRANULE_SIZE aligned.
+ */
+ do {
+ /*
+ * 'asm volatile' is required to prevent the compiler to move
+ * the statement outside of the loop.
+ */
+ asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
+ :
+ : "r" (_addr)
+ : "memory");
+
+ _addr += MTE_GRANULE_SIZE;
+ } while (_addr != _end);
+}
+
#else /* CONFIG_ARM64_MTE */
diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
index 9e1a12e10053..a0a650451510 100644
--- a/arch/arm64/lib/mte.S
+++ b/arch/arm64/lib/mte.S
@@ -150,18 +150,3 @@ SYM_FUNC_START(mte_restore_page_tags)
ret
SYM_FUNC_END(mte_restore_page_tags)
-/*
- * Assign allocation tags for a region of memory based on the pointer tag
- * x0 - source pointer
- * x1 - size
- *
- * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
- * size must be non-zero and MTE_GRANULE_SIZE aligned.
- */
-SYM_FUNC_START(mte_assign_mem_tag_range)
-1: stg x0, [x0]
- add x0, x0, #MTE_GRANULE_SIZE
- subs x1, x1, #MTE_GRANULE_SIZE
- b.gt 1b
- ret
-SYM_FUNC_END(mte_assign_mem_tag_range)
--
2.30.0
On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
> mte_assign_mem_tag_range() is called on production KASAN HW hot
> paths. It makes sense to inline it in an attempt to reduce the
> overhead.
>
> Inline mte_assign_mem_tag_range() based on the indications provided at
> [1].
>
> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
> ---
> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
> arch/arm64/lib/mte.S | 15 ---------------
> 2 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 237bb2f7309d..1a6fd53f82c3 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> unsigned long addr, unsigned long data);
>
> -void mte_assign_mem_tag_range(void *addr, size_t size);
> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
> +{
> + u64 _addr = (u64)addr;
> + u64 _end = _addr + size;
> +
> + /*
> + * This function must be invoked from an MTE enabled context.
> + *
> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
> + */
> + do {
> + /*
> + * 'asm volatile' is required to prevent the compiler to move
> + * the statement outside of the loop.
> + */
> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
> + :
> + : "r" (_addr)
> + : "memory");
> +
> + _addr += MTE_GRANULE_SIZE;
> + } while (_addr != _end);
> +}
While I'm ok with moving this function to C, I don't think it solves the
inlining in the kasan code. The only interface we have to kasan is via
mte_{set,get}_mem_tag_range(), so the above function doesn't need to
live in a header.
If you do want inlining all the way to the kasan code, we should
probably move the mte_{set,get}_mem_tag_range() functions to the header
as well (and ideally backed by some numbers to show that it matters).
Moving it to mte.c also gives us more control on how it's called (we
have the WARN_ONs in place in the callers).
--
Catalin
Hi Catalin,
On 1/19/21 2:45 PM, Catalin Marinas wrote:
> On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
>> mte_assign_mem_tag_range() is called on production KASAN HW hot
>> paths. It makes sense to inline it in an attempt to reduce the
>> overhead.
>>
>> Inline mte_assign_mem_tag_range() based on the indications provided at
>> [1].
>>
>> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
>>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Signed-off-by: Vincenzo Frascino <[email protected]>
>> ---
>> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
>> arch/arm64/lib/mte.S | 15 ---------------
>> 2 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
>> index 237bb2f7309d..1a6fd53f82c3 100644
>> --- a/arch/arm64/include/asm/mte.h
>> +++ b/arch/arm64/include/asm/mte.h
>> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
>> int mte_ptrace_copy_tags(struct task_struct *child, long request,
>> unsigned long addr, unsigned long data);
>>
>> -void mte_assign_mem_tag_range(void *addr, size_t size);
>> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
>> +{
>> + u64 _addr = (u64)addr;
>> + u64 _end = _addr + size;
>> +
>> + /*
>> + * This function must be invoked from an MTE enabled context.
>> + *
>> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
>> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
>> + */
>> + do {
>> + /*
>> + * 'asm volatile' is required to prevent the compiler to move
>> + * the statement outside of the loop.
>> + */
>> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
>> + :
>> + : "r" (_addr)
>> + : "memory");
>> +
>> + _addr += MTE_GRANULE_SIZE;
>> + } while (_addr != _end);
>> +}
>
> While I'm ok with moving this function to C, I don't think it solves the
> inlining in the kasan code. The only interface we have to kasan is via
> mte_{set,get}_mem_tag_range(), so the above function doesn't need to
> live in a header.
>
> If you do want inlining all the way to the kasan code, we should
> probably move the mte_{set,get}_mem_tag_range() functions to the header
> as well (and ideally backed by some numbers to show that it matters).
>
> Moving it to mte.c also gives us more control on how it's called (we
> have the WARN_ONs in place in the callers).
>
Based on the thread [1] this patch contains only an intermediate step to allow
KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
think that mte_set_mem_tag_range() can be removed.
If you agree, I would live the things like this to give to Andrey a chance to
execute on the original plan with a separate series.
I agree though that this change alone does not bring huge benefits but
regressions neither.
If you want I can add something to the commit message in the next version to
make this more explicit.
Let me know how do you want me to proceed.
--
Regards,
Vincenzo
On Tue, Jan 19, 2021 at 4:45 PM Vincenzo Frascino
<[email protected]> wrote:
>
> Hi Catalin,
>
> On 1/19/21 2:45 PM, Catalin Marinas wrote:
> > On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
> >> mte_assign_mem_tag_range() is called on production KASAN HW hot
> >> paths. It makes sense to inline it in an attempt to reduce the
> >> overhead.
> >>
> >> Inline mte_assign_mem_tag_range() based on the indications provided at
> >> [1].
> >>
> >> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
> >>
> >> Cc: Catalin Marinas <[email protected]>
> >> Cc: Will Deacon <[email protected]>
> >> Signed-off-by: Vincenzo Frascino <[email protected]>
> >> ---
> >> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
> >> arch/arm64/lib/mte.S | 15 ---------------
> >> 2 files changed, 25 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> >> index 237bb2f7309d..1a6fd53f82c3 100644
> >> --- a/arch/arm64/include/asm/mte.h
> >> +++ b/arch/arm64/include/asm/mte.h
> >> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
> >> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> >> unsigned long addr, unsigned long data);
> >>
> >> -void mte_assign_mem_tag_range(void *addr, size_t size);
> >> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
> >> +{
> >> + u64 _addr = (u64)addr;
> >> + u64 _end = _addr + size;
> >> +
> >> + /*
> >> + * This function must be invoked from an MTE enabled context.
> >> + *
> >> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
> >> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
> >> + */
> >> + do {
> >> + /*
> >> + * 'asm volatile' is required to prevent the compiler to move
> >> + * the statement outside of the loop.
> >> + */
> >> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
> >> + :
> >> + : "r" (_addr)
> >> + : "memory");
> >> +
> >> + _addr += MTE_GRANULE_SIZE;
> >> + } while (_addr != _end);
> >> +}
> >
> > While I'm ok with moving this function to C, I don't think it solves the
> > inlining in the kasan code. The only interface we have to kasan is via
> > mte_{set,get}_mem_tag_range(), so the above function doesn't need to
> > live in a header.
> >
> > If you do want inlining all the way to the kasan code, we should
> > probably move the mte_{set,get}_mem_tag_range() functions to the header
> > as well (and ideally backed by some numbers to show that it matters).
> >
> > Moving it to mte.c also gives us more control on how it's called (we
> > have the WARN_ONs in place in the callers).
> >
>
> Based on the thread [1] this patch contains only an intermediate step to allow
> KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
> think that mte_set_mem_tag_range() can be removed.
>
> If you agree, I would live the things like this to give to Andrey a chance to
> execute on the original plan with a separate series.
I think we should drop this patch from this series as it's unrelated.
I will pick it up into my future optimization series. Then it will be
easier to discuss it in the context. The important part that I needed
is an inlinable C implementation of mte_assign_mem_tag_range(), which
I now have with this patch.
Thanks, Vincenzo!
On Mon, Jan 18, 2021 at 7:30 PM Vincenzo Frascino
<[email protected]> wrote:
>
> This patchset implements the asynchronous mode support for ARMv8.5-A
> Memory Tagging Extension (MTE), which is a debugging feature that allows
> to detect with the help of the architecture the C and C++ programmatic
> memory errors like buffer overflow, use-after-free, use-after-return, etc.
>
> MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
> (Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
> subset of its address space that is multiple of a 16 bytes granule. MTE
> is based on a lock-key mechanism where the lock is the tag associated to
> the physical memory and the key is the tag associated to the virtual
> address.
> When MTE is enabled and tags are set for ranges of address space of a task,
> the PE will compare the tag related to the physical memory with the tag
> related to the virtual address (tag check operation). Access to the memory
> is granted only if the two tags match. In case of mismatch the PE will raise
> an exception.
>
> The exception can be handled synchronously or asynchronously. When the
> asynchronous mode is enabled:
> - Upon fault the PE updates the TFSR_EL1 register.
> - The kernel detects the change during one of the following:
> - Context switching
> - Return to user/EL0
> - Kernel entry from EL1
> - Kernel exit to EL1
> - If the register has been updated by the PE the kernel clears it and
> reports the error.
>
> The series contains as well an optimization to mte_assign_mem_tag_range().
>
> The series is based on linux 5.11-rc3.
>
> To simplify the testing a tree with the new patches on top has been made
> available at [1].
>
> [1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async
Hi Vincenzo,
This change has multiple conflicts with the KASAN testing patches that
are currently in the mm tree. If Andrew decides to send all of them
during RC, then this should be good to go through arm64. Otherwise, I
guess this will need to go through mm as well. So you probably need to
rebase this on top of those patches in any case.
Thanks!
On Tue, Jan 19, 2021 at 07:12:40PM +0100, Andrey Konovalov wrote:
> On Tue, Jan 19, 2021 at 4:45 PM Vincenzo Frascino
> <[email protected]> wrote:
> > On 1/19/21 2:45 PM, Catalin Marinas wrote:
> > > On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
> > >> mte_assign_mem_tag_range() is called on production KASAN HW hot
> > >> paths. It makes sense to inline it in an attempt to reduce the
> > >> overhead.
> > >>
> > >> Inline mte_assign_mem_tag_range() based on the indications provided at
> > >> [1].
> > >>
> > >> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
> > >>
> > >> Cc: Catalin Marinas <[email protected]>
> > >> Cc: Will Deacon <[email protected]>
> > >> Signed-off-by: Vincenzo Frascino <[email protected]>
> > >> ---
> > >> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
> > >> arch/arm64/lib/mte.S | 15 ---------------
> > >> 2 files changed, 25 insertions(+), 16 deletions(-)
> > >>
> > >> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> > >> index 237bb2f7309d..1a6fd53f82c3 100644
> > >> --- a/arch/arm64/include/asm/mte.h
> > >> +++ b/arch/arm64/include/asm/mte.h
> > >> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
> > >> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> > >> unsigned long addr, unsigned long data);
> > >>
> > >> -void mte_assign_mem_tag_range(void *addr, size_t size);
> > >> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
> > >> +{
> > >> + u64 _addr = (u64)addr;
> > >> + u64 _end = _addr + size;
> > >> +
> > >> + /*
> > >> + * This function must be invoked from an MTE enabled context.
> > >> + *
> > >> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
> > >> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
> > >> + */
> > >> + do {
> > >> + /*
> > >> + * 'asm volatile' is required to prevent the compiler to move
> > >> + * the statement outside of the loop.
> > >> + */
> > >> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
> > >> + :
> > >> + : "r" (_addr)
> > >> + : "memory");
> > >> +
> > >> + _addr += MTE_GRANULE_SIZE;
> > >> + } while (_addr != _end);
> > >> +}
> > >
> > > While I'm ok with moving this function to C, I don't think it solves the
> > > inlining in the kasan code. The only interface we have to kasan is via
> > > mte_{set,get}_mem_tag_range(), so the above function doesn't need to
> > > live in a header.
> > >
> > > If you do want inlining all the way to the kasan code, we should
> > > probably move the mte_{set,get}_mem_tag_range() functions to the header
> > > as well (and ideally backed by some numbers to show that it matters).
> > >
> > > Moving it to mte.c also gives us more control on how it's called (we
> > > have the WARN_ONs in place in the callers).
> > >
> >
> > Based on the thread [1] this patch contains only an intermediate step to allow
> > KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
> > think that mte_set_mem_tag_range() can be removed.
> >
> > If you agree, I would live the things like this to give to Andrey a chance to
> > execute on the original plan with a separate series.
>
> I think we should drop this patch from this series as it's unrelated.
>
> I will pick it up into my future optimization series. Then it will be
> easier to discuss it in the context. The important part that I needed
> is an inlinable C implementation of mte_assign_mem_tag_range(), which
> I now have with this patch.
That's fine by me but we may want to add some forced-alignment on the
addr and size as the loop here depends on them being aligned, otherwise
it gets stuck. The mte_set_mem_tag_range() at least had a WARN_ON in
place. Here we could do:
addr &= MTE_GRANULE_MASK;
size = ALIGN(size, MTE_GRANULE_SIZE);
(or maybe trim "size" with MTE_GRANULE_MASK)
That's unless the call places are well known and guarantee this
alignment (only had a very brief look).
--
Catalin
On Tue, Jan 19, 2021 at 8:00 PM Catalin Marinas <[email protected]> wrote:
>
> On Tue, Jan 19, 2021 at 07:12:40PM +0100, Andrey Konovalov wrote:
> > On Tue, Jan 19, 2021 at 4:45 PM Vincenzo Frascino
> > <[email protected]> wrote:
> > > On 1/19/21 2:45 PM, Catalin Marinas wrote:
> > > > On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
> > > >> mte_assign_mem_tag_range() is called on production KASAN HW hot
> > > >> paths. It makes sense to inline it in an attempt to reduce the
> > > >> overhead.
> > > >>
> > > >> Inline mte_assign_mem_tag_range() based on the indications provided at
> > > >> [1].
> > > >>
> > > >> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
> > > >>
> > > >> Cc: Catalin Marinas <[email protected]>
> > > >> Cc: Will Deacon <[email protected]>
> > > >> Signed-off-by: Vincenzo Frascino <[email protected]>
> > > >> ---
> > > >> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
> > > >> arch/arm64/lib/mte.S | 15 ---------------
> > > >> 2 files changed, 25 insertions(+), 16 deletions(-)
> > > >>
> > > >> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> > > >> index 237bb2f7309d..1a6fd53f82c3 100644
> > > >> --- a/arch/arm64/include/asm/mte.h
> > > >> +++ b/arch/arm64/include/asm/mte.h
> > > >> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
> > > >> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> > > >> unsigned long addr, unsigned long data);
> > > >>
> > > >> -void mte_assign_mem_tag_range(void *addr, size_t size);
> > > >> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
> > > >> +{
> > > >> + u64 _addr = (u64)addr;
> > > >> + u64 _end = _addr + size;
> > > >> +
> > > >> + /*
> > > >> + * This function must be invoked from an MTE enabled context.
> > > >> + *
> > > >> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
> > > >> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
> > > >> + */
> > > >> + do {
> > > >> + /*
> > > >> + * 'asm volatile' is required to prevent the compiler to move
> > > >> + * the statement outside of the loop.
> > > >> + */
> > > >> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
> > > >> + :
> > > >> + : "r" (_addr)
> > > >> + : "memory");
> > > >> +
> > > >> + _addr += MTE_GRANULE_SIZE;
> > > >> + } while (_addr != _end);
> > > >> +}
> > > >
> > > > While I'm ok with moving this function to C, I don't think it solves the
> > > > inlining in the kasan code. The only interface we have to kasan is via
> > > > mte_{set,get}_mem_tag_range(), so the above function doesn't need to
> > > > live in a header.
> > > >
> > > > If you do want inlining all the way to the kasan code, we should
> > > > probably move the mte_{set,get}_mem_tag_range() functions to the header
> > > > as well (and ideally backed by some numbers to show that it matters).
> > > >
> > > > Moving it to mte.c also gives us more control on how it's called (we
> > > > have the WARN_ONs in place in the callers).
> > > >
> > >
> > > Based on the thread [1] this patch contains only an intermediate step to allow
> > > KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
> > > think that mte_set_mem_tag_range() can be removed.
> > >
> > > If you agree, I would live the things like this to give to Andrey a chance to
> > > execute on the original plan with a separate series.
> >
> > I think we should drop this patch from this series as it's unrelated.
> >
> > I will pick it up into my future optimization series. Then it will be
> > easier to discuss it in the context. The important part that I needed
> > is an inlinable C implementation of mte_assign_mem_tag_range(), which
> > I now have with this patch.
>
> That's fine by me but we may want to add some forced-alignment on the
> addr and size as the loop here depends on them being aligned, otherwise
> it gets stuck. The mte_set_mem_tag_range() at least had a WARN_ON in
> place. Here we could do:
>
> addr &= MTE_GRANULE_MASK;
> size = ALIGN(size, MTE_GRANULE_SIZE);
>
> (or maybe trim "size" with MTE_GRANULE_MASK)
>
> That's unless the call places are well known and guarantee this
> alignment (only had a very brief look).
No problem. I'll either add the ALIGN or change the call site to
ensure alignment.
Hi Andrey,
On 1/19/21 6:09 PM, Andrey Konovalov wrote:
> Hi Vincenzo,
>
> This change has multiple conflicts with the KASAN testing patches that
> are currently in the mm tree. If Andrew decides to send all of them
> during RC, then this should be good to go through arm64. Otherwise, I
> guess this will need to go through mm as well. So you probably need to
> rebase this on top of those patches in any case.
>
Could you please let me know on which tree do you want me to rebase my patches?
I almost completed the requested changes.
Thank you!
> Thanks!
--
Regards,
Vincenzo
On Thu, Jan 21, 2021 at 12:31 PM Vincenzo Frascino
<[email protected]> wrote:
>
> Hi Andrey,
>
> On 1/19/21 6:09 PM, Andrey Konovalov wrote:
> > Hi Vincenzo,
> >
> > This change has multiple conflicts with the KASAN testing patches that
> > are currently in the mm tree. If Andrew decides to send all of them
> > during RC, then this should be good to go through arm64. Otherwise, I
> > guess this will need to go through mm as well. So you probably need to
> > rebase this on top of those patches in any case.
> >
>
> Could you please let me know on which tree do you want me to rebase my patches?
> I almost completed the requested changes.
linux-next/akpm should work. Thanks!