[PATCH] powerpc/pseries: read the lpar name from the firmware

[PATCH] powerpc/pseries: read the lpar name from the firmware

[Laurent Dufour]

The LPAR name may be changed after the LPAR has been started in the HMC.
In that case lparstat command is not reporting the updated value because it
reads it from the device tree which is read at boot time.

However this value could be read from RTAS.

Adding this value in the /proc/powerpc/lparcfg output allows to read the
updated value.

Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/lparcfg.c | 50 ++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index f71eac74ea92..b597b132ce32 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -311,6 +311,55 @@ static void parse_mpp_x_data(struct seq_file *m)
 		seq_printf(m, "coalesce_pool_spurr=%ld\n", mpp_x_data.pool_spurr_cycles);
 }
 
+/*
+ * PAPR defines no maximum for the LPAR name, and defines that the maximum
+ * length of the get-system-parameter's output buffer is 4000 plus 2 bytes for
+ * the length. Limit LPAR's name size to 1024
+ */
+#define SPLPAR_LPAR_NAME_MAXLEN	1026
+#define SPLPAR_LPAR_NAME_TOKEN	55
+static void parse_lpar_name(struct seq_file *m)
+{
+	int call_status, len;
+	unsigned char *local_buffer;
+
+	local_buffer = kmalloc(SPLPAR_LPAR_NAME_MAXLEN, GFP_KERNEL);
+	if (!local_buffer) {
+		pr_err("%s %s kmalloc failure at line %d\n",
+		       __FILE__, __func__, __LINE__);
+		return;
+	}
+
+	spin_lock(&rtas_data_buf_lock);
+	memset(rtas_data_buf, 0, RTAS_DATA_BUF_SIZE);
+	call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+				NULL,
+				SPLPAR_LPAR_NAME_TOKEN,
+				__pa(rtas_data_buf),
+				RTAS_DATA_BUF_SIZE);
+	memcpy(local_buffer, rtas_data_buf, SPLPAR_LPAR_NAME_MAXLEN);
+	spin_unlock(&rtas_data_buf_lock);
+
+	if (call_status != 0) {
+		pr_err("%s %s Error calling get-system-parameter (0x%x)\n",
+		       __FILE__, __func__, call_status);
+	} else {
+		local_buffer[SPLPAR_LPAR_NAME_MAXLEN - 1] = '\0';
+		len = local_buffer[0] * 256 + local_buffer[1];
+
+		/*
+		 * Forces an empty string in the weird case fw reports no data.
+		 */
+		if (!len)
+			local_buffer[2] = '\0';
+
+		seq_printf(m, "lpar_name=%s\n", local_buffer + 2);
+	}
+
+	kfree(local_buffer);
+}
+
+
 #define SPLPAR_CHARACTERISTICS_TOKEN 20
 #define SPLPAR_MAXLENGTH 1026*(sizeof(char))
 
@@ -496,6 +545,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v)
 
 	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
 		/* this call handles the ibm,get-system-parameter contents */
+		parse_lpar_name(m);
 		parse_system_parameter_string(m);
 		parse_ppp_data(m);
 		parse_mpp_data(m);
-- 
2.34.1

Re: [PATCH] powerpc/pseries: read the lpar name from the firmware

[Nathan Lynch]

Laurent Dufour <ldufour@linux.ibm.com> writes:
> The LPAR name may be changed after the LPAR has been started in the HMC.
> In that case lparstat command is not reporting the updated value because it
> reads it from the device tree which is read at boot time.

Could lparstat be changed to make the appropriate get-system-parameter
call via librtas, avoiding a kernel change?


