All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Branislav Rankov <Branislav.Rankov@arm.com>,
	Marco Elver <elver@google.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend
Date: Fri, 12 Feb 2021 13:19:44 +0000	[thread overview]
Message-ID: <20210212131944.GB7718@arm.com> (raw)
In-Reply-To: <20210212120015.GA18281@e121166-lin.cambridge.arm.com>

On Fri, Feb 12, 2021 at 12:00:15PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
> > +void mte_suspend_enter(void)
> > +{
> > +	if (!system_supports_mte())
> > +		return;
> > +
> > +	/*
> > +	 * The barriers are required to guarantee that the indirect writes
> > +	 * to TFSR_EL1 are synchronized before we report the state.
> > +	 */
> > +	dsb(nsh);
> > +	isb();
> > +
> > +	/* Report SYS_TFSR_EL1 before suspend entry */
> > +	mte_check_tfsr_el1();
> > +}
> > +
> >  void mte_suspend_exit(void)
> >  {
> >  	if (!system_supports_mte())
> >  		return;
> >  
> >  	update_gcr_el1_excl(gcr_kernel_excl);
> > +
> > +	/* Clear SYS_TFSR_EL1 after suspend exit */
> > +	write_sysreg_s(0, SYS_TFSR_EL1);
> 
> AFAICS it is not needed, it is done already in __cpu_setup() (that is
> called by cpu_resume on return from cpu_suspend() from firmware).
> 
> However, I have a question. We are relying on context switch to set
> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
> low power switches context we are running with SCTLR_EL1_TCF0 not
> reflecting the actual value.

I think you have a point here, though not for SCTLR_EL1 as it is already
restored. GCR_EL1 is only updated after some C code has run and may mess
up stack tagging when/if we ever support it. Anyway, something to worry
about later, I think even the boot path gets this wrong.

-- 
Catalin

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

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Will Deacon <will@kernel.org>, Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Branislav Rankov <Branislav.Rankov@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>
Subject: Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend
Date: Fri, 12 Feb 2021 13:19:44 +0000	[thread overview]
Message-ID: <20210212131944.GB7718@arm.com> (raw)
In-Reply-To: <20210212120015.GA18281@e121166-lin.cambridge.arm.com>

On Fri, Feb 12, 2021 at 12:00:15PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Feb 11, 2021 at 03:33:52PM +0000, Vincenzo Frascino wrote:
> > +void mte_suspend_enter(void)
> > +{
> > +	if (!system_supports_mte())
> > +		return;
> > +
> > +	/*
> > +	 * The barriers are required to guarantee that the indirect writes
> > +	 * to TFSR_EL1 are synchronized before we report the state.
> > +	 */
> > +	dsb(nsh);
> > +	isb();
> > +
> > +	/* Report SYS_TFSR_EL1 before suspend entry */
> > +	mte_check_tfsr_el1();
> > +}
> > +
> >  void mte_suspend_exit(void)
> >  {
> >  	if (!system_supports_mte())
> >  		return;
> >  
> >  	update_gcr_el1_excl(gcr_kernel_excl);
> > +
> > +	/* Clear SYS_TFSR_EL1 after suspend exit */
> > +	write_sysreg_s(0, SYS_TFSR_EL1);
> 
> AFAICS it is not needed, it is done already in __cpu_setup() (that is
> called by cpu_resume on return from cpu_suspend() from firmware).
> 
> However, I have a question. We are relying on context switch to set
> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
> low power switches context we are running with SCTLR_EL1_TCF0 not
> reflecting the actual value.

I think you have a point here, though not for SCTLR_EL1 as it is already
restored. GCR_EL1 is only updated after some C code has run and may mess
up stack tagging when/if we ever support it. Anyway, something to worry
about later, I think even the boot path gets this wrong.

-- 
Catalin

  parent reply	other threads:[~2021-02-12 13:21 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 15:33 [PATCH v13 0/7] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-02-11 15:33 ` Vincenzo Frascino
2021-02-11 15:33 ` [PATCH v13 1/7] arm64: mte: Add asynchronous " Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-12 21:21   ` Andrey Konovalov
2021-02-12 21:21     ` Andrey Konovalov
2021-02-22 11:14     ` Vincenzo Frascino
2021-02-22 11:14       ` Vincenzo Frascino
2021-02-11 15:33 ` [PATCH v13 2/7] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-11 17:50   ` Andrey Konovalov
2021-02-11 17:50     ` Andrey Konovalov
2021-02-12 11:26     ` Vincenzo Frascino
2021-02-12 11:26       ` Vincenzo Frascino
2021-02-11 15:33 ` [PATCH v13 3/7] kasan: Add report for async mode Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-11 20:03   ` kernel test robot
2021-02-11 20:03     ` kernel test robot
2021-02-11 20:03     ` kernel test robot
2021-02-11 20:13     ` Andrey Konovalov
2021-02-11 20:13       ` Andrey Konovalov
2021-02-11 20:13       ` Andrey Konovalov
2021-02-12 11:25       ` Vincenzo Frascino
2021-02-12 11:25         ` Vincenzo Frascino
2021-02-12 11:25         ` Vincenzo Frascino
2021-02-12 14:47         ` Andrey Konovalov
2021-02-12 14:47           ` Andrey Konovalov
2021-02-12 14:47           ` Andrey Konovalov
2021-02-11 23:03   ` kernel test robot
2021-02-11 23:03     ` kernel test robot
2021-02-11 15:33 ` [PATCH v13 4/7] arm64: mte: Enable TCO in functions that can read beyond buffer limits Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-12 17:21   ` Catalin Marinas
2021-02-12 17:21     ` Catalin Marinas
2021-02-22 12:08     ` Vincenzo Frascino
2021-02-22 12:08       ` Vincenzo Frascino
2021-02-22 17:58       ` Catalin Marinas
2021-02-22 17:58         ` Catalin Marinas
2021-02-23 10:56         ` Vincenzo Frascino
2021-02-23 10:56           ` Vincenzo Frascino
2021-02-23 12:05           ` Catalin Marinas
2021-02-23 12:05             ` Catalin Marinas
2021-02-23 12:22             ` Vincenzo Frascino
2021-02-23 12:22               ` Vincenzo Frascino
2021-02-23 12:49             ` Will Deacon
2021-02-23 12:49               ` Will Deacon
2021-02-23 13:14               ` Catalin Marinas
2021-02-23 13:14                 ` Catalin Marinas
2021-02-23 14:25               ` Vincenzo Frascino
2021-02-23 14:25                 ` Vincenzo Frascino
2021-02-11 15:33 ` [PATCH v13 5/7] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-11 15:33 ` [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-12 12:00   ` Lorenzo Pieralisi
2021-02-12 12:00     ` Lorenzo Pieralisi
2021-02-12 12:30     ` Lorenzo Pieralisi
2021-02-12 12:30       ` Lorenzo Pieralisi
2021-02-12 13:05       ` Vincenzo Frascino
2021-02-12 13:05         ` Vincenzo Frascino
2021-02-12 13:19     ` Catalin Marinas [this message]
2021-02-12 13:19       ` Catalin Marinas
2021-02-12 17:24   ` Catalin Marinas
2021-02-12 17:24     ` Catalin Marinas
2021-02-11 15:33 ` [PATCH v13 7/7] kasan: don't run tests in async mode Vincenzo Frascino
2021-02-11 15:33   ` Vincenzo Frascino
2021-02-12  0:49   ` kernel test robot
2021-02-12  0:49     ` kernel test robot
2021-02-12 17:22   ` Catalin Marinas
2021-02-12 17:22     ` Catalin Marinas
2021-02-12 21:44     ` Andrey Konovalov
2021-02-12 21:44       ` Andrey Konovalov
2021-02-22 11:17       ` Vincenzo Frascino
2021-02-22 11:17         ` Vincenzo Frascino
2021-02-22 13:35         ` Andrey Konovalov
2021-02-22 13:35           ` Andrey Konovalov
2021-02-11 15:45 ` [PATCH v13 0/7] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-02-11 15:45   ` Vincenzo Frascino

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=20210212131944.GB7718@arm.com \
    --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=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will@kernel.org \
    /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.