On Wed, Mar 06, 2019 at 08:38:27AM -0700, Eric Snowberg wrote: > > 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.
OK, thanks. Then Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> If there are no objections I will apply this patch next week. Thank you for doing the work. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
