linux-kernel - Re: [PATCH v6 07/24] arm64: Make PMR part of task context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d7651225-97ed-02a1-dd87-87cdbd36bca5@arm.com>
Date:   Tue, 4 Dec 2018 17:30:53 +0000
From:   Julien Thierry <julien.thierry@....com>
To:     Catalin Marinas <catalin.marinas@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, daniel.thompson@...aro.org,
        marc.zyngier@....com, will.deacon@....com,
        linux-kernel@...r.kernel.org, christoffer.dall@....com,
        james.morse@....com, joel@...lfernandes.org,
        Dave Martin <Dave.Martin@....com>
Subject: Re: [PATCH v6 07/24] arm64: Make PMR part of task context



On 04/12/18 17:09, Catalin Marinas wrote:
> On Mon, Nov 12, 2018 at 11:56:58AM +0000, Julien Thierry wrote:
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 039144e..eb8120e 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -249,6 +249,12 @@ alternative_else_nop_endif
>>  	msr	sp_el0, tsk
>>  	.endif
>>  
>> +	/* Save pmr */
>> +alternative_if ARM64_HAS_IRQ_PRIO_MASKING
>> +	mrs_s	x20, SYS_ICC_PMR_EL1
>> +	str	x20, [sp, #S_PMR_SAVE]
>> +alternative_else_nop_endif
>> +
>>  	/*
>>  	 * Registers that may be useful after this macro is invoked:
>>  	 *
>> @@ -269,6 +275,13 @@ alternative_else_nop_endif
>>  	/* No need to restore UAO, it will be restored from SPSR_EL1 */
>>  	.endif
>>  
>> +	/* Restore pmr */
>> +alternative_if ARM64_HAS_IRQ_PRIO_MASKING
>> +	ldr	x20, [sp, #S_PMR_SAVE]
>> +	msr_s	SYS_ICC_PMR_EL1, x20
>> +	dsb	sy
>> +alternative_else_nop_endif
> 
> What's this DSB for? If it's needed, please add a comment.
> 

The DSB is to make sure that, in the case we are unmasking interrupt
priorities, the unmasking is seen by the redistributor. Without it the
redistributor might only start forwarding interrupts (if their
priorities are too low) to the CPU once it has seen that PMR was
modified, which could happen after the CPU has returned from the exception.

I'll add a comment.

> I would have expected an ISB (or none at all as we are going to return
> from an exception).

So the two reasons we don't need an ISB are:
- the only thing that matter is that PMR modification + DSB happens
before exception return
- writes to ICC_PMR_EL1 are self synchronizing so we don't need an ISB
before the DSB

Thanks,

-- 
Julien Thierry