Re: [PATCH] irqchip/gic-v3: Fix priority comparison when non-secure priorities are used

Hi Alex,

On Thu, 12 Aug 2021 15:24:03 +0100,
Alexandru Elisei [off-list ref] wrote:

Hi Marc,

On 8/12/21 2:09 PM, Marc Zyngier wrote:

quoted

On Thu, 12 Aug 2021 12:51:34 +0100,
Alexandru Elisei [off-list ref] wrote:

quoted

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