[MODERATED] Re: [PATCH v5 16/27] MDSv5 10

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Jan 18, 2019 at 04:50:31PM -0800, speck for Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> Subject:  mds: Mark interrupts clear cpu, unless opted-out
> 
> Interrupts might touch user data from other processes
> in any context.
> 
> By default we clear the CPU on the next kernel exit.
> 
> Add a new IRQ_F_NO_USER interrupt flag. When the flag
> is not set on interrupt execution we clear the cpu state on

s/we clear the cpu state/we flush the CPU's MDS state's/ ?

'cpu state' implies (At least to me) - everything - like even cache.
But that is what not what we do - we do our lazy CPU flushing.
> next kernel exit.
> 
> This allows interrupts to opt-out from the extra clearing
> overhead, but is safe by default.

s/but is safe by default/if they are sanitized and carry no user data./ ?

> 
> Over time as more interrupt code is audited it can set the opt-out.

s/it can set the opt-out/we can opt-out various code/ ?

Either way:
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!
> ---
>  include/linux/interrupt.h | 2 ++
>  kernel/irq/handle.c       | 8 ++++++++
>  kernel/irq/manage.c       | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index c672f34235e7..291b7fee3afe 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -61,6 +61,7 @@
>   *                interrupt handler after suspending interrupts. For system
>   *                wakeup devices users need to implement wakeup detection in
>   *                their interrupt handlers.
> + * IRQF_NO_USER	- Interrupt does not touch user data
>   */
>  #define IRQF_SHARED		0x00000080
>  #define IRQF_PROBE_SHARED	0x00000100
> @@ -74,6 +75,7 @@
>  #define IRQF_NO_THREAD		0x00010000
>  #define IRQF_EARLY_RESUME	0x00020000
>  #define IRQF_COND_SUSPEND	0x00040000
> +#define IRQF_NO_USER		0x00080000
>  
>  #define IRQF_TIMER		(__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
>  
> diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> index 38554bc35375..e5910938ce2b 100644
> --- a/kernel/irq/handle.c
> +++ b/kernel/irq/handle.c
> @@ -13,6 +13,7 @@
>  #include <linux/sched.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel_stat.h>
> +#include <linux/clearcpu.h>
>  
>  #include <trace/events/irq.h>
>  
> @@ -149,6 +150,13 @@ irqreturn_t __handle_irq_event_percpu(struct irq_desc *desc, unsigned int *flags
>  		res = action->handler(irq, action->dev_id);
>  		trace_irq_handler_exit(irq, action, res);
>  
> +		/*
> +		 * We aren't sure if the interrupt handler did or did not
> +		 * touch user data. Schedule a cpu clear just in case.
> +		 */
> +		if (!(action->flags & IRQF_NO_USER))
> +			lazy_clear_cpu();
> +
>  		if (WARN_ONCE(!irqs_disabled(),"irq %u handler %pF enabled interrupts\n",
>  			      irq, action->handler))
>  			local_irq_disable();
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index a4888ce4667a..3f0c99240638 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1793,6 +1793,7 @@ EXPORT_SYMBOL(free_irq);
>   *
>   *	IRQF_SHARED		Interrupt is shared
>   *	IRQF_TRIGGER_*		Specify active edge(s) or level
> + *	IRQF_NOUSER		Does not touch user data.
>   *
>   */
>  int request_threaded_irq(unsigned int irq, irq_handler_t handler,
> -- 
> 2.17.2