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

[Greg KH <gregkh@linuxfoundation.org>]

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
> next kernel exit.
> 
> This allows interrupts to opt-out from the extra clearing
> overhead, but is safe by default.
> 
> Over time as more interrupt code is audited it can set the opt-out.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  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

I know you want to be "safe", but I think Linus's comment about having
this be "opt in" is better from an auditing point of view in that you
should mark an IRQ as touching user data as you should know that you are
doing that.

> --- 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();

We should be sure.  Why can't we be sure?  We know what the irq did,
right, we wrote it :)

thanks,

greg k-h