* [PATCH v06 5/5] migration/memory: Support 'ibm,dynamic-memory-v2'
From: Michael Bringmann @ 2018-10-15 19:55 UTC (permalink / raw)
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot, linuxppc-dev
In-Reply-To: <20181015194806.5705.95242.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds recognition for changes to the
associativity of memory blocks described by 'ibm,dynamic-memory-v2'.
If the associativity of an LMB has changed, it should be readded to
the system in order to update local and general kernel data structures.
This patch builds upon previous enhancements that scan the device-tree
"ibm,dynamic-memory" properties using the base LMB array, and a copy
derived from the updated properties.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index dc2aa34..8c08eb2 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -1184,7 +1184,8 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory") ||
+ !strcmp(rd->prop->name, "ibm,dynamic-memory-v2")) {
struct drmem_lmb_info *dinfo =
drmem_lmbs_init(rd->prop);
if (!dinfo)
^ permalink raw reply related
* [PATCH v06 4/5] migration/memory: Evaluate LMB assoc changes
From: Michael Bringmann @ 2018-10-15 19:54 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot
In-Reply-To: <20181015194754.5705.39622.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds code that recognizes changes to
the associativity of memory blocks described by the device-tree
properties in order to drive equivalent 'hotplug' operations to
update local and general kernel data structures to reflect those
changes. These differences may include:
* Evaluate 'ibm,dynamic-memory' properties when processing the
updated device-tree properties of the system during Post Migration
events (migration_store). The new functionality looks for changes
to the aa_index values for each drc_index/LMB to identify any memory
blocks that should be readded.
* In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
property may change. In the event that a row of the array differs,
locate all assigned memory blocks with that 'aa_index' and 're-add'
them to the system memory block data structures. In the process of
the 're-add', the system routines will update the corresponding entry
for the memory in the LMB structures and any other relevant kernel
data structures.
A number of previous extensions made to the DRMEM code for scanning
device-tree properties and creating LMB arrays are used here to
ensure that the resulting code is simpler and more usable:
* Use new paired list iterator for the DRMEM LMB info arrays to find
differences in old and new versions of properties.
* Use new iterator for copies of the DRMEM info arrays to evaluate
completely new structures.
* Combine common code for parsing and evaluating memory description
properties based on the DRMEM LMB array model to greatly simplify
extension from the older property 'ibm,dynamic-memory' to the new
property model of 'ibm,dynamic-memory-v2'.
For support, add a new pseries hotplug action for DLPAR operations,
PSERIES_HP_ELOG_ACTION_READD_MULTIPLE. It is a variant of the READD
operation which performs the action upon multiple instances of the
resource at one time. The operation is to be triggered by device-tree
analysis of updates by RTAS events analyzed by 'migation_store' during
post-migration processing. It will be used for memory updates,
initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
Changes in v04:
-- Move dlpar_memory_readd_multiple() function definition and use
into previous patch along with action constant definition.
-- Correct spacing in patch
Changes in v03:
-- Modify the code that parses the memory affinity attributes to
mark relevant DRMEM LMB array entries using the internal_flags
mechanism instead of generate unique hotplug actions for each
memory block to be readded. The change is intended to both
simplify the code, and to require fewer resources on systems
with huge amounts of memory.
-- Save up notice about any all LMB entries until the end of the
'migration_store' operation at which point a single action is
queued to scan the entire DRMEM array.
-- Add READD_MULTIPLE function for memory that scans the DRMEM
array to identify multiple entries that were marked previously.
The corresponding memory blocks are to be readded to the system
to update relevant data structures outside of the powerpc-
specific code.
-- Change dlpar_memory_pmt_changes_action to directly queue worker
to pseries work queue.
---
arch/powerpc/include/asm/topology.h | 7 +
arch/powerpc/mm/numa.c | 6 -
arch/powerpc/platforms/pseries/hotplug-memory.c | 207 +++++++++++++++++++----
arch/powerpc/platforms/pseries/mobility.c | 3
arch/powerpc/platforms/pseries/pseries.h | 8 +
5 files changed, 186 insertions(+), 45 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index a4a718d..fbe03df 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -135,5 +135,12 @@ static inline void shared_proc_topology_init(void) {}
#endif
#endif
+
+struct assoc_arrays {
+ u32 n_arrays;
+ u32 array_sz;
+ const __be32 *arrays;
+};
+
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_TOPOLOGY_H */
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 693ae1c..f1e7287 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -368,12 +368,6 @@ static unsigned long read_n_cells(int n, const __be32 **buf)
return result;
}
-struct assoc_arrays {
- u32 n_arrays;
- u32 array_sz;
- const __be32 *arrays;
-};
-
/*
* Retrieve and validate the list of associativity arrays for drconf
* memory from the ibm,associativity-lookup-arrays property of the
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 9c76345..dc2aa34 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -537,8 +537,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
}
}
- if (!lmb_found)
- rc = -EINVAL;
+ if (!lmb_found) {
+ pr_info("Failed to update memory for drc index %lx\n",
+ (unsigned long) drc_index);
+ return -EINVAL;
+ }
if (rc)
pr_info("Failed to update memory at %llx\n",
@@ -998,55 +1001,173 @@ static int pseries_add_mem_node(struct device_node *np)
return (ret < 0) ? -EINVAL : 0;
}
-static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
+static int pmt_changes = 0;
+
+void dlpar_memory_pmt_changes_set(void)
+{
+ pmt_changes = 1;
+}
+
+void dlpar_memory_pmt_changes_clear(void)
{
- struct of_drconf_cell_v1 *new_drmem, *old_drmem;
+ pmt_changes = 0;
+}
+
+int dlpar_memory_pmt_changes(void)
+{
+ return pmt_changes;
+}
+
+void dlpar_memory_pmt_changes_action(void)
+{
+ if (dlpar_memory_pmt_changes()) {
+ struct pseries_hp_errorlog hp_errlog;
+
+ hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
+ hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
+ hp_errlog.id_type = 0;
+
+ queue_hotplug_event(&hp_errlog);
+
+ dlpar_memory_pmt_changes_clear();
+ }
+}
+
+struct dlpar_memory_lmb_chk_affinity_data {
unsigned long memblock_size;
- u32 entries;
- __be32 *p;
- int i, rc = -EINVAL;
+};
+
+int dlpar_memory_lmb_chk_affinity(struct drmem_lmb *old_lmb,
+ struct drmem_lmb *new_lmb,
+ void *data)
+{
+ struct dlpar_memory_lmb_chk_affinity_data *ldata = data;
+ int rc = 1;
+
+ if (new_lmb->drc_index != old_lmb->drc_index)
+ return rc;
+
+ if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
+ (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
+ rc = pseries_remove_memblock(
+ old_lmb->base_addr,
+ ldata->memblock_size);
+ } else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ rc = memblock_add(old_lmb->base_addr,
+ ldata->memblock_size);
+ rc = (rc < 0) ? -EINVAL : 0;
+ } else if ((old_lmb->aa_index != new_lmb->aa_index) &&
+ (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(old_lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+
+ return rc;
+}
+
+static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
+{
+ struct dlpar_memory_lmb_chk_affinity_data data;
+ int rc = 0;
if (rtas_hp_event)
return 0;
- memblock_size = pseries_memory_block_size();
- if (!memblock_size)
+ data.memblock_size = pseries_memory_block_size();
+ if (!data.memblock_size)
return -EINVAL;
+ /* Arrays should have the same size and DRC indexes */
+ rc = walk_drmem_lmbs_pairs(new_dinfo,
+ dlpar_memory_lmb_chk_affinity, &data);
+ return rc;
+}
+
+static void pseries_update_ala_memory_aai(int aa_index)
+{
+ struct drmem_lmb *lmb;
+
+ /* Readd all LMBs which were previously using the
+ * specified aa_index value.
+ */
+ for_each_drmem_lmb(lmb) {
+ if ((lmb->aa_index == aa_index) &&
+ (lmb->flags & DRCONF_MEM_ASSIGNED)) {
+ drmem_mark_lmb_update(lmb);
+ dlpar_memory_pmt_changes_set();
+ }
+ }
+}
+
+static int pseries_update_ala_memory(struct of_reconfig_data *pr)
+{
+ struct assoc_arrays new_ala, old_ala;
+ __be32 *p;
+ int i, lim;
+
+ if (rtas_hp_event)
+ return 0;
+
+ /*
+ * The layout of the ibm,associativity-lookup-arrays
+ * property is a number N indicating the number of
+ * associativity arrays, followed by a number M
+ * indicating the size of each associativity array,
+ * followed by a list of N associativity arrays.
+ */
+
p = (__be32 *) pr->old_prop->value;
if (!p)
return -EINVAL;
+ old_ala.n_arrays = of_read_number(p++, 1);
+ old_ala.array_sz = of_read_number(p++, 1);
+ old_ala.arrays = p;
- /* The first int of the property is the number of lmb's described
- * by the property. This is followed by an array of of_drconf_cell
- * entries. Get the number of entries and skip to the array of
- * of_drconf_cell's.
- */
- entries = be32_to_cpu(*p++);
- old_drmem = (struct of_drconf_cell_v1 *)p;
-
- p = (__be32 *)pr->prop->value;
- p++;
- new_drmem = (struct of_drconf_cell_v1 *)p;
-
- for (i = 0; i < entries; i++) {
- if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
- (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
- rc = pseries_remove_memblock(
- be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- break;
- } else if ((!(be32_to_cpu(old_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) &&
- (be32_to_cpu(new_drmem[i].flags) &
- DRCONF_MEM_ASSIGNED)) {
- rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
- memblock_size);
- rc = (rc < 0) ? -EINVAL : 0;
- break;
+ p = (__be32 *) pr->prop->value;
+ if (!p)
+ return -EINVAL;
+ new_ala.n_arrays = of_read_number(p++, 1);
+ new_ala.array_sz = of_read_number(p++, 1);
+ new_ala.arrays = p;
+
+ lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
+ new_ala.n_arrays;
+
+ if (old_ala.array_sz == new_ala.array_sz) {
+
+ /* Iterate comparing rows of the old and new
+ * ibm,associativity-lookup-arrays looking for
+ * changes. If a row has changed, then mark all
+ * memory blocks using that index for readd.
+ */
+ for (i = 0; i < lim; i++) {
+ int index = (i * new_ala.array_sz);
+
+ if (!memcmp(&old_ala.arrays[index],
+ &new_ala.arrays[index],
+ new_ala.array_sz))
+ continue;
+
+ pseries_update_ala_memory_aai(i);
}
+
+ /* Reset any entries representing the extra rows.
+ * There shouldn't be any, but just in case ...
+ */
+ for (i = lim; i < new_ala.n_arrays; i++)
+ pseries_update_ala_memory_aai(i);
+
+ } else {
+ /* Update all entries representing these rows;
+ * as all rows have different sizes, none can
+ * have equivalent values.
+ */
+ for (i = 0; i < lim; i++)
+ pseries_update_ala_memory_aai(i);
}
- return rc;
+
+ return 0;
}
static int pseries_memory_notifier(struct notifier_block *nb,
@@ -1063,8 +1184,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
err = pseries_remove_mem_node(rd->dn);
break;
case OF_RECONFIG_UPDATE_PROPERTY:
- if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
- err = pseries_update_drconf_memory(rd);
+ if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
+ struct drmem_lmb_info *dinfo =
+ drmem_lmbs_init(rd->prop);
+ if (!dinfo)
+ return -EINVAL;
+ err = pseries_update_drconf_memory(dinfo);
+ drmem_lmbs_free(dinfo);
+ } else if (!strcmp(rd->prop->name,
+ "ibm,associativity-lookup-arrays"))
+ err = pseries_update_ala_memory(rd);
break;
}
return notifier_from_errno(err);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 7da222d..2557a42 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -380,6 +380,9 @@ static ssize_t migration_store(struct class *class,
post_mobility_fixup();
+ /* Apply any necessary changes identified during fixup */
+ dlpar_memory_pmt_changes_action();
+
start_topology_update();
return count;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 72c0b89..0c33f79 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -63,13 +63,21 @@ extern struct device_node *dlpar_configure_connector(__be32,
void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
+void dlpar_memory_pmt_changes_set(void);
+void dlpar_memory_pmt_changes_clear(void);
+int dlpar_memory_pmt_changes(void);
+
#ifdef CONFIG_MEMORY_HOTPLUG
int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
+void dlpar_memory_pmt_changes_action(void);
#else
static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
{
return -EOPNOTSUPP;
}
+static inline void dlpar_memory_pmt_changes_action(void)
+{
+}
#endif
#ifdef CONFIG_HOTPLUG_CPU
^ permalink raw reply related
* [PATCH v06 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2018-10-15 19:53 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot
In-Reply-To: <20181015194738.5705.88953.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
migration/memory: This patch adds a new pseries hotplug action
for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
This is a variant of the READD operation which performs the action
upon multiple instances of the resource at one time. The operation
is to be triggered by device-tree analysis of updates by RTAS events
analyzed by 'migation_store' during post-migration processing. It
will be used for memory updates, initially.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v05:
-- Provide dlpar_memory_readd_helper routine to compress some common code
Changes in v04:
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
previous patch.
-- Pull in implementation of dlpar_memory_readd_multiple() to go
with operation flag.
---
arch/powerpc/include/asm/rtas.h | 1 +
arch/powerpc/platforms/pseries/hotplug-memory.c | 44 ++++++++++++++++++++---
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 0183e95..cc00451 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
#define PSERIES_HP_ELOG_ACTION_ADD 1
#define PSERIES_HP_ELOG_ACTION_REMOVE 2
#define PSERIES_HP_ELOG_ACTION_READD 3
+#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE 4
#define PSERIES_HP_ELOG_ID_DRC_NAME 1
#define PSERIES_HP_ELOG_ID_DRC_INDEX 2
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 2b796da..9c76345 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -507,6 +507,19 @@ static int dlpar_memory_remove_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_helper(struct drmem_lmb *lmb)
+{
+ int rc;
+
+ rc = dlpar_remove_lmb(lmb);
+ if (!rc) {
+ rc = dlpar_add_lmb(lmb);
+ if (rc)
+ dlpar_release_drc(lmb->drc_index);
+ }
+ return rc;
+}
+
static int dlpar_memory_readd_by_index(u32 drc_index)
{
struct drmem_lmb *lmb;
@@ -519,12 +532,7 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
for_each_drmem_lmb(lmb) {
if (lmb->drc_index == drc_index) {
lmb_found = 1;
- rc = dlpar_remove_lmb(lmb);
- if (!rc) {
- rc = dlpar_add_lmb(lmb);
- if (rc)
- dlpar_release_drc(lmb->drc_index);
- }
+ rc = dlpar_memory_readd_helper(lmb);
break;
}
}
@@ -541,6 +549,23 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
return rc;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ struct drmem_lmb *lmb;
+ int rc;
+
+ pr_info("Attempting to update multiple LMBs\n");
+
+ for_each_drmem_lmb(lmb) {
+ if (drmem_lmb_update(lmb)) {
+ rc = dlpar_memory_readd_helper(lmb);
+ drmem_remove_lmb_update(lmb);
+ }
+ }
+
+ return rc;
+}
+
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
struct drmem_lmb *lmb, *start_lmb, *end_lmb;
@@ -641,6 +666,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
{
return -EOPNOTSUPP;
}
+static int dlpar_memory_readd_multiple(void)
+{
+ return -EOPNOTSUPP;
+}
static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
{
@@ -918,6 +947,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
drc_index = hp_elog->_drc_u.drc_index;
rc = dlpar_memory_readd_by_index(drc_index);
break;
+ case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
+ rc = dlpar_memory_readd_multiple();
+ break;
default:
pr_err("Invalid action (%d) specified\n", hp_elog->action);
rc = -EINVAL;
^ permalink raw reply related
* Fwd: [PATCH v06 2/5] powerpc/drmem: Add internal_flags feature
From: Michael Bringmann @ 2018-10-15 19:52 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot
In-Reply-To: <20181015194723.5705.51193.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
powerpc/drmem: Add internal_flags field to each LMB to allow
marking of kernel software-specific operations that need not
be exported to other users. For instance, if information about
selected LMBs needs to be maintained for subsequent passes
through the system, it can be encoded into the LMB array itself
without requiring the allocation and maintainance of additional
data structures.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
---
Changes in v04:
-- Add another initialization of 'lmb->internal_flags' to
init_drmem_v2_lmbs.
---
arch/powerpc/include/asm/drmem.h | 18 ++++++++++++++++++
arch/powerpc/mm/drmem.c | 3 +++
2 files changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index cfe8598..dbb3e6c 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -17,6 +17,7 @@ struct drmem_lmb {
u32 drc_index;
u32 aa_index;
u32 flags;
+ u32 internal_flags;
};
struct drmem_lmb_info {
@@ -94,6 +95,23 @@ static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
return lmb->flags & DRMEM_LMB_RESERVED;
}
+#define DRMEM_LMBINT_UPDATE 0x00000001
+
+static inline void drmem_mark_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags |= DRMEM_LMBINT_UPDATE;
+}
+
+static inline void drmem_remove_lmb_update(struct drmem_lmb *lmb)
+{
+ lmb->internal_flags &= ~DRMEM_LMBINT_UPDATE;
+}
+
+static inline bool drmem_lmb_update(struct drmem_lmb *lmb)
+{
+ return lmb->internal_flags & DRMEM_LMBINT_UPDATE;
+}
+
u64 drmem_lmb_memory_max(void);
void __init walk_drmem_lmbs(struct device_node *dn,
void (*func)(struct drmem_lmb *, const __be32 **));
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index ded9dbf..f199fe5 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -207,6 +207,7 @@ static void read_drconf_v1_cell(struct drmem_lmb *lmb,
lmb->aa_index = of_read_number(p++, 1);
lmb->flags = of_read_number(p++, 1);
+ lmb->internal_flags = 0;
*prop = p;
}
@@ -265,6 +266,7 @@ static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
lmb.aa_index = dr_cell.aa_index;
lmb.flags = dr_cell.flags;
+ lmb.internal_flags = 0;
func(&lmb, &usm);
}
@@ -441,6 +443,7 @@ static void init_drmem_v2_lmbs(const __be32 *prop,
lmb->aa_index = dr_cell.aa_index;
lmb->flags = dr_cell.flags;
+ lmb->internal_flags = 0;
}
}
}
^ permalink raw reply related
* [PATCH v06 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Bringmann @ 2018-10-15 19:50 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Michael Bringmann, Juliet Kim, Tyrel Datwyler,
Nathan Fontenot
In-Reply-To: <20181015194704.5705.54278.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>
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 iterator function to work through a pair of drmem_info arrays
with a callback function to apply specific tests.
* 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 <mwb@linux.vnet.ibm.com>
---
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
---
arch/powerpc/include/asm/drmem.h | 13 +++++
arch/powerpc/mm/drmem.c | 96 ++++++++++++++++++++++++++++++--------
2 files changed, 89 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 7c1d8e7..cfe8598 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -35,6 +35,11 @@ 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])
+
/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
* specified in the ibm,dynamic-memory device tree property.
@@ -94,6 +99,14 @@ 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);
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data);
+
#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..ded9dbf 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 == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
if (!prop || len < dt_root_size_cells * sizeof(__be32))
return;
@@ -353,24 +359,47 @@ void __init walk_drmem_lmbs(struct device_node *dn,
}
}
-static void __init init_drmem_v1_lmbs(const __be32 *prop)
+int walk_drmem_lmbs_pairs(struct drmem_lmb_info *dinfo_oth,
+ int (*func)(struct drmem_lmb *cnt,
+ struct drmem_lmb *oth,
+ void *data),
+ void *data)
+{
+ struct drmem_lmb *lmb1, *lmb2;
+ int rc = 0;
+
+ for ((lmb1) = (&drmem_info->lmbs[0]), (lmb2) = (&dinfo_oth->lmbs[0]);
+ ((lmb1) <= (&drmem_info->lmbs[drmem_info->n_lmbs - 1])) &&
+ ((lmb2) <= (&dinfo_oth->lmbs[dinfo_oth->n_lmbs - 1]));
+ (lmb1)++, (lmb2)++)
+ {
+ rc = func(lmb1, lmb2, data);
+ if (rc <= 0)
+ break;
+ }
+ return rc;
+}
+
+static void init_drmem_v1_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
- drmem_info->n_lmbs = of_read_number(prop++, 1);
- if (drmem_info->n_lmbs == 0)
+ dinfo->n_lmbs = of_read_number(prop++, 1);
+ if (dinfo->n_lmbs == 0)
return;
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
- for_each_drmem_lmb(lmb)
+ for_each_dinfo_lmb(dinfo, lmb)
read_drconf_v1_cell(lmb, &prop);
}
-static void __init init_drmem_v2_lmbs(const __be32 *prop)
+static void init_drmem_v2_lmbs(const __be32 *prop,
+ struct drmem_lmb_info *dinfo)
{
struct drmem_lmb *lmb;
struct of_drconf_cell_v2 dr_cell;
@@ -386,12 +415,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
p = prop;
for (i = 0; i < lmb_sets; i++) {
read_drconf_v2_cell(&dr_cell, &p);
- drmem_info->n_lmbs += dr_cell.seq_lmbs;
+ dinfo->n_lmbs += dr_cell.seq_lmbs;
}
- drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
+ dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
GFP_KERNEL);
- if (!drmem_info->lmbs)
+ if (!dinfo->lmbs)
return;
/* second pass, read in the LMB information */
@@ -402,10 +431,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
read_drconf_v2_cell(&dr_cell, &p);
for (j = 0; j < dr_cell.seq_lmbs; j++) {
- lmb = &drmem_info->lmbs[lmb_index++];
+ lmb = &dinfo->lmbs[lmb_index++];
lmb->base_addr = dr_cell.base_addr;
- dr_cell.base_addr += drmem_info->lmb_size;
+ dr_cell.base_addr += dinfo->lmb_size;
lmb->drc_index = dr_cell.drc_index;
dr_cell.drc_index++;
@@ -416,11 +445,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
}
}
+void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
+{
+ if (dinfo) {
+ kfree(dinfo->lmbs);
+ kfree(dinfo);
+ }
+}
+
+struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
+{
+ struct drmem_lmb_info *dinfo;
+
+ dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
+ if (!dinfo)
+ return NULL;
+
+ if (!strcmp("ibm,dynamic-memory", prop->name))
+ init_drmem_v1_lmbs(prop->value, dinfo);
+ else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
+ init_drmem_v2_lmbs(prop->value, dinfo);
+
+ return dinfo;
+}
+
static int __init drmem_init(void)
{
struct device_node *dn;
const __be32 *prop;
+ if (n_root_addr_cells == 0)
+ n_root_addr_cells = dt_root_addr_cells;
+
dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (!dn) {
pr_info("No dynamic reconfiguration memory found\n");
@@ -434,11 +490,11 @@ static int __init drmem_init(void)
prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
if (prop) {
- init_drmem_v1_lmbs(prop);
+ init_drmem_v1_lmbs(prop, drmem_info);
} else {
prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
if (prop)
- init_drmem_v2_lmbs(prop);
+ init_drmem_v2_lmbs(prop, drmem_info);
}
of_node_put(dn);
^ permalink raw reply related
* [PATCH v06 0/5] powerpc/migration: Affinity fix for memory
From: Michael Bringmann @ 2018-10-15 19:45 UTC (permalink / raw)
To: linuxppc-dev, mwb
Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, Nathan Fontenot
The migration of LPARs across Power systems affects many attributes
including that of the associativity of memory blocks. The patches
in this set execute when a system is coming up fresh upon a migration
target. They are intended to,
* Recognize changes to the associativity of memory recorded in
internal data structures when compared to the latest copies in
the device tree (e.g. ibm,dynamic-memory, ibm,dynamic-memory-v2).
* Recognize changes to the associativity mapping (e.g. ibm,
associativity-lookup-arrays), locate all assigned memory blocks
corresponding to each changed row, and readd all such blocks.
* Generate calls to other code layers to reset the data structures
related to associativity of memory.
* Re-register the 'changed' entities into the target system.
Re-registration of memory blocks mostly entails acting as if they
have been newly hot-added into the target system.
This code builds upon features introduced in a previous patch set
that updates CPUs for affinity changes that may occur during LPM.
Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
Michael Bringmann (5):
powerpc/drmem: Export 'dynamic-memory' loader
powerpc/drmem: Add internal_flags feature
migration/memory: Add hotplug flags READD_MULTIPLE
migration/memory: Evaluate LMB assoc changes
migration/memory: Support 'ibm,dynamic-memory-v2'
---
Changes in v06:
-- Rebase to powerpc next branch to account for recent code changes.
-- Fix prototype problem when CONFIG_MEMORY_HOTPLUG not defined.
Changes in v05:
-- Add walk_drmem_lmbs_pairs to replace macro for_each_pair_lmb
-- Use walk_drmem_lmbs_pairs and callback instead of local loop
-- Provide dlpar_memory_readd_helper routine to compress some common code
-- Move common structure from numa.c + hotplug-memory.c to header file.
-- Clarify some comments.
Changes in v04:
-- Move dlpar_memory_readd_multiple() to patch with new ACTION
constant.
-- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
patch with other references to flag.
-- Correct spacing in one of the patches
Changes in v03:
-- Change operation to tag changed LMBs in DRMEM array instead of
queuing a potentially huge number of structures.
-- Added another hotplug queue event for CPU/memory operations
-- Added internal_flags feature to DRMEM
-- Improve the patch description language for the patch set.
-- Revise patch set to queue worker for memory association
updates directly to pseries worker queue.
^ permalink raw reply
* Re: [PATCH v3 13/18] of: overlay: check prevents multiple fragments touching same property
From: Frank Rowand @ 2018-10-15 19:30 UTC (permalink / raw)
To: Joe Perches, Rob Herring, Pantelis Antoniou, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, Alan Tull, Moritz Fischer
Cc: devicetree, linux-fpga, linuxppc-dev, linux-kernel
In-Reply-To: <e045d69a-7896-3ff1-8472-2f51c2468d7f@gmail.com>
On 10/14/18 20:21, Frank Rowand wrote:
> On 10/14/18 18:55, Joe Perches wrote:
>> On Sun, 2018-10-14 at 18:52 -0700, Frank Rowand wrote:
>>> On 10/14/18 18:06, Joe Perches wrote:
>>>> On Sun, 2018-10-14 at 17:24 -0700, frowand.list@gmail.com wrote:
>>>>> From: Frank Rowand <frank.rowand@sony.com>
>>>>>
>>>>> Add test case of two fragments updating the same property. After
>>>>> adding the test case, the system hangs at end of boot, after
>>>>> after slub stack dumps from kfree() in crypto modprobe code.
>> []
>>>> I think this is worse performance than before.
>>>>
>>>> This now walks all entries when before it would
>>>> return -EINVAL directly when it found a match.
>>>
>>> Yes, it is worse performance, but that is OK.
>>>
>>> This is a check that is done when a devicetree overlay is applied.
>>> If an error occurs then that means that the overlay was incorrectly
>>> specified. The file drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
>>> in this patch provides an example of how a bad overlay can be created.
>>>
>>> Once an error was detected, the check could return immediately, or it
>>> could continue to give a complete list of detected errors. I chose to
>>> give the complete list of detected errors.
>>
>> Swell. Please describe that in the commit message.
>
> If a version 4 of the series is created I will update the commit
> message. As a stand alone item I do not think it is worth a
> new version.
And there will be a version 4, so I will update the commit message.
-Frank
^ permalink raw reply
* Re: [PATCH v3 00/18] of: overlay: validation checks, subsequent fixes
From: Alan Tull @ 2018-10-15 19:21 UTC (permalink / raw)
To: Frank Rowand
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-fpga, Pantelis Antoniou, linux-kernel, Rob Herring,
Moritz Fischer, Paul Mackerras, linuxppc-dev
In-Reply-To: <1539563070-12969-1-git-send-email-frowand.list@gmail.com>
On Sun, Oct 14, 2018 at 7:26 PM <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> Add checks to (1) overlay apply process and (2) memory freeing
> triggered by overlay release. The checks are intended to detect
> possible memory leaks and invalid overlays.
>
> The checks revealed bugs in existing code. Fixed the bugs.
>
> While fixing bugs, noted other issues, which are fixed in
> separate patches.
>
> ***** Powerpc folks: I was not able to test the patches that
> ***** directly impact Powerpc systems that use dynamic
> ***** devicetree. Please review that code carefully and
> ***** test. The specific patches are: 03/16, 04/16, 07/16
>
> FPGA folks:
>
> I made the validation checks that should result in an
> invalid live devicetree report "ERROR" and cause the overlay apply
> to fail.
>
> I made the memory leak validation tests report "WARNING" and allow
> the overlay apply to complete successfully. Please let me know
> if you encounter the warnings. There are at least two paths
> forward to deal with the cases that trigger the warning: (1) change
> the warning to an error and fail the overlay apply, or (2) find a
> way to detect the potential memory leaks and free the memory
> appropriately.
I reran my FPGA testing. The strings are fixed, no longer NULL. I
have functionality back, my test passes now. I'm seeing the intended
warnings about any properties added to existing nodes. That includes
warnings about added symbols. Below is a simplified part to show some
of what I'm seeing.
By the way my testing is all using Pantelis' DT overlay configfs interface.
root@arria10:~# ./apply-static-region.sh
Applying dtbo: socfpga_arria10_socdk_sdmmc_ghrd_ovl_ext_cfg.dtb
[ 1821.088640] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/soc/base_fpga_region/ranges
[ 1821.103307] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/soc/base_fpga_region/external-fpga-config
[ 1821.117359] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/soc/base_fpga_region/clocks
[ 1821.130130] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/soc/base_fpga_region/clock-names
[ 1821.143449] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property: /__symbols__/clk_0
[ 1821.155357] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property: /__symbols__/ILC
[ 1821.167074] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/__symbols__/freeze_controller_0
[ 1821.180171] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property:
/__symbols__/sysid_qsys_0
[ 1821.192662] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property: /__symbols__/led_pio
[ 1821.204720] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property: /__symbols__/button_pio
[ 1821.217034] OF: overlay: WARNING: add_changeset_property(), memory
leak will occur if overlay removed. Property: /__symbols__/dipsw_pio
[ 1821.231977] of-fpga-region soc:base_fpga_region:fpga_pr_region0:
FPGA Region probed
[ 1821.240144] altera_freeze_br ff200450.freeze_controller: fpga
bridge [freeze] registered
root@arria10:~# ./apply-static-region.sh root@arria10:~# rmdir
/sys/kernel/config/device-tree/overlays/1-socfpga_arria10_socdk_sdmmc_ghrd_ovl_ext_cfg.dtb
[ 1823.805564] OF: ERROR: memory leak - destroy cset entry: attach
overlay node /soc/base_fpga_region/clk_0 with refcount 2
Alan
Alan
>
> ALL people:
>
> The validations do _not_ address another major concern I have with
> releasing overlays, which is use after free errors.
>
> Changes since v2:
>
> - 13/18: Use continue to reduce indentation in find_dup_cset_node_entry()
> and find_dup_cset_prop()
>
> Changes since v1:
>
> - move patch 16/16 to 17/18
> - move patch 15/16 to 18/18
> - new patch 15/18
> - new patch 16/18
>
> - 05/18: add_changeset_node() header comment: incorrect comment for @target
>
> - 18/18: add same fix for of_parse_phandle_with_args()
> - 18/18: add same fix for of_parse_phandle_with_args_map()
>
> Frank Rowand (18):
> of: overlay: add tests to validate kfrees from overlay removal
> of: overlay: add missing of_node_put() after add new node to changeset
> of: overlay: add missing of_node_get() in __of_attach_node_sysfs
> powerpc/pseries: add of_node_put() in dlpar_detach_node()
> of: overlay: use prop add changeset entry for property in new nodes
> of: overlay: do not duplicate properties from overlay for new nodes
> of: dynamic: change type of of_{at,de}tach_node() to void
> of: overlay: reorder fields in struct fragment
> of: overlay: validate overlay properties #address-cells and
> #size-cells
> of: overlay: make all pr_debug() and pr_err() messages unique
> of: overlay: test case of two fragments adding same node
> of: overlay: check prevents multiple fragments add or delete same node
> of: overlay: check prevents multiple fragments touching same property
> of: unittest: remove unused of_unittest_apply_overlay() argument
> of: overlay: set node fields from properties when add new overlay node
> of: unittest: allow base devicetree to have symbol metadata
> of: unittest: find overlays[] entry by name instead of index
> of: unittest: initialize args before calling of_*parse_*()
>
> arch/powerpc/platforms/pseries/dlpar.c | 15 +-
> arch/powerpc/platforms/pseries/reconfig.c | 6 +-
> drivers/of/dynamic.c | 68 +++--
> drivers/of/kobj.c | 4 +-
> drivers/of/overlay.c | 298 ++++++++++++++++-----
> drivers/of/unittest-data/Makefile | 2 +
> .../of/unittest-data/overlay_bad_add_dup_node.dts | 28 ++
> .../of/unittest-data/overlay_bad_add_dup_prop.dts | 24 ++
> drivers/of/unittest-data/overlay_base.dts | 1 +
> drivers/of/unittest.c | 96 +++++--
> include/linux/of.h | 19 +-
> 11 files changed, 439 insertions(+), 122 deletions(-)
> create mode 100644 drivers/of/unittest-data/overlay_bad_add_dup_node.dts
> create mode 100644 drivers/of/unittest-data/overlay_bad_add_dup_prop.dts
>
> --
> Frank Rowand <frank.rowand@sony.com>
>
^ permalink raw reply
* Re: [PATCH v3 09/18] of: overlay: validate overlay properties #address-cells and #size-cells
From: Alan Tull @ 2018-10-15 19:01 UTC (permalink / raw)
To: Frank Rowand
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-fpga, Pantelis Antoniou, linux-kernel, Rob Herring,
Moritz Fischer, Paul Mackerras, linuxppc-dev
In-Reply-To: <1539563070-12969-10-git-send-email-frowand.list@gmail.com>
On Sun, Oct 14, 2018 at 7:26 PM <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> If overlay properties #address-cells or #size-cells are already in
> the live devicetree for any given node, then the values in the
> overlay must match the values in the live tree.
>
> If the properties are already in the live tree then there is no
> need to create a changeset entry to add them since they must
> have the same value. This reduces the memory used by the
> changeset and eliminates a possible memory leak. This is
> verified by 12 fewer warnings during the devicetree unittest,
> as the possible memory leak warnings about #address-cells and
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
> drivers/of/overlay.c | 38 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 272a0d1a5e18..ee66651db553 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -287,7 +287,12 @@ static struct property *dup_and_fixup_symbol_prop(
> * @target may be either in the live devicetree or in a new subtree that
> * is contained in the changeset.
> *
> - * Some special properties are not updated (no error returned).
> + * Some special properties are not added or updated (no error returned):
> + * "name", "phandle", "linux,phandle".
> + *
> + * Properties "#address-cells" and "#size-cells" are not updated if they
> + * are already in the live tree, but if present in the live tree, the values
> + * in the overlay must match the values in the live tree.
> *
> * Update of property in symbols node is not allowed.
> *
> @@ -300,6 +305,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
> {
> struct property *new_prop = NULL, *prop;
> int ret = 0;
> + bool check_for_non_overlay_node = false;
>
> if (!of_prop_cmp(overlay_prop->name, "name") ||
> !of_prop_cmp(overlay_prop->name, "phandle") ||
> @@ -322,13 +328,39 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
> if (!new_prop)
> return -ENOMEM;
>
> - if (!prop)
> + if (!prop) {
> +
> + check_for_non_overlay_node = true;
> ret = of_changeset_add_property(&ovcs->cset, target->np,
> new_prop);
> - else
> +
> + } else if (!of_prop_cmp(prop->name, "#address-cells")) {
> +
Hi Frank,
If we get these ERROR messages, I suggest that this function should
return an error so the overlay will be rejected.
> + if (prop->length != 4 || new_prop->length != 4 ||
> + *(u32 *)prop->value != *(u32 *)new_prop->value)
*(u32 *)prop->value != *(u32 *)new_prop->value) {
> + pr_err("ERROR: overlay and/or live tree #address-cells invalid in node %pOF\n",
> + target->np);
ret = -EINVAL;
}
Otherwise there is an ERROR message, but it continues trying to apply
the invalid overlay anyway and I get an oops. By adding the ret =
-EINVAL, the overlay gets rejected and the oops is avoided.
> +
> + } else if (!of_prop_cmp(prop->name, "#size-cells")) {
> +
> + if (prop->length != 4 || new_prop->length != 4 ||
> + *(u32 *)prop->value != *(u32 *)new_prop->value)
> + pr_err("ERROR: overlay and/or live tree #size-cells invalid in node %pOF\n",
> + target->np);
Add the ret = -EINVAL here also. This give me the following (if my
overlay changes #address-cells):
[ 21.167551] OF: overlay: ERROR: overlay and/or live tree
#address-cells invalid in node /soc/base_fpga_region
[ 21.177442] OF: overlay: add_changeset_property ret=-22
[ 21.182656] create_overlay: Failed to create overlay (err=-22)
Also, I wonder if the ERROR message could be more direct. Currently
it says the #address-cells property is invalid but that doesn't say
anything about why it's invalid. How about something like:
OF: overlay: ERROR: changing #address-cells not allowed (/soc/base_fpga_region)
The 'OF: overlay' part still makes it clear it's overlay related. The
rest of it makes it clear *why* it's invalid. This ERROR will be a
surprise for people who have been using overlays, so that could be
helpful to light the way a bit.
Alan
> +
> + } else {
> +
> + check_for_non_overlay_node = true;
> ret = of_changeset_update_property(&ovcs->cset, target->np,
> new_prop);
>
> + }
> +
> + if (check_for_non_overlay_node &&
> + !of_node_check_flag(target->np, OF_OVERLAY))
> + pr_err("WARNING: %s(), memory leak will occur if overlay removed. Property: %pOF/%s\n",
> + __func__, target->np, new_prop->name);
> +
> if (ret) {
> kfree(new_prop->name);
> kfree(new_prop->value);
> --
> Frank Rowand <frank.rowand@sony.com>
>
^ permalink raw reply
* [PATCH RFC] powerpc/ftrace: Handle large kernel configs
From: Naveen N. Rao @ 2018-10-15 18:57 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, mikey; +Cc: linuxppc-dev
Currently, we expect to be able to reach ftrace_caller() from all
ftrace-enabled functions through a single relative branch. With large
kernel configs, we see functions farther than 32MB of ftrace_caller()
causing ftrace_init() to bail.
One way to solve this is by adding additional trampolines around .text,
.init.text and any other sections with profiled functions. However,
such trampolines only help if a section does not exceed 64MB. With
allyesconfig, .text section alone can grow upwards of 100MB, which will
then require us to insert trampolines in the middle of .text... somehow.
In such configurations, gcc/ld emits two types of trampolines for mcount():
1. A long_branch, which has a single branch to mcount() for functions that
are one hop away from mcount():
c0000000019e8544 <00031b56.long_branch._mcount>:
c0000000019e8544: 4a 69 3f ac b c00000000007c4f0 <._mcount>
2. A plt_branch, for functions that are farther away from mcount():
c0000000051f33f8 <0008ba04.plt_branch._mcount>:
c0000000051f33f8: 3d 82 ff a4 addis r12,r2,-92
c0000000051f33fc: e9 8c 04 20 ld r12,1056(r12)
c0000000051f3400: 7d 89 03 a6 mtctr r12
c0000000051f3404: 4e 80 04 20 bctr
We can reuse those trampolines for ftrace if we can have those
trampolines go to ftrace_caller() instead. On powerpc, we don't support
!CONFIG_DYNAMIC_FTRACE anymore. As such, we can simply patch mcount() to
branch to ftrace_caller() (or to ftrace_regs_caller() on
-mprofile-kernel) allowing us to use those gcc-generated trampolines for
ftrace.
We note down all the existing gcc-generated trampolines during
ftrace_init() and patch branches to those if ftrace_caller() is not
reachable.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
The one aspect I am not entirely sure about is if the plt_branch is fine
for -mprofile-kernel as it depends on r2 being properly setup. If it
isn't, we will have to setup separate trampolines just for
-mprofile-kernel.
- Naveen
arch/powerpc/kernel/trace/ftrace.c | 131 ++++++++++++++++++++++++++++-
1 file changed, 129 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 4bfbb54dee51..5fcc05866a23 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -30,6 +30,10 @@
#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define NUM_FTRACE_TRAMPS 8
+static unsigned long ftrace_cc_tramps[NUM_FTRACE_TRAMPS];
+
static unsigned int
ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
{
@@ -270,6 +274,52 @@ __ftrace_make_nop(struct module *mod,
#endif /* PPC64 */
#endif /* CONFIG_MODULES */
+static void add_ftrace_cc_tramp(unsigned long tramp)
+{
+ int i;
+
+ for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
+ if (!ftrace_cc_tramps[i]) {
+ ftrace_cc_tramps[i] = tramp;
+ return;
+ } else if (ftrace_cc_tramps[i] == tramp)
+ return;
+
+ WARN(1, "No ftrace cc tramp slots available");
+}
+
+static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
+{
+ unsigned long tramp, ip = rec->ip;
+ unsigned int op;
+
+ /* read where this goes */
+ if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
+ pr_err("Fetching opcode failed.\n");
+ return -EFAULT;
+ }
+
+ /* Make sure that that this is still a 24bit jump */
+ if (!is_bl_op(op)) {
+ pr_err("Not expected bl: opcode is %x\n", op);
+ return -EINVAL;
+ }
+
+ /* lets find where the pointer goes */
+ tramp = find_bl_target(ip, op);
+
+ pr_devel("ip:%lx jumps to %lx", ip, tramp);
+
+ add_ftrace_cc_tramp(tramp);
+
+ if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
+ pr_err("Patching NOP failed.\n");
+ return -EPERM;
+ }
+
+ return 0;
+}
+
int ftrace_make_nop(struct module *mod,
struct dyn_ftrace *rec, unsigned long addr)
{
@@ -286,7 +336,8 @@ int ftrace_make_nop(struct module *mod,
old = ftrace_call_replace(ip, addr, 1);
new = PPC_INST_NOP;
return ftrace_modify_code(ip, old, new);
- }
+ } else if (core_kernel_text(ip))
+ return __ftrace_make_nop_kernel(rec, addr);
#ifdef CONFIG_MODULES
/*
@@ -456,6 +507,40 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
#endif /* CONFIG_PPC64 */
#endif /* CONFIG_MODULES */
+static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
+{
+ unsigned int op, i;
+ void *ip = (void *)rec->ip;
+
+ /* read where this goes */
+ if (probe_kernel_read(&op, ip, sizeof(op)))
+ return -EFAULT;
+
+ if (op != PPC_INST_NOP) {
+ pr_err("Unexpected call sequence at %p: %x\n", ip, op);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < NUM_FTRACE_TRAMPS; i++) {
+ if (!ftrace_cc_tramps[i])
+ break;
+
+ if (!create_branch(ip, ftrace_cc_tramps[i], BRANCH_SET_LINK))
+ continue;
+
+ if (patch_branch(ip, ftrace_cc_tramps[i], BRANCH_SET_LINK)) {
+ pr_err("Error patching branch to ftrace tramp!\n");
+ return -EINVAL;
+ }
+
+ return 0;
+ }
+
+ pr_err("No trampolines are reachable from %p\n", ip);
+
+ return -EINVAL;
+}
+
int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
{
unsigned long ip = rec->ip;
@@ -471,7 +556,8 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
old = PPC_INST_NOP;
new = ftrace_call_replace(ip, addr, 1);
return ftrace_modify_code(ip, old, new);
- }
+ } else if (core_kernel_text(ip))
+ return __ftrace_make_call_kernel(rec, addr);
#ifdef CONFIG_MODULES
/*
@@ -603,6 +689,12 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
old = ftrace_call_replace(ip, old_addr, 1);
new = ftrace_call_replace(ip, addr, 1);
return ftrace_modify_code(ip, old, new);
+ } else if (core_kernel_text(ip)) {
+ /*
+ * We always patch out of range locations to go to the regs
+ * variant, so there is nothing to do here
+ */
+ return 0;
}
#ifdef CONFIG_MODULES
@@ -654,8 +746,43 @@ void arch_ftrace_update_code(int command)
ftrace_modify_all_code(command);
}
+/*
+ * Patch _mcount() to jump to ftrace_caller/ftrace_regs_caller for catching
+ * ftrace entry from far functions in a large kernel.
+ */
int __init ftrace_dyn_arch_init(void)
{
+ unsigned long ip = ppc_global_function_entry((void *)_mcount);
+ unsigned long ftrace_call_entry;
+ unsigned int op;
+
+#ifdef CONFIG_MPROFILE_KERNEL
+ ftrace_call_entry = (unsigned long)ftrace_regs_caller;
+#else
+ ftrace_call_entry = ppc_global_function_entry((void *)ftrace_caller);
+#endif
+
+ if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE)) {
+ pr_err("Fetching instruction at %lx failed.\n", ip);
+ return -EFAULT;
+ }
+
+ /* We expect either a mflr r0 (ppc32), or a mflr r12 (ppc64) */
+ if ((op & 0xfc1fffff) != PPC_INST_MFLR) {
+ pr_err("Unexpected instruction %08x in _mcount()\n", op);
+ return -EINVAL;
+ }
+
+ if (!create_branch((unsigned int *)ip, ftrace_call_entry, 0)) {
+ pr_err("Branch out of range\n");
+ return -EINVAL;
+ }
+
+ if (patch_branch((unsigned int *)ip, ftrace_call_entry, 0)) {
+ pr_err("REL24 out of range!\n");
+ return -EINVAL;
+ }
+
return 0;
}
#endif /* CONFIG_DYNAMIC_FTRACE */
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 1/7] dmaengine: fsldma: Replace DMA_IN/OUT by FSL_DMA_IN/OUT
From: Vinod @ 2018-10-15 17:07 UTC (permalink / raw)
To: Peng Ma
Cc: mark.rutland, devicetree, Wen He, linuxppc-dev, linux-kernel,
leoyang.li, zw, robh+dt, dmaengine, dan.j.williams, shawnguo,
linux-arm-kernel
In-Reply-To: <20181011094655.45707-1-peng.ma@nxp.com>
On 11-10-18, 17:46, Peng Ma wrote:
> From: Wen He <wen.he_1@nxp.com>
>
> This patch implement a standard macro call functions is
> used to NXP dma drivers.
>
> Signed-off-by: Wen He <wen.he_1@nxp.com>
Please read Documentation/process/submitting-patches.rst, we expect each
patch you send to have your signed off and signed off from orignal
authors
Also as Rob pointed out this patch series should be v8 or so as this is
following the previous work submitted to the list
Please also indicated changes from last rev which help in the review.
--
~Vinod
^ permalink raw reply
* RE: [PATCH v4 5/6] arm64: dts: add QorIQ LX2160A SoC support
From: Stephen Boyd @ 2018-10-15 16:48 UTC (permalink / raw)
To: Shawn Guo, Vabhav Sharma
Cc: mark.rutland@arm.com, kstewart@linuxfoundation.org,
Yogesh Narayan Gaur, linux-kernel-owner@vger.kernel.org,
catalin.marinas@arm.com, mturquette@baylibre.com,
will.deacon@arm.com, yamada.masahiro@socionext.com, Sriram Dash,
linux-clk@vger.kernel.org, Pankaj Bansal, Udit Kumar,
linux@armlinux.org.uk, Priyanka Jain, viresh.kumar@linaro.org,
devicetree@vger.kernel.org, arnd@arndb.de,
linux-pm@vger.kernel.org, oss@buserror.net, robh+dt@kernel.org,
Varun Sethi, Nipun Gupta, linux-arm-kernel@lists.infradead.org,
Ramneek Mehresh, gregkh@linuxfoundation.org, Ying Zhang,
rjw@rjwysocki.net, linux-kernel@vger.kernel.org, Leo Li,
sudeep.holla@arm.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <VI1PR04MB4800C0AB214B258793AF9ABAF3FD0@VI1PR04MB4800.eurprd04.prod.outlook.com>
Quoting Vabhav Sharma (2018-10-14 19:58:15)
> > > +
> > > + pmu {
> > > + compatible = "arm,cortex-a72-pmu";
> > > + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> > > + };
> > > +
> > > + psci {
> > > + compatible = "arm,psci-0.2";
> > > + method = "smc";
> > > + };
> > > +
> > > + memory@80000000 {
> > > + // DRAM space - 1, size : 2 GB DRAM
> > > + device_type = "memory";
> > > + reg = <0x00000000 0x80000000 0 0x80000000>;
> > > + };
> > > +
> > > + ddr1: memory-controller@1080000 {
> > > + compatible = "fsl,qoriq-memory-controller";
> > > + reg = <0x0 0x1080000 0x0 0x1000>;
> > > + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> > > + little-endian;
> > > + };
> > > +
> > > + ddr2: memory-controller@1090000 {
> > > + compatible = "fsl,qoriq-memory-controller";
> > > + reg = <0x0 0x1090000 0x0 0x1000>;
> > > + interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> > > + little-endian;
> > > + };
> > > +
> > > + sysclk: sysclk {
> >
> > Name the node a bit generic like clock-xxx.
> There is only one clock-unit, Bootloader(U-boot) require sysclk node during device tree fix-up as different platform support varied platform frequency as per reset configuration word used.
> Referred other ARM based platform with one clock using name as x: x
Please add a comment above this node with this information. Newcomers
reading this DTS file won't have any idea why this node is specially
named and a comment will help tremendously here.
^ permalink raw reply
* Re: Stack protector crash in pnv_smp_cpu_kill_self()
From: Christophe LEROY @ 2018-10-15 13:29 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev@ozlabs.org; +Cc: Abdul Haleem
In-Reply-To: <87k1mjz6hd.fsf@concordia.ellerman.id.au>
Looks like a lack of initialisation of the canary for the non-boot CPUs
on SMP, you applied this morning the patch I sent you for that.
Is the patch in ?
Christophe
Le 15/10/2018 à 15:26, Michael Ellerman a écrit :
> Hi all,
>
> Spotted this today, haven't had time to debug it further, just FYI in
> case anyone else sees it.
>
> Running tests in cpufreq
> ========================================
> selftests: cpufreq: main.sh
> pid 9727's current affinity mask: ffffffffffffffffffffffffffffffffffffffffffff
> pid 9727's new affinity mask: 1
> Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: pnv_smp_cpu_kill_self+0x2a0/0x2b0
>
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc3-gcc-7.3.1-00168-g4ffe713b7587 #94
> Call Trace:
> [c000000007a1fb00] [c000000000ae7b4c] dump_stack+0xb0/0xf4 (unreliable)
> [c000000007a1fb40] [c0000000000e59cc] panic+0x144/0x328
> [c000000007a1fbe0] [c0000000000e544c] __stack_chk_fail+0x2c/0x30
> [c000000007a1fc40] [c00000000009eca0] pnv_smp_cpu_kill_self+0x2a0/0x2b0
> [c000000007a1fe10] [c0000000000475f8] cpu_die+0x48/0x70
> [c000000007a1fe30] [c000000000020620] arch_cpu_idle_dead+0x20/0x40
> [c000000007a1fe50] [c00000000012da94] do_idle+0x274/0x390
> [c000000007a1fec0] [c00000000012de08] cpu_startup_entry+0x38/0x50
> [c000000007a1fef0] [c000000000047334] start_secondary+0x5e4/0x600
> [c000000007a1ff90] [c00000000000ac70] start_secondary_prolog+0x10/0x14
> Rebooting in 10 seconds..
> [39378.502863506,5] OPAL: Reboot request
>
>
>
> cheers
>
^ permalink raw reply
* Stack protector crash in pnv_smp_cpu_kill_self()
From: Michael Ellerman @ 2018-10-15 13:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Abdul Haleem
Hi all,
Spotted this today, haven't had time to debug it further, just FYI in
case anyone else sees it.
Running tests in cpufreq
========================================
selftests: cpufreq: main.sh
pid 9727's current affinity mask: ffffffffffffffffffffffffffffffffffffffffffff
pid 9727's new affinity mask: 1
Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: pnv_smp_cpu_kill_self+0x2a0/0x2b0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc3-gcc-7.3.1-00168-g4ffe713b7587 #94
Call Trace:
[c000000007a1fb00] [c000000000ae7b4c] dump_stack+0xb0/0xf4 (unreliable)
[c000000007a1fb40] [c0000000000e59cc] panic+0x144/0x328
[c000000007a1fbe0] [c0000000000e544c] __stack_chk_fail+0x2c/0x30
[c000000007a1fc40] [c00000000009eca0] pnv_smp_cpu_kill_self+0x2a0/0x2b0
[c000000007a1fe10] [c0000000000475f8] cpu_die+0x48/0x70
[c000000007a1fe30] [c000000000020620] arch_cpu_idle_dead+0x20/0x40
[c000000007a1fe50] [c00000000012da94] do_idle+0x274/0x390
[c000000007a1fec0] [c00000000012de08] cpu_startup_entry+0x38/0x50
[c000000007a1fef0] [c000000000047334] start_secondary+0x5e4/0x600
[c000000007a1ff90] [c00000000000ac70] start_secondary_prolog+0x10/0x14
Rebooting in 10 seconds..
[39378.502863506,5] OPAL: Reboot request
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/book3s64: fix dump_linuxpagetables "present" flag
From: Aneesh Kumar K.V @ 2018-10-15 12:55 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <23d27ae15a7b0389fc26f2265f969c6c69f44009.1539433567.git.christophe.leroy@c-s.fr>
On 10/15/18 12:07 PM, Christophe Leroy wrote:
> Since commit bd0dbb73e013 ("powerpc/mm/books3s: Add new pte bit to
> mark pte temporarily invalid."), _PAGE_PRESENT doesn't mean exactly
> that a page is present. A page is also considered preset when
> _PAGE_INVALID is set.
>
> This patch changes the meaning of "present" and adds a status "valid"
> associated to the _PAGE_PRESENT flag.
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Fixes: bd0dbb73e013 ("powerpc/mm/books3s: Add new pte bit to mark pte temporarily invalid.")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/mm/dump_linuxpagetables-book3s64.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/dump_linuxpagetables-book3s64.c b/arch/powerpc/mm/dump_linuxpagetables-book3s64.c
> index a637e612b205..ed6fcf78256e 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables-book3s64.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables-book3s64.c
> @@ -38,8 +38,13 @@ static const struct flag_info flag_array[] = {
> }, {
> .mask = _PAGE_PRESENT,
> .val = _PAGE_PRESENT,
> - .set = "present",
> - .clear = " ",
> + .set = "valid",
> + .clear = " ",
> + }, {
> + .mask = _PAGE_PRESENT | _PAGE_INVALID,
> + .val = 0,
> + .set = " ",
> + .clear = "present",
> }, {
> .mask = H_PAGE_HASHPTE,
> .val = H_PAGE_HASHPTE,
>
^ permalink raw reply
* Re: linux-next: qemu boot failures with today's linux-next
From: Michael Ellerman @ 2018-10-15 12:45 UTC (permalink / raw)
To: Stephen Rothwell, Benjamin Herrenschmidt, PowerPC
Cc: Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20181015191859.2f600de5@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi all,
>
> Today's linux-next tree does not boot in qemu (with kvm or without).
> Here is the (non-kvm) boot log (the kvm one is basically the same):
>
> ------------------------------------------------------
...
> Welcome to Open Firmware
>
> Copyright (c) 2004, 2017 IBM Corporation All rights reserved.
> This program and the accompanying materials are made available
> under the terms of the BSD License available at
> http://www.opensource.org/licenses/bsd-license.php
>
> Booting from memory...
> OF stdout device is: /vdevice/vty@71000000
> Preparing to boot Linux version 4.19.0-rc7 (sfr@ash) (gcc version 8.2.0 (Debian 8.2.0-4)) #2 SMP Mon Oct 15 18:53:28 AEDT 2018
^^^^^^^^^^
I assume that's wrong, this is actually linux-next you're booting?
> Detected machine type: 0000000000000101
> command line:
> Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
> Calling ibm,client-architecture-support... done
> memory layout at init:
> memory_limit : 0000000000000000 (16 MB aligned)
> alloc_bottom : 0000000001640000
> alloc_top : 0000000030000000
> alloc_top_hi : 0000000080000000
> rmo_top : 0000000030000000
> ram_top : 0000000080000000
> instantiating rtas at 0x000000002fff0000... done
> prom_hold_cpus: skipped
> copying OF device tree...
> Building dt strings...
> Building dt structure...
> Device tree strings 0x0000000001850000 -> 0x0000000001850a02
> Device tree struct 0x0000000001860000 -> 0x0000000001870000
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x0000000000400000 ...
> ------------------------------------------------------
>
> I have no idea what may have caused this.
If you git Ctrl-a-c you should get the qemu prompt. Then you can run
'info registers' to print the regs and maybe see where it's stuck.
And/or build with EARLY_DEBUG_LPAR to get early console output.
cheers
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs
From: Madhavan Srinivasan @ 2018-10-15 12:39 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <87sh17zcur.fsf@concordia.ellerman.id.au>
On Monday 15 October 2018 04:38 PM, Michael Ellerman wrote:
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
>
>> On Saturday 13 October 2018 04:26 PM, Michael Ellerman wrote:
>>> We use a shared definition for struct pt_regs in uapi/asm/ptrace.h.
>>> That means the layout of the structure is ABI, ie. we can't change it.
>>>
>>> That would be fine if it was only used to describe the user-visible
>>> register state of a process, but it's also the struct we use in the
>>> kernel to describe the registers saved in an interrupt frame.
>>>
>>> We'd like more flexibility in the content (and possibly layout) of the
>>> kernel version of the struct, but currently that's not possible.
>>>
>>> So split the definition into a user-visible definition which remains
>>> unchanged, and a kernel internal one.
>>>
>>> At the moment they're still identical, and we check that at build
>>> time. That's because we have code (in ptrace etc.) that assumes that
>>> they are the same. We will fix that code in future patches, and then
>>> we can break the strict symmetry between the two structs.
>> Nice and awesome. But just trying to understand. What will
>> *regs will point to in the "struct sigcontext".
> Yeah that's a bit fishy.
>
> It should always point to a user_pt_regs.
>
> So in the kernel we want:
>
> struct sigcontext {
> ...
> struct user_pt_regs __user *regs;
>
> And in userspace we want:
>
> struct sigcontext {
> ...
> struct pt_regs __user *regs;
>
>
> I think it's not actually broken at the moment, because it's just a
> pointer, and we don't do anything based on the sizeof() the type.
yes. This clarifies. But still perf/perf_regs.c needs changes.
Because perf support dumping user_space regs and interrupt regs.
Once again, we dont use any sizeof(), but need to handle the
user_pt_regs changes.
I will have a look at that in the morning.
Thanks for clarification.
Maddy
>
> But still we should fix it.
>
> I guess I'll do this:
>
> diff --git a/arch/powerpc/include/uapi/asm/sigcontext.h b/arch/powerpc/include/uapi/asm/sigcontext.h
> index 2fbe485acdb4..630aeda56d59 100644
> --- a/arch/powerpc/include/uapi/asm/sigcontext.h
> +++ b/arch/powerpc/include/uapi/asm/sigcontext.h
> @@ -22,7 +22,11 @@ struct sigcontext {
> #endif
> unsigned long handler;
> unsigned long oldmask;
> - struct pt_regs __user *regs;
> +#ifdef __KERNEL__
> + struct user_pt_regs __user *regs;
> +#else
> + struct pt_regs *regs;
> +#endif
> #ifdef __powerpc64__
> elf_gregset_t gp_regs;
> elf_fpregset_t fp_regs;
>
>
> Thanks for the review.
>
> cheers
>
^ permalink raw reply
* Re: 32-bit poweroc compile failure in v4.19-rc7-166-g7ec21823634d
From: Meelis Roos @ 2018-10-15 12:20 UTC (permalink / raw)
To: Michael Ellerman, linux-kernel, linuxppc-dev
In-Reply-To: <87tvlnzegi.fsf@concordia.ellerman.id.au>
>> I tried to test the fix to 32-bit poweroc boot hang but found that current git does not compile on 32bit poweroc at all for me:
>
> That's GCC 8 I think?
Yes, gcc version 8.2.0 (Debian 8.2.0-7).
> I have seen that but couldn't work out what the hell GCC is thinking.
>
> I don't think there's an actual bug, the size of the copy is bounded by
> the count parameter, which *doesn't* come from user space, it's hard
> coded.
>
> I suspect the logic is just to convoluted for GCC.
>
> We should fix it somehow, but I haven't worked out what's the best option.
>
> cheers
--
Meelis Roos <mroos@linux.ee>
^ permalink raw reply
* Re: [PATCH V4 00/15] x86/KVM/Hyper-v: Add HV ept tlb range flush hypercall support in KVM
From: Paolo Bonzini @ 2018-10-15 12:04 UTC (permalink / raw)
To: lantianyu1986
Cc: linux-mips, linux, kvm, rkrcmar, will.deacon, linux-kernel, hpa,
kys, kvmarm, sthemmin, x86, michael.h.kelley, mingo,
catalin.marinas, jhogan, Lan Tianyu, marc.zyngier, haiyangz,
kvm-ppc, tglx, linux-arm-kernel, christoffer.dall, ralf,
paul.burton, devel, vkuznets, linuxppc-dev
In-Reply-To: <20181013145406.4911-1-Tianyu.Lan@microsoft.com>
On 13/10/2018 16:53, lantianyu1986@gmail.com wrote:
> From: Lan Tianyu <Tianyu.Lan@microsoft.com>
>
> For nested memory virtualization, Hyper-v doesn't set write-protect
> L1 hypervisor EPT page directory and page table node to track changes
> while it relies on guest to tell it changes via HvFlushGuestAddressLlist
> hypercall. HvFlushGuestAddressLlist hypercall provides a way to flush
> EPT page table with ranges which are specified by L1 hypervisor.
>
> If L1 hypervisor uses INVEPT or HvFlushGuestAddressSpace hypercall to
> flush EPT tlb, Hyper-V will invalidate associated EPT shadow page table
> and sync L1's EPT table when next EPT page fault is triggered.
> HvFlushGuestAddressLlist hypercall helps to avoid such redundant EPT
> page fault and synchronization of shadow page table.
So I just told you that the first part is well understood but I must
retract that; after carefully reviewing the whole series, I think one of
us is actually very confused.
I am not afraid to say it can be me, but my understanding is that you're
passing L1 GPAs to the hypercall and instead the spec says it expects L2
GPAs. (Consider that, because KVM's shadow paging code is shared
between nested EPT and !EPT cases, every time you see gpa/gfn in the
code it is for L1, while nested EPT GPAs are really what the code calls
gva.)
What's going on?
Paolo
^ permalink raw reply
* Re: [PATCH V4 2/15] KVM/MMU: Add tlb flush with range helper function
From: Paolo Bonzini @ 2018-10-15 12:02 UTC (permalink / raw)
To: Thomas Gleixner, Liran Alon
Cc: linux-mips, linux, kvm, rkrcmar, catalin.marinas, will.deacon,
linux-kernel, hpa, kys, kvmarm, lantianyu1986, sthemmin, x86,
michael.h.kelley, mingo, jhogan, Lan Tianyu, marc.zyngier,
haiyangz, kvm-ppc, linux-arm-kernel, christoffer.dall, ralf,
paul.burton, devel, vkuznets, linuxppc-dev
In-Reply-To: <alpine.DEB.2.21.1810141014350.1438@nanos.tec.linutronix.de>
On 14/10/2018 10:16, Thomas Gleixner wrote:
>>> +static inline bool kvm_available_flush_tlb_with_range(void)
>>> +{
>>> + return kvm_x86_ops->tlb_remote_flush_with_range;
>>> +}
>> Seems that kvm_available_flush_tlb_with_range() is not used in this patch…
> What's wrong with that?
>
> It provides the implementation and later patches make use of it. It's a
> sensible way to split patches into small, self contained entities.
That's true, on the other hand I have indeed a concerns with this patch:
this series is not bisectable at all, because all the new code is dead
until the very last patch. Uses of the new feature should come _after_
the implementation.
I don't have any big problem with what Liran pointed out (and I can live
with the unused static functions that would warn with -Wunused, too),
but the above should be fixed in v5, basically by moving patches 12-15
at the beginning of the series.
Paolo
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs
From: Michael Ellerman @ 2018-10-15 12:02 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20181013222756.2e9ca459@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Sat, 13 Oct 2018 21:56:44 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> We use a shared definition for struct pt_regs in uapi/asm/ptrace.h.
>> That means the layout of the structure is ABI, ie. we can't change it.
>>
>> That would be fine if it was only used to describe the user-visible
>> register state of a process, but it's also the struct we use in the
>> kernel to describe the registers saved in an interrupt frame.
>>
>> We'd like more flexibility in the content (and possibly layout) of the
>> kernel version of the struct, but currently that's not possible.
>>
>> So split the definition into a user-visible definition which remains
>> unchanged, and a kernel internal one.
>>
>> At the moment they're still identical, and we check that at build
>> time. That's because we have code (in ptrace etc.) that assumes that
>> they are the same. We will fix that code in future patches, and then
>> we can break the strict symmetry between the two structs.
>>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Yeah this looks much better than my int_frame thing, thanks.
Thanks. More bug prone to :)
But hopefully it will pay off in the long run.
cheers
^ permalink raw reply
* [PATCH 2/2] powerpc/aout: Fix struct user definition to use user_pt_regs
From: Michael Ellerman @ 2018-10-15 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: maddy
In-Reply-To: <20181015120143.1248-1-mpe@ellerman.id.au>
I'm pretty sure this is dead code, it's only used by the a.out core
dump code, and we don't support a.out. We should remove it.
But while it's in the tree it should be using the ABI version of
pt_regs which is called user_pt_regs in the kernel, because the whole
struct is written to the core dump and so its size shouldn't change.
Note this isn't a uapi header so we don't need an ifdef.
Fixes: 002af9391bfb ("powerpc: Split user/kernel definitions of struct pt_regs")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/user.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/user.h b/arch/powerpc/include/asm/user.h
index 5c0e082eae7b..99443b8594e7 100644
--- a/arch/powerpc/include/asm/user.h
+++ b/arch/powerpc/include/asm/user.h
@@ -31,7 +31,7 @@
* to write an integer number of pages.
*/
struct user {
- struct pt_regs regs; /* entire machine state */
+ struct user_pt_regs regs; /* entire machine state */
size_t u_tsize; /* text size (pages) */
size_t u_dsize; /* data size (pages) */
size_t u_ssize; /* stack size (pages) */
--
2.17.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/uapi: Fix sigcontext definition to use user_pt_regs
From: Michael Ellerman @ 2018-10-15 12:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: maddy
My recent patch to split pt_regs between user and kernel missed
the usage in struct sigcontext.
Because this is a user visible struct it should be using the user
visible definition, which when we're building for the kernel is called
struct user_pt_regs.
As far as I can see this hasn't actually caused a bug (yet), because
we don't use the sizeof() the sigcontext->regs anywhere. But we should
still fix it to avoid confusion and future bugs.
Fixes: 002af9391bfb ("powerpc: Split user/kernel definitions of struct pt_regs")
Reported-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/uapi/asm/sigcontext.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/uapi/asm/sigcontext.h b/arch/powerpc/include/uapi/asm/sigcontext.h
index 2fbe485acdb4..630aeda56d59 100644
--- a/arch/powerpc/include/uapi/asm/sigcontext.h
+++ b/arch/powerpc/include/uapi/asm/sigcontext.h
@@ -22,7 +22,11 @@ struct sigcontext {
#endif
unsigned long handler;
unsigned long oldmask;
- struct pt_regs __user *regs;
+#ifdef __KERNEL__
+ struct user_pt_regs __user *regs;
+#else
+ struct pt_regs *regs;
+#endif
#ifdef __powerpc64__
elf_gregset_t gp_regs;
elf_fpregset_t fp_regs;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs
From: Michael Ellerman @ 2018-10-15 11:08 UTC (permalink / raw)
To: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <458fb5e1-3654-e8db-dfa7-074195decb11@linux.vnet.ibm.com>
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> On Saturday 13 October 2018 04:26 PM, Michael Ellerman wrote:
>> We use a shared definition for struct pt_regs in uapi/asm/ptrace.h.
>> That means the layout of the structure is ABI, ie. we can't change it.
>>
>> That would be fine if it was only used to describe the user-visible
>> register state of a process, but it's also the struct we use in the
>> kernel to describe the registers saved in an interrupt frame.
>>
>> We'd like more flexibility in the content (and possibly layout) of the
>> kernel version of the struct, but currently that's not possible.
>>
>> So split the definition into a user-visible definition which remains
>> unchanged, and a kernel internal one.
>>
>> At the moment they're still identical, and we check that at build
>> time. That's because we have code (in ptrace etc.) that assumes that
>> they are the same. We will fix that code in future patches, and then
>> we can break the strict symmetry between the two structs.
>
> Nice and awesome. But just trying to understand. What will
> *regs will point to in the "struct sigcontext".
Yeah that's a bit fishy.
It should always point to a user_pt_regs.
So in the kernel we want:
struct sigcontext {
...
struct user_pt_regs __user *regs;
And in userspace we want:
struct sigcontext {
...
struct pt_regs __user *regs;
I think it's not actually broken at the moment, because it's just a
pointer, and we don't do anything based on the sizeof() the type.
But still we should fix it.
I guess I'll do this:
diff --git a/arch/powerpc/include/uapi/asm/sigcontext.h b/arch/powerpc/include/uapi/asm/sigcontext.h
index 2fbe485acdb4..630aeda56d59 100644
--- a/arch/powerpc/include/uapi/asm/sigcontext.h
+++ b/arch/powerpc/include/uapi/asm/sigcontext.h
@@ -22,7 +22,11 @@ struct sigcontext {
#endif
unsigned long handler;
unsigned long oldmask;
- struct pt_regs __user *regs;
+#ifdef __KERNEL__
+ struct user_pt_regs __user *regs;
+#else
+ struct pt_regs *regs;
+#endif
#ifdef __powerpc64__
elf_gregset_t gp_regs;
elf_fpregset_t fp_regs;
Thanks for the review.
cheers
^ permalink raw reply related
* Re: 32-bit poweroc compile failure in v4.19-rc7-166-g7ec21823634d
From: Michael Ellerman @ 2018-10-15 10:33 UTC (permalink / raw)
To: Meelis Roos, linux-kernel, linuxppc-dev
In-Reply-To: <17d36e6c-f99a-abfb-12cd-fa4b14bc7e09@linux.ee>
Meelis Roos <mroos@linux.ee> writes:
> I tried to test the fix to 32-bit poweroc boot hang but found that current git does not compile on 32bit poweroc at all for me:
That's GCC 8 I think?
I have seen that but couldn't work out what the hell GCC is thinking.
I don't think there's an actual bug, the size of the copy is bounded by
the count parameter, which *doesn't* come from user space, it's hard
coded.
I suspect the logic is just to convoluted for GCC.
We should fix it somehow, but I haven't worked out what's the best option.
cheers
> CC arch/powerpc/kernel/ptrace.o
> In file included from ./include/linux/bitmap.h:9,
> from ./include/linux/cpumask.h:12,
> from ./include/linux/rcupdate.h:44,
> from ./include/linux/rculist.h:11,
> from ./include/linux/pid.h:5,
> from ./include/linux/sched.h:14,
> from arch/powerpc/kernel/ptrace.c:19:
> In function ‘memcpy’,
> inlined from ‘user_regset_copyin’ at ./include/linux/regset.h:295:4,
> inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:619:9:
> ./include/linux/string.h:345:9: error: ‘__builtin_memcpy’ offset [-527, -529] is out of the bounds [0, 16] of object ‘vrsave’ with type ‘union <anonymous>’ [-Werror=array-bounds]
> return __builtin_memcpy(p, q, size);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
> arch/powerpc/kernel/ptrace.c:614:5: note: ‘vrsave’ declared here
> } vrsave;
> ^~~~~~
> In file included from ./include/linux/bitmap.h:9,
> from ./include/linux/cpumask.h:12,
> from ./include/linux/rcupdate.h:44,
> from ./include/linux/rculist.h:11,
> from ./include/linux/pid.h:5,
> from ./include/linux/sched.h:14,
> from arch/powerpc/kernel/ptrace.c:19:
> In function ‘memcpy’,
> inlined from ‘user_regset_copyout’ at ./include/linux/regset.h:270:4,
> inlined from ‘vr_get’ at arch/powerpc/kernel/ptrace.c:572:9:
> ./include/linux/string.h:345:9: error: ‘__builtin_memcpy’ offset [-527, -529] is out of the bounds [0, 16] of object ‘vrsave’ with type ‘union <anonymous>’ [-Werror=array-bounds]
> return __builtin_memcpy(p, q, size);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/ptrace.c: In function ‘vr_get’:
> arch/powerpc/kernel/ptrace.c:567:5: note: ‘vrsave’ declared here
> } vrsave;
> ^~~~~~
> cc1: all warnings being treated as errors
> make[1]: *** [scripts/Makefile.build:306: arch/powerpc/kernel/ptrace.o] Error 1
> make: *** [Makefile:1052: arch/powerpc/kernel] Error 2
>
> --
> Meelis Roos <mroos@linux.ee>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox