> On Mar 6, 2019, at 4:32 AM, Daniel Kiper <daniel.ki...@oracle.com> wrote: > > On Mon, Mar 04, 2019 at 05:27:39PM -0800, 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 current IEEE1275 ofdisk driver may be suitable for PPC and x86, it >> 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> > > In the general I am OK with the patch but... > >> --- >> Changes since V2 (all requested in the last review): >> >> Changed all if (x) to if (x != NULL) >> >> Removed static NULL initializations >> >> Removed all code to allow a user to easily retrieve the disk name thru the >> shell without needing to understand the grub2 escaping policy for >> disk names with commas. SPARC disks frequently contain commas, figuring >> this out is now up to the user. Cut and pasting the disk name is no >> longer possible. Also, this will make automated testing thru the shell >> extremely difficult. >> >> Changed the order of the functions to escape_commas(), >> replace_escaped_commas(), and finally count_commas(). > > ... I am a bit confused with this. You left escape_commas() and > count_commas(). Both are used in encode_grub_devname(). IMO this does > not agree with "Removed all code to allow a user to easily retrieve the > disk name thru the shell without needing to understand the grub2 > escaping policy for disk names with commas." Does it? >
The disk enumeration code within this driver retrieves device names. For example it can find all SAS disks in a system. These device names are not in a format understandable by GRUB2. These functions are required for converting a device name OpenFirmware understands to a device name GRUB2 understands. These functions have nothing to do with the code for a user to easily retrieve the disk name thru the shell. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
