lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200720060900.GC4000@drishya.in.ibm.com>
Date:   Mon, 20 Jul 2020 11:39:00 +0530
From:   Vaidyanathan Srinivasan <svaidy@...ux.ibm.com>
To:     "Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>
Cc:     Nicholas Piggin <npiggin@...il.com>,
        Anton Blanchard <anton@...abs.org>,
        Nathan Lynch <nathanl@...ux.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Michael Neuling <mikey@...ling.org>, linuxppc-dev@...abs.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 2/5] cpuidle-pseries: Add function to parse extended CEDE
 records

* Gautham R Shenoy <ego@...ux.vnet.ibm.com> [2020-07-07 16:41:36]:

> From: "Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>
> 
> Currently we use CEDE with latency-hint 0 as the only other idle state
> on a dedicated LPAR apart from the polling "snooze" state.
> 
> The platform might support additional extended CEDE idle states, which
> can be discovered through the "ibm,get-system-parameter" rtas-call
> made with CEDE_LATENCY_TOKEN.
> 
> This patch adds a function to obtain information about the extended
> CEDE idle states from the platform and parse the contents to populate
> an array of extended CEDE states. These idle states thus discovered
> will be added to the cpuidle framework in the next patch.
> 
> dmesg on a POWER9 LPAR, demonstrating the output of parsing the
> extended CEDE latency parameters.
> 
> [    5.913180] xcede : xcede_record_size = 10
> [    5.913183] xcede : Record 0 : hint = 1, latency =0x400 tb-ticks, Wake-on-irq = 1
> [    5.913188] xcede : Record 1 : hint = 2, latency =0x3e8000 tb-ticks, Wake-on-irq = 0
> [    5.913193] cpuidle : Skipping the 2 Extended CEDE idle states
> 
> Signed-off-by: Gautham R. Shenoy <ego@...ux.vnet.ibm.com>

Reviewed-by: Vaidyanathan Srinivasan <svaidy@...ux.ibm.com>

>
> ---
>  drivers/cpuidle/cpuidle-pseries.c | 129 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 127 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
> index 39d4bb6..c13549b 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -21,6 +21,7 @@
>  #include <asm/runlatch.h>
>  #include <asm/idle.h>
>  #include <asm/plpar_wrappers.h>
> +#include <asm/rtas.h>
> 
>  struct cpuidle_driver pseries_idle_driver = {
>  	.name             = "pseries_idle",
> @@ -105,9 +106,120 @@ static void check_and_cede_processor(void)
>  	}
>  }
> 
> -#define NR_CEDE_STATES		1  /* CEDE with latency-hint 0 */
> +struct xcede_latency_records {
> +	u8  latency_hint;
> +	u64 wakeup_latency_tb_ticks;
> +	u8  responsive_to_irqs;
> +};
> +
> +/*
> + * XCEDE : Extended CEDE states discovered through the
> + *         "ibm,get-systems-parameter" rtas-call with the token
> + *         CEDE_LATENCY_TOKEN
> + */
> +#define MAX_XCEDE_STATES		4
> +#define	XCEDE_LATENCY_RECORD_SIZE	10
> +#define XCEDE_LATENCY_PARAM_MAX_LENGTH	(2 + 2 + \
> +					(MAX_XCEDE_STATES * XCEDE_LATENCY_RECORD_SIZE))
> +
> +#define CEDE_LATENCY_TOKEN		45
> +
> +#define NR_CEDE_STATES		(MAX_XCEDE_STATES + 1) /* CEDE with latency-hint 0 */
>  #define NR_DEDICATED_STATES	(NR_CEDE_STATES + 1) /* Includes snooze */
> 
> +struct xcede_latency_records xcede_records[MAX_XCEDE_STATES];
> +unsigned int nr_xcede_records;
> +char xcede_parameters[XCEDE_LATENCY_PARAM_MAX_LENGTH];
> +
> +static int parse_cede_parameters(void)
> +{
> +	int ret = -1, i;
> +	u16 payload_length;
> +	u8 xcede_record_size;
> +	u32 total_xcede_records_size;
> +	char *payload;
> +
> +	memset(xcede_parameters, 0, XCEDE_LATENCY_PARAM_MAX_LENGTH);
> +
> +	ret = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
> +			NULL, CEDE_LATENCY_TOKEN, __pa(xcede_parameters),
> +			XCEDE_LATENCY_PARAM_MAX_LENGTH);
> +
> +	if (ret) {
> +		pr_err("xcede: Error parsing CEDE_LATENCY_TOKEN\n");
> +		return ret;
> +	}
> +
> +	payload_length = be16_to_cpu(*(__be16 *)(&xcede_parameters[0]));
> +	payload = &xcede_parameters[2];
> +
> +	/*
> +	 * If the platform supports the cede latency settings
> +	 * information system parameter it must provide the following
> +	 * information in the NULL terminated parameter string:
> +	 *
> +	 * a. The first byte is the length ???N??? of each cede
> +	 *    latency setting record minus one (zero indicates a length
> +	 *    of 1 byte).
> +	 *
> +	 * b. For each supported cede latency setting a cede latency
> +	 *    setting record consisting of the first ???N??? bytes as per
> +	 *    the following table.
> +	 *
> +	 *	-----------------------------
> +	 *	| Field           | Field  |
> +	 *	| Name            | Length |
> +	 *	-----------------------------
> +	 *	| Cede Latency    | 1 Byte |
> +	 *	| Specifier Value |        |
> +	 *	-----------------------------
> +	 *	| Maximum wakeup  |        |
> +	 *	| latency in      | 8 Bytes|
> +	 *	| tb-ticks        |        |
> +	 *	-----------------------------
> +	 *	| Responsive to   |        |
> +	 *	| external        | 1 Byte |
> +	 *	| interrupts      |        |
> +	 *	-----------------------------
> +	 *
> +	 * This version has cede latency record size = 10.
> +	 */
> +	xcede_record_size = (u8)payload[0] + 1;

