lkml.org 
[lkml]   [2018]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 06/24] arm64: ptrace: Provide definitions for PMR values
From
Date


On 30/11/18 10:38, Daniel Thompson wrote:
> 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).
>

If you were using boot-wrapper, that might have been the case as
SCR_EL3.FIQ is not getting set.

The fun bit is that under this configuration the magic bit still happens
for non-secure accesses to priorities configured in the
distributor/redistributor, but it disables the magic for non-secure PMR
and RPR accesses. So you can easily end up masking too much stuff when
writting to PMR when SCR_EL1.FIQ is cleared if you don't realize that
what non-secure sees in the distributor is not aligned with what will be
masked by PMR or presented in RPR.

> 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.
>

--
Julien Thierry

\
 
 \ /
  Last update: 2018-11-30 12:04    [W:0.085 / U:2.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site