On Thu, Feb 08, 2018 at 08:37:27AM -0700, Eric Snowberg wrote: > > On Feb 8, 2018, at 8:11 AM, Daniel Kiper <daniel.ki...@oracle.com> wrote: > > On Wed, Feb 07, 2018 at 10:39:00AM -0700, Eric Snowberg wrote: > >>> On Feb 7, 2018, at 4:53 AM, Daniel Kiper <daniel.ki...@oracle.com> wrote: > >>> On Tue, Jan 30, 2018 at 08:49:48PM -0800, Eric Snowberg wrote: > >>>> Fix the Open Firmware (OF) path property for sun4v SPARC systems. > >>>> These platforms do not have a /sas/ within their path. Over time > >>>> different OF addressing schemes have been supported. There > >>>> is no generic addressing scheme that works across every HBA. > >>>> > >>>> Signed-off-by: Eric Snowberg <eric.snowb...@oracle.com> > >>> > >>> In general Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>... > >>> > >>> Just a few comments below... > >>> > >>>> --- > >>>> V2: > >>>> - Requested coding style changes > >>>> --- > >>>> grub-core/osdep/linux/ofpath.c | 148 > >>>> +++++++++++++++++++++++++++++++++++++++- > >>>> 1 files changed, 145 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/grub-core/osdep/linux/ofpath.c > >>>> b/grub-core/osdep/linux/ofpath.c > >>>> index dce4e59..5369508 100644 > >>>> --- a/grub-core/osdep/linux/ofpath.c > >>>> +++ b/grub-core/osdep/linux/ofpath.c > >>>> @@ -38,6 +38,46 @@ > >>>> #include <errno.h> > >>>> #include <ctype.h> > >>>> > >>>> +#ifdef __sparc__ > >>> > >>> I have discussed this with Vladimir. It looks that this will not > >>> work if you try to cross-install SPARC GRUB2 binary using e.g. > >>> x86 grub-install. By default it should work. However, we will also > >>> have other issues here, like lack of access to OF firmware/paths, > >>> which make such configs unusable anyway. So, I would leave this patch > >>> as is for time being. If somebody cares then he/she should fix it. > >>> > >>> Anyway, I will add something like that into the commit message before > >>> committing. > >> > >> That is fine with me. > > > > Great! Thanks! > > > >>>> +typedef enum > >>>> + { > >>>> + GRUB_OFPATH_SPARC_WWN_ADDR = 1, > >>>> + GRUB_OFPATH_SPARC_TGT_LUN, > >>>> + } ofpath_sparc_addressing; > >>>> + > >>>> +struct ofpath_sparc_hba > >>>> +{ > >>>> + grub_uint32_t device_id; > >>>> + ofpath_sparc_addressing addressing; > >>>> +}; > >>>> + > >>>> +static struct ofpath_sparc_hba sparc_lsi_hba[] = { > >>>> + /* Rhea, Jasper 320, LSI53C1020/1030. */ > >>>> + {0x30, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* SAS-1068E. */ > >>>> + {0x50, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* SAS-1064E. */ > >>>> + {0x56, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* Pandora SAS-1068E. */ > >>>> + {0x58, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* Aspen, Invader, LSI SAS-3108. */ > >>>> + {0x5d, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* Niwot, SAS 2108. */ > >>>> + {0x79, GRUB_OFPATH_SPARC_TGT_LUN}, > >>>> + /* Erie, Falcon, LSI SAS 2008. */ > >>>> + {0x72, GRUB_OFPATH_SPARC_WWN_ADDR}, > >>>> + /* LSI WarpDrive 6203. */ > >>>> + {0x7e, GRUB_OFPATH_SPARC_WWN_ADDR}, > >>>> + /* LSI SAS 2308. */ > >>>> + {0x87, GRUB_OFPATH_SPARC_WWN_ADDR}, > >>>> + /* LSI SAS 3008. */ > >>>> + {0x97, GRUB_OFPATH_SPARC_WWN_ADDR}, > >>>> + {0, 0} > >>>> +}; > >>>> + > >>>> +static const int LSI_VENDOR_ID = 0x1000; > >>>> +#endif > >>>> + > >>>> #ifdef OFPATH_STANDALONE > >>>> #define xmalloc malloc > >>>> void > >>>> @@ -338,6 +378,67 @@ vendor_is_ATA(const char *path) > >>>> return (memcmp(bufcont, "ATA", 3) == 0); > >>>> } > >>>> > >>>> +#ifdef __sparc__ > >>>> +static void > >>>> +check_hba_identifiers (const char *sysfs_path, int *vendor, int > >>>> *device_id) > >>>> +{ > >>>> + char *ed = strstr (sysfs_path, "host"); > >>>> + size_t path_size; > >>>> + char *p, *path; > >>>> + char buf[8]; > >>>> + int fd; > >>>> + > >>>> + if (!ed) > >>>> + return; > >>>> + > >>>> + p = xstrdup (sysfs_path); > >>>> + ed = strstr (p, "host"); > >>>> + > >>>> + if (!ed) > >>>> + goto out; > >>> > >>> In normal circumstances this will never ever fail. > >>> If you are OK I will drop this "if" before committing. > >> > >> I put that in for the case if the xstrdup above can not allocate memory. > >> Then p will be null, the strstr will fail and then we should jump to out. > > > > If this would be the case then you should check p for NULL before > > strstr() call. AIUI its behavior is not well defined if one of > > arguments is NULL. Anyway, xstrdup() does not return if it fails. > > So, we do not have such problem here. > > Ok, do you want to remove it? Or should I send out an updated patch?
I will remove it before commit. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel