[PATCH 0/8] drmgr: Add multipath partner device support

46 views
Skip to first unread message

Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:10:23 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:14:05 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

get_node_by_name() should return dr_node if the DRC name or DRC
index is matched. But the current code returns only if the DRC
name is matched. This patch fixes this issue and returns dr_node
if the index is matched.

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/common_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/drmgr/common_pci.c b/src/drmgr/common_pci.c
index c6dcfdf..2e0e5fb 100644
--- a/src/drmgr/common_pci.c
+++ b/src/drmgr/common_pci.c
@@ -969,7 +969,7 @@ get_node_by_name(const char *drc_name, uint32_t node_type)
/* See if the drc index was specified */
drc_index = strtoul(drc_name, NULL, 0);
if (node->drc_index == drc_index)
- continue;
+ break;

for (child = node->children; child; child = child->next) {
if (strcmp(drc_name, child->drc_name) == 0)
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:17:05 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

search_drc_list() provides the search based on the key type such
as DRC_NAME, DRC_TYPE, DRC_INDEX or DRC_POWERDOMAIN. But the
current code has get_drc_by_index() which looks in to the DRC list
passed to this function instead of all DRC lists and is not used
right now.

This patch modifies get_drc_by_index() and the corresponding
changes such that it retrieves dr_connector with the specified DRC
index as does in the case of get_drc_by_name().

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/common_ofdt.c | 78 +++++++++++++++++++++++++++++++----------
src/drmgr/ofdt.h | 1 +
2 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/src/drmgr/common_ofdt.c b/src/drmgr/common_ofdt.c
index 1d9b7e7..655c9d2 100644
--- a/src/drmgr/common_ofdt.c
+++ b/src/drmgr/common_ofdt.c
@@ -650,23 +650,24 @@ drc_index_to_name(uint32_t index, struct
dr_connector *drc_list)
}

