Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Balbir Singh <sblbir@amazon.com>, linux-kernel@vger.kernel.org
Cc:     jpoimboe@redhat.com, tony.luck@intel.com, keescook@chromium.org,
        benh@kernel.crashing.org, x86@kernel.org, dave.hansen@intel.com,
        thomas.lendacky@amd.com, Balbir Singh <sblbir@amazon.com>
Subject: Re: [PATCH v6 5/6] Optionally flush L1D on context switch
In-Reply-To: <20200510014803.12190-6-sblbir@amazon.com>
References: <20200510014803.12190-1-sblbir@amazon.com> <20200510014803.12190-6-sblbir@amazon.com>
Date:   Wed, 13 May 2020 18:16:56 +0200
Message-ID: <875zcz3j47.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Balbir Singh <sblbir@amazon.com> writes:

This part:

> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -238,4 +238,8 @@ struct prctl_mm_map {
>  #define PR_SET_IO_FLUSHER		57
>  #define PR_GET_IO_FLUSHER		58
>  
> +/* Flush L1D on context switch (mm) */
> +#define PR_SET_L1D_FLUSH		59
> +#define PR_GET_L1D_FLUSH		60

...

> @@ -2514,6 +2524,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>  
>  		error = (current->flags & PR_IO_FLUSHER) == PR_IO_FLUSHER;
>  		break;
> +	case PR_SET_L1D_FLUSH:
> +		if (arg3 || arg4 || arg5)
> +			return -EINVAL;
> +		error = arch_prctl_l1d_flush_set(me, arg2);
> +		break;
> +	case PR_GET_L1D_FLUSH:
> +		if (arg2 || arg3 || arg4 || arg5)
> +			return -EINVAL;
> +		error = arch_prctl_l1d_flush_get(me);
> +		break;
>  	default:
>  		error = -EINVAL;
>  		break;

wants to be split into a separate patch, really. Then we get a proper
subject lines with proper subsystem prefixes. This part also lacks a
description in Documentation/userspace-api/ and function prototypes for
the arch_prctl* functions.

But looking at this deeper (yes I should have noticed earlier):

    Why do we need yet another PRCTL?

We already have PR_SET_SPECULATION_CTRL/PR_GET_SPECULATION_CTRL. That
L1D flush thingy fits into this category, right?

This makes even more sense if you think about the second use case for
L1D flush, i.e. the flush when a untrusted task comes in. If we ever
want to support that case then this will be imposed by seccomp and then
we'd need yet another interface there.

And for this reason we should also name that current opt-in thingy:
L1D_FLUSH_OUT in the prctl and also for the TIF bits.

Hmm? Kees?

I've applied the first 4 patches to:

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/mm

so the polishing I did gets preserved and you don't have to resend the
whole pile.

Thanks,

        tglx