On Tue, Sep 15, 2020 at 05:43:40PM +0200, Heiko Carstens wrote:
> Address spaces still have to switched/changed for machines without the
> mvcos instructions and especially for instructions like e.g. compare
> and swap (-> futex) which must be executed in kernel address space but
> access user address space. For such instructions enable_sacf_uaccess()
> and disable_sacf_uaccess() must be used like before.
That logic always confused me and still keeps confusing me,
dumb questions below:
> int oldval = 0, newval, ret;
> - mm_segment_t old_fs;
> + bool old;
>
> - old_fs = enable_sacf_uaccess();
> + old = enable_sacf_uaccess();
> switch (op) {
> case FUTEX_OP_SET:
> __futex_atomic_op("lr %2,%5\n",
> @@ -53,7 +53,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
> default:
> ret = -ENOSYS;
> }
> - disable_sacf_uaccess(old_fs);
> + disable_sacf_uaccess(old);
Do we need to return the old value here? The way I understand it
this is context switched with the thread, and given that only small
isolated code bases now use it, sacf use can't nest, can it?
> @@ -116,7 +114,7 @@ struct thread_struct {
> unsigned long hardirq_timer; /* task cputime in hardirq context */
> unsigned long softirq_timer; /* task cputime in softirq context */
> unsigned long sys_call_table; /* system call table address */
> - mm_segment_t mm_segment;
> + bool sacf_uaccess; /* uaccess with sacf enabled */
Isn't there a flags field somewhere where this could be folded into?
> -void set_fs_fixup(void)
> -{
> - struct pt_regs *regs = current_pt_regs();
> - static bool warned;
> -
> - set_fs(USER_DS);
> - if (warned)
> - return;
> - WARN(1, "Unbalanced set_fs - int code: 0x%x\n", regs->int_code);
> - show_registers(regs);
> - warned = true;
Would a warning about an unbalanced sacf flag still make sense? Or
just objtool for compile time checks similar to the unsafe uaccess
routines on x86?
> +bool enable_sacf_uaccess(void)
Maybe add a little comment documenting when to use the function
> }
> EXPORT_SYMBOL(enable_sacf_uaccess);
Neither enable_sacf_uaccess nor disable_sacf_uaccess appear to be
used in modular code, so these exports can probably be dropped.
