2022-01-06 16:13:53

by Laurent Dufour

[permalink] [raw]
Subject: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

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.

However the hypervisor, like Qemu/KVM, may not support this RTAS
parameter. In that case the value reported in lparcfg is read from the
device tree and so is not updated accordingly.

Cc: Nathan Lynch <[email protected]>
Signed-off-by: Laurent Dufour <[email protected]>
---
v5:
fallback to the device tree value if RTAS is not providing the value.
v4:
address Nathan's new comments limiting size of the buffer.
v3:
address Michael's comments.
v2:
address Nathan's comments.
change title to partition_name aligning with existing partition_id
---
arch/powerpc/platforms/pseries/lparcfg.c | 93 ++++++++++++++++++++++++
1 file changed, 93 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index c7940fcfc911..8ca08fc306e7 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -311,6 +311,98 @@ 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, in section "7.3.16 System Parameters Option", the token 55 to
+ * read the LPAR name, and the largest output data to 4000 + 2 bytes length.
+ */
+#define SPLPAR_LPAR_NAME_TOKEN 55
+#define GET_SYS_PARM_BUF_SIZE 4002
+#if GET_SYS_PARM_BUF_SIZE > RTAS_DATA_BUF_SIZE
+#error "GET_SYS_PARM_BUF_SIZE is larger than RTAS_DATA_BUF_SIZE"
+#endif
+
+/**
+ * Read the lpar name using the RTAS ibm,get-system-parameter call.
+ *
+ * The name read through this call is updated if changes are made by the end
+ * user on the hypervisor side.
+ *
+ * Some hypervisor (like Qemu) may not provide this value. In that case, a non
+ * null value is returned.
+ */
+static int read_RTAS_lpar_name(struct seq_file *m)
+{
+ int rc, len, token;
+ union {
+ char raw_buffer[GET_SYS_PARM_BUF_SIZE];
+ struct {
+ __be16 len;
+ char name[GET_SYS_PARM_BUF_SIZE-2];
+ };
+ } *local_buffer;
+
+ token = rtas_token("ibm,get-system-parameter");
+ if (token == RTAS_UNKNOWN_SERVICE)
+ return -EINVAL;
+
+ local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
+ if (!local_buffer)
+ return -ENOMEM;
+
+ do {
+ spin_lock(&rtas_data_buf_lock);
+ memset(rtas_data_buf, 0, sizeof(*local_buffer));
+ rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
+ __pa(rtas_data_buf), sizeof(*local_buffer));
+ if (!rc)
+ memcpy(local_buffer->raw_buffer, rtas_data_buf,
+ sizeof(local_buffer->raw_buffer));
+ spin_unlock(&rtas_data_buf_lock);
+ } while (rtas_busy_delay(rc));
+
+ if (!rc) {
+ /* Force end of string */
+ len = min((int) be16_to_cpu(local_buffer->len),
+ (int) sizeof(local_buffer->name)-1);
+ local_buffer->name[len] = '\0';
+
+ seq_printf(m, "partition_name=%s\n", local_buffer->name);
+ } else
+ rc = -ENODATA;
+
+ kfree(local_buffer);
+ return rc;
+}
+
+/**
+ * Read the LPAR name from the Device Tree.
+ *
+ * The value read in the DT is not updated if the end-user is touching the LPAR
+ * name on the hypervisor side.
+ */
+static int read_DT_lpar_name(struct seq_file *m)
+{
+ struct device_node *rootdn;
+ const char *name;
+
+ rootdn = of_find_node_by_path("/");
+ if (!rootdn)
+ return -ENOENT;
+
+ name = of_get_property(rootdn, "ibm,partition-name", NULL);
+ if (!name)
+ return -ENOENT;
+
+ seq_printf(m, "partition_name=%s\n", name);
+ return 0;
+}
+
+static void read_lpar_name(struct seq_file *m)
+{
+ if (read_RTAS_lpar_name(m) && read_DT_lpar_name(m))
+ pr_err_once("Error can't get the LPAR name");
+}
+
#define SPLPAR_CHARACTERISTICS_TOKEN 20
#define SPLPAR_MAXLENGTH 1026*(sizeof(char))