/**
- * get_drc_by_name
- * @brief Retrieve a dr_connector with the specified drc_name
+ * search_drc_by_key
+ * @brief Retrieve a dr_connector based on DRC name or DRC index
*
* This routine searches the drc lists for a dr_connector with the
- * specified name starting at the specified directory. If a
dr_connector
- * is found the root_dir that the dr_connector was found in is also
- * filled out.
+ * specified name or index starting at the specified directory. If
+ * a dr_connector is found the root_dir that the dr_connector was
+ * found in is also filled out.
*
- * @param drc_name name of the dr_connector to search for
+ * @param key to serach for the dr_connector
* @param drc pointer to a drc to point to the found dr_connector
* @param root_dir pointer to buf to fill in with root directory
* @param start_dir, directory to start searching
+ * @param key_type whether the key is DRC name or DRC index
* @returns 0 on success (drc and root_dir filled in), !0 on failure
*/
int
-get_drc_by_name(char *drc_name, struct dr_connector *drc, char
*root_dir,
- char *start_dir)
+search_drc_by_key(void *key, struct dr_connector *drc, char *root_dir,
+ char *start_dir, int key_type)
{
struct dr_connector *drc_list = NULL;
struct dr_connector *drc_entry;
@@ -681,7 +682,7 @@ get_drc_by_name(char *drc_name, struct dr_connector
*drc, char *root_dir,
if (drc_list == NULL)
return -1;

- drc_entry = search_drc_list(drc_list, NULL, DRC_NAME,
drc_name);
+ drc_entry = search_drc_list(drc_list, NULL, key_type, key);
if (drc_entry != NULL) {
memcpy(drc, drc_entry, sizeof(*drc));
sprintf(root_dir, "%s", start_dir);
@@ -700,7 +701,8 @@ get_drc_by_name(char *drc_name, struct dr_connector
*drc, char *root_dir,
continue;

sprintf(dir_path, "%s/%s", start_dir, de->d_name);
- rc = get_drc_by_name(drc_name, drc, root_dir,
dir_path);
+ rc = search_drc_by_key(key, drc, root_dir, dir_path,
+ key_type);
if (rc == 0)
break;
}
@@ -709,17 +711,57 @@ get_drc_by_name(char *drc_name, struct
dr_connector *drc, char *root_dir,
return rc;
}

-struct dr_connector *
-get_drc_by_index(uint32_t drc_index, struct dr_connector *drc_list)
+/**
+ * get_drc_by_name
+ * @brief Retrieve a dr_connector with the specified drc_name
+ *
+ * This routine searches the drc lists for a dr_connector with the
+ * specified name starting at the specified directory. If a
dr_connector
+ * is found the root_dir that the dr_connector was found in is also
+ * filled out.
+ *
+ * @param drc_name name of the dr_connector to search for
+ * @param drc pointer to a drc to point to the found dr_connector
+ * @param root_dir pointer to buf to fill in with root directory
+ * @param start_dir, directory to start searching
+ * @returns 0 on success (drc and root_dir filled in), !0 on failure
+ */
+int
+get_drc_by_name(char *drc_name, struct dr_connector *drc, char
*root_dir,
+ char *start_dir)
{
- struct dr_connector *drc;
+ int rc;

- for (drc = drc_list; drc; drc = drc->next) {
- if (drc->index == drc_index)
- return drc;
- }
+ rc = search_drc_by_key(drc_name, drc, root_dir, start_dir,
DRC_NAME);

- return NULL;
+ return rc;
+}
+
+/**
+ * get_drc_by_index
+ * @brief Retrieve a dr_connector with the specified index
+ *
+ * This routine searches the drc lists for a dr_connector with the
+ * specified index starting at the specified directory. If a
dr_connector
+ * is found the root_dir that the dr_connector was found in is also
+ * filled out.
+ *
+ * @param index of the dr_connector to search for
+ * @param drc pointer to a drc to point to the found dr_connector
+ * @param root_dir pointer to buf to fill in with root directory
+ * @param start_dir, directory to start searching
+ * @returns 0 on success (drc and root_dir filled in), !0 on failure
+ */
+int
+get_drc_by_index(uint32_t index, struct dr_connector *drc, char
*root_dir,
+ char *start_dir)
+{
+ int rc;
+
+ rc = search_drc_by_key((void *)&index, drc, root_dir,
start_dir,
+ DRC_INDEX);
+
+ return rc;
}

/*
diff --git a/src/drmgr/ofdt.h b/src/drmgr/ofdt.h
index bd90810..6c1b961 100644
--- a/src/drmgr/ofdt.h
+++ b/src/drmgr/ofdt.h
@@ -177,6 +177,7 @@ int get_my_drc_index(char *, uint32_t *);
int drc_name_to_index(const char *, struct dr_connector *);
char * drc_index_to_name(uint32_t, struct dr_connector *);
int get_drc_by_name(char *, struct dr_connector *, char *, char *);
+int get_drc_by_index(uint32_t, struct dr_connector *, char *, char *);

int get_min_common_depth(void);
int get_assoc_arrays(const char *dir, struct assoc_arrays *aa,
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:18:15 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

get_my_partner_drc_index() is called to retrieve DRC index from the
"ibm,multipath-partner-drc" property. This property is available
in the parent device node if the device has miltipath partner device.
"ibm,multipath-partner-drc" has the DRC index of the partner device.

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/common_ofdt.c | 20 ++++++++++++++++++++
src/drmgr/ofdt.h | 1 +
2 files changed, 21 insertions(+)

diff --git a/src/drmgr/common_ofdt.c b/src/drmgr/common_ofdt.c
index 655c9d2..1e5fe53 100644
--- a/src/drmgr/common_ofdt.c
+++ b/src/drmgr/common_ofdt.c
@@ -609,6 +609,26 @@ get_my_drc_index(char *of_path, uint32_t *index)
return rc;
}

+/**
+ * get_my_partner_drc_index
+ *
+ * @param of_full_path
+ * @param index
+ * @returns 0 on success, !0 otherwise
+ */
+int get_my_partner_drc_index(struct dr_node *node, uint32_t *index)
+{
+ int rc;
+
+ if (node == NULL)
+ return -1;
+
+ rc = get_ofdt_uint_property(node->ofdt_path,
+ "ibm,multipath-partner-drc", index);
+
+ return rc;
+}
+
/**
* drc_name_to_index
* @brief Find the drc index for the given name
diff --git a/src/drmgr/ofdt.h b/src/drmgr/ofdt.h
index 6c1b961..08d34e1 100644
--- a/src/drmgr/ofdt.h
+++ b/src/drmgr/ofdt.h
@@ -174,6 +174,7 @@ struct dr_connector *search_drc_list(struct dr_connector *,
struct dr_connector *, int, void *);

int get_my_drc_index(char *, uint32_t *);
+int get_my_partner_drc_index(struct dr_node *, uint32_t *);
int drc_name_to_index(const char *, struct dr_connector *);
char * drc_index_to_name(uint32_t, struct dr_connector *);
int get_drc_by_name(char *, struct dr_connector *, char *, char *);
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:19:37 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

The PHB node has "ibm,multipath-partner-drc" property if the device
has multipath partner device. This property provides the DRC index
of the partner device. For the hotplug removal, both paths must be
removed and the following steps will be executed:

- Find the partner path DRC index from "ibm,multipath-partner-drc"
property for the specified device
- Notify user about the removal of partner path if available
- Remove the primary path
- Remove the partner path

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/drslot_chrp_phb.c | 71 ++++++++++++++++++++++++++++++-------
1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/src/drmgr/drslot_chrp_phb.c b/src/drmgr/drslot_chrp_phb.c
index f59baa4..39f6cc9 100644
--- a/src/drmgr/drslot_chrp_phb.c
+++ b/src/drmgr/drslot_chrp_phb.c
@@ -276,24 +276,18 @@ static int disable_os_hp_children(struct dr_node *phb)
return rc;
}

-/**
- * remove_phb
+/*
+ * find_remove_phb_slot - Find phb children and remove the slot
*
* @returns 0 on success, !0 otherwise
*/
-static int remove_phb(void)
+static int
+find_remove_phb_slot(struct dr_node *phb)
{
- struct dr_node *phb;
+ struct dr_node *hp_list = NULL;
struct dr_node *child;
- struct dr_node *hp_list = NULL;
int rc = 0;

- phb = get_node_by_name(usr_drc_name, PHB_NODES);
- if (phb == NULL) {
- say(ERROR, "Could not find PHB %s\n", usr_drc_name);
- return RC_NONEXISTENT;
- }
-
if (phb_has_display_adapter(phb)) {
say(ERROR, "This PHB contains a display adapter, DLPAR "
"remove of display adapters is not supported.\n");
@@ -354,12 +348,63 @@ static int remove_phb(void)

rc = release_phb(phb);

+phb_remove_error:
+ if (hp_list)
+ free_node(hp_list);
+
+ return rc;
+}
+
+/**
+ * remove_phb
+ *
+ * @returns 0 on success, !0 otherwise
+ */
+static int remove_phb(void)
+{
+ uint32_t partner_drc_index = 0;
+ char drc_index_str[10];
+ struct dr_node *phb, *partner_phb = NULL;
+ int rc = 0;
+
+ phb = get_node_by_name(usr_drc_name, PHB_NODES);
+ if (phb == NULL) {
+ say(ERROR, "Could not find PHB %s\n", usr_drc_name);
+ return RC_NONEXISTENT;
+ }
+
+ /* Find the multipath partner device index if available */
+ rc = get_my_partner_drc_index(phb, &partner_drc_index);
+
+ if (!rc && partner_drc_index) {
+ sprintf(drc_index_str, "%d", partner_drc_index);
+
+ /* Find the partner phb device */
+ partner_phb = get_node_by_name(drc_index_str, PHB_NODES);
+ if (partner_phb == NULL) {
+ say(ERROR, "Could not find partner PHB\n");
+ rc = RC_NONEXISTENT;
+ goto phb_remove_error;
+ }
+ printf("<%s> and <%s> are multipath partner devices.\n"
+ "<%s> is also removed.\n", phb->drc_name,
+ partner_phb->drc_name, partner_phb->drc_name);
+
+ }
+
+ rc = find_remove_phb_slot(phb);
+ /*
+ * Also remove partner device if available
+ */
+ if (!rc && partner_phb)
+ rc = find_remove_phb_slot(partner_phb);
+
phb_remove_error:
if (phb)
free_node(phb);

- if (hp_list)
- free_node(hp_list);
+ if (partner_phb)
+ free_node(partner_phb);

return rc;
}
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:20:27 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

The PHB node has "ibm,multipath-partner-drc" property if the device
has multipath partner device. This property provides the DRC index
of the partner device. So if the PHB node has this property, the
partner path should also be added when the user initiated PHB add.

So the following steps will be executed for the PHB add:
- Add the specified PHB device
- Find the partner path DRC index from "ibm,multipath-partner-drc"
property for the specified device
- Add the partner path
- Notify user about adding both paths

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/drslot_chrp_phb.c | 117 ++++++++++++++++++++++++++----------
1 file changed, 85 insertions(+), 32 deletions(-)

diff --git a/src/drmgr/drslot_chrp_phb.c b/src/drmgr/drslot_chrp_phb.c
index 39f6cc9..6180d70 100644
--- a/src/drmgr/drslot_chrp_phb.c
+++ b/src/drmgr/drslot_chrp_phb.c
@@ -31,7 +31,7 @@
static char *usagestr = "-c phb [-Q | -r | -a] -s <drc_name | drc_index>";

static int release_phb(struct dr_node *);
-static int acquire_phb(char *, struct dr_node **);
+static int acquire_phb(struct dr_connector *, char *, struct dr_node **);


/**
@@ -413,27 +413,19 @@ phb_remove_error:
* acquire_phb
*
*/
-static int acquire_phb(char *drc_name, struct dr_node **phb)
+static int acquire_phb(struct dr_connector *drc, char *path,
+ struct dr_node **phb)
{
- struct dr_connector drc;
struct of_node *of_nodes;
- char path[DR_PATH_MAX];
int rc;

- rc = get_drc_by_name(drc_name, &drc, path, OFDT_BASE);
- if (rc) {
- say(ERROR, "Could not find drc index for %s, unable to add the"
- "PHB.\n", drc_name);
- return rc;
- }
-
- rc = acquire_drc(drc.index);
+ rc = acquire_drc(drc->index);
if (rc)
return rc;

- of_nodes = configure_connector(drc.index);
+ of_nodes = configure_connector(drc->index);
if (of_nodes == NULL) {
- release_drc(drc.index, PHB_DEV);
+ release_drc(drc->index, PHB_DEV);
return -1;
}

@@ -441,7 +433,7 @@ static int acquire_phb(char *drc_name, struct dr_node **phb)
free_of_node(of_nodes);
if (rc) {
say(ERROR, "add_device_tree_nodes failed at %s\n", path);
- release_drc(drc.index, PHB_DEV);
+ release_drc(drc->index, PHB_DEV);
return -1;
}

@@ -449,35 +441,30 @@ static int acquire_phb(char *drc_name, struct dr_node **phb)
* This also acts as a sanity check that everything up to this
* point has succeeded.
*/
- *phb = get_node_by_name(drc_name, PHB_NODES);
+ *phb = get_node_by_name(drc->name, PHB_NODES);
if (*phb == NULL) {
- say(ERROR, "Could not get find \"%s\"\n", drc_name);
+ say(ERROR, "Could not get find \"%s\"\n", drc->name);
/* or should we call release_drc? but need device type */
- release_drc(drc.index, PHB_DEV);
+ release_drc(drc->index, PHB_DEV);
return -1;
}

return 0;
}

-/**
- * add_phb
+/*
+ * add_enable_phb - Add PHB and its children and enable the device
*
* @returns 0 on success, !0 otherwise
*/
-static int add_phb(void)
+static int
+add_enable_phb(struct dr_connector *drc, char *path,
+ struct dr_node **phb_node)
{
- struct dr_node *phb = NULL;
int rc, n_children = 0;
+ struct dr_node *phb;

- phb = get_node_by_name(usr_drc_name, PHB_NODES);
- if (phb) {
- say(ERROR, "PHB is already owned by this partition\n");
- rc = RC_ALREADY_OWN;
- goto phb_add_error;
- }
-
- rc = acquire_phb(usr_drc_name, &phb);
+ rc = acquire_phb(drc, path, &phb);
if (rc)
return rc;

@@ -487,7 +474,7 @@ static int add_phb(void)
say(ERROR, "Unknown failure. Data may be out of sync "
"and\nthe system may require a reboot.\n");
}
- goto phb_add_error;
+ return rc;
}

rc = dlpar_add_slot(phb->drc_name);
@@ -504,7 +491,7 @@ static int add_phb(void)
say(ERROR, "Unknown failure. Data may be out of sync "
"and\nthe system may require a reboot.\n");
}
- goto phb_add_error;
+ return rc;
}

if (n_children) {
@@ -533,9 +520,75 @@ static int add_phb(void)
}
}

