Hi Thor,
(CC: +Mark, Lorenzo and Sudeep for PSCI.
How should SYSTEM_RESET2 be used for a vendor-specific reset?
The original patch is:
lore.kernel.org/r/1559594269-10077-1-git-send-email-thor.tha...@linux.intel.com
)
On 03/06/2019 21:37, thor.tha...@linux.intel.com wrote:
> From: Thor Thayer <thor.tha...@linux.intel.com>
>
> The Stratix10 peripheral FIFO memories can recover from double
> bit errors with a warm reset instead of a cold reset.
> Add the option of a warm reset for peripheral (USB, Ethernet)
> memories.
>
> CPU memories such as SDRAM and OCRAM require a cold reset for
> DBEs.
> Filter on whether the error is a SDRAM/OCRAM or a peripheral
> FIFO memory to determine which reset to use when the warm
> reset option is configured.
... so you want to make different SMC calls on each CPU after panic()?
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index 8816f74a22b4..179601f14b48 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -2036,6 +2036,19 @@ static const struct irq_domain_ops a10_eccmgr_ic_ops =
> {
> /* panic routine issues reboot on non-zero panic_timeout */
> extern int panic_timeout;
>
> +#ifdef CONFIG_EDAC_ALTERA_ARM64_WARM_RESET
> +/* EL3 SMC call to setup CPUs for warm reset */
> +void panic_smp_self_stop(void)
> +{
> + struct arm_smccc_res result;
> +
> + __cpu_disable();
> + cpu_relax();
> + arm_smccc_smc(INTEL_SIP_SMC_ECC_DBE, S10_WARM_RESET_WFI_FLAG,
> + S10_WARM_RESET_WFI_FLAG, 0, 0, 0, 0, 0, &result);
> +}
> +#endif
Oooer!
panic_smp_self_stop() isn't for drivers to override: only the arch code.
__cpu_disable() is only for the cpu-hotplug machinery. Nothing else should
touch it.
Isn't this thing only called if another CPU out there is panic()ing too?
I think one of the problems here is arm64 leaves secondary CPUs running after
panic().
This would be better fixed by using the appropriate cpu_ops[]->cpu_die() call
in arm64's
ipi_cpu_stop().
As for passing platform-specific options, PSCI[0] has a 'reset_type' for
SYSTEM_RESET2,
which looks suspiciously like what you want here. I'm not sure how its expected
to be
used... hopefully the PSCI maintainers can give us some pointers.
(The existing support is commit 4302e381a870 ("firmware/psci: add support for
SYSTEM_RESET2"))
Is it possible for firmware to do both the cold/warm reset work when
SYSTEM_RESET is
called? This would mean you don't have to care here and there are fewer choices
to be made
overall.
If not, is there anything left behind that can give it the hint? Like non-zero
error
counters for the USB/Ethernet devices?
> @@ -2067,14 +2080,28 @@ static int s10_edac_dberr_handler(struct
> notifier_block *this,
> regmap_write(edac->ecc_mgr_map,
> S10_SYSMGR_UE_ADDR_OFST, err_addr);
> edac_printk(KERN_ERR, EDAC_DEVICE,
> - "EDAC: [Fatal DBE on %s @ 0x%08X]\n",
> - ed->edac_dev_name, err_addr);
> + "EDAC: [Fatal DBE on %s [CPU=%d] @
> 0x%08X]\n",
> + ed->edac_dev_name, raw_smp_processor_id(),
> + err_addr);
> break;
> }
> /* Notify the System through SMC. Reboot delay = 1 second */
> +#ifdef CONFIG_EDAC_ALTERA_ARM64_WARM_RESET
> + /* Handle peripheral FIFO DBE as Warm Resets */
> + if (dberror & S10_COLD_RESET_MASK) {
> + panic_timeout = 1;
Isn't this value supposed to be provided on the kernel commandline? Surely this
prevents
debug using the commandline option to increase the delay?
(I see you already change it)
> + arm_smccc_smc(INTEL_SIP_SMC_ECC_DBE, dberror, 0, 0, 0,
> + 0, 0, 0, &result);
> + } else {
> + arm_smccc_smc(INTEL_SIP_SMC_ECC_DBE,
> + S10_WARM_RESET_WFI_FLAG | dberror, 0, 0,
> + 0, 0, 0, 0, &result);
> + }
> +#else
> panic_timeout = 1;
> arm_smccc_smc(INTEL_SIP_SMC_ECC_DBE, dberror, 0, 0, 0, 0,
> 0, 0, &result);
> +#endif
> }
>
> return NOTIFY_DONE;
What do these SMC do? Are they equivalent to the PSCI CPU online/offline calls?
panic() notifiers aren't robust as they can be skipped if kdump is loaded.
Thanks,
James
[0]
https://static.docs.arm.com/den0022/d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf
