From patchwork Tue May 26 07:21:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gautham R Shenoy X-Patchwork-Id: 1297765 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49WQRg5Lnnz9sRK for ; Tue, 26 May 2020 17:23:27 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.vnet.ibm.com Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 49WQRg482TzDqMV for ; Tue, 26 May 2020 17:23:27 +1000 (AEST) X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.vnet.ibm.com (client-ip=148.163.158.5; helo=mx0a-001b2d01.pphosted.com; envelope-from=ego@linux.vnet.ibm.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=linux.vnet.ibm.com Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 49WQRR6mgZzDqLl; Tue, 26 May 2020 17:23:15 +1000 (AEST) Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 04Q733hT023433; Tue, 26 May 2020 03:23:12 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 318vckufn2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 26 May 2020 03:23:12 -0400 Received: from m0098420.ppops.net (m0098420.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 04Q75D6j034583; Tue, 26 May 2020 03:23:11 -0400 Received: from ppma04dal.us.ibm.com (7a.29.35a9.ip4.static.sl-reverse.com [169.53.41.122]) by mx0b-001b2d01.pphosted.com with ESMTP id 318vckufmu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 26 May 2020 03:23:11 -0400 Received: from pps.filterd (ppma04dal.us.ibm.com [127.0.0.1]) by ppma04dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 04Q7LBW4028696; Tue, 26 May 2020 07:23:11 GMT Received: from b03cxnp07029.gho.boulder.ibm.com (b03cxnp07029.gho.boulder.ibm.com [9.17.130.16]) by ppma04dal.us.ibm.com with ESMTP id 316uf9gsa9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 26 May 2020 07:23:11 +0000 Received: from b03ledav001.gho.boulder.ibm.com (b03ledav001.gho.boulder.ibm.com [9.17.130.232]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 04Q7M9hI47120714 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 26 May 2020 07:22:09 GMT Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 85B996E04C; Tue, 26 May 2020 07:22:09 +0000 (GMT) Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7AFC66E04E; Tue, 26 May 2020 07:22:07 +0000 (GMT) Received: from sofia.ibm.com (unknown [9.199.56.71]) by b03ledav001.gho.boulder.ibm.com (Postfix) with ESMTP; Tue, 26 May 2020 07:22:07 +0000 (GMT) Received: by sofia.ibm.com (Postfix, from userid 1000) id E035A2E48B5; Tue, 26 May 2020 12:52:02 +0530 (IST) From: "Gautham R. Shenoy" To: skiboot@lists.ozlabs.org, Vaidyanathan Srinivasan , "Oliver O'Halloran" , Vasant Hegde Date: Tue, 26 May 2020 12:51:50 +0530 Message-Id: <1590477710-24770-1-git-send-email-ego@linux.vnet.ibm.com> X-Mailer: git-send-email 1.8.3.1 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.216, 18.0.687 definitions=2020-05-25_12:2020-05-25, 2020-05-25 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxscore=0 suspectscore=0 phishscore=0 lowpriorityscore=0 mlxlogscore=999 adultscore=0 priorityscore=1501 spamscore=0 clxscore=1015 bulkscore=0 malwarescore=0 cotscore=-2147483648 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2004280000 definitions=main-2005260049 Subject: [Skiboot] [PATCH] occ: Fix false negatives in wait_for_all_occ_init() X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Gautham R. Shenoy" , skiboot-stable@lists.ozlabs.org MIME-Version: 1.0 Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" From: "Gautham R. Shenoy" Currently the wait_for_all_occ_init() function determines that the OCCs associated with every Chip has been initialized by verifying if the "Valid" bit in pstate table of that OCC is set. However, on chips where all the EX units are guarded, the OCC, even though it is active, does not update the pstate_table. Currently as a result of this, OPAL concludes that the OCC is not functional and not only disable Pstate initialization, but incorrectly report that that OCCs were not initialized, thereby cutting other features such as sensors. Fix this by ensuring that * We check if there is atleast one active EX unit in the chip before checking if the OCC is active. * On platforms with OCC-OPAL communication interface version 0x90 * wait_for_all_occ_init() only checks if the occ_state in the OCC dynamic area is set to "Active State". * move the "Valid" bit check to add_cpu_pstate_properties(), which is where we create the device-tree entries for frequency scaling. Cc: Vaidyanathan Srinivasan Tested-by: Pavaman Subramaniyam Signed-off-by: Gautham R. Shenoy --- core/cpu.c | 6 +- hw/occ.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++---------- include/chip.h | 3 + 3 files changed, 154 insertions(+), 32 deletions(-) diff --git a/core/cpu.c b/core/cpu.c index 37d9f41..1b38124 100644 --- a/core/cpu.c +++ b/core/cpu.c @@ -1171,10 +1171,12 @@ void init_all_cpus(void) chip_id = dt_get_chip_id(cpu); /* Only use operational CPUs */ - if (!strcmp(dt_prop_get(cpu, "status"), "okay")) + if (!strcmp(dt_prop_get(cpu, "status"), "okay")) { state = cpu_state_present; - else + get_chip(chip_id)->ex_present = true; + } else { state = cpu_state_unavailable; + } prlog(PR_INFO, "CPU: CPU from DT PIR=0x%04x Server#=0x%x" " State=%d\n", pir, server_no, state); diff --git a/hw/occ.c b/hw/occ.c index 76c7736..a1e4982 100644 --- a/hw/occ.c +++ b/hw/occ.c @@ -312,12 +312,17 @@ struct occ_dynamic_data *get_occ_dynamic_data(struct proc_chip *chip) OPAL_DYNAMIC_DATA_OFFSET); } -/* Check each chip's HOMER/Sapphire area for PState valid bit */ +/* + * On Chips which have at least one active EX unit, check the + * HOMER area for pstate-table valid bit on versions 0x1 and 0x2, or + * HOMER dynamic area occ_state on version 0x90. + */ static bool wait_for_all_occ_init(void) { struct proc_chip *chip; struct dt_node *xn; struct occ_pstate_table *occ_data; + struct occ_dynamic_data *occ_dyn_data; int tries; uint64_t start_time, end_time; uint32_t timeout = 0; @@ -327,6 +332,19 @@ static bool wait_for_all_occ_init(void) start_time = mftb(); for_each_chip(chip) { + u8 version; + + /* + * If the chip doesn't any EX unit present, then OCC + * will not update the pstate-table. So, skip the + * check. + */ + if (!chip->ex_present) { + prlog(PR_DEBUG, "OCC: Chip %02x has no active EX units. Skipping check\n", + chip->id); + continue; + } + /* Check for valid homer address */ if (!chip->homer_base) { /** @@ -347,27 +365,108 @@ static bool wait_for_all_occ_init(void) occ_data = get_occ_pstate_table(chip); /* - * Checking for occ_data->valid == 1 is ok because we clear all - * homer_base+size before passing memory to host services. - * This ensures occ_data->valid == 0 before OCC load + * Wait for the OCC to set an appropriate version bit. + * The wait is needed since on some platforms (such P8 + * Tuletta), OCC is not loaded before OPAL boot. Hence + * initialization can take a while. + * + * Note: Checking for occ_data->version == (0x01/0x02/0x90) + * is ok because we clear all of + * homer_base+size before passing memory to host + * services. This ensures occ_data->version == 0x0 + * before OCC load. */ tries = timeout * 10; - while((occ_data->valid != 1) && tries--) { + while (tries--) { + version = occ_data->version; + + if (version == 0x01 || version == 0x02 || + version == 0x90) + break; + time_wait_ms(100); } - if (occ_data->valid != 1) { - /** - * @fwts-label OCCInvalidPStateTable - * @fwts-advice The pstate table for a chip - * was not valid. This means that OCC (On Chip - * Controller) will be non-functional and CPU - * frequency scaling will not be functional. CPU may - * be set to a low, safe frequency. This means - * that CPU idle states and CPU frequency scaling - * may not be functional. + + version = occ_data->version; + switch (version) { + case 0x1: + case 0x2: + /* + * OCC-OPAL interface version 0x1 and 0x2 do not have + * the dynamic data. Hence the the only way to figure out + * if the OCC is up or not is to check the valid-bit + * in the pstate table. + */ + if (occ_data->valid != 1) { + /** + * @fwts-label OCCInvalidPStateTable + * @fwts-advice The pstate table for a chip + * was not valid. This means that OCC (On Chip + * Controller) will be non-functional and CPU + * frequency scaling will not be functional. CPU may + * be set to a low, safe frequency. This means + * that CPU idle states and CPU frequency scaling + * may not be functional. + */ + prlog(PR_ERR, "OCC: Chip: %x PState table is not valid\n", + chip->id); + return false; + } + break; + + case 0x90: + /* + * OCC-OPAL interface version 0x90 has a + * dynamic data section. This has an + * occ_state field whose values inform about + * the state of the OCC. + * + * 0x00 = OCC not running. No communication + * allowed. + * + * 0x01 = Standby. No communication allowed. + * + * 0x02 = Observation State. Communication + * allowed and is command dependent. + * + * 0x03 = Active State. Communication allowed + * and is command dependent. + * + * 0x04 = Safe State. No communication + * allowed. Just like CPU throttle + * status, some failures will not allow + * for OCC to update state to safe. + * + * 0x05 = Characterization State. + * Communication allowed and is command + * dependent. + * + * We will error out if OCC is not in the + * Active State. + * + * XXX : Should we error out only if no + * communication is allowed with the + * OCC ? */ - prlog(PR_ERR, "OCC: Chip: %x PState table is not valid\n", - chip->id); + occ_dyn_data = get_occ_dynamic_data(chip); + if (occ_dyn_data->occ_state != 0x3) { + /** + * @fwts-label OCCInactive + * @fwts-advice The OCC for a chip was not active. + * This means that CPU frequency scaling will + * not be functional. CPU may be set to a low, + * safe frequency. This means that CPU idle + * states and CPU frequency scaling may not be + * functional. + */ + prlog(PR_ERR, "OCC: Chip: %x: OCC not active\n", + chip->id); + return false; + } + break; + + default: + prlog(PR_ERR, "OCC: Unknown OCC-OPAL interface version.\n"); return false; } @@ -376,7 +475,15 @@ static bool wait_for_all_occ_init(void) prlog(PR_DEBUG, "OCC: Chip %02x Data (%016llx) = %016llx\n", chip->id, (uint64_t)occ_data, be64_to_cpu(*(__be64 *)occ_data)); + + if (version == 0x90) { + occ_dyn_data = get_occ_dynamic_data(chip); + prlog(PR_DEBUG, "OCC: Chip %02x Dynamic Data (%016llx) = %016llx\n", + chip->id, (uint64_t)occ_dyn_data, + be64_to_cpu(*(__be64 *)occ_dyn_data)); + } } + end_time = mftb(); prlog(PR_NOTICE, "OCC: All Chip Rdy after %lu ms\n", tb_to_msecs(end_time - start_time)); @@ -480,7 +587,7 @@ static bool add_cpu_pstate_properties(struct dt_node *power_mgt, { struct proc_chip *chip; uint64_t occ_data_area; - struct occ_pstate_table *occ_data; + struct occ_pstate_table *occ_data = NULL; /* Arrays for device tree */ __be32 *dt_id, *dt_freq; int pmax, pmin, pnom; @@ -490,24 +597,34 @@ static bool add_cpu_pstate_properties(struct dt_node *power_mgt, prlog(PR_DEBUG, "OCC: CPU pstate state device tree init\n"); - /* Find first chip */ - chip = next_chip(NULL); + /* + * Find first chip with an OCC which has as a valid + * pstate-table + */ + for_each_chip(chip) { + occ_data = get_occ_pstate_table(chip); - /* Extract PState information from OCC */ - occ_data = get_occ_pstate_table(chip); + /* Dump first 16 bytes of PState table */ + occ_data_area = (uint64_t)occ_data; + prlog(PR_DEBUG, "OCC: Chip %02d :Data (%16llx) = %16llx %16llx\n", + chip->id, occ_data_area, + be64_to_cpu(*(__be64 *)occ_data_area), + be64_to_cpu(*(__be64 *)(occ_data_area + 8))); - /* Dump first 16 bytes of PState table */ - occ_data_area = (uint64_t)occ_data; - prlog(PR_DEBUG, "OCC: Data (%16llx) = %16llx %16llx\n", - occ_data_area, - be64_to_cpu(*(__be64 *)occ_data_area), - be64_to_cpu(*(__be64 *)(occ_data_area + 8))); + if (occ_data->valid) + break; + /* + * XXX : Error out if !occ_data->valid but Chip has at + * least one EX Unit? + */ + } + assert(occ_data); if (!occ_data->valid) { /** * @fwts-label OCCInvalidPStateTableDT - * @fwts-advice The pstate table for the first chip - * was not valid. This means that OCC (On Chip + * @fwts-advice The pstate tables for none of the chips + * are valid. This means that OCC (On Chip * Controller) will be non-functional. This means * that CPU idle states and CPU frequency scaling * will not be functional as OPAL doesn't populate diff --git a/include/chip.h b/include/chip.h index f76c837..b79b63e 100644 --- a/include/chip.h +++ b/include/chip.h @@ -192,6 +192,9 @@ struct proc_chip { /* Used by hw/dio-p9.c */ struct p9_dio *dio; + + /* Used during OCC init */ + bool ex_present; }; extern uint32_t pir_to_chip_id(uint32_t pir);