This is standard PAPR interface that has been defined long time ago.
However, new H_CEDE hints that map to new platform features will
appear in the same interface and Linux needs to prepare and be ready
to check and exploit the new hints if they are useful for the given
setup.


> +
> +	if (xcede_record_size != XCEDE_LATENCY_RECORD_SIZE) {
> +		pr_err("xcede : Expected record-size %d. Observed size %d.\n",
> +		       XCEDE_LATENCY_RECORD_SIZE, xcede_record_size);
> +		return -EINVAL;
> +	}
> +
> +	pr_info("xcede : xcede_record_size = %d\n", xcede_record_size);
> +
> +	/*
> +	 * Since the payload_length includes the last NULL byte and
> +	 * the xcede_record_size, the remaining bytes correspond to
> +	 * array of all cede_latency settings.
> +	 */
> +	total_xcede_records_size = payload_length - 2;
> +	nr_xcede_records = total_xcede_records_size / xcede_record_size;
> +
> +	payload++;
> +	for (i = 0; i < nr_xcede_records; i++) {
> +		struct xcede_latency_records *record = &xcede_records[i];
> +
> +		record->latency_hint = (u8)payload[0];
> +		record->wakeup_latency_tb_ticks  =
> +			be64_to_cpu(*(__be64 *)(&payload[1]));
> +		record->responsive_to_irqs = (u8)payload[9];
> +		payload += xcede_record_size;
> +		pr_info("xcede : Record %d : hint = %u, latency =0x%llx tb-ticks, Wake-on-irq = %u\n",
> +			i, record->latency_hint,
> +			record->wakeup_latency_tb_ticks,
> +			record->responsive_to_irqs);
> +	}
> +
> +	return 0;
> +}
> +
>  u8 cede_latency_hint[NR_DEDICATED_STATES];
>  static int dedicated_cede_loop(struct cpuidle_device *dev,
>  				struct cpuidle_driver *drv,
> @@ -238,6 +350,19 @@ static int pseries_cpuidle_driver_init(void)
>  	return 0;
>  }
> 
> +static int add_pseries_idle_states(void)
> +{
> +	int nr_states = 2; /* By default we have snooze, CEDE */
> +
> +	if (parse_cede_parameters())
> +		return nr_states;
> +
> +	pr_info("cpuidle : Skipping the %d Extended CEDE idle states\n",
> +		nr_xcede_records);
> +
> +	return nr_states;

More logic will be added to this function in the subsequent patches to
actually make use of the information that is obtained from the platform
firmware.

--Vaidy

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