mbox series

[v6,00/16] arm64/firmware: Software Delegated Exception Interface

Message ID 20180108153818.22743-1-james.morse@arm.com
Headers show
Series arm64/firmware: Software Delegated Exception Interface | expand

Message

James Morse Jan. 8, 2018, 3:38 p.m. UTC
This series is rebased onto arm64's kpti-base tag, with v2 of Will's kpti
series[0]. The changes were only due to code moving around, but its now
all been tested together. As before patches 15 and 16 contain the extra
kpti trampoline stuff.

The Software Delegated Exception Interface (SDEI) is an ARM specification
for registering callbacks from the platform firmware into the OS.
This is intended to be used to implement firmware-first RAS notifications,
but also supports vendor-defined events and binding IRQs as events.

The document is here:
http://infocenter.arm.com/help/topic/com.arm.doc.den0054a/ARM_DEN0054A_Software_
Delegated_Exception_Interface.pdf

I anticipate once reviewed this series will go via the arm64 tree as it has
an ever closer interaction with the arch code. Despite the bulk of the code
being under /drivers/firmware/.

The relevant change in v5 was a three-way lockdep issue via the hotplug
state machine. (I argue it can't happen, but lockdep doesn't care.) Now the
hotplug callbacks save nothing, and restore the OS-view of registered/enabled.
This makes bound-interrupts harder to work with. I dropped Catalin's ack on
all affected patches.

For your regular boiler-plate cover-letter, please see [1].

Thanks,

James

[0] https://www.spinics.net/lists/arm-kernel/msg626876.html
[1] https://patchwork.ozlabs.org/cover/845290/


James Morse (16):
  KVM: arm64: Store vcpu on the stack during __guest_enter()
  KVM: arm/arm64: Convert kvm_host_cpu_state to a static per-cpu
    allocation
  KVM: arm64: Change hyp_panic()s dependency on tpidr_el2
  arm64: alternatives: use tpidr_el2 on VHE hosts
  KVM: arm64: Stop save/restoring host tpidr_el1 on VHE
  Docs: dt: add devicetree binding for describing arm64 SDEI firmware
  firmware: arm_sdei: Add driver for Software Delegated Exceptions
  arm64: Add vmap_stack header file
  arm64: uaccess: Add PAN helper
  arm64: kernel: Add arch-specific SDEI entry code and CPU masking
  firmware: arm_sdei: Add support for CPU and system power states
  firmware: arm_sdei: add support for CPU private events
  arm64: acpi: Remove __init from acpi_psci_use_hvc() for use by SDEI
  firmware: arm_sdei: Discover SDEI support via ACPI
  arm64: mmu: add the entry trampolines start/end section markers into
    sections.h
  arm64: sdei: Add trampoline code for remapping the kernel

 .../devicetree/bindings/arm/firmware/sdei.txt      |   42 +
 MAINTAINERS                                        |    9 +
 arch/arm64/include/asm/alternative.h               |    2 +
 arch/arm64/include/asm/assembler.h                 |    8 +
 arch/arm64/include/asm/kvm_host.h                  |    2 +
 arch/arm64/include/asm/mmu.h                       |    3 +-
 arch/arm64/include/asm/percpu.h                    |   11 +-
 arch/arm64/include/asm/sdei.h                      |   57 +
 arch/arm64/include/asm/sections.h                  |    1 +
 arch/arm64/include/asm/stacktrace.h                |    3 +
 arch/arm64/include/asm/uaccess.h                   |   12 +
 arch/arm64/include/asm/vmap_stack.h                |   28 +
 arch/arm64/kernel/Makefile                         |    1 +
 arch/arm64/kernel/acpi.c                           |    2 +-
 arch/arm64/kernel/alternative.c                    |    9 +-
 arch/arm64/kernel/asm-offsets.c                    |    5 +
 arch/arm64/kernel/cpufeature.c                     |   17 +
 arch/arm64/kernel/entry.S                          |  177 ++++
 arch/arm64/kernel/irq.c                            |   13 +-
 arch/arm64/kernel/sdei.c                           |  235 +++++
 arch/arm64/kernel/smp.c                            |   11 +-
 arch/arm64/kernel/suspend.c                        |    4 +-
 arch/arm64/kvm/hyp-init.S                          |    4 +
 arch/arm64/kvm/hyp/entry.S                         |   10 +-
 arch/arm64/kvm/hyp/hyp-entry.S                     |   18 +-
 arch/arm64/kvm/hyp/switch.c                        |   25 +-
 arch/arm64/kvm/hyp/sysreg-sr.c                     |   16 +-
 arch/arm64/mm/mmu.c                                |    2 -
 arch/arm64/mm/proc.S                               |    8 +
 drivers/firmware/Kconfig                           |    8 +
 drivers/firmware/Makefile                          |    1 +
 drivers/firmware/arm_sdei.c                        | 1091 ++++++++++++++++++++
 include/linux/arm_sdei.h                           |   79 ++
 include/linux/cpuhotplug.h                         |    1 +
 include/linux/psci.h                               |    3 +-
 include/uapi/linux/arm_sdei.h                      |   73 ++
 virt/kvm/arm/arm.c                                 |   18 +-
 37 files changed, 1943 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/sdei.txt
 create mode 100644 arch/arm64/include/asm/sdei.h
 create mode 100644 arch/arm64/include/asm/vmap_stack.h
 create mode 100644 arch/arm64/kernel/sdei.c
 create mode 100644 drivers/firmware/arm_sdei.c
 create mode 100644 include/linux/arm_sdei.h
 create mode 100644 include/uapi/linux/arm_sdei.h

