From: Marc Zyngier <maz@kernel.org> To: Alexandru Elisei <alexandru.elisei@arm.com> Cc: Chen-Yu Tsai <wenst@chromium.org>, Thomas Gleixner <tglx@linutronix.de>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] irqchip/gic-v3: Fix priority comparison when non-secure priorities are used Date: Fri, 20 Aug 2021 13:58:56 +0100 [thread overview] Message-ID: <87pmu8qnlb.wl-maz@kernel.org> (raw) In-Reply-To: <061bafb6-b0fe-bd7c-1567-a642d3ddd4a5@arm.com> Hi Alex, On Thu, 12 Aug 2021 15:24:03 +0100, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi Marc, > > On 8/12/21 2:09 PM, Marc Zyngier wrote: > > On Thu, 12 Aug 2021 12:51:34 +0100, > > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > >> Hi, > >> > >> After re-familiarizing myself with the spec, it starting to look to > >> me like indeed there's something not quite right (read as: totally > >> broken) with my patch. > >> > >> Arm IHI 0069F, the pseudocode for reading ICC_RPR_EL1 (page 11-797), > >> says that the priority returned is unchanged if SCTLR_EL3.FIQ == > >> 0. > > Sure, but look at what ICC_RPR_EL1 does for FIQ==1: > > > > <quote> > > if HaveEL(EL3) && !IsSecure() && SCR_EL3.FIQ == '1' then > > // A Non-secure GIC access and Group 0 inaccessible to Non-secure. > > if pPriority<7> == '0' then > > // Priority is in Secure half and not visible to Non-secure > > Priority = Zeros(); > > elsif !IsOnes(pPriority) then > > // Non-secure access and not idle, so physical priority must be shifted > > pPriority<7:0> = (pPriority AND PRIMask())<6:0>:'0'; > > > > return ZeroExtend(pPriority); > > </quote> > > > > See how the the priority is shifted *left* (bits [6:0] end up in > > [7:1])? > > Yes, when SCR_EL3.FIQ=1, but gic_nonsecure_priorities is enabled > when SCR_EL3.FIQ=0 (gic_has_group0()). In that case ICC_RPR_EL1 > returns (what I assume to be) the highest priority interrupt from > ICC_AP0R_EL1, ICC_AP1R_EL1NS and ICC_AP1R_EL1S. Isn't that the > secure view (or Distributor value) of the priority? Yup. I guess I got confused with what "non-secure" priorities mean in this context. [...] > I don't see how that is the case - ICC_RPR_EL1 contains the priority > value as seen by the Distributor, and non-secure priorities get > right-shifted when SCR_EL3.FIQ=0, meaning that GICD_INT_NMI_PRI > becomes (GICD_INT_NMI_PRI >> 1) | 0x80 in the Distributor. Can you > elaborate where I'm contradicting myself? I think I know why I confused myself. When FIQ==0, G0 is NS. On the face of it, this should mean that no shift occurs. However, G1S is still in the picture, and we get the extra shift to preserve the ordering with G1S. This is a different configuration from that of a guest, where G0 is also NS, but there is no shift at all, as there is no G1S. The GIC strikes back. Again. I run some more tests with this patch, and merge it of nothing breaks. Thanks, M. -- Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org> To: Alexandru Elisei <alexandru.elisei@arm.com> Cc: Chen-Yu Tsai <wenst@chromium.org>, Thomas Gleixner <tglx@linutronix.de>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] irqchip/gic-v3: Fix priority comparison when non-secure priorities are used Date: Fri, 20 Aug 2021 13:58:56 +0100 [thread overview] Message-ID: <87pmu8qnlb.wl-maz@kernel.org> (raw) In-Reply-To: <061bafb6-b0fe-bd7c-1567-a642d3ddd4a5@arm.com> Hi Alex, On Thu, 12 Aug 2021 15:24:03 +0100, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi Marc, > > On 8/12/21 2:09 PM, Marc Zyngier wrote: > > On Thu, 12 Aug 2021 12:51:34 +0100, > > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > >> Hi, > >> > >> After re-familiarizing myself with the spec, it starting to look to > >> me like indeed there's something not quite right (read as: totally > >> broken) with my patch. > >> > >> Arm IHI 0069F, the pseudocode for reading ICC_RPR_EL1 (page 11-797), > >> says that the priority returned is unchanged if SCTLR_EL3.FIQ == > >> 0. > > Sure, but look at what ICC_RPR_EL1 does for FIQ==1: > > > > <quote> > > if HaveEL(EL3) && !IsSecure() && SCR_EL3.FIQ == '1' then > > // A Non-secure GIC access and Group 0 inaccessible to Non-secure. > > if pPriority<7> == '0' then > > // Priority is in Secure half and not visible to Non-secure > > Priority = Zeros(); > > elsif !IsOnes(pPriority) then > > // Non-secure access and not idle, so physical priority must be shifted > > pPriority<7:0> = (pPriority AND PRIMask())<6:0>:'0'; > > > > return ZeroExtend(pPriority); > > </quote> > > > > See how the the priority is shifted *left* (bits [6:0] end up in > > [7:1])? > > Yes, when SCR_EL3.FIQ=1, but gic_nonsecure_priorities is enabled > when SCR_EL3.FIQ=0 (gic_has_group0()). In that case ICC_RPR_EL1 > returns (what I assume to be) the highest priority interrupt from > ICC_AP0R_EL1, ICC_AP1R_EL1NS and ICC_AP1R_EL1S. Isn't that the > secure view (or Distributor value) of the priority? Yup. I guess I got confused with what "non-secure" priorities mean in this context. [...] > I don't see how that is the case - ICC_RPR_EL1 contains the priority > value as seen by the Distributor, and non-secure priorities get > right-shifted when SCR_EL3.FIQ=0, meaning that GICD_INT_NMI_PRI > becomes (GICD_INT_NMI_PRI >> 1) | 0x80 in the Distributor. Can you > elaborate where I'm contradicting myself? I think I know why I confused myself. When FIQ==0, G0 is NS. On the face of it, this should mean that no shift occurs. However, G1S is still in the picture, and we get the extra shift to preserve the ordering with G1S. This is a different configuration from that of a guest, where G0 is also NS, but there is no shift at all, as there is no G1S. The GIC strikes back. Again. I run some more tests with this patch, and merge it of nothing breaks. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-08-20 12:59 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-11 17:15 [PATCH] irqchip/gic-v3: Fix priority comparison when non-secure priorities are used Chen-Yu Tsai 2021-08-11 17:15 ` Chen-Yu Tsai 2021-08-11 18:31 ` Marc Zyngier 2021-08-11 18:31 ` Marc Zyngier 2021-08-12 11:51 ` Alexandru Elisei 2021-08-12 11:51 ` Alexandru Elisei 2021-08-12 13:09 ` Marc Zyngier 2021-08-12 13:09 ` Marc Zyngier 2021-08-12 14:24 ` Alexandru Elisei 2021-08-12 14:24 ` Alexandru Elisei 2021-08-20 12:58 ` Marc Zyngier [this message] 2021-08-20 12:58 ` Marc Zyngier 2021-08-16 15:11 ` Chen-Yu Tsai 2021-08-16 15:11 ` Chen-Yu Tsai 2021-08-20 13:31 ` Alexandru Elisei 2021-08-20 13:31 ` Alexandru Elisei 2021-08-20 13:55 ` Marc Zyngier 2021-08-20 13:55 ` Marc Zyngier 2021-08-20 13:34 ` [irqchip: irq/irqchip-next] " irqchip-bot for Chen-Yu Tsai 2021-08-20 14:05 ` irqchip-bot for Chen-Yu Tsai
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=87pmu8qnlb.wl-maz@kernel.org \ --to=maz@kernel.org \ --cc=alexandru.elisei@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=wenst@chromium.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: 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.