From: Daniel Thompson <daniel.thompson@linaro.org>
To: Julien Thierry <julien.thierry@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, joel@joelfernandes.org,
marc.zyngier@arm.com, christoffer.dall@arm.com,
james.morse@arm.com, catalin.marinas@arm.com,
will.deacon@arm.com, Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH v6 06/24] arm64: ptrace: Provide definitions for PMR values
Date: Fri, 30 Nov 2018 10:38:11 +0000 [thread overview]
Message-ID: <20181130103811.lushdja552xevghm@holly.lan> (raw)
In-Reply-To: <79145359-3594-3dac-1123-cec552c2b13e@arm.com>
On Fri, Nov 30, 2018 at 08:53:47AM +0000, Julien Thierry wrote:
>
>
> On 29/11/18 16:40, Mark Rutland wrote:
> > On Mon, Nov 12, 2018 at 11:56:57AM +0000, Julien Thierry wrote:
> >> Introduce fixed values for PMR that are going to be used to mask and
> >> unmask interrupts by priority. These values are chosent in such a way
> >
> > Nit: s/chosent/chosen/
> >
> >> that a single bit (GIC_PMR_UNMASKED_BIT) encodes the information whether
> >> interrupts are masked or not.
> >
> > There's no GIC_PMR_UNMASKED_BIT in this patch. Should that be
> > GIC_PRIO_STATUS_BIT?
> >
>
> Yep, forgot to update the commit message when renaming, thanks.
>
> >> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> >> Suggested-by: Daniel Thompson <daniel.thompson@linaro.org>
> >> Cc: Oleg Nesterov <oleg@redhat.com>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will.deacon@arm.com>
> >> ---
> >> arch/arm64/include/asm/ptrace.h | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> >> index fce22c4..ce6998c 100644
> >> --- a/arch/arm64/include/asm/ptrace.h
> >> +++ b/arch/arm64/include/asm/ptrace.h
> >> @@ -25,6 +25,12 @@
> >> #define CurrentEL_EL1 (1 << 2)
> >> #define CurrentEL_EL2 (2 << 2)
> >>
> >> +/* PMR values used to mask/unmask interrupts */
> >> +#define GIC_PRIO_IRQON 0xf0
> >> +#define GIC_PRIO_STATUS_SHIFT 6
> >> +#define GIC_PRIO_STATUS_BIT (1 << GIC_PRIO_STATUS_SHIFT)
> >> +#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON ^ GIC_PRIO_STATUS_BIT)
> >
> > Could you elaborate on the GIC priority logic a bit?
> >
>
> Yes, I'll give details below.
>
> > Are lower numbers higher priority?
> >
>
> Yes, that is the case.
>
> > Are there restrictions on valid PMR values?
> >
>
> Yes, there are at most 8 priority bits but implementations are free to
> implement a number of priority bits:
> - between 5 and 8 when GIC runs two security states (bits [7:3] always
> being implemented and [2:0] being optional), but non-secure side is
> always deprived or the lowest implemented bit
> - between 4 and 8 when GIC runs only one security state (bits [7:4]
> implemented, bits [3:0] optional)
>
> This is detailed in section 4.8 "Interrupt prioritization" of the GICv3
> architecture specification.
>
> So Linux should always be able to see bits [7:4].
>
> > IIUC GIC_PRIO_IRQOFF is 0xb0 (aka 0b10110000), which seems a little
> > surprising. I'd have expected that we'd use the most signficant bit.
> >
>
> So, re-reading the GICv3 spec, I believe this sparked from a confusion...
>
> The idea was that the GICv3 specification would recommend to keep
> non-secure group-1 interrupts at a lower priority that group-0 (and
> secure group-1 interrupts) interrupts, and to do so the idea was to
> always keep bit[7] == 1 for non-secure group-1.
>
> So, we would need to have priority bit[7] == 1 for both normal
> interrupts and pseudo-NMIs, and using the most significant bit to mask
> would mean masking pseudo-NMIs as well.
>
> However, I only find mention of this in the notes of section 4.8.6
> "Software accesses of interrupt priority". The section only applies to
> GIC with two security states, and the recommendation of writing
> non-secure group-1 priorities with bit[7] == 1 is only directed at
> writes from the secure side. From the non-secure side, the GIC already
> does some magic to enforce that the value kept in the distributor has
> bit[7] == 1.
>
> So, I believe that from the non-secure point of view, we could define
> pseudo-NMI priority as e.g. 0x40 (which the GIC will convert to 0xa0)
> and use the most significant bit of PMR to mask normal interrupts which
> would be more intuitive.
>
> Marc, as GIC expert do you agree with this? Or is there a reason we
> should keep bit[7] == 1 for non-secure group-1 priorities?
I think selecting bit 6 dates back to when I was working on this.
I originally used bit 7 but switched due to problems on the FVP at the
time (my memory is a little hazy here but it felt like it wasn't
doing the magic shift properly when running in non-secure mode).
Once the patchset was running on real hardware I kept on with bit 6
figuring that, given the magic shift from non-secure mode is a little
odd, it would remain furtile soil for future silicon bugs (I was
watching a lot of patches go past on the ML working round bugs in
non-Arm GIC implementations and ended up feeling rather paranoid
about things like that).
Daniel.
next prev parent reply other threads:[~2018-11-30 10:38 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-12 11:56 [PATCH v6 00/24] arm64: provide pseudo NMI with GICv3 Julien Thierry
2018-11-12 11:56 ` [PATCH v6 01/24] arm64: Remove unused daif related functions/macros Julien Thierry
2018-11-29 16:26 ` Mark Rutland
2018-11-30 18:03 ` Catalin Marinas
2018-11-12 11:56 ` [PATCH v6 02/24] arm64: cpufeature: Set SYSREG_GIC_CPUIF as a boot system feature Julien Thierry
2018-11-12 18:00 ` Suzuki K Poulose
2018-11-29 16:27 ` Mark Rutland
2018-11-30 18:07 ` Catalin Marinas
2018-11-12 11:56 ` [PATCH v6 03/24] arm64: cpufeature: Add cpufeature for IRQ priority masking Julien Thierry
2018-11-12 18:02 ` Suzuki K Poulose
2018-11-29 17:12 ` Mark Rutland
2018-12-03 10:33 ` Julien Thierry
2018-11-30 18:07 ` Catalin Marinas
2018-11-12 11:56 ` [PATCH v6 04/24] arm/arm64: gic-v3: Add PMR and RPR accessors Julien Thierry
2018-11-29 16:32 ` Mark Rutland
2018-11-30 18:07 ` Catalin Marinas
2018-11-12 11:56 ` [PATCH v6 05/24] irqchip/gic-v3: Switch to PMR masking before calling IRQ handler Julien Thierry
2018-11-29 18:12 ` Mark Rutland
2018-11-30 9:18 ` Julien Thierry
2018-12-04 16:21 ` Catalin Marinas
2018-11-12 11:56 ` [PATCH v6 06/24] arm64: ptrace: Provide definitions for PMR values Julien Thierry
2018-11-29 16:40 ` Mark Rutland
2018-11-30 8:53 ` Julien Thierry
2018-11-30 10:38 ` Daniel Thompson [this message]
2018-11-30 11:03 ` Julien Thierry
2018-11-12 11:56 ` [PATCH v6 07/24] arm64: Make PMR part of task context Julien Thierry
2018-11-29 16:46 ` Mark Rutland
2018-11-30 9:25 ` Julien Thierry
2018-12-04 17:09 ` Catalin Marinas
2018-12-04 17:30 ` Julien Thierry
2018-11-12 11:56 ` [PATCH v6 08/24] arm64: Unmask PMR before going idle Julien Thierry
2018-11-29 17:44 ` Mark Rutland
2018-11-30 10:55 ` Julien Thierry
2018-11-30 13:37 ` Mark Rutland
2018-12-03 10:38 ` Julien Thierry
2018-11-12 11:57 ` [PATCH v6 09/24] arm64: kvm: Unmask PMR before entering guest Julien Thierry
2018-11-12 11:57 ` [PATCH v6 10/24] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking Julien Thierry
2018-12-04 17:36 ` Catalin Marinas
2018-12-05 16:55 ` Julien Thierry
2018-12-05 18:26 ` Catalin Marinas
2018-12-06 9:50 ` Julien Thierry
2018-12-10 14:39 ` Catalin Marinas
2018-11-12 11:57 ` [PATCH v6 11/24] arm64: daifflags: Include PMR in daifflags restore operations Julien Thierry
2018-11-12 11:57 ` [PATCH v6 12/24] arm64: alternative: Allow alternative status checking per cpufeature Julien Thierry
2018-11-12 11:57 ` [PATCH v6 13/24] arm64: alternative: Apply alternatives early in boot process Julien Thierry
2018-11-12 11:57 ` [PATCH v6 14/24] irqchip/gic-v3: Factor group0 detection into functions Julien Thierry
2018-11-12 11:57 ` [PATCH v6 15/24] arm64: Switch to PMR masking when starting CPUs Julien Thierry
2018-12-04 17:51 ` Catalin Marinas
2018-12-04 18:11 ` Julien Thierry
2018-11-12 11:57 ` [PATCH v6 16/24] arm64: gic-v3: Implement arch support for priority masking Julien Thierry
2018-11-12 11:57 ` [PATCH v6 17/24] irqchip/gic-v3: Detect if GIC can support pseudo-NMIs Julien Thierry
2018-11-12 11:57 ` [PATCH v6 18/24] irqchip/gic-v3: Handle pseudo-NMIs Julien Thierry
2018-11-12 11:57 ` [PATCH v6 19/24] irqchip/gic: Add functions to access irq priorities Julien Thierry
2018-11-12 11:57 ` [PATCH v6 20/24] irqchip/gic-v3: Allow interrupts to be set as pseudo-NMI Julien Thierry
2018-11-12 11:57 ` [PATCH v6 21/24] arm64: Handle serror in NMI context Julien Thierry
2018-12-04 18:09 ` Catalin Marinas
2018-12-05 13:02 ` James Morse
2018-11-12 11:57 ` [PATCH v6 22/24] arm64: Skip preemption when exiting an NMI Julien Thierry
2018-11-12 11:57 ` [PATCH v6 23/24] arm64: Skip irqflags tracing for NMI in IRQs disabled context Julien Thierry
2018-11-12 11:57 ` [PATCH v6 24/24] arm64: Enable the support of pseudo-NMIs Julien Thierry
2018-11-12 12:00 ` [PATCH v6 00/24] arm64: provide pseudo NMI with GICv3 Julien Thierry
2018-11-13 14:43 ` 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=20181130103811.lushdja552xevghm@holly.lan \
--to=daniel.thompson@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@arm.com \
--cc=james.morse@arm.com \
--cc=joel@joelfernandes.org \
--cc=julien.thierry@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mark.rutland@arm.com \
--cc=oleg@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).