Re: [PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
Checked my notes. I changed read_drconf_v1_cell/read_drconf_v2_cell
to use of_read_number in place of dt_mem_next_cell, because the function
is marked __init, and is not loaded in memory after a migration, so the
system crashes. So, we need that modification, unless we also add some
changes to kernel/drivers/of/fdt.c to the mix.
On 10/10/2018 12:34 PM, Michael Bringmann wrote:
> On 10/10/2018 11:54 AM, Nathan Fontenot wrote:
>> On 10/09/2018 03:36 PM, Michael Bringmann wrote:
>>> powerpc/drmem: Export many of the functions of DRMEM to parse
>>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>>> operations and for Post Migration events.
>>>
>>> Also modify the DRMEM initialization code to allow it to,
>>>
>>> * Be called after system initialization
>>> * Provide a separate user copy of the LMB array that is produces
>>> * Free the user copy upon request
>>>
>>> In addition, a couple of changes were made to make the creation
>>> of additional copies of the LMB array more useful including,
>>>
>>> * Add new iterator to work through a pair of drmem_info arrays.
>>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>>> dt_mem_next_cell, as these are only available at first boot.
>>>
>>> Signed-off-by: Michael Bringmann
>>> ---
>>> arch/powerpc/include/asm/drmem.h | 15
>>> arch/powerpc/mm/drmem.c | 75
>>> --
>>> 2 files changed, 70 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/drmem.h
>>> b/arch/powerpc/include/asm/drmem.h
>>> index 7c1d8e7..1fbb684 100644
>>> --- a/arch/powerpc/include/asm/drmem.h
>>> +++ b/arch/powerpc/include/asm/drmem.h
>>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>>> &drmem_info->lmbs[0], \
>>> &drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>>
>>> +#define for_each_dinfo_lmb(dinfo, lmb) \
>>> + for_each_drmem_lmb_in_range((lmb), \
>>> + &dinfo->lmbs[0],\
>>> + &dinfo->lmbs[dinfo->n_lmbs - 1])
>>> +
>>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)\
>>> + for ((lmb1) = (&dinfo1->lmbs[0]), \
>>> +(lmb2) = (&dinfo2->lmbs[0]); \
>>> +((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
>>> +((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
>>> +(lmb1)++, (lmb2)++)
>>> +
>>
>> The macros for traversing seem to be getting a bit unwieldy with these
>> updates. I wonder if we should move to just using walk routine
>> for all traversing of the drmem lmbs.
>
> We can do that. One new routine + one API - several macros + 2 files changed.
>
>>
>>> /*
>>> * The of_drconf_cell_v1 struct defines the layout of the LMB data
>>> * specified in the ibm,dynamic-memory device tree property.
>>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>> void (*func)(struct drmem_lmb *, const __be32 **));
>>> int drmem_update_dt(void);
>>>
>>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>>> +
>>> #ifdef CONFIG_PPC_PSERIES
>>> void __init walk_drmem_lmbs_early(unsigned long node,
>>> void (*func)(struct drmem_lmb *, const __be32 **));
>>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>>> index 3f18036..13d2abb 100644
>>> --- a/arch/powerpc/mm/drmem.c
>>> +++ b/arch/powerpc/mm/drmem.c
>>> @@ -20,6 +20,7 @@
>>>
>>> static struct drmem_lmb_info __drmem_info;
>>> struct drmem_lmb_info *drmem_info = &__drmem_info;
>>> +static int n_root_addr_cells;
>>>
>>> u64 drmem_lmb_memory_max(void)
>>> {
>>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>>> return rc;
>>> }
>>>
>>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>>>const __be32 **prop)
>>> {
>>> const __be32 *p = *prop;
>>>
>>> - lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>>> + lmb->base_addr = of_read_number(p, n_root_addr_cells);
>>> + p += n_root_addr_cells;
>>
>> Any reason this can't just be
>> lmb->base_addr= dt_mem_next_cell(n_root_addr_cells, &p);
>
> Probably, not. I will rebuild/retest with this.
>
>>
>>> lmb->drc_index = of_read_number(p++, 1);
>>>
>>> p++; /* skip reserved field */
>>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb
>>> *lmb,
>>> *prop = p;
>>> }
>>>
>>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32
>>> *usm,
>>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>> void (*func)(struct drmem_lmb *, const __be32 **))
>>> {
>>> struct drmem_lmb lmb;
>>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32
Re: [PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
On 10/10/2018 11:54 AM, Nathan Fontenot wrote:
> On 10/09/2018 03:36 PM, Michael Bringmann wrote:
>> powerpc/drmem: Export many of the functions of DRMEM to parse
>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>> operations and for Post Migration events.
>>
>> Also modify the DRMEM initialization code to allow it to,
>>
>> * Be called after system initialization
>> * Provide a separate user copy of the LMB array that is produces
>> * Free the user copy upon request
>>
>> In addition, a couple of changes were made to make the creation
>> of additional copies of the LMB array more useful including,
>>
>> * Add new iterator to work through a pair of drmem_info arrays.
>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>> dt_mem_next_cell, as these are only available at first boot.
>>
>> Signed-off-by: Michael Bringmann
>> ---
>> arch/powerpc/include/asm/drmem.h | 15
>> arch/powerpc/mm/drmem.c | 75
>> --
>> 2 files changed, 70 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/drmem.h
>> b/arch/powerpc/include/asm/drmem.h
>> index 7c1d8e7..1fbb684 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>> &drmem_info->lmbs[0], \
>> &drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>
>> +#define for_each_dinfo_lmb(dinfo, lmb) \
>> +for_each_drmem_lmb_in_range((lmb), \
>> +&dinfo->lmbs[0],\
>> +&dinfo->lmbs[dinfo->n_lmbs - 1])
>> +
>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2) \
>> +for ((lmb1) = (&dinfo1->lmbs[0]), \
>> + (lmb2) = (&dinfo2->lmbs[0]); \
>> + ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
>> + ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
>> + (lmb1)++, (lmb2)++)
>> +
>
> The macros for traversing seem to be getting a bit unwieldy with these
> updates. I wonder if we should move to just using walk routine
> for all traversing of the drmem lmbs.
We can do that. One new routine + one API - several macros + 2 files changed.
>
>> /*
>> * The of_drconf_cell_v1 struct defines the layout of the LMB data
>> * specified in the ibm,dynamic-memory device tree property.
>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>> void (*func)(struct drmem_lmb *, const __be32 **));
>> int drmem_update_dt(void);
>>
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>> +
>> #ifdef CONFIG_PPC_PSERIES
>> void __init walk_drmem_lmbs_early(unsigned long node,
>> void (*func)(struct drmem_lmb *, const __be32 **));
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index 3f18036..13d2abb 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -20,6 +20,7 @@
>>
>> static struct drmem_lmb_info __drmem_info;
>> struct drmem_lmb_info *drmem_info = &__drmem_info;
>> +static int n_root_addr_cells;
>>
>> u64 drmem_lmb_memory_max(void)
>> {
>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>> return rc;
>> }
>>
>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>> const __be32 **prop)
>> {
>> const __be32 *p = *prop;
>>
>> -lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +lmb->base_addr = of_read_number(p, n_root_addr_cells);
>> +p += n_root_addr_cells;
>
> Any reason this can't just be
> lmb->base_addr= dt_mem_next_cell(n_root_addr_cells, &p);
Probably, not. I will rebuild/retest with this.
>
>> lmb->drc_index = of_read_number(p++, 1);
>>
>> p++; /* skip reserved field */
>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb
>> *lmb,
>> *prop = p;
>> }
>>
>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32
>> *usm,
>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> void (*func)(struct drmem_lmb *, const __be32 **))
>> {
>> struct drmem_lmb lmb;
>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32
>> *prop, const __be32 *usm,
>> }
>> }
>>
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> const __be32 **prop)
>> {
>> const __be32 *p = *prop;
>>
>> dr_cell->seq_lmbs = of_read_number(p++, 1);
>> -dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +dr_cell->base_addr = of_read_number(p, n_root_addr_ce
Re: [PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
On 10/09/2018 03:36 PM, Michael Bringmann wrote:
> powerpc/drmem: Export many of the functions of DRMEM to parse
> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
> operations and for Post Migration events.
>
> Also modify the DRMEM initialization code to allow it to,
>
> * Be called after system initialization
> * Provide a separate user copy of the LMB array that is produces
> * Free the user copy upon request
>
> In addition, a couple of changes were made to make the creation
> of additional copies of the LMB array more useful including,
>
> * Add new iterator to work through a pair of drmem_info arrays.
> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
> dt_mem_next_cell, as these are only available at first boot.
>
> Signed-off-by: Michael Bringmann
> ---
> arch/powerpc/include/asm/drmem.h | 15
> arch/powerpc/mm/drmem.c | 75
> --
> 2 files changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/drmem.h
> b/arch/powerpc/include/asm/drmem.h
> index 7c1d8e7..1fbb684 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
> &drmem_info->lmbs[0], \
> &drmem_info->lmbs[drmem_info->n_lmbs - 1])
>
> +#define for_each_dinfo_lmb(dinfo, lmb) \
> + for_each_drmem_lmb_in_range((lmb), \
> + &dinfo->lmbs[0],\
> + &dinfo->lmbs[dinfo->n_lmbs - 1])
> +
> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2) \
> + for ((lmb1) = (&dinfo1->lmbs[0]), \
> + (lmb2) = (&dinfo2->lmbs[0]); \
> + ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
> + ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
> + (lmb1)++, (lmb2)++)
> +
The macros for traversing seem to be getting a bit unwieldy with these
updates. I wonder if we should move to just using walk routine
for all traversing of the drmem lmbs.
> /*
> * The of_drconf_cell_v1 struct defines the layout of the LMB data
> * specified in the ibm,dynamic-memory device tree property.
> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
> void (*func)(struct drmem_lmb *, const __be32 **));
> int drmem_update_dt(void);
>
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
> +
> #ifdef CONFIG_PPC_PSERIES
> void __init walk_drmem_lmbs_early(unsigned long node,
> void (*func)(struct drmem_lmb *, const __be32 **));
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 3f18036..13d2abb 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -20,6 +20,7 @@
>
> static struct drmem_lmb_info __drmem_info;
> struct drmem_lmb_info *drmem_info = &__drmem_info;
> +static int n_root_addr_cells;
>
> u64 drmem_lmb_memory_max(void)
> {
> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
> return rc;
> }
>
> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
> const __be32 **prop)
> {
> const __be32 *p = *prop;
>
> - lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> + lmb->base_addr = of_read_number(p, n_root_addr_cells);
> + p += n_root_addr_cells;
Any reason this can't just be
lmb->base_addr= dt_mem_next_cell(n_root_addr_cells, &p);
> lmb->drc_index = of_read_number(p++, 1);
>
> p++; /* skip reserved field */
> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb
> *lmb,
> *prop = p;
> }
>
> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32
> *usm,
> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
> void (*func)(struct drmem_lmb *, const __be32 **))
> {
> struct drmem_lmb lmb;
> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32
> *prop, const __be32 *usm,
> }
> }
>
> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> const __be32 **prop)
> {
> const __be32 *p = *prop;
>
> dr_cell->seq_lmbs = of_read_number(p++, 1);
> - dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> + dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
> + p += n_root_addr_cells;
Same here.
-Nathan
> dr_cell->drc_index = of_read_number(p++, 1);
> dr_cell->aa_index = of_read_number(p++, 1);
> dr_cell->flags = of_read_number(p++, 1);
> @@ -239,7 +242,7 @@ static void __init read_d
[PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
powerpc/drmem: Export many of the functions of DRMEM to parse
"ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
operations and for Post Migration events.
Also modify the DRMEM initialization code to allow it to,
* Be called after system initialization
* Provide a separate user copy of the LMB array that is produces
* Free the user copy upon request
In addition, a couple of changes were made to make the creation
of additional copies of the LMB array more useful including,
* Add new iterator to work through a pair of drmem_info arrays.
* Modify DRMEM code to replace usages of dt_root_addr_cells, and
dt_mem_next_cell, as these are only available at first boot.
Signed-off-by: Michael Bringmann
---
arch/powerpc/include/asm/drmem.h | 15
arch/powerpc/mm/drmem.c | 75 --
2 files changed, 70 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 7c1d8e7..1fbb684 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,18 @@ struct drmem_lmb_info {
&drmem_info->lmbs[0], \
&drmem_info->lmbs[drmem_info->n_lmbs - 1])
+#define for_each_dinfo_lmb(dinfo, lmb) \
+ for_each_drmem_lmb_in_range((lmb), \
+ &dinfo->lmbs[0],\
+ &dinfo->lmbs[dinfo->n_lmbs - 1])
+
+#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)\
+ for ((lmb1) = (&dinfo1->lmbs[0]), \
+(lmb2) = (&dinfo2->lmbs[0]); \
+((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) && \
+((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1])); \
+(lmb1)++, (lmb2)++)
+
/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
* specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
int drmem_update_dt(void);
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
+
#ifdef CONFIG_PPC_PSERIES
void __init walk_drmem_lmbs_early(unsigned long node,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..13d2abb 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -20,6 +20,7 @@
static struct drmem_lmb_info __drmem_info;
struct drmem_lmb_info *drmem_info = &__drmem_info;
+static int n_root_addr_cells;
u64 drmem_lmb_memory_max(void)
{
@@ -193,12 +194,13 @@ int drmem_update_dt(void)
return rc;
}
-static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
+static void read_drconf_v1_cell(struct drmem_lmb *lmb,
const __be32 **prop)
{
const __be32 *p = *prop;
- lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ lmb->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
lmb->drc_index = of_read_number(p++, 1);
p++; /* skip reserved field */
@@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb
*lmb,
*prop = p;
}
-static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct drmem_lmb lmb;
@@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32
*prop, const __be32 *usm,
}
}
-static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
+static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
const __be32 **prop)
{
const __be32 *p = *prop;
dr_cell->seq_lmbs = of_read_number(p++, 1);
- dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
+ dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
+ p += n_root_addr_cells;
dr_cell->drc_index = of_read_number(p++, 1);
dr_cell->aa_index = of_read_number(p++, 1);
dr_cell->flags = of_read_number(p++, 1);
@@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct
of_drconf_cell_v2 *dr_cell,
*prop = p;
}
-static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
+static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
void (*func)(struct drmem_lmb *, const __be32 **))
{
struct of_drconf_cell_v2 dr_cell;
@@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
const __be32 *prop, *usm;
int len;
+ if (n_root_addr_cells =
