[RFC PATCH v2 21/41] arm64/sve: Enable SVE on demand for userspace
Mark Rutland
mark.rutland at arm.com
Wed Mar 22 09:48:10 PDT 2017
Hi,
On Wed, Mar 22, 2017 at 02:50:51PM +0000, Dave Martin wrote:
> This patch tracks whether a task has ever attempted to use the
> Scalable Vector Extension. If and only if SVE is in use by a task,
> it will be enabled for userspace when scheduling the task in. For
> other tasks, SVE is disabled when scheduling in.
> #define TIF_SYSCALL_AUDIT 9
> #define TIF_SYSCALL_TRACEPOINT 10
> #define TIF_SECCOMP 11
> +#define TIF_SVE 17 /* Scalable Vector Extension in use */
Please don't skip ahead to bit 17. I see why you're doing that, but I
don't think that's a good idea. More on that below.
> +void do_sve_acc(unsigned int esr, struct pt_regs *regs)
> +{
> + unsigned long tmp;
> +
> + if (test_and_set_thread_flag(TIF_SVE))
> + BUG();
> +
> + asm ("mrs %0, cpacr_el1" : "=r" (tmp));
> + asm volatile ("msr cpacr_el1, %0" :: "r" (tmp | (1 << 17)));
Please de-magic the number, e.g. with something like:
#define CPACR_SVE_EN (1 << 17)
... in <asm/sysreg.h>.
Please also use {read,write}_sysreg(), e.g.
val = read_sysreg(cpacr_el1);
tmp |= CPACR_SVE_EN;
write_sysreg(tmp, cpacr_el1);
[...]
> + if (IS_ENABLED(CONFIG_ARM64_SVE)) {
> + /*
> + * Flip SVE enable for userspace if it doesn't
> + * match the current_task.
> + */
> + asm ("mrs %0, cpacr_el1" : "=r" (tmp));
> + flags = current_thread_info()->flags;
> + if ((tmp ^ (unsigned long)flags) & (1 << 17)) {
> + tmp ^= 1 << 17;
> + asm volatile ("msr cpacr_el1, %0" :: "r" (tmp));
> + }
I think it's a bad idea to rely on the TIF flag and CPACR bit to have
the same index. It makes this painful to read, and it leaves us fragile
if the TIF bits are reorganised.
How about:
unsigned long cpacr = read_sysreg(cpacr_el1);
bool hw_enabled = !!(cpacr & CPACR_SVE_EN);
bool sw_enabled = test_thread_flag(TIF_SVE);
/*
* Flip SVE enable for userspace if it doesn't
* match the current_task.
*/
if (hw_enabled != sw_enabled) {
cpacr ^= CPACR_SVE_EN;
write_sysreg(cpacr, cpacr_el1);
}
Thanks,
Mark.
More information about the linux-arm-kernel
mailing list