Comments

Lorenzo Pieralisi Jan. 8, 2018, 3:51 p.m. UTC | #1
On Mon, Jan 08, 2018 at 03:38:15PM +0000, James Morse wrote:
> SDEI inherits the 'use hvc' bit that is also used by PSCI. PSCI does all
> its initialisation early, SDEI does its late.
> 
> Remove the __init annotation from acpi_psci_use_hvc().
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> The function name is unchanged as this bit is named 'PSCI_USE_HVC'
> in table 5-37 of ACPIv6.2.
> 
>  arch/arm64/kernel/acpi.c | 2 +-
>  include/linux/psci.h     | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..252396a96c78 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -117,7 +117,7 @@ bool __init acpi_psci_present(void)
>  }
>  
>  /* Whether HVC must be used instead of SMC as the PSCI conduit */
> -bool __init acpi_psci_use_hvc(void)
> +bool acpi_psci_use_hvc(void)
>  {
>  	return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
>  }
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index 6306ab10af18..f724fd8c78e8 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -47,10 +47,11 @@ static inline int psci_dt_init(void) { return 0; }
>  #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)
>  int __init psci_acpi_init(void);
>  bool __init acpi_psci_present(void);
> -bool __init acpi_psci_use_hvc(void);
> +bool acpi_psci_use_hvc(void);
>  #else
>  static inline int psci_acpi_init(void) { return 0; }
>  static inline bool acpi_psci_present(void) { return false; }
> +static inline bool acpi_psci_use_hvc(void) {return false; }
>  #endif
>  
>  #endif /* __LINUX_PSCI_H */
> -- 
> 2.15.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi Jan. 8, 2018, 5:22 p.m. UTC | #2
On Mon, Jan 08, 2018 at 03:38:13PM +0000, James Morse wrote:

[...]

> +/* When entering idle, mask/unmask events for this cpu */

Just mentioning (since I know you know), this notifier is called also
through syscore_ops so it is not just idle (CPUidle or Suspend-to-idle).

