On Thu, Apr 05, 2018 at 10:53:55AM -0700, Eric Snowberg wrote:
> Add a new disk driver called obdisk for IEEE1275 platforms. Currently
> the only platform using this disk driver is SPARC, however other IEEE1275
> platforms could start using it if they so choose. While the functionality
> within the other IEEE1275 ofdisk driver may be suitable for PPC and x86, it
s/other/current/?
> presented too many problems on SPARC hardware.
>
> Within the old ofdisk, there is not a way to determine the true canonical
> name for the disk. Within Open Boot, the same disk can have multiple names
> but all reference the same disk. For example the same disk can be referenced
> by its SAS WWN, using this form:
>
> /pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@w5000cca02f037d6d,0
>
> It can also be referenced by its PHY identifier using this form:
>
> /pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@p0
>
> It can also be referenced by its Target identifier using this form:
>
> /pci@302/pci@2/pci@0/pci@17/LSI,sas@0/disk@0
>
> Also, when the LUN=0, it is legal to omit the ,0 from the device name. So
> with
> the disk above, before taking into account the device aliases, there are 6
> ways
> to reference the same disk.
>
> Then it is possible to have 0 .. n device aliases all representing the same
> disk.
> Within this new driver the true canonical name is determined using the the
> IEEE1275 encode-unit and decode-unit commands when address_cells == 4. This
> will determine the true single canonical name for the device so multiple
> ihandles
> are not opened for the same device. This is what frequently happens with the
> old
> ofdisk driver. With some devices when they are opened multiple times it
> causes
> the entire system to hang.
>
> Another problem solved with this driver is devices that do not have a device
> alias can be booted and used within GRUB. Within the old ofdisk, this was not
> possible, unless it was the original boot device. All devices behind a SAS
> or SCSI parent can be found. Within the old ofdisk, finding these disks
> relied on there being an alias defined. The alias requirement is not
> necessary with this new driver. It can also find devices behind a parent
> after they have been hot-plugged. This is something that is not possible
> with the old ofdisk driver.
>
> The old ofdisk driver also incorrectly assumes that the device pointing to by
> a
> device alias is in its true canonical form. This assumption is never made with
> this new driver.
>
> Another issue solved with this driver is that it properly caches the ihandle
> for all open devices. The old ofdisk tries to do this by caching the last
> opened ihandle. However this does not work properly because the layer above
> does not use a consistent device name for the same disk when calling into the
> driver. This is because the upper layer uses the bootpath value returned
> within
> /chosen, other times it uses the device alias, and other times it uses the
> value within grub.cfg. It does not have a way to figure out that these
> devices
> are the same disk. This is not a problem with this new driver.
>
> Due to the way GRUB repeatedly opens and closes the same disk. Caching the
> ihandle is important on SPARC. Without caching, some SAS devices can take
> 15 - 20 minutes to get to the GRUB menu. This ihandle caching is not possible
> without correctly having the canonical disk name.
>
> When available, this driver also tries to use the deblocker #blocks and
> a way of determining the disk size.
>
> Finally and probably most importantly, this new driver is also capable of
> seeing all partitions on a GPT disk. With the old driver, the GPT
> partition table can not be read and only the first partition on the disk
> can be seen.
>
> Signed-off-by: Eric Snowberg <eric.snowb...@oracle.com>
> ---
> grub-core/Makefile.core.def | 1 +
> grub-core/commands/nativedisk.c | 1 +
> grub-core/disk/ieee1275/obdisk.c | 1093
> ++++++++++++++++++++++++++++++++++++++
> grub-core/kern/ieee1275/cmain.c | 3 +
> grub-core/kern/ieee1275/init.c | 12 +-
> include/grub/disk.h | 1 +
> include/grub/ieee1275/ieee1275.h | 2 +
> include/grub/ieee1275/obdisk.h | 25 +
> 8 files changed, 1137 insertions(+), 1 deletions(-)
> create mode 100644 grub-core/disk/ieee1275/obdisk.c
> create mode 100644 include/grub/ieee1275/obdisk.h
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 2c1d62c..14471c0 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -292,6 +292,7 @@ kernel = {
> sparc64_ieee1275 = kern/sparc64/cache.S;
> sparc64_ieee1275 = kern/sparc64/dl.c;
> sparc64_ieee1275 = kern/sparc64/ieee1275/ieee1275.c;
> + sparc64_ieee1275 = disk/ieee1275/obdisk.c;
>
> arm = kern/arm/dl.c;
> arm = kern/arm/dl_helper.c;
> diff --git a/grub-core/commands/nativedisk.c b/grub-core/commands/nativedisk.c
> index 2f56a87..2f2315d 100644
> --- a/grub-core/commands/nativedisk.c
> +++ b/grub-core/commands/nativedisk.c
> @@ -66,6 +66,7 @@ get_uuid (const char *name, char **uuid, int getnative)
> /* Firmware disks. */
> case GRUB_DISK_DEVICE_BIOSDISK_ID:
> case GRUB_DISK_DEVICE_OFDISK_ID:
> + case GRUB_DISK_DEVICE_OBDISK_ID:
> case GRUB_DISK_DEVICE_EFIDISK_ID:
> case GRUB_DISK_DEVICE_NAND_ID:
> case GRUB_DISK_DEVICE_ARCDISK_ID:
> diff --git a/grub-core/disk/ieee1275/obdisk.c
> b/grub-core/disk/ieee1275/obdisk.c
> new file mode 100644
> index 0000000..1ebf237
> --- /dev/null
> +++ b/grub-core/disk/ieee1275/obdisk.c
> @@ -0,0 +1,1093 @@
> +/* obdisk.c - Open Boot disk access. */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2018 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/disk.h>
> +#include <grub/env.h>
> +#include <grub/i18n.h>
> +#include <grub/kernel.h>
> +#include <grub/list.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/scsicmd.h>
> +#include <grub/time.h>
> +#include <grub/ieee1275/ieee1275.h>
> +#include <grub/ieee1275/obdisk.h>
> +
> +struct disk_dev
> +{
> + struct disk_dev *next;
> + struct disk_dev **prev;
> + /* open boot canonical name */
> + char *name;
> + /* open boot raw disk name to access entire disk */
> + char *raw_name;
> + /* grub encoded device name */
> + char *grub_devpath;
> + /* grub encoded alias name */
> + char *grub_alias_devpath;
> + /* grub unescaped name */
> + char *grub_decoded_devpath;
> + grub_ieee1275_ihandle_t ihandle;
> + grub_uint32_t block_size;
> + grub_uint64_t num_blocks;
> + unsigned int log_sector_size;
> + grub_uint32_t opened;
> + grub_uint32_t valid;
> + grub_uint32_t boot_dev;
> +};
> +
> +struct parent_dev
> +{
> + struct parent_dev *next;
> + struct parent_dev **prev;
> + /* canonical parent name */
> + char *name;
> + char *type;
> + grub_ieee1275_ihandle_t ihandle;
> + grub_uint32_t address_cells;
> +};
Could you align all members names in one column for these structures?
> +static struct grub_scsi_test_unit_ready tur =
> +{
> + .opcode = grub_scsi_cmd_test_unit_ready,
> + .lun = 0,
> + .reserved1 = 0,
> + .reserved2 = 0,
> + .reserved3 = 0,
> + .control = 0,
> +};
> +
> +static int disks_enumerated = 0;
> +static struct disk_dev *disk_devs = NULL;
> +static struct parent_dev *parent_devs = NULL;
> +
> +static const char *block_blacklist[] = {
> + /* Requires addition work in grub before being able to be used. */
> + "/iscsi-hba",
> + /* This block device should never be used by grub. */
> + "/reboot-memory@0",
> + 0
> +};
I do not think that you need to set values to 0/NULL above.
Compiler should do work for you.
> +#define STRCMP(a, b) ((a) && (b) && (grub_strcmp (a, b) == 0))
> +
> +static char *
> +strip_ob_partition (char *path)
> +{
> + char *sptr;
> +
> + sptr = grub_strstr (path, ":");
> +
> + if (sptr)
> + *sptr = '\0';
> +
> + return path;
> +}
> +
> +static char *
> +remove_escaped_commas (char *src)
s/remove/replace/?
Hmmm... Why do not really remove them?
> +{
> + char *iptr;
> +
> + if (src == NULL)
> + return NULL;
> + for (iptr = src; *iptr; )
> + {
> + if ((*iptr == '\\') && (*(iptr + 1) == ','))
> + {
> + *iptr++ = '_';
> + *iptr++ = '_';
> + }
> + iptr++;
> + }
> +
> + return src;
> +}
> +
> +static int
> +count_commas (const char *src)
> +{
> + int count = 0;
> +
> + for ( ; *src; src++)
> + if (*src == ',')
> + count++;
> +
> + return count;
> +}
> +
> +static void
> +escape_commas (const char *src, char *dest)
> +{
> + const char *iptr;
> +
> + for (iptr = src; *iptr; )
> + {
> + if (*iptr == ',')
> + *dest++ ='\\';
> +
> + *dest++ = *iptr++;
> + }
> +
> + *dest = '\0';
> +}
> +
> +static char *
> +decode_grub_devname (const char *name)
> +{
> + char *devpath = grub_malloc (grub_strlen (name) + 1);
> + char *p, c;
> +
> + if (!devpath)
Please be consistent. Use !devpath or devpath != NULL in entire file.
Same applies to comparison with 0.
> + return NULL;
> +
> + /* Un-escape commas. */
Is not it related to the issue which you have tried to
fix by the patch for shell parser?
> + p = devpath;
> + while ((c = *name++) != '\0')
> + {
> + if (c == '\\' && *name == ',')
> + {
> + *p++ = ',';
> + name++;
> + }
> + else
> + *p++ = c;
> + }
> +
> + *p++ = '\0';
> +
> + return devpath;
> +}
> +
> +static char *
> +encode_grub_devname (const char *path)
> +{
> + char *encoding, *optr;
> +
> + if (path == NULL)
Yes, like this one please. If you like it/it is in line with other files of
course...
> + return NULL;
> +
> + encoding = grub_malloc (sizeof ("ieee1275/") + count_commas (path) +
> + grub_strlen (path) + 1);
> +
> + if (encoding == NULL)
Ditto.
> + {
> + grub_print_error ();
> + return NULL;
> + }
> +
> + optr = grub_stpcpy (encoding, "ieee1275/");
You use "ieee1275/" in various places. Please define it as a constant.
> + escape_commas (path, optr);
> + return encoding;
> +}
> +
> +static char *
> +get_parent_devname (const char *devname)
> +{
> + char *parent, *pptr;
> +
> + parent = grub_strdup (devname);
> +
> + if (parent == NULL)
> + {
> + grub_print_error ();
> + return NULL;
> + }
> +
> + pptr = grub_strstr (parent, "/disk@");
A constant please...
> +
> + if (pptr)
> + *pptr = '\0';
> +
> + return parent;
> +}
> +
> +static void
> +free_parent_dev (struct parent_dev *parent)
> +{
> + if (parent)
> + {
> + grub_free (parent->name);
> + grub_free (parent->type);
> + grub_free (parent);
> + }
> +}
> +
> +static struct parent_dev *
> +init_parent (const char *parent)
> +{
> + struct parent_dev *op;
> +
> + op = grub_zalloc (sizeof (struct parent_dev));
> +
> + if (op == NULL)
> + {
> + grub_print_error ();
> + return NULL;
> + }
> +
> + op->name = grub_strdup (parent);
> + op->type = grub_malloc (IEEE1275_MAX_PROP_LEN);
> +
> + if ((op->name == NULL) || (op->type == NULL))
> + {
> + grub_print_error ();
> + free_parent_dev (op);
> + return NULL;
> + }
> +
> + return op;
> +}
> +
> +static struct parent_dev *
> +open_new_parent (const char *parent)
> +{
> + struct parent_dev *op = init_parent(parent);
> + grub_ieee1275_ihandle_t ihandle;
> + grub_ieee1275_phandle_t phandle;
> + grub_uint32_t address_cells = 2;
> + grub_ssize_t actual = 0;
> +
> + if (op == NULL)
> + return NULL;
> +
> + grub_ieee1275_open (parent, &ihandle);
> +
> + if (ihandle == 0)
> + {
> + grub_error (GRUB_ERR_BAD_DEVICE, "unable to open %s", parent);
> + grub_print_error ();
> + free_parent_dev (op);
> + return NULL;
> + }
> +
> + if (grub_ieee1275_instance_to_package (ihandle, &phandle))
> + {
> + grub_error (GRUB_ERR_BAD_DEVICE, "unable to get parent %s", parent);
> + grub_print_error ();
> + free_parent_dev (op);
> + return NULL;
> + }
> +
> + /* IEEE Std 1275-1994 page 110: A missing "address-cells" property
> + signifies that the number of address cells is two. So ignore on error.
> */
> + grub_ieee1275_get_integer_property (phandle, "#address-cells",
> &address_cells,
> + sizeof (address_cells), 0);
> +
> + grub_ieee1275_get_property (phandle, "device_type", op->type,
> + IEEE1275_MAX_PROP_LEN, &actual);
> + op->ihandle = ihandle;
> + op->address_cells = address_cells;
> + return op;
> +}
> +
> +static struct parent_dev *
> +open_parent (const char *parent)
> +{
> + struct parent_dev *op;
> +
> + if ((op =
> + grub_named_list_find (GRUB_AS_NAMED_LIST (parent_devs), parent)) ==
> NULL)
Oh no, please call grub_named_list_find() in separate line and then check op
value.
> + {
> + op = open_new_parent (parent);
> +
> + if (op)
> + grub_list_push (GRUB_AS_LIST_P (&parent_devs), GRUB_AS_LIST (op));
Something is wrong with alignment.
> + }
> +
> + return op;
> +}
> +
> +static void
> +display_parents (void)
> +{
> + struct parent_dev *parent;
> +
> + grub_printf ("-------------------- PARENTS --------------------\n");
> +
> + FOR_LIST_ELEMENTS (parent, parent_devs)
> + {
> + grub_printf ("name: %s\n", parent->name);
> + grub_printf ("type: %s\n", parent->type);
> + grub_printf ("address_cells %x\n", parent->address_cells);
Values in one column?
> + }
> +
> + grub_printf ("-------------------------------------------------\n");
> +}
> +
> +static char *
> +canonicalise_4cell_ua (grub_ieee1275_ihandle_t ihandle, char *unit_address)
> +{
> + grub_uint32_t phy_lo, phy_hi, lun_lo, lun_hi;
> + int valid_phy = 0;
> + grub_size_t size;
> + char *canon = NULL;
> +
> + valid_phy = grub_ieee1275_decode_unit4 (ihandle, unit_address,
> + grub_strlen (unit_address),
> &phy_lo,
> + &phy_hi, &lun_lo, &lun_hi);
> +
> + if ((!valid_phy) && (phy_hi != 0xffffffff))
valid_phy == 0
> + canon = grub_ieee1275_encode_uint4 (ihandle, phy_lo, phy_hi,
> + lun_lo, lun_hi, &size);
> +
> + return canon;
> +}
> +
> +static char *
> +canonicalise_disk (const char *devname)
> +{
> + char *canon, *parent;
> + struct parent_dev *op;
> +
> + canon = grub_ieee1275_canonicalise_devname (devname);
> +
> + if (canon == NULL)
> + {
> + /* This should not happen. */
> + grub_error (GRUB_ERR_BAD_DEVICE, "canonicalise devname failed");
> + grub_print_error ();
> + return NULL;
> + }
> +
> + /* Don't try to open the parent of a virtual device. */
> + if (grub_strstr (canon, "virtual-devices"))
> + return canon;
> +
> + parent = get_parent_devname (canon);
> +
> + if (parent == NULL)
> + return NULL;
> +
> + op = open_parent (parent);
> +
> + /* Devices with 4 address cells can have many different types of addressing
> + (phy, wwn, and target lun). Use the parents encode-unit / decode-unit
> + to find the true canonical name. */
> + if ((op) && (op->address_cells == 4))
> + {
> + char *unit_address, *real_unit_address, *real_canon;
> +
> + unit_address = grub_strstr (canon, "/disk@");
> + unit_address += grub_strlen ("/disk@");
> +
> + if (unit_address == NULL)
> + {
> + /* This should not be possible, but return the canonical name for
> + the non-disk block device. */
> + grub_free (parent);
> + return (canon);
> + }
> +
> + real_unit_address = canonicalise_4cell_ua (op->ihandle, unit_address);
> +
> + if (real_unit_address == NULL)
> + {
> + /* This is not an error, since this function could be called with
> a devalias
> + containing a drive that isn't installed in the system. */
> + grub_free (parent);
> + return NULL;
> + }
> +
> + real_canon = grub_malloc (grub_strlen (op->name) + sizeof ("/disk@") +
> + grub_strlen (real_unit_address));
Please calculate and assign the size in separate line and use it in
grub_malloc() and grub_snprintf().
> +
> + grub_snprintf (real_canon, grub_strlen (op->name) + sizeof ("/disk@") +
> + grub_strlen (real_unit_address), "%s/disk@%s",
> + op->name, real_unit_address);
> +
> + grub_free (canon);
> + canon = real_canon;
> + }
> +
> + grub_free (parent);
> + return (canon);
> +}
> +
> +static struct disk_dev *
> +add_canon_disk (const char *cname)
> +{
> + struct disk_dev *dev;
> +
> + dev = grub_zalloc (sizeof (struct disk_dev));
> +
> + if (!dev)
> + goto failed;
> +
> + if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_RAW_DEVNAMES))
> + {
> + /* Append :nolabel to the end of all SPARC disks.
> + nolabel is mutually exclusive with all other
> + arguments and allows a client program to open
> + the entire (raw) disk. Any disk label is ignored. */
> +
> + dev->raw_name = grub_malloc (grub_strlen (cname) + sizeof
> (":nolabel"));
> +
> + if (dev->raw_name == NULL)
> + goto failed;
> +
> + grub_snprintf (dev->raw_name, grub_strlen (cname) + sizeof
> (":nolabel"),
> + "%s:nolabel", cname);
> + }
> +
> + /* Don't use grub_ieee1275_encode_devname here, the devpath in grub.cfg
> doesn't
> + understand device aliases, which the layer above sometimes sends us. */
> + dev->grub_devpath = encode_grub_devname(cname);
> +
> + if (dev->grub_devpath == NULL)
> + goto failed;
> +
> + dev->name = grub_strdup (cname);
> +
> + if (dev->name == NULL)
> + goto failed;
> +
> + dev->valid = 1;
> + grub_list_push (GRUB_AS_LIST_P (&disk_devs), GRUB_AS_LIST (dev));
> + return dev;
> +
> +failed:
Use one space before each label.
> + grub_print_error ();
> +
> + if (dev)
> + {
> + grub_free (dev->name);
> + grub_free (dev->grub_devpath);
> + grub_free (dev->raw_name);
> + }
> +
> + grub_free (dev);
> + return NULL;
> +}
> +
> +static grub_err_t
> +add_disk (const char *path)
> +{
> + grub_err_t rval = GRUB_ERR_NONE;
> + struct disk_dev *dev;
> + char *canon;
> +
> + canon = canonicalise_disk (path);
> + dev = grub_named_list_find (GRUB_AS_NAMED_LIST (disk_devs), canon);
> +
> + if ((canon != NULL) && (dev == NULL))
> + {
> + struct disk_dev *ob_device;
> +
> + ob_device = add_canon_disk (canon);
> +
> + if (ob_device == NULL)
> + rval = grub_error (GRUB_ERR_OUT_OF_MEMORY, "failure to add disk");
> + }
> + else if (dev)
> + dev->valid = 1;
> +
> + grub_free (canon);
> + return (rval);
> +}
> +
> +static grub_err_t
> +grub_obdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
> + grub_size_t size, char *dest)
> +{
> + grub_err_t rval = GRUB_ERR_NONE;
I would use ret instead of rval here and there.
> + struct disk_dev *dev;
> + unsigned long long pos;
> + grub_ssize_t result = 0;
> +
> + if (disk->data == NULL)
> + return grub_error (GRUB_ERR_BAD_DEVICE, "invalid disk data");
> +
> + dev = (struct disk_dev *)disk->data;
> + pos = sector << disk->log_sector_size;
> + grub_ieee1275_seek (dev->ihandle, pos, &result);
> +
> + if (result < 0)
> + {
> + dev->opened = 0;
> + return grub_error (GRUB_ERR_READ_ERROR, "seek error, can't seek block
> %llu",
> + (long long) sector);
> + }
> +
> + grub_ieee1275_read (dev->ihandle, dest, size << disk->log_sector_size,
> + &result);
> +
> + if (result != (grub_ssize_t) (size << disk->log_sector_size))
s/ / /
> + {
> + dev->opened = 0;
> + return grub_error (GRUB_ERR_READ_ERROR, N_("failure reading sector
> 0x%llx "
> + "from `%s'"),
> + (unsigned long long) sector,
> + disk->name);
> + }
> + return rval;
> +}
> +
> +static void
> +grub_obdisk_close (grub_disk_t disk)
s/grub_obdisk_close/grub_obdisk_clear/?
> +{
> + disk->data = NULL;
> + disk->id = 0;
> + disk->total_sectors = 0;
> + disk->log_sector_size = 0;
grub_memset (disk, 0, sizeof (*disk));?
> +}
> +
> +static void
> +scan_usb_disk (const char *parent)
> +{
> + struct parent_dev *op;
> + grub_ssize_t result;
> +
> + op = open_parent (parent);
> +
> + if (op == NULL)
> + {
> + grub_error (GRUB_ERR_BAD_DEVICE, "unable to open %s", parent);
> + grub_print_error ();
> + return;
> + }
> +
> + if ((grub_ieee1275_set_address (op->ihandle, 0, 0) == 0) &&
> + (grub_ieee1275_no_data_command (op->ihandle, &tur, &result) == 0) &&
> + (result == 0))
> + {
> + char *buf;
> +
> + buf = grub_malloc (IEEE1275_MAX_PATH_LEN);
> +
> + if (buf == NULL)
> + {
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "disk scan failure");
> + grub_print_error ();
> + return;
> + }
> +
> + grub_snprintf (buf, IEEE1275_MAX_PATH_LEN, "%s/disk@0", parent);
> + add_disk (buf);
> + grub_free (buf);
> + }
> +}
> +
> +static void
> +scan_nvme_disk (const char *path)
> +{
> + char *buf;
> +
> + buf = grub_malloc (IEEE1275_MAX_PATH_LEN);
> +
> + if (buf == NULL)
> + {
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "disk scan failure");
> + grub_print_error ();
> + return;
> + }
> +
> + grub_snprintf (buf, IEEE1275_MAX_PATH_LEN, "%s/disk@1", path);
> + add_disk (buf);
> + grub_free (buf);
> +}
> +
> +static void
> +scan_sparc_sas_2cell (struct parent_dev *op)
> +{
> + grub_ssize_t result;
> + grub_uint8_t tgt;
> + char *buf;
> +
> + buf = grub_malloc (IEEE1275_MAX_PATH_LEN);
> +
> + if (buf == NULL)
> + {
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "disk scan failure");
> + grub_print_error ();
> + return;
> + }
> +
> + for (tgt = 0; tgt < 0xf; tgt++)
> + {
> +
> + if ((grub_ieee1275_set_address(op->ihandle, tgt, 0) == 0) &&
> + (grub_ieee1275_no_data_command (op->ihandle, &tur, &result) == 0)
> &&
> + (result == 0))
> + {
> +
> + grub_snprintf (buf, IEEE1275_MAX_PATH_LEN, "%s/disk@%"
> + PRIxGRUB_UINT32_T, op->name, tgt);
> +
> + add_disk (buf);
> + }
> + }
> +}
> +
> +static void
> +scan_sparc_sas_4cell (struct parent_dev *op)
> +{
> + grub_uint16_t exp;
> + grub_uint8_t phy;
> + char *buf;
> +
> + buf = grub_malloc (IEEE1275_MAX_PATH_LEN);
> +
> + if (buf == NULL)
> + {
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "disk scan failure");
> + grub_print_error ();
> + return;
> + }
> +
> + for (exp = 0; exp <= 0x100; exp+=0x100)
> +
> + for (phy = 0; phy < 0x20; phy++)
> + {
> + char *canon = NULL;
> +
> + grub_snprintf (buf, IEEE1275_MAX_PATH_LEN, "p%" PRIxGRUB_UINT32_T
> ",0",
> + exp | phy);
> +
> + canon = canonicalise_4cell_ua (op->ihandle, buf);
> +
> + if (canon)
> + {
> + grub_snprintf (buf, IEEE1275_MAX_PATH_LEN, "%s/disk@%s",
> + op->name, canon);
> +
> + add_disk (buf);
> + grub_free (canon);
> + }
> + }
> +
> + grub_free (buf);
> +}
> +
> +static void
> +scan_sparc_sas_disk (const char *parent)
> +{
> + struct parent_dev *op;
> +
> + op = open_parent (parent);
> +
> + if ((op) && (op->address_cells == 4))
> + scan_sparc_sas_4cell (op);
> + else if ((op) && (op->address_cells == 2))
> + scan_sparc_sas_2cell (op);
> +}
> +
> +static void
> +iterate_devtree (const struct grub_ieee1275_devalias *alias)
> +{
> + struct grub_ieee1275_devalias child;
> +
> + if ((grub_strcmp (alias->type, "scsi-2") == 0) ||
> + (grub_strcmp (alias->type, "scsi-sas") == 0))
> + return scan_sparc_sas_disk (alias->path);
> +
> + else if (grub_strcmp (alias->type, "nvme") == 0)
> + return scan_nvme_disk (alias->path);
> +
> + else if (grub_strcmp (alias->type, "scsi-usb") == 0)
> + return scan_usb_disk (alias->path);
> +
> + else if (grub_strcmp (alias->type, "block") == 0)
> + {
> + const char **bl = block_blacklist;
> +
> + while (*bl != NULL)
> + {
> + if (grub_strstr (alias->path, *bl))
> + return;
> + bl++;
> + }
> +
> + add_disk (alias->path);
> + return;
> + }
> +
> + FOR_IEEE1275_DEVCHILDREN (alias->path, child)
> + iterate_devtree (&child);
> +}
> +
> +static void
> +unescape_devices (void)
> +{
> + struct disk_dev *dev;
> +
> + FOR_LIST_ELEMENTS (dev, disk_devs)
> + {
> + grub_free (dev->grub_decoded_devpath);
> +
> + if ((dev->grub_alias_devpath) &&
> + (grub_strcmp (dev->grub_alias_devpath, dev->grub_devpath) != 0))
> + dev->grub_decoded_devpath =
> + remove_escaped_commas (grub_strdup (dev->grub_alias_devpath));
> + else
> + dev->grub_decoded_devpath =
> + remove_escaped_commas (grub_strdup (dev->grub_devpath));
> + }
> +}
> +
> +static void
> +enumerate_disks (void)
> +{
> + struct grub_ieee1275_devalias alias;
> +
> + FOR_IEEE1275_DEVCHILDREN("/", alias)
> + iterate_devtree (&alias);
> +}
> +
> +static grub_err_t
> +add_bootpath (void)
> +{
> + struct disk_dev *ob_device;
> + grub_err_t rval = GRUB_ERR_NONE;
> + char *dev, *alias;
> + char *type;
> +
> + dev = grub_ieee1275_get_boot_dev ();
> +
> + if (dev == NULL)
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "failure adding boot device");
> +
> + type = grub_ieee1275_get_device_type (dev);
> +
> + if (type == NULL)
> + {
> + grub_free (dev);
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "failure adding boot
> device");
> + }
> +
> + alias = NULL;
> +
> + if (!(grub_strcmp (type, "network") == 0))
grub_strcmp (type, "network") != 0
> + {
> + dev = strip_ob_partition (dev);
> + ob_device = add_canon_disk (dev);
> +
> + if (ob_device == NULL)
> + rval = grub_error (GRUB_ERR_OUT_OF_MEMORY, "failure adding boot
> device");
> +
> + ob_device->valid = 1;
> +
> + alias = grub_ieee1275_get_devname (dev);
> +
> + if (alias && grub_strcmp (alias, dev) != 0)
> + ob_device->grub_alias_devpath = grub_ieee1275_encode_devname (dev);
> +
> + ob_device->boot_dev = 1;
> + }
> +
> + grub_free (type);
> + grub_free (dev);
> + grub_free (alias);
> + return rval;
> +}
> +
> +static void
> +enumerate_aliases (void)
> +{
> + struct grub_ieee1275_devalias alias;
> +
> + /* Some block device aliases are not in canonical form
> +
> + For example:
> +
> + disk3 /pci@301/pci@1/scsi@0/disk@p3
> + disk2 /pci@301/pci@1/scsi@0/disk@p2
> + disk1 /pci@301/pci@1/scsi@0/disk@p1
> + disk /pci@301/pci@1/scsi@0/disk@p0
> + disk0 /pci@301/pci@1/scsi@0/disk@p0
> +
> + None of these devices are in canonical form.
> +
> + Also, just because there is a devalias, doesn't mean there is a disk
> + at that location. And a valid boot block device doesn't have to have
> + a devalias at all.
> +
> + At this point, all valid disks have been found in the system
> + and devaliases that point to canonical names are stored in the
> + disk_devs list already. */
I do not like comments formated in that way because it is difficult to
differentiate code from comments at first sight. I know that coding style
says something different but I am going to change it. So, please adhere
to Linux kernel comments style. Here and there.
> + FOR_IEEE1275_DEVALIASES (alias)
> + {
> + struct disk_dev *dev;
> + char *canon;
> +
> + if (grub_strcmp (alias.type, "block") != 0)
> + continue;
> +
> + canon = canonicalise_disk (alias.name);
> +
> + if (canon == NULL)
> + /* This is not an error, a devalias could point to a
> + nonexistent disk */
> + continue;
> +
> + dev = grub_named_list_find (GRUB_AS_NAMED_LIST (disk_devs), canon);
> +
> + if (dev)
> + {
> + /* If more than one alias points to the same device,
> + remove the previous one unless it is the boot dev,
> + since the upper level will use the first one. The reason
> + all the others are redone is in the case of hot-plugging
> + a disk. If the boot disk gets hot-plugged, it will come
> + thru here with a different name without the boot_dev flag
> + set. */
Ditto...
> + if ((dev->boot_dev) && (dev->grub_alias_devpath))
> + continue;
> +
> + grub_free (dev->grub_alias_devpath);
> + dev->grub_alias_devpath = grub_ieee1275_encode_devname
> (alias.path);
> + }
> + grub_free (canon);
> + }
> +}
> +
> +static void
> +display_disks (void)
> +{
> + struct disk_dev *dev;
> +
> + grub_printf ("--------------------- DISKS ---------------------\n");
> +
> + FOR_LIST_ELEMENTS (dev, disk_devs)
> + {
> + grub_printf ("name: %s\n", dev->name);
> + grub_printf ("grub_devpath: %s\n", dev->grub_devpath);
> + grub_printf ("grub_alias_devpath: %s\n", dev->grub_alias_devpath);
> + grub_printf ("grub_decoded_devpath: %s\n", dev->grub_decoded_devpath);
> + grub_printf ("valid: %s\n", (dev->valid) ? "yes" : "no");
> + grub_printf ("boot_dev: %s\n", (dev->boot_dev) ? "yes" : "no");
> + grub_printf ("opened: %s\n", (dev->ihandle) ? "yes" : "no");
> + grub_printf ("block size: %" PRIuGRUB_UINT32_T "\n", dev->block_size);
> + grub_printf ("num blocks: %" PRIuGRUB_UINT64_T "\n", dev->num_blocks);
> + grub_printf ("log sector size: %" PRIuGRUB_UINT32_T "\n",
> + dev->log_sector_size);
> + grub_printf ("\n");
Could not you put all values in one column?
> + }
> +
> + grub_printf ("-------------------------------------------------\n");
> +}
> +
> +static void
> +display_stats (void)
> +{
> + const char *debug = grub_env_get ("debug");
> +
> + if (! debug)
Please be consistent...
> + return;
> +
> + if (grub_strword (debug, "all") || grub_strword (debug, "obdisk"))
> + {
> + display_parents ();
> + display_disks ();
> + }
> +}
> +
> +static void
> +invalidate_all_disks (void)
> +{
> + struct disk_dev *dev = NULL;
> +
> + if (disks_enumerated)
> + FOR_LIST_ELEMENTS (dev, disk_devs)
> + dev->valid = 0;
> +}
> +
> +/* This is for backwards compatibility, since the path should be generated
> + correctly now. */
> +static struct disk_dev *
> +find_legacy_grub_devpath (const char *name)
> +{
> + struct disk_dev *dev = NULL;
> + char *canon, *devpath = NULL;
> +
> + devpath = decode_grub_devname (name + sizeof ("ieee1275"));
> + canon = canonicalise_disk (devpath);
> +
> + if (canon != NULL)
> + dev = grub_named_list_find (GRUB_AS_NAMED_LIST (disk_devs), canon);
> +
> + grub_free (devpath);
> + grub_free (canon);
> + return dev;
> +}
> +
> +static void
> +enumerate_devices (void)
> +{
> + invalidate_all_disks ();
> + enumerate_disks ();
> + enumerate_aliases ();
> + unescape_devices ();
> + disks_enumerated = 1;
> + display_stats ();
> +}
> +
> +static struct disk_dev *
> +find_grub_devpath_real (const char *name)
> +{
> + struct disk_dev *dev = NULL;
> +
> + FOR_LIST_ELEMENTS (dev, disk_devs)
> + {
> + if ((STRCMP (dev->grub_devpath, name))
> + || (STRCMP (dev->grub_alias_devpath, name))
> + || (STRCMP (dev->grub_decoded_devpath, name)))
> + break;
> + }
> +
> + return dev;
> +}
> +
> +static struct disk_dev *
> +find_grub_devpath (const char *name)
> +{
> + struct disk_dev *dev = NULL;
> + int enumerated;
> +
> + do {
> + enumerated = disks_enumerated;
> + dev = find_grub_devpath_real (name);
> +
> + if (dev)
> + break;
> +
> + dev = find_legacy_grub_devpath (name);
> +
> + if (dev)
> + break;
> +
> + enumerate_devices ();
> + } while (enumerated == 0);
> +
> + return dev;
> +}
> +
> +static int
> +grub_obdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
> + grub_disk_pull_t pull)
> +{
> + struct disk_dev *dev;
> +
> + if (pull != GRUB_DISK_PULL_NONE)
> + return 0;
> +
> + enumerate_devices ();
> +
> + FOR_LIST_ELEMENTS (dev, disk_devs)
> + {
> + if (dev->valid == 1)
> + if (hook (dev->grub_decoded_devpath, hook_data))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static grub_err_t
> +grub_obdisk_open (const char *name, grub_disk_t disk)
> +{
> + grub_ieee1275_ihandle_t ihandle = 0;
> + struct disk_dev *dev = NULL;
> +
> + if (grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/") - 1) != 0)
> + return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not IEEE1275 device");
> +
> + dev = find_grub_devpath (name);
> +
> + if (dev == NULL)
> + {
> + grub_printf ("UNKNOWN DEVICE: %s\n", name);
> + return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "%s", name);
> + }
> +
> + if (dev->opened == 0)
> + {
> + if (dev->raw_name)
> + grub_ieee1275_open (dev->raw_name, &ihandle);
> + else
> + grub_ieee1275_open (dev->name, &ihandle);
> +
> + if (ihandle == 0)
> + {
> + grub_printf ("Can't open device %s\n", name);
> + return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "can't open device
> %s", name);
> + }
> +
> + dev->block_size = grub_ieee1275_get_block_size (ihandle);
> + dev->num_blocks = grub_ieee1275_num_blocks (ihandle);
> +
> + if (dev->num_blocks == 0)
> + dev->num_blocks = grub_ieee1275_num_blocks64 (ihandle);
> +
> + if (dev->num_blocks == 0)
> + dev->num_blocks = GRUB_DISK_SIZE_UNKNOWN;
> +
> + if (dev->block_size != 0)
> + {
> + for (dev->log_sector_size = 0;
> + (1U << dev->log_sector_size) < dev->block_size;
> + dev->log_sector_size++);
> + }
> + else
> + dev->log_sector_size = 9;
> +
> + dev->ihandle = ihandle;
> + dev->opened = 1;
> + }
> +
> + disk->total_sectors = dev->num_blocks;
> + disk->id = dev->ihandle;
> + disk->data = dev;
> + disk->log_sector_size = dev->log_sector_size;
> + return GRUB_ERR_NONE;
> +}
> +
> +
> +static struct grub_disk_dev grub_obdisk_dev =
> + {
> + .name = "obdisk",
> + .id = GRUB_DISK_DEVICE_OBDISK_ID,
> + .iterate = grub_obdisk_iterate,
> + .open = grub_obdisk_open,
> + .close = grub_obdisk_close,
> + .read = grub_obdisk_read,
> + .next = 0
Assignments in one column please... And drop 0 assignment.
Compiler is your friend.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
