All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Andrey Konovalov <andreyknvl@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev@googlegroups.com,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Elena Petrova <lenaptr@google.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 29/39] arm64: mte: Switch GCR_EL1 in kernel entry and exit
Date: Fri, 9 Oct 2020 11:16:43 +0100	[thread overview]
Message-ID: <20201009101643.GG23638@gaia> (raw)
In-Reply-To: <106f8670-3dd0-70ad-91ac-4f419585df50@arm.com>

On Fri, Oct 09, 2020 at 10:56:02AM +0100, Vincenzo Frascino wrote:
> On 10/9/20 9:11 AM, Catalin Marinas wrote:
> > On Thu, Oct 08, 2020 at 07:24:12PM +0100, Vincenzo Frascino wrote:
> >> On 10/2/20 3:06 PM, Catalin Marinas wrote:
> >>> On Fri, Oct 02, 2020 at 01:10:30AM +0200, Andrey Konovalov wrote:
> >>>> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> >>>> index 7c67ac6f08df..d1847f29f59b 100644
> >>>> --- a/arch/arm64/kernel/mte.c
> >>>> +++ b/arch/arm64/kernel/mte.c
> >>>> @@ -23,6 +23,8 @@
> >>>>  #include <asm/ptrace.h>
> >>>>  #include <asm/sysreg.h>
> >>>>  
> >>>> +u64 gcr_kernel_excl __ro_after_init;
> >>>> +
> >>>>  static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
> >>>>  {
> >>>>  	pte_t old_pte = READ_ONCE(*ptep);
> >>>> @@ -120,6 +122,13 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
> >>>>  	return ptr;
> >>>>  }
> >>>>  
> >>>> +void mte_init_tags(u64 max_tag)
> >>>> +{
> >>>> +	u64 incl = GENMASK(max_tag & MTE_TAG_MAX, 0);
> >>>
> >>> Nitpick: it's not obvious that MTE_TAG_MAX is a mask, so better write
> >>> this as GENMASK(min(max_tag, MTE_TAG_MAX), 0).
> >>
> >> The two things do not seem equivalent because the format of the tags in KASAN is
> >> 0xFF and in MTE is 0xF, hence if extract the minimum whatever is the tag passed
> >> by KASAN it will always be MTE_TAG_MAX.
> >>
> >> To make it cleaner I propose: GENMASK(FIELD_GET(MTE_TAG_MAX, max_tag), 0);
> > 
> > I don't think that's any clearer since FIELD_GET still assumes that
> > MTE_TAG_MAX is a mask. I think it's better to add a comment on why this
> > is needed, as you explained above that the KASAN tags go to 0xff.
> > 
> > If you want to get rid of MTE_TAG_MAX altogether, just do a
> > 
> > 	max_tag &= (1 << MAX_TAG_SIZE) - 1;
> > 
> > before setting incl (a comment is still useful).
> > 
> 
> Agree, but still think we should use FIELD_GET here since it is common language
> in the kernel.
> 
> How about we get rid of MTE_TAG_MAX and we do something like:
> 
> GENMASK(FIELD_GET(MTE_TAG_MASK >> MTE_TAG_SHIFT, max_tag), 0);

It works for me and you can drop the MTE_TAG_MAX definition (I think
it's only used here).

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	Marco Elver <elver@google.com>,
	Elena Petrova <lenaptr@google.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Alexander Potapenko <glider@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dmitry Vyukov <dvyukov@google.com>
Subject: Re: [PATCH v4 29/39] arm64: mte: Switch GCR_EL1 in kernel entry and exit
Date: Fri, 9 Oct 2020 11:16:43 +0100	[thread overview]
Message-ID: <20201009101643.GG23638@gaia> (raw)
In-Reply-To: <106f8670-3dd0-70ad-91ac-4f419585df50@arm.com>

On Fri, Oct 09, 2020 at 10:56:02AM +0100, Vincenzo Frascino wrote:
> On 10/9/20 9:11 AM, Catalin Marinas wrote:
> > On Thu, Oct 08, 2020 at 07:24:12PM +0100, Vincenzo Frascino wrote:
> >> On 10/2/20 3:06 PM, Catalin Marinas wrote:
> >>> On Fri, Oct 02, 2020 at 01:10:30AM +0200, Andrey Konovalov wrote:
> >>>> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> >>>> index 7c67ac6f08df..d1847f29f59b 100644
> >>>> --- a/arch/arm64/kernel/mte.c
> >>>> +++ b/arch/arm64/kernel/mte.c
> >>>> @@ -23,6 +23,8 @@
> >>>>  #include <asm/ptrace.h>
> >>>>  #include <asm/sysreg.h>
> >>>>  
> >>>> +u64 gcr_kernel_excl __ro_after_init;
> >>>> +
> >>>>  static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
> >>>>  {
> >>>>  	pte_t old_pte = READ_ONCE(*ptep);
> >>>> @@ -120,6 +122,13 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
> >>>>  	return ptr;
> >>>>  }
> >>>>  
> >>>> +void mte_init_tags(u64 max_tag)
> >>>> +{
> >>>> +	u64 incl = GENMASK(max_tag & MTE_TAG_MAX, 0);
> >>>
> >>> Nitpick: it's not obvious that MTE_TAG_MAX is a mask, so better write
> >>> this as GENMASK(min(max_tag, MTE_TAG_MAX), 0).
> >>
> >> The two things do not seem equivalent because the format of the tags in KASAN is
> >> 0xFF and in MTE is 0xF, hence if extract the minimum whatever is the tag passed
> >> by KASAN it will always be MTE_TAG_MAX.
> >>
> >> To make it cleaner I propose: GENMASK(FIELD_GET(MTE_TAG_MAX, max_tag), 0);
> > 
> > I don't think that's any clearer since FIELD_GET still assumes that
> > MTE_TAG_MAX is a mask. I think it's better to add a comment on why this
> > is needed, as you explained above that the KASAN tags go to 0xff.
> > 
> > If you want to get rid of MTE_TAG_MAX altogether, just do a
> > 
> > 	max_tag &= (1 << MAX_TAG_SIZE) - 1;
> > 
> > before setting incl (a comment is still useful).
> > 
> 
> Agree, but still think we should use FIELD_GET here since it is common language
> in the kernel.
> 
> How about we get rid of MTE_TAG_MAX and we do something like:
> 
> GENMASK(FIELD_GET(MTE_TAG_MASK >> MTE_TAG_SHIFT, max_tag), 0);

It works for me and you can drop the MTE_TAG_MAX definition (I think
it's only used here).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-10-09 10:16 UTC|newest]

Thread overview: 138+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 23:10 [PATCH v4 00/39] kasan: add hardware tag-based mode for arm64 Andrey Konovalov
2020-10-01 23:10 ` Andrey Konovalov
2020-10-01 23:10 ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 01/39] kasan: drop unnecessary GPL text from comment headers Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 02/39] kasan: KASAN_VMALLOC depends on KASAN_GENERIC Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 03/39] kasan: group vmalloc code Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 04/39] kasan: shadow declarations only for software modes Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 05/39] kasan: rename (un)poison_shadow to (un)poison_memory Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 06/39] kasan: rename KASAN_SHADOW_* to KASAN_GRANULE_* Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 07/39] kasan: only build init.c for software modes Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 08/39] kasan: split out shadow.c from common.c Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 09/39] kasan: define KASAN_GRANULE_PAGE Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 10/39] kasan: rename report and tags files Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 11/39] kasan: don't duplicate config dependencies Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 12/39] kasan: hide invalid free check implementation Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 13/39] kasan: decode stack frame only with KASAN_STACK_ENABLE Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 14/39] kasan, arm64: only init shadow for software modes Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 15/39] kasan, arm64: only use kasan_depth " Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 16/39] kasan: rename addr_has_shadow to addr_has_metadata Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 17/39] kasan: rename print_shadow_for_address to print_memory_metadata Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 18/39] kasan: kasan_non_canonical_hook only for software modes Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 19/39] kasan: rename SHADOW layout macros to META Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 20/39] kasan: separate metadata_fetch_row for each mode Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 21/39] kasan, arm64: don't allow SW_TAGS with ARM64_MTE Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 22/39] kasan: introduce CONFIG_KASAN_HW_TAGS Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 23/39] arm64: Enable armv8.5-a asm-arch option Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 24/39] arm64: mte: Add in-kernel MTE helpers Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-02 13:51   ` Catalin Marinas
2020-10-02 13:51     ` Catalin Marinas
2020-10-01 23:10 ` [PATCH v4 25/39] arm64: kasan: Add arch layer for memory tagging helpers Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 26/39] arm64: mte: Add in-kernel tag fault handler Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 27/39] arm64: kasan: Enable in-kernel MTE Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-02 14:00   ` Catalin Marinas
2020-10-02 14:00     ` Catalin Marinas
2020-10-01 23:10 ` [PATCH v4 28/39] arm64: mte: Convert gcr_user into an exclude mask Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 29/39] arm64: mte: Switch GCR_EL1 in kernel entry and exit Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-02 14:06   ` Catalin Marinas
2020-10-02 14:06     ` Catalin Marinas
2020-10-08 18:24     ` Vincenzo Frascino
2020-10-08 18:24       ` Vincenzo Frascino
2020-10-09  8:11       ` Catalin Marinas
2020-10-09  8:11         ` Catalin Marinas
2020-10-09  9:56         ` Vincenzo Frascino
2020-10-09  9:56           ` Vincenzo Frascino
2020-10-09 10:16           ` Catalin Marinas [this message]
2020-10-09 10:16             ` Catalin Marinas
2020-10-09 10:21             ` Vincenzo Frascino
2020-10-09 10:21               ` Vincenzo Frascino
2020-10-01 23:10 ` [PATCH v4 30/39] arm64: kasan: Enable TBI EL1 Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-02 14:10   ` Catalin Marinas
2020-10-02 14:10     ` Catalin Marinas
2020-10-01 23:10 ` [PATCH v4 31/39] arm64: kasan: Align allocations for HW_TAGS Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 32/39] kasan: define KASAN_GRANULE_SIZE " Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 33/39] kasan, x86, s390: update undef CONFIG_KASAN Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 34/39] kasan, arm64: expand CONFIG_KASAN checks Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 35/39] kasan, arm64: implement HW_TAGS runtime Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 36/39] kasan, arm64: print report from tag fault handler Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 37/39] kasan, mm: reset tags when accessing metadata Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 38/39] kasan, arm64: enable CONFIG_KASAN_HW_TAGS Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10 ` [PATCH v4 39/39] kasan: add documentation for hardware tag-based mode Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov
2020-10-01 23:10   ` Andrey Konovalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201009101643.GG23638@gaia \
    --to=catalin.marinas@arm.com \
    --cc=Branislav.Rankov@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kevin.brodsky@arm.com \
    --cc=lenaptr@google.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=vincenzo.frascino@arm.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.