[Skiboot] [PATCH 58/61] P10 Cleanup special wakeup and xive stop api usage
Nicholas Piggin
npiggin at gmail.com
Wed Jul 21 21:32:09 AEST 2021
Excerpts from Vasant Hegde's message of July 19, 2021 11:20 pm:
> From: Vaidyanathan Srinivasan <svaidy at linux.ibm.com>
>
> Cleanup P9 code pending implementation for P10.
>
> P10 stop-api integration will be needed for STOP11
> support only. STOP11 will be a restricted usage
> for testing core re-init on P10. Only stop0,2,3
> will be available for general usage.
>
> Also, do not treat gated core with SPW done as error.
> Core gating bit is a software state updated by microcode,
> while SPWU done bit comes from hardware logic to indicate
> successful operation. Print a warning if the status bits
> are out of sync, but no need to fail the special wakeup
> operation.
Hmm, I wonder if similar should be done with P9 code first,
and then some of the P10 updates merged into the P10 enable
patches.
>
> Signed-off-by: Vaidyanathan Srinivasan <svaidy at linux.ibm.com>
> Signed-off-by: Pratik R. Sampat <psampat at linux.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> ---
> core/direct-controls.c | 34 +++++++++++++++++++++++++++-------
> hw/slw.c | 30 +++++++++---------------------
> hw/xive2.c | 26 +-------------------------
> 3 files changed, 37 insertions(+), 53 deletions(-)
>
> diff --git a/core/direct-controls.c b/core/direct-controls.c
> index 879a537af..4795c19dc 100644
> --- a/core/direct-controls.c
> +++ b/core/direct-controls.c
> @@ -600,15 +600,35 @@ static int p10_core_set_special_wakeup(struct cpu_thread *cpu)
> * CORE_GATED will be unset on a successful special
> * wakeup of the core which indicates that the core is
> * out of stop state. If CORE_GATED is still set then
> - * raise error.
> + * check SPWU register and raise error only if SPWU_DONE
> + * is not set, else print a warning and consider SPWU
> + * operation as successful.
> */
> if (p10_core_is_gated(cpu)) {
> - /* Deassert spwu for this strange error */
> - xscom_write(chip_id, spwu_addr, 0);
> - prlog(PR_ERR, "Failed special wakeup on %u:%u"
> - " core remains gated.\n",
> - chip_id, core_id);
> - return OPAL_HARDWARE;
> + if(xscom_read(chip_id, spwu_addr, &val)) {
> + prlog(PR_ERR, "Core %u:%u:"
> + " unable to read QME_SPWU_HYP\n",
> + chip_id, core_id);
> + return OPAL_HARDWARE;
> + }
> + if (val & P10_SPWU_DONE) {
> + /*
> + * If SPWU DONE bit is set then
> + * SPWU operation is complete
> + */
> + prlog(PR_WARNING, "Special wakeup on "
> + "%u:%u: core remains gated while"
> + " SPWU_HYP DONE set\n",
> + chip_id, core_id);
> + return 0;
> + }
> + /* Deassert spwu for this strange error */
> + xscom_write(chip_id, spwu_addr, 0);
> + prlog(PR_ERR,
> + "Failed special wakeup on %u:%u"
> + " core remains gated.\n",
> + chip_id, core_id);
> + return OPAL_HARDWARE;
Should the equivalent be done for P9 here?
> } else {
> return 0;
> }
> diff --git a/hw/slw.c b/hw/slw.c
> index e22d1bdde..52536db06 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -228,32 +228,20 @@ static bool slw_set_overrides_p10(struct proc_chip *chip, struct cpu_thread *c)
> int rc;
> uint32_t core = pir_to_core_id(c->pir);
>
> - /* Clear special wakeup bits that could hold power mgt */
> - rc = xscom_write(chip->id,
> - XSCOM_ADDR_P10_QME_CORE(core, P10_QME_SPWU_HYP),
> - 0);
> - if (rc) {
> - log_simple_error(&e_info(OPAL_RC_SLW_SET),
> - "SLW: Failed to write P10_QME_SPWU_HYP\n");
> - return false;
> - }
> - /* Read back for debug */
> + /* Special wakeup bits that could hold power mgt */
> rc = xscom_read(chip->id,
> XSCOM_ADDR_P10_QME_CORE(core, P10_QME_SPWU_HYP),
> &tmp);
> - if (tmp)
> + if (rc) {
> + log_simple_error(&e_info(OPAL_RC_SLW_SET),
> + "SLW: Failed to read P10_QME_SPWU_HYP\n");
> + return false;
> + }
Ditto here -- should the p9 code be changed? I wonder if the special
wakeup check should be made in direct controls init code rather than
here.
> + if (tmp & P10_SPWU_REQ)
> prlog(PR_WARNING,
> - "SLW: core %d P10_QME_SPWU_HYP read 0x%016llx\n",
> - core, tmp);
> -#if 0
> - rc = xscom_read(chip->id,
> - XSCOM_ADDR_P10_QME_CORE(core, P10_QME_SPWU_OTR),
> - &tmp);
> - if (tmp)
> - prlog(PR_WARNING,
> - "SLW: core %d P10_QME_SPWU_OTR read 0x%016llx\n",
> + "SLW: core %d P10_QME_SPWU_HYP requested 0x%016llx\n",
> core, tmp);
> -#endif
> +
> return true;
> }
>
> diff --git a/hw/xive2.c b/hw/xive2.c
> index b79635cc9..f559b0f9a 100644
> --- a/hw/xive2.c
> +++ b/hw/xive2.c
> @@ -3022,34 +3022,10 @@ static void xive_configure_ex_special_bar(struct xive *x, struct cpu_thread *c)
>
> void xive2_late_init(void)
> {
> - struct cpu_thread *c;
> -
> prlog(PR_INFO, "SLW: Configuring self-restore for NCU_SPEC_BAR\n");
> - for_each_present_cpu(c) {
> - if(cpu_is_thread0(c)) {
> - struct proc_chip *chip = get_chip(c->chip_id);
> - struct xive *x = chip->xive;
> - uint64_t xa, val, rc;
> - xa = XSCOM_ADDR_P10_NCU(pir_to_core_id(c->pir), P10_NCU_SPEC_BAR);
> - val = (uint64_t)x->tm_base | P10_NCU_SPEC_BAR_ENABLE;
> - /* Bail out if wakeup engine has already failed */
> - if ( wakeup_engine_state != WAKEUP_ENGINE_PRESENT) {
> - prlog(PR_ERR, "XIVE p9_stop_api fail detected\n");
> - break;
> - }
> /*
> - * TODO (p10): need P10 stop state engine
> + * TODO (p10): need P10 stop state engine and fix for STOP11
> */
> - rc = p9_stop_save_scom((void *)chip->homer_base, xa, val,
> - P9_STOP_SCOM_REPLACE, P9_STOP_SECTION_EQ_SCOM);
> - if (rc) {
> - xive_cpu_err(c, "p9_stop_api failed for NCU_SPEC_BAR rc=%lld\n",
> - rc);
> - wakeup_engine_state = WAKEUP_ENGINE_FAILED;
> - }
> - }
> - }
This should probably be merged withthe xive2 code that added it.
Thanks,
Nick
More information about the Skiboot
mailing list