@@ -496,6 +588,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 */
+ read_lpar_name(m);
parse_system_parameter_string(m);
parse_ppp_data(m);
parse_mpp_data(m);
--
2.34.1



2022-01-06 21:13:52

by Nathan Lynch

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

Laurent Dufour <[email protected]> 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.
>
> However this value could be read from RTAS.
>
> Adding this value in the /proc/powerpc/lparcfg output allows to read the
> updated value.
>
> However the hypervisor, like Qemu/KVM, may not support this RTAS
> parameter. In that case the value reported in lparcfg is read from the
> device tree and so is not updated accordingly.
>
> Cc: Nathan Lynch <[email protected]>
> Signed-off-by: Laurent Dufour <[email protected]>
> ---
> v5:
> fallback to the device tree value if RTAS is not providing the value.
> v4:
> address Nathan's new comments limiting size of the buffer.
> v3:
> address Michael's comments.
> v2:
> address Nathan's comments.
> change title to partition_name aligning with existing partition_id

Thanks Laurent.

Reviewed-by: Nathan Lynch <[email protected]>

2022-01-06 21:38:12

by Tyrel Datwyler

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

On 1/6/22 8:13 AM, Laurent Dufour wrote:
> 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.
>
> However the hypervisor, like Qemu/KVM, may not support this RTAS
> parameter. In that case the value reported in lparcfg is read from the
> device tree and so is not updated accordingly.
>
> Cc: Nathan Lynch <[email protected]>
> Signed-off-by: Laurent Dufour <[email protected]>

My only nit would be that in general for consistency with other function names
_RTAS_ and _DT_ should be lowercase. Seeing as they are statically scoped within
lparcfg.c maybe its ok. Otherwise,

Reviewed-by: Tyrel Datwyler <[email protected]>

> ---
> v5:
> fallback to the device tree value if RTAS is not providing the value.
> v4:
> address Nathan's new comments limiting size of the buffer.
> v3:
> address Michael's comments.
> v2:
> address Nathan's comments.
> change title to partition_name aligning with existing partition_id
> ---
> arch/powerpc/platforms/pseries/lparcfg.c | 93 ++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
> index c7940fcfc911..8ca08fc306e7 100644
> --- a/arch/powerpc/platforms/pseries/lparcfg.c
> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
> @@ -311,6 +311,98 @@ 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, in section "7.3.16 System Parameters Option", the token 55 to
> + * read the LPAR name, and the largest output data to 4000 + 2 bytes length.
> + */
> +#define SPLPAR_LPAR_NAME_TOKEN 55
> +#define GET_SYS_PARM_BUF_SIZE 4002
> +#if GET_SYS_PARM_BUF_SIZE > RTAS_DATA_BUF_SIZE
> +#error "GET_SYS_PARM_BUF_SIZE is larger than RTAS_DATA_BUF_SIZE"
> +#endif
> +
> +/**
> + * Read the lpar name using the RTAS ibm,get-system-parameter call.
> + *
> + * The name read through this call is updated if changes are made by the end
> + * user on the hypervisor side.
> + *
> + * Some hypervisor (like Qemu) may not provide this value. In that case, a non
> + * null value is returned.
> + */
> +static int read_RTAS_lpar_name(struct seq_file *m)
> +{
> + int rc, len, token;
> + union {
> + char raw_buffer[GET_SYS_PARM_BUF_SIZE];
> + struct {
> + __be16 len;
> + char name[GET_SYS_PARM_BUF_SIZE-2];
> + };
> + } *local_buffer;
> +
> + token = rtas_token("ibm,get-system-parameter");
> + if (token == RTAS_UNKNOWN_SERVICE)
> + return -EINVAL;
> +
> + local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
> + if (!local_buffer)
> + return -ENOMEM;
> +
> + do {
> + spin_lock(&rtas_data_buf_lock);
> + memset(rtas_data_buf, 0, sizeof(*local_buffer));
> + rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
> + __pa(rtas_data_buf), sizeof(*local_buffer));
> + if (!rc)
> + memcpy(local_buffer->raw_buffer, rtas_data_buf,
> + sizeof(local_buffer->raw_buffer));
> + spin_unlock(&rtas_data_buf_lock);
> + } while (rtas_busy_delay(rc));
> +
> + if (!rc) {
> + /* Force end of string */
> + len = min((int) be16_to_cpu(local_buffer->len),
> + (int) sizeof(local_buffer->name)-1);
> + local_buffer->name[len] = '\0';
> +
> + seq_printf(m, "partition_name=%s\n", local_buffer->name);
> + } else
> + rc = -ENODATA;
> +
> + kfree(local_buffer);
> + return rc;
> +}
> +
> +/**
> + * Read the LPAR name from the Device Tree.
> + *
> + * The value read in the DT is not updated if the end-user is touching the LPAR
> + * name on the hypervisor side.
> + */
> +static int read_DT_lpar_name(struct seq_file *m)
> +{
> + struct device_node *rootdn;
> + const char *name;
> +
> + rootdn = of_find_node_by_path("/");
> + if (!rootdn)
> + return -ENOENT;
> +
> + name = of_get_property(rootdn, "ibm,partition-name", NULL);
> + if (!name)
> + return -ENOENT;
> +
> + seq_printf(m, "partition_name=%s\n", name);
> + return 0;
> +}
> +
> +static void read_lpar_name(struct seq_file *m)
> +{
> + if (read_RTAS_lpar_name(m) && read_DT_lpar_name(m))
> + pr_err_once("Error can't get the LPAR name");
> +}
> +
> #define SPLPAR_CHARACTERISTICS_TOKEN 20
> #define SPLPAR_MAXLENGTH 1026*(sizeof(char))
>
> @@ -496,6 +588,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 */
> + read_lpar_name(m);
> parse_system_parameter_string(m);
> parse_ppp_data(m);
> parse_mpp_data(m);
>


2022-01-10 13:34:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

Hi Laurent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linux/master linus/master v5.16 next-20220110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Laurent-Dufour/powerpc-pseries-read-the-lpar-name-from-the-firmware/20220107-001503
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r026-20220106 (https://download.01.org/0day-ci/archive/20220110/[email protected]/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5cf0dea6e919e93ff3088f87acd40e84608a6861
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Laurent-Dufour/powerpc-pseries-read-the-lpar-name-from-the-firmware/20220107-001503
git checkout 5cf0dea6e919e93ff3088f87acd40e84608a6861
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/platforms/pseries/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

arch/powerpc/platforms/pseries/lparcfg.c:257: warning: Function parameter or member 'm' not described in 'parse_mpp_data'
arch/powerpc/platforms/pseries/lparcfg.c:295: warning: Function parameter or member 'm' not described in 'parse_mpp_x_data'
arch/powerpc/platforms/pseries/lparcfg.c:334: warning: Function parameter or member 'm' not described in 'read_RTAS_lpar_name'
>> arch/powerpc/platforms/pseries/lparcfg.c:334: warning: expecting prototype for Read the lpar name using the RTAS ibm,get-system(). Prototype was for read_RTAS_lpar_name() instead
>> arch/powerpc/platforms/pseries/lparcfg.c:378: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Read the LPAR name from the Device Tree.
arch/powerpc/platforms/pseries/lparcfg.c:678: warning: Function parameter or member 'entitlement' not described in 'update_mpp'
arch/powerpc/platforms/pseries/lparcfg.c:678: warning: Function parameter or member 'weight' not described in 'update_mpp'


vim +334 arch/powerpc/platforms/pseries/lparcfg.c

323
324 /**
325 * Read the lpar name using the RTAS ibm,get-system-parameter call.
326 *
327 * The name read through this call is updated if changes are made by the end
328 * user on the hypervisor side.
329 *
330 * Some hypervisor (like Qemu) may not provide this value. In that case, a non
331 * null value is returned.
332 */
333 static int read_RTAS_lpar_name(struct seq_file *m)
> 334 {
335 int rc, len, token;
336 union {
337 char raw_buffer[GET_SYS_PARM_BUF_SIZE];
338 struct {
339 __be16 len;
340 char name[GET_SYS_PARM_BUF_SIZE-2];
341 };
342 } *local_buffer;
343
344 token = rtas_token("ibm,get-system-parameter");
345 if (token == RTAS_UNKNOWN_SERVICE)
346 return -EINVAL;
347
348 local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
349 if (!local_buffer)
350 return -ENOMEM;
351
352 do {
353 spin_lock(&rtas_data_buf_lock);
354 memset(rtas_data_buf, 0, sizeof(*local_buffer));
355 rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
356 __pa(rtas_data_buf), sizeof(*local_buffer));
357 if (!rc)
358 memcpy(local_buffer->raw_buffer, rtas_data_buf,
359 sizeof(local_buffer->raw_buffer));
360 spin_unlock(&rtas_data_buf_lock);
361 } while (rtas_busy_delay(rc));
362
363 if (!rc) {
364 /* Force end of string */
365 len = min((int) be16_to_cpu(local_buffer->len),
366 (int) sizeof(local_buffer->name)-1);
367 local_buffer->name[len] = '\0';
368
369 seq_printf(m, "partition_name=%s\n", local_buffer->name);
370 } else
371 rc = -ENODATA;
372
373 kfree(local_buffer);
374 return rc;
375 }
376
377 /**
> 378 * Read the LPAR name from the Device Tree.
379 *
380 * The value read in the DT is not updated if the end-user is touching the LPAR
381 * name on the hypervisor side.
382 */
383 static int read_DT_lpar_name(struct seq_file *m)
384 {
385 struct device_node *rootdn;
386 const char *name;
387
388 rootdn = of_find_node_by_path("/");
389 if (!rootdn)
390 return -ENOENT;
391
392 name = of_get_property(rootdn, "ibm,partition-name", NULL);
393 if (!name)
394 return -ENOENT;
395
396 seq_printf(m, "partition_name=%s\n", name);
397 return 0;
398 }
399

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-01-11 22:40:46

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

Tyrel Datwyler <[email protected]> writes:
> On 1/6/22 8:13 AM, Laurent Dufour wrote:
>> 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.
>>
>> However the hypervisor, like Qemu/KVM, may not support this RTAS
>> parameter. In that case the value reported in lparcfg is read from the
>> device tree and so is not updated accordingly.
>>
>> Cc: Nathan Lynch <[email protected]>
>> Signed-off-by: Laurent Dufour <[email protected]>
>
> My only nit would be that in general for consistency with other function names
> _RTAS_ and _DT_ should be lowercase. Seeing as they are statically scoped within
> lparcfg.c maybe its ok. Otherwise,

Yeah I agree, I changed them to lower case when applying.

cheers

2022-01-12 13:10:31

by Laurent Dufour

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

On 11/01/2022, 23:40:27, Michael Ellerman wrote:
> Tyrel Datwyler <[email protected]> writes:
>> On 1/6/22 8:13 AM, Laurent Dufour wrote:
>>> 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.
>>>
>>> However the hypervisor, like Qemu/KVM, may not support this RTAS
>>> parameter. In that case the value reported in lparcfg is read from the
>>> device tree and so is not updated accordingly.
>>>
>>> Cc: Nathan Lynch <[email protected]>
>>> Signed-off-by: Laurent Dufour <[email protected]>
>>
>> My only nit would be that in general for consistency with other function names
>> _RTAS_ and _DT_ should be lowercase. Seeing as they are statically scoped within
>> lparcfg.c maybe its ok. Otherwise,
>
> Yeah I agree, I changed them to lower case when applying.

Thanks Michael and Tyrel.


2022-02-15 11:01:37

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

On Thu, 6 Jan 2022 17:13:39 +0100, Laurent Dufour wrote:
> 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.
>
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries: read the lpar name from the firmware
https://git.kernel.org/powerpc/c/eddaa9a402758d379520f6511fb61e89990698aa

cheers