> +static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
> +			    void *data)
> +{
> +	int rv;
> +
> +	switch (action) {
> +	case CPU_PM_ENTER:
> +		rv = sdei_mask_local_cpu();
> +		break;
> +	case CPU_PM_EXIT:

You should handle CPU_PM_ENTER_FAILED here for correctness,
in case the notifier chain fails.

Other than that:

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> +		rv = sdei_unmask_local_cpu();
> +		break;
> +	default:
> +		return NOTIFY_DONE;
> +	}
> +
> +	if (rv)
> +		return notifier_from_errno(rv);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block sdei_pm_nb = {
> +	.notifier_call = sdei_pm_notifier,
> +};
> +
> +static int sdei_device_suspend(struct device *dev)
> +{
> +	on_each_cpu(_ipi_mask_cpu, NULL, true);
> +
> +	return 0;
> +}
> +
> +static int sdei_device_resume(struct device *dev)
> +{
> +	on_each_cpu(_ipi_unmask_cpu, NULL, true);
> +
> +	return 0;
> +}
> +
> +/*
> + * We need all events to be reregistered when we resume from hibernate.
> + *
> + * The sequence is freeze->thaw. Reboot. freeze->restore. We unregister
> + * events during freeze, then re-register and re-enable them during thaw
> + * and restore.
> + */
> +static int sdei_device_freeze(struct device *dev)
> +{
> +	int err;
> +
> +	cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING);
> +
> +	err = sdei_unregister_shared();
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int sdei_device_thaw(struct device *dev)
> +{
> +	int err;
> +
> +	/* re-register shared events */
> +	err = sdei_reregister_shared();
> +	if (err) {
> +		pr_warn("Failed to re-register shared events...\n");
> +		sdei_mark_interface_broken();
> +		return err;
> +	}
> +
> +	err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI",
> +				&sdei_cpuhp_up, &sdei_cpuhp_down);
> +	if (err)
> +		pr_warn("Failed to re-register CPU hotplug notifier...\n");
> +
> +	return err;
> +}
> +
> +static int sdei_device_restore(struct device *dev)
> +{
> +	int err;
> +
> +	err = sdei_platform_reset();
> +	if (err)
> +		return err;
> +
> +	return sdei_device_thaw(dev);
> +}
> +
> +static const struct dev_pm_ops sdei_pm_ops = {
> +	.suspend = sdei_device_suspend,
> +	.resume = sdei_device_resume,
> +	.freeze = sdei_device_freeze,
> +	.thaw = sdei_device_thaw,
> +	.restore = sdei_device_restore,
> +};
> +
> +/*
> + * Mask all CPUs and unregister all events on panic, reboot or kexec.
> + */
> +static int sdei_reboot_notifier(struct notifier_block *nb, unsigned long action,
> +				void *data)
> +{
> +	/*
> +	 * We are going to reset the interface, after this there is no point
> +	 * doing work when we take CPUs offline.
> +	 */
> +	cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING);
> +
> +	sdei_platform_reset();
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block sdei_reboot_nb = {
> +	.notifier_call = sdei_reboot_notifier,
> +};
> +
>  static void sdei_smccc_smc(unsigned long function_id,
>  			   unsigned long arg0, unsigned long arg1,
>  			   unsigned long arg2, unsigned long arg3,
> @@ -547,9 +772,36 @@ static int sdei_probe(struct platform_device *pdev)
>  		return 0;
>  	}
>  
> -	on_each_cpu(&_ipi_unmask_cpu, NULL, false);
> +	err = cpu_pm_register_notifier(&sdei_pm_nb);
> +	if (err) {
> +		pr_warn("Failed to register CPU PM notifier...\n");
> +		goto error;
> +	}
> +
> +	err = register_reboot_notifier(&sdei_reboot_nb);
> +	if (err) {
> +		pr_warn("Failed to register reboot notifier...\n");
> +		goto remove_cpupm;
> +	}
> +
> +	err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI",
> +				&sdei_cpuhp_up, &sdei_cpuhp_down);
> +	if (err) {
> +		pr_warn("Failed to register CPU hotplug notifier...\n");
> +		goto remove_reboot;
> +	}
>  
>  	return 0;
> +
> +remove_reboot:
> +	unregister_reboot_notifier(&sdei_reboot_nb);
> +
> +remove_cpupm:
> +	cpu_pm_unregister_notifier(&sdei_pm_nb);
> +
> +error:
> +	sdei_mark_interface_broken();
> +	return err;
>  }
>  
>  static const struct of_device_id sdei_of_match[] = {
> @@ -560,6 +812,7 @@ static const struct of_device_id sdei_of_match[] = {
>  static struct platform_driver sdei_driver = {
>  	.driver		= {
>  		.name			= "sdei",
> +		.pm			= &sdei_pm_ops,
>  		.of_match_table		= sdei_of_match,
>  	},
>  	.probe		= sdei_probe,
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 201ab7267986..87b505a48a94 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -109,6 +109,7 @@ enum cpuhp_state {
>  	CPUHP_AP_PERF_XTENSA_STARTING,
>  	CPUHP_AP_PERF_METAG_STARTING,
>  	CPUHP_AP_MIPS_OP_LOONGSON3_STARTING,
> +	CPUHP_AP_ARM_SDEI_STARTING,
>  	CPUHP_AP_ARM_VFP_STARTING,
>  	CPUHP_AP_ARM64_DEBUG_MONITORS_STARTING,
>  	CPUHP_AP_PERF_ARM_HW_BREAKPOINT_STARTING,
> -- 
> 2.15.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi Jan. 8, 2018, 5:56 p.m. UTC | #3
On Mon, Jan 08, 2018 at 03:38:16PM +0000, James Morse wrote:
> SDEI defines a new ACPI table to indicate the presence of the interface.
> The conduit is discovered in the same way as PSCI.
> 
> For ACPI we need to create the platform device ourselves as SDEI doesn't
> have an entry in the DSDT.
> 
> The SDEI platform device should be created after ACPI has been initialised
> so that we can parse the table, but before GHES devices are created, which
> may register SDE events if they use SDEI as their notification type.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  drivers/firmware/arm_sdei.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index 10a8bfa7339a..fb7caa3628b9 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -907,6 +907,14 @@ static int sdei_get_conduit(struct platform_device *pdev)
>  		}
>  
>  		pr_warn("invalid \"method\" property: %s\n", method);
> +	} else if (IS_ENABLED(CONFIG_ACPI) && !acpi_disabled) {
> +		if (acpi_psci_use_hvc()) {
> +			sdei_firmware_call = &sdei_smccc_hvc;
> +			return CONDUIT_HVC;
> +		} else {
> +			sdei_firmware_call = &sdei_smccc_smc;
> +			return CONDUIT_SMC;
> +		}
>  	}
>  
>  	return CONDUIT_INVALID;
> @@ -1020,14 +1028,45 @@ static bool __init sdei_present_dt(void)
>  	return true;
>  }
>  
> +static bool __init sdei_present_acpi(void)
> +{
> +	acpi_status status;
> +	struct platform_device *pdev;
> +	struct acpi_table_header *sdei_table_header;
> +
> +	if (acpi_disabled)
> +		return false;
> +
> +	status = acpi_get_table(ACPI_SIG_SDEI, 0, &sdei_table_header);
> +	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_info("Failed to get ACPI:SDEI table, %s\n", msg);
> +	}
> +	if (ACPI_FAILURE(status))
> +		return false;
> +
> +	pdev = platform_device_register_simple(sdei_driver.driver.name, 0, NULL,
> +					       0);
> +	if (IS_ERR(pdev))
> +		return false;
> +
> +	return true;
> +}
> +
>  static int __init sdei_init(void)
>  {
> -	if (sdei_present_dt())
> +	if (sdei_present_dt() || sdei_present_acpi())
>  		platform_driver_register(&sdei_driver);
>  
>  	return 0;
>  }
>  
> +/*
> + * On an ACPI system SDEI needs to be ready before HEST:GHES tries to register
> + * its events. ACPI is initialised from a subsys_initcall(), GHES is initialised
> + * by device_initcall(). We want to be called in the middle.
> + */
>  subsys_initcall_sync(sdei_init);
>  
>  int sdei_event_handler(struct pt_regs *regs,
> -- 
> 2.15.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Catalin Marinas Jan. 13, 2018, noon UTC | #4
On Mon, Jan 08, 2018 at 05:22:26PM +0000, Lorenzo Pieralisi wrote:
> On Mon, Jan 08, 2018 at 03:38:13PM +0000, James Morse wrote:
> 
> [...]
> 
> > +/* When entering idle, mask/unmask events for this cpu */
> 
> Just mentioning (since I know you know), this notifier is called also
> through syscore_ops so it is not just idle (CPUidle or Suspend-to-idle).
> 
> > +static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
> > +			    void *data)
> > +{
> > +	int rv;
> > +
> > +	switch (action) {
> > +	case CPU_PM_ENTER:
> > +		rv = sdei_mask_local_cpu();
> > +		break;
> > +	case CPU_PM_EXIT:
> > +		rv = sdei_unmask_local_cpu();
> 
> You should handle CPU_PM_ENTER_FAILED here for correctness,
> in case the notifier chain fails.

So, just to confirm, the CPU_PM_ENTER_FAILED case goes together with
CPU_PM_EXIT so that we unmask the SDE again for the CPU.
Lorenzo Pieralisi Jan. 14, 2018, 12:20 p.m. UTC | #5
On Sat, Jan 13, 2018 at 12:00:31PM +0000, Catalin Marinas wrote:
> On Mon, Jan 08, 2018 at 05:22:26PM +0000, Lorenzo Pieralisi wrote:
> > On Mon, Jan 08, 2018 at 03:38:13PM +0000, James Morse wrote:
> > 
> > [...]
> > 
> > > +/* When entering idle, mask/unmask events for this cpu */
> > 
> > Just mentioning (since I know you know), this notifier is called also
> > through syscore_ops so it is not just idle (CPUidle or Suspend-to-idle).
> > 
> > > +static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
> > > +			    void *data)
> > > +{
> > > +	int rv;
> > > +
> > > +	switch (action) {
> > > +	case CPU_PM_ENTER:
> > > +		rv = sdei_mask_local_cpu();
> > > +		break;
> > > +	case CPU_PM_EXIT:
> > > +		rv = sdei_unmask_local_cpu();
> > 
> > You should handle CPU_PM_ENTER_FAILED here for correctness,
> > in case the notifier chain fails.
> 
> So, just to confirm, the CPU_PM_ENTER_FAILED case goes together with
> CPU_PM_EXIT so that we unmask the SDE again for the CPU.

Yes, that's correct (I do not think that's a problem with current
mainline but that ought to be handled correctly anyway).

Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Morse Jan. 17, 2018, 12:01 p.m. UTC | #6
Hi Lorenzo, Catalin,

On 14/01/18 12:20, Lorenzo Pieralisi wrote:
> On Sat, Jan 13, 2018 at 12:00:31PM +0000, Catalin Marinas wrote:
>> On Mon, Jan 08, 2018 at 05:22:26PM +0000, Lorenzo Pieralisi wrote:
>>> On Mon, Jan 08, 2018 at 03:38:13PM +0000, James Morse wrote:
>>>> +/* When entering idle, mask/unmask events for this cpu */
>>>
>>> Just mentioning (since I know you know), this notifier is called also
>>> through syscore_ops so it is not just idle (CPUidle or Suspend-to-idle).
>>>
>>>> +static int sdei_pm_notifier(struct notifier_block *nb, unsigned long action,
>>>> +			    void *data)
>>>> +{
>>>> +	int rv;
>>>> +
>>>> +	switch (action) {
>>>> +	case CPU_PM_ENTER:
>>>> +		rv = sdei_mask_local_cpu();
>>>> +		break;
>>>> +	case CPU_PM_EXIT:
>>>> +		rv = sdei_unmask_local_cpu();
>>>
>>> You should handle CPU_PM_ENTER_FAILED here for correctness,
>>> in case the notifier chain fails.

(not seen this before, I mirrored what KVM does here ... which may have the same
bug)


>> So, just to confirm, the CPU_PM_ENTER_FAILED case goes together with
>> CPU_PM_EXIT so that we unmask the SDE again for the CPU.
> 
> Yes, that's correct (I do not think that's a problem with current
> mainline but that ought to be handled correctly anyway).

I'll send a fix..,
... oh wait Catalin already did this.


Thanks!

James
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html