> However this value could be read from RTAS.
>
> Adding this value in the /proc/powerpc/lparcfg output allows to read the
> updated value.
>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/lparcfg.c | 50 ++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
> index f71eac74ea92..b597b132ce32 100644
> --- a/arch/powerpc/platforms/pseries/lparcfg.c
> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
> @@ -311,6 +311,55 @@ static void parse_mpp_x_data(struct seq_file *m)
>  		seq_printf(m, "coalesce_pool_spurr=%ld\n", mpp_x_data.pool_spurr_cycles);
>  }
>  
> +/*
> + * PAPR defines no maximum for the LPAR name, and defines that the maximum
> + * length of the get-system-parameter's output buffer is 4000 plus 2 bytes for
> + * the length. Limit LPAR's name size to 1024
> + */
> +#define SPLPAR_LPAR_NAME_MAXLEN	1026
> +#define SPLPAR_LPAR_NAME_TOKEN	55
> +static void parse_lpar_name(struct seq_file *m)
> +{
> +	int call_status, len;
> +	unsigned char *local_buffer;
> +
> +	local_buffer = kmalloc(SPLPAR_LPAR_NAME_MAXLEN, GFP_KERNEL);
> +	if (!local_buffer) {
> +		pr_err("%s %s kmalloc failure at line %d\n",
> +		       __FILE__, __func__, __LINE__);
> +		return;
> +	}

No error prints on memory allocation failure, the mm code will do that.

> +
> +	spin_lock(&rtas_data_buf_lock);
> +	memset(rtas_data_buf, 0, RTAS_DATA_BUF_SIZE);
> +	call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
> +				NULL,
> +				SPLPAR_LPAR_NAME_TOKEN,
> +				__pa(rtas_data_buf),
> +				RTAS_DATA_BUF_SIZE);
> +	memcpy(local_buffer, rtas_data_buf, SPLPAR_LPAR_NAME_MAXLEN);
> +	spin_unlock(&rtas_data_buf_lock);
> +
> +	if (call_status != 0) {
> +		pr_err("%s %s Error calling get-system-parameter (0x%x)\n",
> +		       __FILE__, __func__, call_status);

If this yields an error then it should fall back to the device tree.

ibm,get-system-parameter can return -2 or 990x, which are not errors.
Callers of ibm,get-system-parameter must handle these statuses using
rtas_busy_delay() or similar, which potentially involves sleeping.
Granted this is inconvenient when dealing the rtas_data_buf and
rtas_data_buf_lock - you can't call rtas_busy_delay() before you've
released the buffer lock. See dlpar_configure_connector() for an example
of how this can be structured.

It looks like most existing users of ibm,get-system-parameter have this
bug, unfortunately.

Re: [PATCH] powerpc/pseries: read the lpar name from the firmware

[Laurent Dufour]

On 01/12/2021, 16:30:29, Nathan Lynch wrote:
> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> The LPAR name may be changed after the LPAR has been started in the HMC.
>> In that case lparstat command is not reporting the updated value because it
>> reads it from the device tree which is read at boot time.
> Could lparstat be changed to make the appropriate get-system-parameter
> call via librtas, avoiding a kernel change?

You're right, I could do that directly in lparstat without changes in the
kernel.

Thanks,
Laurent.

Re: [PATCH] powerpc/pseries: read the lpar name from the firmware

[Laurent Dufour]

On 01/12/2021, 16:57:06, Laurent Dufour wrote:
> On 01/12/2021, 16:30:29, Nathan Lynch wrote:
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>> The LPAR name may be changed after the LPAR has been started in the HMC.
>>> In that case lparstat command is not reporting the updated value because it
>>> reads it from the device tree which is read at boot time.
>> Could lparstat be changed to make the appropriate get-system-parameter
>> call via librtas, avoiding a kernel change?
> 
> You're right, I could do that directly in lparstat without changes in the
> kernel.

Indeed, that's not so easy because the RTAS syscall can only be called by
privileged users, even to read some parameters.

So to have the lparstat command reporting the updated LPAR name, the only
way is to report that value in /proc/powerpc/lparcfg.

Furthermore, this would make sense to have this value reported in this file.

That's being said, I'll send a v2 to align the property title to the
existing "partition_id", naming it "partition_name".