RE: [patch 1/6] x86/fpu: Extend fpu_xstate_prctl() with guest permissions

Tian, Kevin
2021-12-14T05:13:14Z

> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Tuesday, December 14, 2021 10:50 AM
>
> KVM requires a clear separation of host user space and guest permissions
> for dynamic XSTATE components.
>
> Add a guest permissions member to struct fpu and a separate set of prctl()
> arguments: ARCH_GET_XCOMP_GUEST_PERM and
> ARCH_REQ_XCOMP_GUEST_PERM.
>
> The semantics are equivalent to the host user space permission control
> except for the following constraints:
>
> 1) Permissions have to be requested before the first vCPU is created
>
> 2) Permissions are frozen when the first vCPU is created to ensure
> consistency. Any attempt to expand permissions via the prctl() after
> that point is rejected.

A curiosity question. Do we allow the user to reduce permissions?

> @@ -477,6 +479,13 @@ struct fpu {
> struct fpu_state_perm perm;
>
> /*
> + * @guest_perm:
> + *
> + * Permission related information for guest pseudo FPUs
> + */

why calling it 'pseudo'? It's real FPU state managed by this series...

> @@ -1742,6 +1751,7 @@ long fpu_xstate_prctl(struct task_struct
> u64 __user *uptr = (u64 __user *)arg2;
> u64 permitted, supported;
> unsigned long idx = arg2;
> + bool guest = false;
>
> if (tsk != current)
> return -EPERM;
> @@ -1760,11 +1770,20 @@ long fpu_xstate_prctl(struct task_struct
> permitted &= XFEATURE_MASK_USER_SUPPORTED;
> return put_user(permitted, uptr);
>
> + case ARCH_GET_XCOMP_GUEST_PERM:
> + permitted = xstate_get_guest_group_perm();
> + permitted &= XFEATURE_MASK_USER_SUPPORTED;
> + return put_user(permitted, uptr);

Similarly as done for ARCH_REQ_XCOMP_GUEST_PERM:

+ case ARCH_GET_XCOMP_GUEST_PERM:
+ guest = true;
+ fallthrough;
+
case ARCH_GET_XCOMP_PERM:
/*
* Lockless snapshot as it can also change right after the
* dropping the lock.
*/
- permitted = xstate_get_host_group_perm();
+ permitted = xstate_get_group_perm(guest);
permitted &= XFEATURE_MASK_USER_SUPPORTED;
return put_user(permitted, uptr);

So the comment about 'lockless' is shared by both.

> +
> + case ARCH_REQ_XCOMP_GUEST_PERM:
> + guest = true;
> + fallthrough;
> +
> case ARCH_REQ_XCOMP_PERM:
> if (!IS_ENABLED(CONFIG_X86_64))
> return -EOPNOTSUPP;
>
> - return xstate_request_perm(idx);
> + return xstate_request_perm(idx, guest);
>
> default:
> return -EINVAL;

Thanks
Kevin