From: Julien Thierry <julien.thierry@arm.com> To: James Morse <james.morse@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, daniel.thompson@linaro.org, joel@joelfernandes.org, marc.zyngier@arm.com, mark.rutland@arm.com, christoffer.dall@arm.com, catalin.marinas@arm.com, will.deacon@arm.com Subject: Re: [PATCH v5 06/27] arm64: Delay daif masking for user return Date: Wed, 12 Sep 2018 14:07:16 +0100 [thread overview] Message-ID: <a206ae6a-e615-96bd-7f9b-90691cd7af8f@arm.com> (raw) In-Reply-To: <59fa96d5-6bfa-c3aa-94d6-5941a7576bfa@arm.com> Hi James, On 12/09/18 11:31, James Morse wrote: > Hi Julien, > > On 28/08/18 16:51, Julien Thierry wrote: >> Masking daif flags is done very early before returning to EL0. >> >> Only toggle the interrupt masking while in the vector entry and mask daif >> once in kernel_exit. > > I had an earlier version that did this, but it showed up as a performance > problem. commit 8d66772e869e ("arm64: Mask all exceptions during kernel_exit") > described it as: > | Adding a naked 'disable_daif' to kernel_exit causes a performance problem > | for micro-benchmarks that do no real work, (e.g. calling getpid() in a > | loop). This is because the ret_to_user loop has already masked IRQs so > | that the TIF_WORK_MASK thread flags can't change underneath it, adding > | disable_daif is an additional self-synchronising operation. > | > | In the future, the RAS APEI code may need to modify the TIF_WORK_MASK > | flags from an SError, in which case the ret_to_user loop must mask SError > | while it examines the flags. > > > We may decide that the benchmark is silly, and we don't care about this. (At the > time it was easy enough to work around). > > We need regular-IRQs masked when we read the TIF flags, and to stay masked until > we return to user-space. > I assume you're changing this so that psuedo-NMI are unmasked for EL0 until > kernel_exit. > Yes. > I'd like to be able to change the TIF flags from the SError handlers for RAS, > which means masking SError for do_notify_resume too. (The RAS code that does > this doesn't exist today, so you can make this my problem to work out later!) > I think we should have psuedo_NMI masked if SError is masked too. > Yes, my intention in the few daif changes was that PseudoNMI would have just a little bit more priority than interrupt: Debug > Abort > FIQ (not used) > NMI (PMR masked, PSR.I == 0) > IRQ (daif + PMR cleared) So if at any point I break this just shout. (I did that change because currently el0_error has everything enabled before returning). > > Is there a strong reason for having psuedo-NMI unmasked during > do_notify_resume(), or is it just for having the maximum amount of code exposed? > As you suspected, this is to have the maximum amount of code exposed to Pseudo-NMIs. Since it is not a strong requirement for Pseudo-NMI, if the perf issue is more important I can drop the patch for now. Although it would be useful to have other opinions to see what makes the most sense. Thanks, > > Thanks, > > James > >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >> index 09dbea22..85ce06ac 100644 >> --- a/arch/arm64/kernel/entry.S >> +++ b/arch/arm64/kernel/entry.S >> @@ -259,9 +259,9 @@ alternative_else_nop_endif >> .endm >> >> .macro kernel_exit, el >> - .if \el != 0 >> disable_daif >> >> + .if \el != 0 >> /* Restore the task's original addr_limit. */ >> ldr x20, [sp, #S_ORIG_ADDR_LIMIT] >> str x20, [tsk, #TSK_TI_ADDR_LIMIT] >> @@ -896,7 +896,7 @@ work_pending: >> * "slow" syscall return path. >> */ >> ret_to_user: >> - disable_daif >> + disable_irq // disable interrupts >> ldr x1, [tsk, #TSK_TI_FLAGS] >> and x2, x1, #_TIF_WORK_MASK >> cbnz x2, work_pending >> > -- Julien Thierry
WARNING: multiple messages have this Message-ID (diff)
From: julien.thierry@arm.com (Julien Thierry) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v5 06/27] arm64: Delay daif masking for user return Date: Wed, 12 Sep 2018 14:07:16 +0100 [thread overview] Message-ID: <a206ae6a-e615-96bd-7f9b-90691cd7af8f@arm.com> (raw) In-Reply-To: <59fa96d5-6bfa-c3aa-94d6-5941a7576bfa@arm.com> Hi James, On 12/09/18 11:31, James Morse wrote: > Hi Julien, > > On 28/08/18 16:51, Julien Thierry wrote: >> Masking daif flags is done very early before returning to EL0. >> >> Only toggle the interrupt masking while in the vector entry and mask daif >> once in kernel_exit. > > I had an earlier version that did this, but it showed up as a performance > problem. commit 8d66772e869e ("arm64: Mask all exceptions during kernel_exit") > described it as: > | Adding a naked 'disable_daif' to kernel_exit causes a performance problem > | for micro-benchmarks that do no real work, (e.g. calling getpid() in a > | loop). This is because the ret_to_user loop has already masked IRQs so > | that the TIF_WORK_MASK thread flags can't change underneath it, adding > | disable_daif is an additional self-synchronising operation. > | > | In the future, the RAS APEI code may need to modify the TIF_WORK_MASK > | flags from an SError, in which case the ret_to_user loop must mask SError > | while it examines the flags. > > > We may decide that the benchmark is silly, and we don't care about this. (At the > time it was easy enough to work around). > > We need regular-IRQs masked when we read the TIF flags, and to stay masked until > we return to user-space. > I assume you're changing this so that psuedo-NMI are unmasked for EL0 until > kernel_exit. > Yes. > I'd like to be able to change the TIF flags from the SError handlers for RAS, > which means masking SError for do_notify_resume too. (The RAS code that does > this doesn't exist today, so you can make this my problem to work out later!) > I think we should have psuedo_NMI masked if SError is masked too. > Yes, my intention in the few daif changes was that PseudoNMI would have just a little bit more priority than interrupt: Debug > Abort > FIQ (not used) > NMI (PMR masked, PSR.I == 0) > IRQ (daif + PMR cleared) So if at any point I break this just shout. (I did that change because currently el0_error has everything enabled before returning). > > Is there a strong reason for having psuedo-NMI unmasked during > do_notify_resume(), or is it just for having the maximum amount of code exposed? > As you suspected, this is to have the maximum amount of code exposed to Pseudo-NMIs. Since it is not a strong requirement for Pseudo-NMI, if the perf issue is more important I can drop the patch for now. Although it would be useful to have other opinions to see what makes the most sense. Thanks, > > Thanks, > > James > >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >> index 09dbea22..85ce06ac 100644 >> --- a/arch/arm64/kernel/entry.S >> +++ b/arch/arm64/kernel/entry.S >> @@ -259,9 +259,9 @@ alternative_else_nop_endif >> .endm >> >> .macro kernel_exit, el >> - .if \el != 0 >> disable_daif >> >> + .if \el != 0 >> /* Restore the task's original addr_limit. */ >> ldr x20, [sp, #S_ORIG_ADDR_LIMIT] >> str x20, [tsk, #TSK_TI_ADDR_LIMIT] >> @@ -896,7 +896,7 @@ work_pending: >> * "slow" syscall return path. >> */ >> ret_to_user: >> - disable_daif >> + disable_irq // disable interrupts >> ldr x1, [tsk, #TSK_TI_FLAGS] >> and x2, x1, #_TIF_WORK_MASK >> cbnz x2, work_pending >> > -- Julien Thierry
next prev parent reply other threads:[~2018-09-12 13:07 UTC|newest] Thread overview: 116+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-28 15:51 [PATCH v5 00/27] arm64: provide pseudo NMI with GICv3 Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 01/27] arm64: cpufeature: Set SYSREG_GIC_CPUIF as a boot system feature Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-21 15:56 ` Marc Zyngier 2018-09-21 15:56 ` Marc Zyngier 2018-09-25 3:10 ` Yao Lihua 2018-09-25 8:13 ` Marc Zyngier 2018-09-25 8:13 ` Marc Zyngier 2018-09-25 10:39 ` Yao Lihua 2018-08-28 15:51 ` [PATCH v5 02/27] arm64: cpufeature: Use alternatives for VHE cpu_enable Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-12 10:28 ` James Morse 2018-09-12 10:28 ` James Morse 2018-09-12 12:03 ` Julien Thierry 2018-09-12 12:03 ` Julien Thierry 2018-09-18 17:46 ` James Morse 2018-09-18 17:46 ` James Morse 2018-09-12 12:37 ` Suzuki K Poulose 2018-09-12 12:37 ` Suzuki K Poulose 2018-08-28 15:51 ` [PATCH v5 03/27] arm64: alternative: Apply alternatives early in boot process Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-12 10:29 ` James Morse 2018-09-12 10:29 ` James Morse 2018-09-12 16:49 ` Julien Thierry 2018-09-12 16:49 ` Julien Thierry 2018-09-17 23:44 ` Daniel Thompson 2018-09-17 23:44 ` Daniel Thompson 2018-09-18 7:37 ` Julien Thierry 2018-09-18 7:37 ` Julien Thierry 2018-09-18 17:47 ` James Morse 2018-09-18 17:47 ` James Morse 2018-09-21 16:05 ` Marc Zyngier 2018-09-21 16:05 ` Marc Zyngier 2018-08-28 15:51 ` [PATCH v5 04/27] arm64: daifflags: Use irqflags functions for daifflags Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-12 12:28 ` James Morse 2018-09-12 12:28 ` James Morse 2018-10-03 15:09 ` Catalin Marinas 2018-10-03 15:09 ` Catalin Marinas 2018-08-28 15:51 ` [PATCH v5 05/27] arm64: Use daifflag_restore after bp_hardening Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-12 10:32 ` James Morse 2018-09-12 10:32 ` James Morse 2018-09-12 11:11 ` Julien Thierry 2018-09-12 11:11 ` Julien Thierry 2018-09-12 12:28 ` James Morse 2018-09-12 12:28 ` James Morse 2018-09-12 13:03 ` Julien Thierry 2018-09-12 13:03 ` Julien Thierry 2018-10-03 15:12 ` Catalin Marinas 2018-10-03 15:12 ` Catalin Marinas 2018-08-28 15:51 ` [PATCH v5 06/27] arm64: Delay daif masking for user return Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-12 10:31 ` James Morse 2018-09-12 10:31 ` James Morse 2018-09-12 13:07 ` Julien Thierry [this message] 2018-09-12 13:07 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 07/27] arm64: xen: Use existing helper to check interrupt status Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-29 21:35 ` Stefano Stabellini 2018-08-29 21:35 ` Stefano Stabellini 2018-10-03 15:14 ` Catalin Marinas 2018-10-03 15:14 ` Catalin Marinas 2018-08-28 15:51 ` [PATCH v5 08/27] irqchip/gic: Unify GIC priority definitions Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-10-03 9:24 ` Marc Zyngier 2018-10-03 9:24 ` Marc Zyngier 2018-08-28 15:51 ` [PATCH v5 09/27] irqchip/gic: Lower priority of GIC interrupts Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 10/27] arm64: cpufeature: Add cpufeature for IRQ priority masking Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 11/27] arm64: Make PMR part of task context Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 12/27] arm64: Unmask PMR before going idle Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 13/27] arm/arm64: gic-v3: Add helper functions to manage IRQ priorities Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 14/27] arm64: kvm: Unmask PMR before entering guest Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 15/27] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-09-21 17:39 ` Julien Thierry 2018-09-21 17:39 ` Julien Thierry 2018-09-21 17:55 ` Julien Thierry 2018-09-21 17:55 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 16/27] arm64: daifflags: Include PMR in daifflags restore operations Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 17/27] irqchip/gic-v3: Factor group0 detection into functions Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 18/27] irqchip/gic-v3: Do not overwrite PMR value Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 19/27] irqchip/gic-v3: Remove acknowledge loop Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-10-03 9:26 ` Marc Zyngier 2018-10-03 9:26 ` Marc Zyngier 2018-08-28 15:51 ` [PATCH v5 20/27] irqchip/gic-v3: Switch to PMR masking after IRQ acknowledge Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 21/27] arm64: Switch to PMR masking when starting CPUs Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 22/27] arm64: Add build option for IRQ masking via priority Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 23/27] arm64: Handle serror in NMI context Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 24/27] irqchip/gic-v3: Detect current view of GIC priorities Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 25/27] irqchip/gic-v3: Add base support for pseudo-NMI Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 26/27] irqchip/gic: Add functions to access irq priorities Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-28 15:51 ` [PATCH v5 27/27] irqchip/gic-v3: Allow interrupts to be set as pseudo-NMI Julien Thierry 2018-08-28 15:51 ` Julien Thierry 2018-08-29 11:37 ` [PATCH v5 00/27] arm64: provide pseudo NMI with GICv3 Daniel Thompson 2018-08-29 11:37 ` Daniel Thompson 2018-08-29 12:58 ` Julien Thierry 2018-08-29 12:58 ` Julien Thierry
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=a206ae6a-e615-96bd-7f9b-90691cd7af8f@arm.com \ --to=julien.thierry@arm.com \ --cc=catalin.marinas@arm.com \ --cc=christoffer.dall@arm.com \ --cc=daniel.thompson@linaro.org \ --cc=james.morse@arm.com \ --cc=joel@joelfernandes.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=mark.rutland@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: linkBe 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.