+ *phb_node = phb;
+
+ return rc;
+}
+
+/**
+ * add_phb
+ *
+ * @returns 0 on success, !0 otherwise
+ */
+static int add_phb(void)
+{
+ struct dr_node *phb = NULL, *partner_phb = NULL;
+ uint32_t partner_drc_index = 0;
+ struct dr_connector drc;
+ char drc_index_str[10];
+ char path[DR_PATH_MAX];
+ int rc;
+
+ phb = get_node_by_name(usr_drc_name, PHB_NODES);
+ if (phb) {
+ say(ERROR, "PHB is already owned by this partition\n");
+ rc = RC_ALREADY_OWN;
+ goto phb_add_error;
+ }
+
+ rc = get_drc_by_name(usr_drc_name, &drc, path, OFDT_BASE);
+ if (rc) {
+ say(ERROR, "Could not find drc index for %s, unable to add "
+ "the PHB.\n", usr_drc_name);
+ return rc;
+ }
+
+ rc = add_enable_phb(&drc, path, &phb);
+ if (rc)
+ goto phb_add_error;
+
+ /* Find the multipath partner device index if available */
+ rc = get_my_partner_drc_index(phb, &partner_drc_index);
+ if (!rc && partner_drc_index) {
+ sprintf(drc_index_str, "%d", partner_drc_index);
+
+ partner_phb = get_node_by_name(drc_index_str, PHB_NODES);
+ if (partner_phb) {
+ say(ERROR, "PHB is already owned by this partition\n");
+ rc = RC_ALREADY_OWN;
+ goto phb_add_error;
+ }
+ rc = get_drc_by_index(partner_drc_index, &drc, path,
+ OFDT_BASE);
+ if (rc) {
+ say(ERROR, "Could not find drc for partner index %x,"
+ " Unable to add PHB.\n", partner_drc_index);
+ goto phb_add_error;
+ }
+
+ rc = add_enable_phb(&drc, path, &partner_phb);
+ if (rc)
+ goto phb_add_error;
+ printf("<%s> and <%s> are multipath partner devices.\n"
+ "<%s> is also added.\n", phb->drc_name,
+ partner_phb->drc_name, partner_phb->drc_name);
+ }
+
phb_add_error:
if (phb)
free_node(phb);

Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:21:56 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

If the PCI device has multipath partner device, its device node
contains "ibm,multipath-partner-drc" property which gives the DRC
index of the partner device. For the hotplug removal, both paths
must be removed before instructing the user to remove the device
from the identified slot.

So the following steps will be executed for the removal:
- Find the partner path DRC index from "ibm,multipath-partner-drc"
property
- Remove the primary path
- Remove the partner path
- Notify user to remove PCI card from the specified slot.

Since both paths will be using the same slot, LED indicators and
the slot identification will be done only for the primary device.

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/drslot_chrp_pci.c | 142 ++++++++++++++++++++++++------------
1 file changed, 95 insertions(+), 47 deletions(-)

diff --git a/src/drmgr/drslot_chrp_pci.c b/src/drmgr/drslot_chrp_pci.c
index f2b76ef..f8f7f99 100644
--- a/src/drmgr/drslot_chrp_pci.c
+++ b/src/drmgr/drslot_chrp_pci.c
@@ -26,6 +26,7 @@
#include <errno.h>
#include <locale.h>
#include <librtas.h>
+#include <stdbool.h>

#include "rtas_calls.h"
#include "dr.h"
@@ -99,9 +100,9 @@ identify_slot(struct dr_node *node)
if (process_led(node, LED_ID))
return USER_QUIT;

- printf("The visual indicator for the specified PCI slot has\n"
- "been set to the identify state. Press Enter to continue\n"
- "or enter x to exit.\n");
+ printf("The visual indicator for the PCI slot <%s>\n"
+ "has been set to the identify state. Press Enter to\n"
+ "continue or enter x to exit.\n", node->drc_name);

if (getchar() == '\n')
return (USER_CONT);
@@ -146,7 +147,8 @@ find_drc_name(uint32_t drc_index, struct dr_node *all_nodes)
* @returns pointer to slot on success, NULL otherwise
*/
static struct dr_node *
-find_slot(char *drc_name, struct dr_node *all_nodes)
+find_slot(char *drc_name, uint32_t drc_index,
+ struct dr_node *all_nodes)
{
struct dr_node *node; /* working pointer */

@@ -157,6 +159,8 @@ find_slot(char *drc_name, struct dr_node *all_nodes)
while (node != NULL) {
if (cmp_drcname(node->drc_name, drc_name))
break;
+ else if (drc_index && (node->drc_index == drc_index))
+ break;
else
node = node->next;
}
@@ -316,7 +320,7 @@ static int do_identify(struct dr_node *all_nodes)
int usr_key;
int led_state;

- node = find_slot(usr_drc_name, all_nodes);
+ node = find_slot(usr_drc_name, 0, all_nodes);
if (node == NULL)
return -1;

@@ -509,6 +513,32 @@ static int do_insert_card_work(struct dr_node *node)
return 0;
}

+/**
+ * find_partner_node
+ *
+ * Find the partner DRC index and retrieve the partner node.
+ */
+static struct dr_node *
+find_partner_node(struct dr_node *node, struct dr_node *all_nodes)
+{
+ struct dr_node *partner_node = NULL;
+ uint32_t partner_index = 0;
+ int rc;
+
+ /*
+ * Expect the partner device only for the PCI node
+ */
+ if (!node->children)
+ return NULL;
+
+ /* Find the multipath partner device index if available */
+ rc = get_my_partner_drc_index(node->children, &partner_index);
+ if (!rc)
+ partner_node = find_slot(NULL, partner_index, all_nodes);
+
+ return partner_node;
+}
+
/**
* do_add
*
@@ -530,7 +560,7 @@ static int do_add(struct dr_node *all_nodes)
int usr_key = USER_CONT;
int rc;

- node = find_slot(usr_drc_name, all_nodes);
+ node = find_slot(usr_drc_name, 0, all_nodes);
if (node == NULL)
return -1;

@@ -602,50 +632,50 @@ static int do_add(struct dr_node *all_nodes)
* Open Firmware device tree. The slot is isolated and powered off,
* and the LED is turned off.
*
- * @returns pointer slot on success, NULL on failure
+ * @returns 0 on success, -1 on failure
*/
-static struct dr_node *remove_work(struct dr_node *all_nodes)
+static int remove_work(struct dr_node *node, bool partner_device)
{
- struct dr_node *node;
struct dr_node *child;
int rc;
int usr_key = USER_CONT;

- node = find_slot(usr_drc_name, all_nodes);
if (node == NULL)
- return NULL;
+ return -1;

say(DEBUG, "found node: drc name=%s, index=0x%x, path=%s\n",
node->drc_name, node->drc_index, node->ofdt_path);

if (is_display_adapter(node)) {
say(ERROR, "DLPAR of display adapters is not supported.\n");
- return NULL;
+ return -1;
}

- if (usr_prompt) {
- if (usr_slot_identification)
- usr_key = identify_slot(node);
+ if (!partner_device) {
+ if (usr_prompt) {
+ if (usr_slot_identification)
+ usr_key = identify_slot(node);

- if (usr_key == USER_QUIT) {
- if (node->children == NULL)
- process_led(node, LED_OFF);
- else
- process_led(node, LED_ON);
- return NULL;
+ if (usr_key == USER_QUIT) {
+ if (node->children == NULL)
+ process_led(node, LED_OFF);
+ else
+ process_led(node, LED_ON);
+ return -1;
+ }
}
- }

- /* Turn on the LED while we go do some work. */
- if (process_led(node, LED_ON))
- return NULL;
+ /* Turn on the LED while we go do some work. */
+ if (process_led(node, LED_ON))
+ return -1;

- /* Make sure there's something there to remove. */
- if (node->children == NULL) {
- process_led(node, LED_OFF);
- say(ERROR, "There is no configured card to remove from the "
- "specified PCI slot.\n");
- return NULL;
+ /* Make sure there's something there to remove. */
+ if (node->children == NULL) {
+ process_led(node, LED_OFF);
+ say(ERROR, "There is no configured card to remove "
+ "from the specified PCI slot.\n");
+ return -1;
+ }
}

if (!pci_virtio) {
@@ -660,7 +690,7 @@ static struct dr_node *remove_work(struct dr_node *all_nodes)
int rc = get_hp_adapter_status(node->drc_name);
if (rc != NOT_CONFIG) {
say(ERROR, "Unconfig adapter failed.\n");
- return NULL;
+ return -1;
}
} else {
/* In certain cases such as a complete failure of the
@@ -697,12 +727,12 @@ static struct dr_node *remove_work(struct dr_node *all_nodes)
rtas_set_indicator(ISOLATION_STATE, node->drc_index,
ISOLATE);
set_power(node->drc_power, POWER_OFF);
- return NULL;
+ return -1;
}
}

if (pci_hotplug_only)
- return node;
+ return 0;

/* We have to isolate and power off before
* allowing the user to physically remove
@@ -719,7 +749,7 @@ static struct dr_node *remove_work(struct dr_node *all_nodes)
say(ERROR, "%s", sw_error);

set_power(node->drc_power, POWER_OFF);
- return NULL;
+ return rc;
}

say(DEBUG, "is calling set_power(POWER_OFF index 0x%x, power_domain "
@@ -733,10 +763,10 @@ static struct dr_node *remove_work(struct dr_node *all_nodes)
say(ERROR, "%s", sw_error);

set_power(node->drc_power, POWER_OFF);
- return NULL;
+ return rc;
}

- return node;
+ return 0;
}

/**
@@ -759,13 +789,27 @@ static struct dr_node *remove_work(struct dr_node *all_nodes)
*/
static int do_remove(struct dr_node *all_nodes)
{
- struct dr_node *node;
+ struct dr_node *node, *partner_node = NULL;

- /* Remove the specified slot and update the device-tree */
- node = remove_work(all_nodes);
+ node = find_slot(usr_drc_name, 0, all_nodes);
if (node == NULL)
return -1;

+ partner_node = find_partner_node(node, all_nodes);
+ if (partner_node)
+ printf("<%s> and <%s> are\nmultipath partner devices. "
+ "So <%s> will\nbe also removed.\n", node->drc_name,
+ partner_node->drc_name, partner_node->drc_name);
+
+ /* Remove the specified slot and update the device-tree */
+ if (remove_work(node, false))
+ return -1;
+
+ if (partner_node) {
+ if (remove_work(partner_node, true))
+ return -1;
+ }
+
/* Prompt user to remove card and to press
* Enter to continue. Can't exit out of here.
*/
@@ -773,10 +817,10 @@ static int do_remove(struct dr_node *all_nodes)
if (process_led(node, LED_ACTION))
return -1;

- printf("The visual indicator for the specified PCI slot "
- "has\nbeen set to the action state. Remove the PCI "
- "card\nfrom the identified slot and press Enter to "
- "continue.\n");
+ printf("The visual indicator for the specified PCI "
+ "slot has\nbeen set to the action state. "
+ "Remove the PCI card\nfrom the identified "
+ "slot and press Enter to continue.\n");
getchar();
if (process_led(node, LED_OFF))
return -1;
@@ -810,11 +854,11 @@ static int do_replace(struct dr_node *all_nodes)
struct dr_node *repl_node;
int rc;

+ repl_node = find_slot(usr_drc_name, 0, all_nodes);
/* Call the routine which does the work of getting the node info,
* then removing it from the OF device tree.
*/
- repl_node = remove_work(all_nodes);
- if (repl_node == NULL)
+ if (remove_work(repl_node, false))
return -1;

if (!repl_node->children) {
@@ -856,12 +900,16 @@ static int do_replace(struct dr_node *all_nodes)

if (repl_node->post_replace_processing) {
int prompt_save = usr_prompt;
+ struct dr_node *node = repl_node;

say(DEBUG, "Doing post replacement processing...\n");
/* disable prompting for post-processing */
usr_prompt = 0;;

- repl_node = remove_work(repl_node);
+ repl_node = find_slot(usr_drc_name, 0, node);
+ if (remove_work(repl_node, false))
+ return -1;
+
rc = add_work(repl_node);
if (!rc)
set_hp_adapter_status(PHP_CONFIG_ADAPTER,
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:22:18 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

If the PCI device has multipath partner device, the firmware
provides partner DRC index in "ibm,multipath-partner-drc"
property and this property is available in the PCI device node.
So when the PCI device add is initiated, both paths will be
added and enabled with the following steps:

- Identify slot and notify user to add the device
- Add the path and enable for the specified device
- Find the partner path DRC index from "ibm,multipath-partner-drc"
property for the specified device
- Add the partner path
- Notify user about adding both paths

Since both paths will be using the same slot, LED indicators and
the slot identification will be done only for the primary device.

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/common_pci.c | 2 +-
src/drmgr/drslot_chrp_pci.c | 162 +++++++++++++++++++++++-------------
src/drmgr/ofdt.h | 1 +
3 files changed, 105 insertions(+), 60 deletions(-)

diff --git a/src/drmgr/common_pci.c b/src/drmgr/common_pci.c
index 2e0e5fb..2411641 100644
--- a/src/drmgr/common_pci.c
+++ b/src/drmgr/common_pci.c
@@ -295,7 +295,7 @@ add_child_node(struct dr_node *parent, char *child_path)
* @param node
* @returns 0 on success, !0 otherwise
*/
-static int
+int
init_node(struct dr_node *node)
{
DIR *d;
diff --git a/src/drmgr/drslot_chrp_pci.c b/src/drmgr/drslot_chrp_pci.c
index f8f7f99..b68b04e 100644
--- a/src/drmgr/drslot_chrp_pci.c
+++ b/src/drmgr/drslot_chrp_pci.c
@@ -31,6 +31,7 @@
#include "rtas_calls.h"
#include "dr.h"
#include "drpci.h"
+#include "ofdt.h"

static char *sw_error = "Internal software error. Contact your service "
"representative.\n";
@@ -363,27 +364,38 @@ static int do_identify(struct dr_node *all_nodes)
* off, isolated, and the LED is turned off.
*
* @param slot
+ * @param partner path or not
* @returns 0 on success, !0 on failure
*/
-static int add_work(struct dr_node *node)
+static int add_work(struct dr_node *node, bool partner_device)
{
- int pow_state; /* Tells us if power was turned on when */
- int iso_state; /* Tells us isolation state after */
+ int pow_state = POWER_OFF; /* Tells us if power was turned */
+ /* on when */
+ int iso_state = ISOLATE; /* Tells us isolation state after */
int rc;
struct of_node *new_nodes;/* nodes returned from configure_connector */

- /* if we're continuing, set LED_ON and see if a card is really there. */
- if (process_led(node, LED_ON))
- return -1;
+ /*
+ * Already checked the card presence for the original device
+ * and both multipaths use the same card. So do not need to
+ * check the card presence again for the partner device.
+ */
+ if (!partner_device) {
+ /* if we're continuing, set LED_ON and see if a card */
+ /* is really there. */
+ if (process_led(node, LED_ON))
+ return -1;

- say(DEBUG, "is calling card_present\n");
- rc = card_present(node, &pow_state, &iso_state);
- if (!rc) {
- say(ERROR, "No PCI card was detected in the specified "
- "PCI slot.\n");
- rtas_set_indicator(ISOLATION_STATE, node->drc_index, ISOLATE);
- set_power(node->drc_power, POWER_OFF);
- return -1;
+ say(DEBUG, "is calling card_present\n");
+ rc = card_present(node, &pow_state, &iso_state);
+ if (!rc) {
+ say(ERROR, "No PCI card was detected in the specified "
+ "PCI slot.\n");
+ rtas_set_indicator(ISOLATION_STATE, node->drc_index,
+ ISOLATE);
+ set_power(node->drc_power, POWER_OFF);
+ return -1;
+ }
}

if (!pow_state) {
@@ -457,7 +469,7 @@ static int add_work(struct dr_node *node)
* power to a slot off. The prompts the user to insert the new card
* into the slot.
*/
-static int do_insert_card_work(struct dr_node *node)
+static int do_insert_card_work(struct dr_node *node, bool partner_device)
{
int rc;

@@ -491,18 +503,18 @@ static int do_insert_card_work(struct dr_node *node)
return -1;
}

- if (usr_prompt) {
+ if (usr_prompt && !partner_device) {
/* Prompt user to put in card and to press
* Enter to continue or other key to exit.
*/
if (process_led(node, LED_ACTION))
return -1;

- printf("The visual indicator for the specified PCI slot has\n"
- "been set to the action state. Insert the PCI card\n"
- "into the identified slot, connect any devices to be\n"
- "configured and press Enter to continue. Enter x to "
- "exit.\n");
+ printf("The visual indicator for the PCI slot <%s>\n"
+ "has been set to the action state. Insert the PCI\n"
+ "card into the identified slot, connect any devices\n"
+ "to be configured and press Enter to continue.\n"
+ "Enter x to exit.\n", node->drc_name);

if (!(getchar() == '\n')) {
process_led(node, LED_OFF);
@@ -539,6 +551,58 @@ find_partner_node(struct dr_node *node, struct dr_node *all_nodes)
return partner_node;
}

+static int insert_add_work(struct dr_node *node, bool partner_device)
+{
+ int usr_key = USER_CONT;
+ int rc;
+
+ if (!partner_device) {
+ /* Prompt user only if in interactive mode. */
+ if (usr_prompt) {
+ if (usr_slot_identification)
+ usr_key = identify_slot(node);
+
+ if (usr_key == USER_QUIT) {
+ if (node->children == NULL)
+ process_led(node, LED_OFF);
+ else
+ process_led(node, LED_ON);
+ return 0;
+ }
+ }
+
+ if (node->children != NULL) {
+ /* If there's already something here, turn the
+ * LED on and exit with user error.
+ */
+ process_led(node, LED_ON);
+ say(ERROR, "The specified PCI slot is already occupied.\n");
+ return -1;
+ }
+ }
+
+ if (!pci_hotplug_only) {
+ rc = do_insert_card_work(node, partner_device);
+ if (rc)
+ return rc;
+ }
+
+ /* Call the routine which determines
+ * what the user wants and does it.
+ */
+ rc = add_work(node, partner_device);
+ if (rc)
+ return rc;
+
+ /*
+ * Need to populate w/ children to retrieve partner device
+ */
+ if (init_node(node))
+ return -1;
+
+ return 1;
+}
+
/**
* do_add
*
@@ -556,8 +620,7 @@ find_partner_node(struct dr_node *node, struct dr_node *all_nodes)
*/
static int do_add(struct dr_node *all_nodes)
{
- struct dr_node *node;
- int usr_key = USER_CONT;
+ struct dr_node *node, *partner_node = NULL;
int rc;

node = find_slot(usr_drc_name, 0, all_nodes);
@@ -569,51 +632,32 @@ static int do_add(struct dr_node *all_nodes)
return -1;
}

- /* Prompt user only if in interactive mode. */
- if (usr_prompt) {
- if (usr_slot_identification)
- usr_key = identify_slot(node);
-
- if (usr_key == USER_QUIT) {
- if (node->children == NULL)
- process_led(node, LED_OFF);
- else
- process_led(node, LED_ON);
- return 0;
- }
- }
-
- if (node->children != NULL) {
- /* If there's already something here, turn the
- * LED on and exit with user error.
- */
- process_led(node, LED_ON);
- say(ERROR, "The specified PCI slot is already occupied.\n");
- return -1;
- }
+ rc = insert_add_work(node, false);
+ if (rc <= 0)
+ return rc;

- if (!pci_hotplug_only) {
- rc = do_insert_card_work(node);
- if (rc)
+ partner_node = find_partner_node(node, all_nodes);
+ if (partner_node) {
+ printf("<%s> and <%s> are\nmultipath partner devices. "
+ "So <%s> is\nalso added.\n", node->drc_name,
+ partner_node->drc_name, partner_node->drc_name);
+ rc = insert_add_work(partner_node, true);
+ if (rc <= 0)
return rc;
}

- /* Call the routine which determines
- * what the user wants and does it.
- */
- rc = add_work(node);
- if (rc)
- return rc;
-
say(DEBUG, "is calling enable_slot to config adapter\n");

/* Try to config the adapter. The rpaphp module doesn't play well with
* qemu pci slots so we let the generic kernel pci code probe the device
* by rescanning the bus in the qemu virtio case.
*/
- if (!pci_virtio)
+ if (!pci_virtio) {
set_hp_adapter_status(PHP_CONFIG_ADAPTER, node->drc_name);
- else
+ if (partner_node)
+ set_hp_adapter_status(PHP_CONFIG_ADAPTER,
+ partner_node->drc_name);
+ } else
pci_rescan_bus();

return 0;
@@ -889,7 +933,7 @@ static int do_replace(struct dr_node *all_nodes)
}
}

- rc = add_work(repl_node);
+ rc = add_work(repl_node, false);
if (rc)
return rc;

@@ -910,7 +954,7 @@ static int do_replace(struct dr_node *all_nodes)
if (remove_work(repl_node, false))
return -1;

- rc = add_work(repl_node);
+ rc = add_work(repl_node, false);
if (!rc)
set_hp_adapter_status(PHP_CONFIG_ADAPTER,
repl_node->drc_name);
diff --git a/src/drmgr/ofdt.h b/src/drmgr/ofdt.h
index 08d34e1..e9ebd03 100644
--- a/src/drmgr/ofdt.h
+++ b/src/drmgr/ofdt.h
@@ -184,6 +184,7 @@ int get_min_common_depth(void);
int get_assoc_arrays(const char *dir, struct assoc_arrays *aa,
int min_common_depth);
int of_associativity_to_node(const char *dir, int min_common_depth);
+int init_node(struct dr_node *);

static inline int aa_index_to_node(struct assoc_arrays *aa, uint32_t aa_index)
{
--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 4:23:03 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@linux.ibm.com

If the PCI device has multipath partner device, its device node
contains "ibm,multipath-partner-drc" property which gives the DRC
index of the partner device.
So for the replace operation, the following steps will be executed:
- Find the device based on the partner DRC index of the requested
device
- Remove the requested device
- Remove the partner path
- Add the new device based on user input
- Find the device based on the partner DRC index of the new device
- Add the partner path of the new device

Since both paths will be using the same slot, LED indicators and
the slot identification will be done only for the primary device
in both removal and add operations.

Signed-off-by: Haren Myneni <ha...@linux.ibm.com>
---
src/drmgr/drslot_chrp_pci.c | 100 +++++++++++++++++++++++++-----------
1 file changed, 70 insertions(+), 30 deletions(-)

diff --git a/src/drmgr/drslot_chrp_pci.c b/src/drmgr/drslot_chrp_pci.c
index b68b04e..0454a14 100644
--- a/src/drmgr/drslot_chrp_pci.c
+++ b/src/drmgr/drslot_chrp_pci.c
@@ -873,6 +873,44 @@ static int do_remove(struct dr_node *all_nodes)
return 0;
}

+static int replace_add_work(struct dr_node *node, bool partner_device)
+{
+
+ say(DEBUG, "repl_node:path=%s node:path=%s\n",
+ node->ofdt_path, node->children->ofdt_path);
+
+ /* Prompt user to replace card and to press
+ * Enter to continue or x to exit. Exiting here
+ * means the original card has been removed.
+ */
+ if (usr_prompt && !partner_device) {
+ if (process_led(node, LED_ACTION))
+ return -1;
+
+ printf("The visual indicator for the specified PCI slot <%s>\n"
+ "has been set to the action state. Replace the PCI\n"
+ "card in the identified slot and press Enter to "
+ "continue.\nEnter x to exit. Exiting now leaves the "
+ "PCI slot\nin the removed state.\n",
+ node->drc_name);
+
+ if (!(getchar() == '\n')) {
+ process_led(node, LED_OFF);
+ return 0;
+ }
+ }
+
+ if (add_work(node, partner_device))
+ return -1;
+
+ say(DEBUG, "CONFIGURING the card in node[name=%s, path=%s]\n",
+ node->drc_name, node->ofdt_path);
+
+ set_hp_adapter_status(PHP_CONFIG_ADAPTER, node->drc_name);
+
+ return 1;
+}
+
/**
* do_replace
* @brief Allows the replacement of an adapter connected to a
@@ -895,52 +933,41 @@ static int do_remove(struct dr_node *all_nodes)
*/
static int do_replace(struct dr_node *all_nodes)
{
- struct dr_node *repl_node;
+ struct dr_node *repl_node, *partner_node = NULL;
int rc;

repl_node = find_slot(usr_drc_name, 0, all_nodes);
+
+ partner_node = find_partner_node(repl_node, all_nodes);
+ if (partner_node)
+ printf("<%s> and <%s> are\nmultipath partner devices. "
+ "So <%s> will\nbe also replaced.\n",
+ repl_node->drc_name, partner_node->drc_name,
+ partner_node->drc_name);
+
/* Call the routine which does the work of getting the node info,
* then removing it from the OF device tree.
*/
if (remove_work(repl_node, false))
return -1;

+ if (partner_node) {
+ if (remove_work(partner_node, true))
+ return -1;
+ }
+
if (!repl_node->children) {
say(ERROR, "Bad node struct.\n");
return -1;
}

- say(DEBUG, "repl_node:path=%s node:path=%s\n",
- repl_node->ofdt_path, repl_node->children->ofdt_path);
-
- /* Prompt user to replace card and to press
- * Enter to continue or x to exit. Exiting here
- * means the original card has been removed.
- */
- if (usr_prompt) {
- if (process_led(repl_node, LED_ACTION))
- return -1;
-
- printf("The visual indicator for the specified PCI slot "
- "has\nbeen set to the action state. Replace the PCI "
- "card\nin the identified slot and press Enter to "
- "continue.\nEnter x to exit. Exiting now leaves the "
- "PCI slot\nin the removed state.\n");
-
- if (!(getchar() == '\n')) {
- process_led(repl_node, LED_OFF);
- return 0;
- }
- }
-
- rc = add_work(repl_node, false);
- if (rc)
+ rc = replace_add_work(repl_node, false);
+ if (rc <= 0)
return rc;

- say(DEBUG, "CONFIGURING the card in node[name=%s, path=%s]\n",
- repl_node->drc_name, repl_node->ofdt_path);
-
- set_hp_adapter_status(PHP_CONFIG_ADAPTER, repl_node->drc_name);
+ rc = replace_add_work(partner_node, true);
+ if (rc <= 0)
+ return rc;

if (repl_node->post_replace_processing) {
int prompt_save = usr_prompt;
@@ -954,11 +981,24 @@ static int do_replace(struct dr_node *all_nodes)
if (remove_work(repl_node, false))
return -1;

+ partner_node = find_partner_node(repl_node, node);
+ if (partner_node) {
+ if (remove_work(partner_node, true))
+ return -1;
+ }
+
rc = add_work(repl_node, false);
if (!rc)
set_hp_adapter_status(PHP_CONFIG_ADAPTER,
repl_node->drc_name);

+ if (partner_node) {
+ rc = add_work(partner_node, true);
+ if (!rc)
+ set_hp_adapter_status(PHP_CONFIG_ADAPTER,
+ partner_node->drc_name);
+ }
+
usr_prompt = prompt_save;
}

--
2.31.1


Haren Myneni

<haren@linux.ibm.com>
unread,
May 10, 2024, 11:17:19 PMMay 10
to tyreld@linux.ibm.com, powerpc-utils-devel@googlegroups.com, nathanl@linux.ibm.com, wenxiong@linux.ibm.com, haren@us.ibm.com

Powerpc Hypervisor enables multi-path to NVME devices and both paths
are exported as PCI devices in the device-tree. Each device node
contains "ibm,multipath-partner-drc" property which has DRC index
value of partner device. So the hypervisor allows the partition to
have redundant path or use both paths for the performance benefit.

For add/remove hotplug, the hypervisor expect OS to update on both
devices based on operations. Both paths will be offline for remove
operation and similarly add them for hotplug add.

This patch series adds these changes for PHB and PCI interfaces.

Thanks to Wen Xiong for suggestions on these interfaces and testing.

Haren Myneni (8):
drmgr: Return from get_node_by_name() if DRC index is matched
drmgr: Retrieve dr_connector with the specified DRC index
drmgr: Introduce get_my_partner_drc_index()
drmgr/phb: Add multipath partner device support for remove
drmgr/phb: Add multipath partner device support for hotplug add
drmgr/pci: Add multipath partner device support for hotplug remove
drmgr/pci: Add multipath partner device support for hotplug add
drmgr/pci: Add multipath partner device support for hotplug replace

src/drmgr/common_ofdt.c | 98 +++++++--
src/drmgr/common_pci.c | 4 +-
src/drmgr/drslot_chrp_pci.c | 402 ++++++++++++++++++++++++------------
src/drmgr/drslot_chrp_phb.c | 188 +++++++++++++----
src/drmgr/ofdt.h | 3 +
5 files changed, 495 insertions(+), 200 deletions(-)

--
2.31.1


Reply all
Reply to author
Forward
0 new messages