[PATCH v3 09/13] arm64: kernel: Add arch-specific SDEI entry code and CPU masking

[Catalin Marinas]

On Fri, Sep 22, 2017 at 07:26:10PM +0100, James Morse wrote:
> diff --git a/arch/arm64/include/asm/sdei.h b/arch/arm64/include/asm/sdei.h
> new file mode 100644
> index 000000000000..ed329e01a301
> --- /dev/null
> +++ b/arch/arm64/include/asm/sdei.h
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright (C) 2017 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ASM_SDEI_H
> +#define __ASM_SDEI_H
> +
> +/* Values for sdei_exit_mode */
> +#define SDEI_EXIT_HVC  0
> +#define SDEI_EXIT_SMC  1
> +
> +#define SDEI_STACK_SIZE		IRQ_STACK_SIZE
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <linux/linkage.h>
> +#include <linux/types.h>
> +
> +#include <asm/virt.h>
> +
> +extern unsigned long sdei_exit_mode;
> +
> +/* Software Delegated Exception entry point from firmware*/
> +asmlinkage void __sdei_asm_handler(unsigned long event_num, unsigned long arg,
> +				   unsigned long pc, unsigned long pstate);
> +
> +/*
> + * The above entry point does the minimum to call C code. This function does
> + * anything else, before calling the driver.
> + */
> +struct sdei_registered_event;
> +asmlinkage unsigned long __sdei_handler(struct pt_regs *regs,
> +					struct sdei_registered_event *arg);
> +
> +extern unsigned long sdei_arch_get_entry_point(int conduit);

Nitpick: drop the "extern" here.

> diff --git a/arch/arm64/kernel/sdei-entry.S b/arch/arm64/kernel/sdei-entry.S
> new file mode 100644
> index 000000000000..cf12f8da0789
> --- /dev/null
> +++ b/arch/arm64/kernel/sdei-entry.S
> @@ -0,0 +1,122 @@
> +/*
> + * Software Delegated Exception entry point from firmware to the kernel.
> + *
> + * Copyright (C) 2017 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/linkage.h>
> +
> +#include <asm/alternative.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/assembler.h>
> +#include <asm/memory.h>
> +#include <asm/ptrace.h>
> +#include <asm/sdei.h>
> +#include <uapi/linux/sdei.h>
> +
> +/*
> + * Software Delegated Exception entry point.
> + *
> + * x0: Event number

Currently the only event number is 0. Shall we plan for having other
events in the future or we just ignore this argument?

> + * x1: struct sdei_registered_event argument from registration time.
> + * x2: interrupted PC
> + * x3: interrupted PSTATE
[...]
> +/*
> + * __sdei_handler() returns one of:
> + *  SDEI_EV_HANDLED -  success, return to the interrupted context.
> + *  SDEI_EV_FAILED  -  failure, return this error code to firmare.
> + *  virtual-address -  success, return to this address.
> + */
> +static __kprobes unsigned long _sdei_handler(struct pt_regs *regs,
> +					     struct sdei_registered_event *arg)
> +{
> +	u32 mode;
> +	int i, err = 0;
> +	int clobbered_registers = 4;

Maybe const int here if you want to avoid magic numbers in the 'for'
loop below. Otherwise it looks like some variable you intend to modify
in this function.

> +	u64 elr = read_sysreg(elr_el1);
> +	u32 kernel_mode = read_sysreg(CurrentEL) | 1;	/* +SPSel */
> +	unsigned long vbar = read_sysreg(vbar_el1);
> +
> +	/* Retrieve the missing registers values */
> +	for (i = 0; i < clobbered_registers; i++) {
> +		/* from within the handler, this call always succeeds */
> +		sdei_api_event_context(i, &regs->regs[i]);
> +	}
> +
> +	/*
> +	 * We didn't take an exception to get here, set PAN. UAO will be cleared
> +	 * by sdei_event_handler()s set_fs(USER_DS) call.
> +	 */
> +	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,
> +			CONFIG_ARM64_PAN));

Can you use uaccess_disable() directly?

> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index d8a9859f6102..1f6633b337aa 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -50,6 +50,7 @@ config ARM_SCPI_POWER_DOMAIN
>  
>  config ARM_SDE_INTERFACE
>  	bool "ARM Software Delegated Exception Interface (SDEI)"
> +	depends on ARM64
>  	help
>  	  The Software Delegated Exception Interface (SDEI) is an ARM
>  	  standard for registering callbacks from the platform firmware
> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index 4b3c7fd99c34..3ea6a19ae439 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -154,6 +154,7 @@ int sdei_api_event_context(u32 query, u64 *result)
>  	return invoke_sdei_fn(SDEI_1_0_FN_SDEI_EVENT_CONTEXT, query, 0, 0, 0, 0,
>  			      result);
>  }
> +NOKPROBE_SYMBOL(sdei_api_event_context);

Should this be part of the patch introducing arm_sdei.c?

>  
>  static int sdei_api_event_get_info(u32 event, u32 info, u64 *result)
>  {
> diff --git a/include/linux/sdei.h b/include/linux/sdei.h
> index bb3dd000771e..df3fe6373e32 100644
> --- a/include/linux/sdei.h
> +++ b/include/linux/sdei.h
> @@ -32,6 +32,8 @@ enum sdei_conduit_types {
>  	CONDUIT_HVC,
>  };
>  
> +#include <asm/sdei.h>
> +
>  /* Arch code should override this to set the entry point from firmware... */
>  #ifndef sdei_arch_get_entry_point
>  #define sdei_arch_get_entry_point(conduit)	(0)

Same here. This should not be built before the Kconfig change anyway.

-- 
Catalin