[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160909171537.GD19083@e104818-lin.cambridge.arm.com>
Date: Fri, 9 Sep 2016 18:15:37 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: linux-arm-kernel@...ts.infradead.org,
Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc: AKASHI Takahiro <takahiro.akashi@...aro.org>,
Will Deacon <will.deacon@....com>,
James Morse <james.morse@....com>,
Kees Cook <keescook@...omium.org>,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v2 3/7] arm64: Introduce uaccess_{disable, enable}
functionality based on TTBR0_EL1
On Fri, Sep 02, 2016 at 04:02:09PM +0100, Catalin Marinas wrote:
> /*
> * User access enabling/disabling.
> */
> +#ifdef CONFIG_ARM64_TTBR0_PAN
> +static inline void uaccess_ttbr0_disable(void)
> +{
> + unsigned long ttbr;
> +
> + /* reserved_ttbr0 placed at the end of swapper_pg_dir */
> + ttbr = read_sysreg(ttbr1_el1) + SWAPPER_DIR_SIZE;
> + write_sysreg(ttbr, ttbr0_el1);
> + isb();
> +}
> +
> +static inline void uaccess_ttbr0_enable(void)
> +{
> + unsigned long flags;
> +
> + /*
> + * Disable interrupts to avoid preemption and potential saved
> + * TTBR0_EL1 updates between reading the variable and the MSR.
> + */
> + local_irq_save(flags);
> + write_sysreg(current_thread_info()->ttbr0, ttbr0_el1);
> + isb();
> + local_irq_restore(flags);
> +}
I followed up with the ARM architects on potential improvements to this
sequence. In summary, changing TCR_EL1.A1 is not guaranteed to have an
effect unless it is followed by TLBI. IOW, we can't use this bit for a
quick switch to the reserved ASID.
Setting TCR_EL1.EPD0 to 1 would work as long as it is followed by an
ASID change to a reserved one with no entries in the TLB. However, the
code sequence above (and the corresponding asm ones) would become even
more complex, so I don't think we gain anything.
Untested, using EPD0 (the assembly version would look sligtly better
than the C version but still a few instructions more than what we
currently have):
static inline void uaccess_ttbr0_disable(void)
{
unsigned long ttbr;
unsigned long tcr;
/* disable TTBR0 page table walks */
tcr = read_sysreg(tcr_el1);
tcr |= TCR_ELD0
write_sysreg(tcr, tcr_el1);
isb();
/* mask out the ASID bits (zero is a reserved ASID) */
ttbr = read_sysreg(ttbr0_el1);
ttbr &= ~ASID_MASK;
write_sysreg(ttbr, ttbr0_el1);
isb();
}
static inline void uaccess_ttbr0_enable(void)
{
unsigned long flags;
local_irq_save(flags);
ttbr = read_sysreg(ttbr0_el1);
ttbr |= current_thread_info()->asid;
write_sysreg(ttbr, ttbr0_el1);
isb();
/* enable TTBR0 page table walks */
tcr = read_sysreg(tcr_el1);
tcr |= TCR_ELD0
write_sysreg(tcr, tcr_el1);
isb();
local_irq_restore(flags);
}
The IRQ disabling for the above sequence is still required since we need
to guarantee the atomicity of the ASID read with the TTBR0_EL1 write.
We may be able to avoid current_thread_info()->asid *if* we find some
other per-CPU place to store the ASID (unused TTBR1_EL1 bits was
suggested, though not sure about the architecture requirements on those
bits being zero when TCR_EL1.A1 is 0). But even with these in place, the
requirement to have to ISBs and the additional TCR_EL1 read/write
doesn't give us anything better.
In conclusion, I propose that we stick to the current TTBR0_EL1 switch
as per these patches.
--
Catalin
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
