Re: [PATCH v3 3/6] ieee1275: implement FCP methods for WWPN and LUNs

[avnish]

On 2024-07-01 13:04, Michael Chang wrote:

On Thu, Jun 06, 2024 at 06:07:24PM GMT, Avnish Chouhan wrote:

This patch enables the fcp-targets and fcp-luns methods which are
responsible to get WWPNs and LUNs for fibre channel devices.

Those methods are specially necessary if the boot directory and grub
installation are in different FCP disks, allowing the dev_iterate()
to find the WWPNs and LUNs when called by searchfs.uuid tool.

Signed-off-by: Diego Domingos <dieg...@br.ibm.com>
Signed-off-by: Avnish Chouhan <avn...@linux.ibm.com>
---

grub-core/disk/ieee1275/ofdisk.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-

 1 file changed, 110 insertions(+), 1 deletion(-)

diff --git a/grub-core/disk/ieee1275/ofdisk.c b/grub-core/disk/ieee1275/ofdisk.c

index 5534684..5958e5e 100644
--- a/grub-core/disk/ieee1275/ofdisk.c
+++ b/grub-core/disk/ieee1275/ofdisk.c

@@ -209,7 +209,116 @@ dev_iterate_real (const char *name, const char *path)

 static void
 dev_iterate (const struct grub_ieee1275_devalias *alias)
 {
-  if (grub_strcmp (alias->type, "vscsi") == 0)
+  if (grub_strcmp (alias->type, "fcp") == 0)
+    {
+      /*
+       * If we are dealing with fcp devices, we need
+       * to find the WWPNs and LUNs to iterate them
+       */
+      grub_ieee1275_ihandle_t ihandle;
+      grub_uint64_t *ptr_targets, *ptr_luns, k, l;
+      unsigned int i, j, pos;
+      char *buf, *bufptr;
+      struct set_fcp_targets_args
+      {
+        struct grub_ieee1275_common_hdr common;
+        grub_ieee1275_cell_t method;
+        grub_ieee1275_cell_t ihandle;
+        grub_ieee1275_cell_t catch_result;
+        grub_ieee1275_cell_t nentries;
+        grub_ieee1275_cell_t table;
+      } args_targets;
+
+      struct set_fcp_luns_args
+      {
+        struct grub_ieee1275_common_hdr common;
+        grub_ieee1275_cell_t method;
+        grub_ieee1275_cell_t ihandle;
+        grub_ieee1275_cell_t wwpn_h;
+        grub_ieee1275_cell_t wwpn_l;
+        grub_ieee1275_cell_t catch_result;
+        grub_ieee1275_cell_t nentries;
+        grub_ieee1275_cell_t table;
+      } args_luns;
+
+      struct args_ret
+      {
+        grub_uint64_t addr;
+        grub_uint64_t len;
+      };
+
+      if (grub_ieee1275_open (alias->path, &ihandle))
+        {

+ grub_dprintf ("disk", "failed to open the disk while iterating FCP disk path=%s\n", alias->path);

+          return;
+        }
+
+      /* Setup the fcp-targets method to call via pfw*/

+ INIT_IEEE1275_COMMON (&args_targets.common, "call-method", 2, 3);

+      args_targets.method = (grub_ieee1275_cell_t) "fcp-targets";
+      args_targets.ihandle = ihandle;
+
+      /* Setup the fcp-luns method to call via pfw */
+      INIT_IEEE1275_COMMON (&args_luns.common, "call-method", 4, 3);
+      args_luns.method = (grub_ieee1275_cell_t) "fcp-luns";
+      args_luns.ihandle = ihandle;
+      if (IEEE1275_CALL_ENTRY_FN (&args_targets) == -1)
+        {

+ grub_dprintf ("disk", "failed to get the targets while iterating FCP disk path=%s\n", alias->path);

+          grub_ieee1275_close (ihandle);
+          return;
+        }
+      buf = grub_malloc (grub_strlen (alias->path) + 32 + 32);
+      if (!buf)
+        {
+          grub_ieee1275_close (ihandle);
+          return;
+        }
+      bufptr = grub_stpcpy (buf, alias->path);
+
+      /*
+       * Iterate over entries returned by pfw. Each entry contains a
+       * pointer to wwpn table and his length.
+       */

+ struct args_ret *targets_table = (struct args_ret *) (args_targets.table);

+      for (i = 0; i < args_targets.nentries; i++)
+        {

+ ptr_targets = (grub_uint64_t*) (grub_uint32_t) targets_table[i].addr;

I believe a comment is needed here to explain why it is necessary to
truncate the 64-bit address into 32-bit. It appears that losing the
higher 32 bits is intentional.

Hi Michael,
Thank you so much for your reviews!

Sure, we'll add a comment here.

+          /* Iterate over all wwpns in given table */
+          for(k = 0; k < targets_table[i].len; k++)
+            {

+ args_luns.wwpn_l = (grub_ieee1275_cell_t) (*ptr_targets); + args_luns.wwpn_h = (grub_ieee1275_cell_t) (*ptr_targets >> 32); + pos = grub_snprintf (bufptr, 32, "/disk@%" PRIxGRUB_UINT64_T,

+                                  *ptr_targets++);

As long as ptr_targets holds a pointer external to grub, the alignment
is neither validated nor guaranteed. IMHO it is safer to use
grub_get_unaligned64(ptr_targets++).

Sure, we can change this as suggested.

Regards,
Avnish Chouhan

+              /* Get the luns for given wwpn target */
+              if (IEEE1275_CALL_ENTRY_FN (&args_luns) == -1)
+                {

+ grub_dprintf ("disk", "failed to get the LUNS while iterating FCP disk path=%s\n", buf);

+                  grub_ieee1275_close (ihandle);
+                  grub_free (buf);
+                  return;
+                }

+ struct args_ret *luns_table = (struct args_ret *) (args_luns.table);

+
+              /* Iterate over all LUNs */
+              for(j = 0; j < args_luns.nentries; j++)
+                {

+ ptr_luns = (grub_uint64_t*) (grub_uint32_t) luns_table[j].addr;

Ditto

+                  for(l = 0; l < luns_table[j].len; l++)
+                    {

+ grub_snprintf (&bufptr[pos], 30, ",%" PRIxGRUB_UINT64_T,

+                                     *ptr_luns++);

Ditto

Thanks,
Michael

+                      dev_iterate_real (buf, buf);
+                    }
+                }
+            }
+        }
+      grub_ieee1275_close (ihandle);
+      grub_free (buf);
+      return;
+    }
+  else if (grub_strcmp (alias->type, "vscsi") == 0)
     {
       static grub_ieee1275_ihandle_t ihandle;
       struct set_color_args
--
2.31.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel