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? Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel