On 07/27/2016 09:23 AM, Michael Bringmann wrote:
> hotplug_init: Simplify the code needed for runtime memory hotplug and
> maintenance with a conversion routine that transforms the compressed
> property "ibm,dynamic-memory-v2" to the form of "ibm,dynamic-memory"
> within the "ibm,dynamic-reconfiguration-memory" property. Thus only
> a single set of routines should be required at runtime to parse, edit,
> and manipulate the memory representation in the device tree. Similarly,
> any userspace applications that need this information will only need
> to recognize the older format to be able to continue to operate.
>
> Signed-off-by: Michael Bringmann <m...@linux.vnet.ibm.com>
> ---
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 2ce1385..f422dcb 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -839,6 +839,95 @@ static int pseries_update_drconf_memory(struct
> of_reconfig_data *pr)
> return rc;
> }
>
> +static int pseries_rewrite_dynamic_memory_v2(void)
> +{
> + unsigned long memblock_size;
> + struct device_node *dn;
> + struct property *prop, *prop_v2;
> + __be32 *p;
> + struct of_drconf_cell *lmbs;
> + u32 num_lmb_desc_sets, num_lmbs;
> + int i;
> +
> + dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> + if (!dn)
> + return -EINVAL;
> +
> + prop_v2 = of_find_property(dn, "ibm,dynamic-memory-v2", NULL);
> + if (!prop_v2)
> + return -EINVAL;
> +
> + memblock_size = pseries_memory_block_size();
> + if (!memblock_size)
> + return -EINVAL;
> +
> + /* The first int of the property is the number of lmb sets
> + * described by the property.
> + */
> + p = (__be32 *)prop_v2->value;
> + num_lmb_desc_sets = be32_to_cpu(*p++);
> +
> + /* Count the number of LMBs for generating the alternate format
> + */
> + for (i = 0, num_lmbs = 0; i < num_lmb_desc_sets; i++) {
> + struct of_drconf_cell_v2 drmem;
> +
> + read_drconf_cell_v2(&drmem, (const __be32 **)&p);
> + num_lmbs += drmem.num_seq_lmbs;
> + }
> +
> + /* Create an empty copy of the new 'ibm,dynamic-memory' property
> + */
> + {
> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> + if (!prop)
> + return -ENOMEM;
> + prop->name = kstrdup("ibm,dynamic-memory", GFP_KERNEL);
> + prop->length = dyn_mem_v2_len(num_lmbs);
I don't think dyn_mem_v2_len() is correct for the length of the new v1
property. After lookiing at that routine though it does seem to compute
the correct value, I think a v1_len call, or directly computing the length,
may make the code clearer.
> + prop->value = kzalloc(prop->length, GFP_KERNEL);
> + }
Is there a reason this is in its own block of code in {}?
Also, you will want to verify that prop->name and prop->value are non-NULL.
> +
> + /* Copy/expand the ibm,dynamic-memory-v2 format to produce the
> + * ibm,dynamic-memory format.
> + */
> + p = (__be32 *)prop->value;
> + *p = cpu_to_be32(num_lmbs);
> + p++;
> + lmbs = (struct of_drconf_cell *)p;
> +
> + p = (__be32 *)prop_v2->value;
> + p++;
> +
> + for (i = 0; i < num_lmb_desc_sets; i++) {
> + struct of_drconf_cell_v2 drmem;
> + int j, k = 0;
Won't this reset k to zero every time through the loop?
-Nathan
> +
> + read_drconf_cell_v2(&drmem, (const __be32 **)&p);
> +
> + for (j = 0; j < drmem.num_seq_lmbs; j++) {
> + lmbs[k+j].base_addr = be64_to_cpu(drmem.base_addr);
> + lmbs[k+j].drc_index = be32_to_cpu(drmem.drc_index);
> + lmbs[k+j].reserved = 0;
> + lmbs[k+j].aa_index = be32_to_cpu(drmem.aa_index);
> + lmbs[k+i].flags = be32_to_cpu(drmem.flags);
> +
> + drmem.base_addr += memblock_size;
> + drmem.drc_index++;
> + }
> +
> + k += drmem.num_seq_lmbs;
> + }
> +
> + of_remove_property(dn, prop_v2);
> +
> + of_add_property(dn, prop);
> +
> + /* And disable feature flag since the property has gone away */
> + powerpc_firmware_features &= ~FW_FEATURE_DYN_MEM_V2;
> +
> + return 0;
> +}
> +
> static int pseries_memory_notifier(struct notifier_block *nb,
> unsigned long action, void *data)
> {
> @@ -866,6 +952,8 @@ static struct notifier_block pseries_mem_nb = {
>
> static int __init pseries_memory_hotplug_init(void)
> {
> + if (firmware_has_feature(FW_FEATURE_DYN_MEM_V2))
> + pseries_rewrite_dynamic_memory_v2();
> if (firmware_has_feature(FW_FEATURE_LPAR))
> of_reconfig_notifier_register(&pseries_mem_nb);
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
