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