Re: [RFC v6 21/62] powerpc: introduce execute-only pkey

[Michael Ellerman]

Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxxxxxxx> writes:

> Michael Ellerman <mpe@xxxxxxxxxxxxxx> writes:
>
>> Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxxxxxxx> writes:
>>> Ram Pai <linuxram@xxxxxxxxxx> writes:
>> ...
>>>> +
>>>> +	/* We got one, store it and use it from here on out */
>>>> +	if (need_to_set_mm_pkey)
>>>> +		mm->context.execute_only_pkey = execute_only_pkey;
>>>> +	return execute_only_pkey;
>>>> +}
>>>
>>> If you follow the code flow in __execute_only_pkey, the AMR and UAMOR
>>> are read 3 times in total, and AMR is written twice. IAMR is read and
>>> written twice. Since they are SPRs and access to them is slow (or isn't
>>> it?),
>>
>> SPRs read/writes are slow, but they're not *that* slow in comparison to
>> a system call (which I think is where this code is being called?).
>
> Yes, this code runs on mprotect and mmap syscalls if the memory is
> requested to have execute but not read nor write permissions.

Yep. That's not in the fast path for key usage, ie. the fast path is
userspace changing the AMR itself, and the overhead of a syscall is
already hundreds of cycles.

>> So we should try to avoid too many SPR read/writes, but at the same time
>> we can accept more than the minimum if it makes the code much easier to
>> follow.
>
> Ok. Ram had asked me to suggest a way to optimize the SPR reads and
> writes and I came up with the patch below. Do you think it's worth it?

At a glance no I don't think it is. Sorry you spent that much time on it.

I think we can probably reduce the number of SPR accesses without
needing to go to that level of complexity.

But don't throw the patch away, I may eat my words once I have the full
series applied and am looking at it hard - at the moment I'm just
reviewing the patches piecemeal as I get time.

cheers