Re: [PATCH] sparc64: fix OF path names for sun4v systems

2018-01-29 Thread John Paul Adrian Glaubitz

Hi!

On 12/19/2017 10:36 PM, Daniel Kiper wrote:

I think that change was suggested by me as the code wouldn't build with
-Werror without the initialization. -Werror is the default when building
GRUB, if I remember correctly.


Eric, if it is true please leave the initialization otherwise drop it.


Any chance we can continue with this patch series?

Thanks,

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2017-12-19 Thread Daniel Kiper
On Mon, Dec 18, 2017 at 11:58:02PM +0100, John Paul Adrian Glaubitz wrote:
> On 12/18/2017 10:30 PM, Eric Snowberg wrote:

[...]

> >>> +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 = NULL, *path = NULL;
> >>
> >> I think that you do not need to initialize *p here.
> >
> > I’ll remove the initialization.
>
> I think that change was suggested by me as the code wouldn't build with
> -Werror without the initialization. -Werror is the default when building
> GRUB, if I remember correctly.

Eric, if it is true please leave the initialization otherwise drop it.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2017-12-19 Thread Daniel Kiper
On Mon, Dec 18, 2017 at 02:30:51PM -0700, Eric Snowberg wrote:
> > On Dec 18, 2017, at 8:22 AM, Daniel Kiper  wrote:
> > On Wed, Dec 06, 2017 at 03:53:30PM -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 
> >> ---
> >> grub-core/osdep/linux/ofpath.c | 147 
> >> -
> >> 1 file changed, 144 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/grub-core/osdep/linux/ofpath.c 
> >> b/grub-core/osdep/linux/ofpath.c
> >> index 3a8bc95a9..525a42ae6 100644
> >> --- a/grub-core/osdep/linux/ofpath.c
> >> +++ b/grub-core/osdep/linux/ofpath.c
> >> @@ -38,6 +38,44 @@
> >> #include 
> >> #include 
> >>
> >> +#ifdef __sparc__
> >
> > This is not good. It will not work if you cross compile.
>
> What error do you see on a cross compile?  I see __sparc__ used throughout 
> the code.

It seems to me that Vladimir (CC-ed) complained about that somewhere.
However, it looks that this check is OK. So, if Vladimir is OK with
that we can leave it as is.

[...]

> >> +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 = NULL, *path = NULL;
> >
> > I think that you do not need to initialize *p here.
>
> I’ll remove the initialization.

Thanks!

> >> +  char buf[8];
> >> +  int fd;
> >> +
> >> +  if (!ed)
> >> +return;
> >> +
> >> +  p = xstrdup (sysfs_path);
> >
> > Why do you need to duplicate sysfs_path?
> > I do not think it is needed. Just p = sysfs_path?
>
> This is duplicated since the result of p is modified ...
>
> >
> >> +  ed = strstr (p, "host");
> >> +
> >> +  if (!ed)
> >> +goto out;
> >> +
> >> +  *ed = '\0’;
>
> right here.  If I didn’t duplicate the string, it would corrupt
> the sysfs_path. Also, sysfs_path is defined as const char *.

Ahhh... Right I have missed that. Sorry.

> >> +
> >> +  path_size = (strlen (p) + sizeof ("vendor"));
> >> +  path = xmalloc (path_size);
> >> +
> >> +  if (!path)
> >> +goto out;
> >> +
> >> +  snprintf (path, path_size, "%svendor", p);
> >> +  fd = open (path, O_RDONLY);
> >> +
> >> +  if (fd < 0)
> >> +goto out;
> >> +
> >> +  memset (buf, 0, sizeof (buf));
> >> +
> >> +  if (read (fd, buf, sizeof (buf) - 1) < 0)
> >> +goto out;
> >> +
> >> +  close (fd);
> >> +  sscanf (buf, "%x", vendor);
> >
> > Please add empty line here.
> >
> >> +  snprintf (path, path_size, "%sdevice", p);
> >
> > I have a feeling that p should be changed somehow here
> > or to be precise a bit earlier... Should not it?
>
> It is being changed above?  *ed is a pointer within it.

Yes, but only once. Later string pointed by p is not
changed. The same value is used for "%svendor" and
"%sdevice". Is it correct? Am I missing something?

[...]

> > In general I am not happy with sscanf() usage as a string to number
> > converter. Especially without any error checking. However, as I can
> > see, it is common here. So, I will accept fixed patch with sscanf()
> > eventually but I would be happy if you replace it everywhere with
> > something more robust in separate patch.
>
> I could look at doing that in the future. I always try to follow the
> coding style within the file.

Great! Thanks!

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2017-12-18 Thread John Paul Adrian Glaubitz
On 12/18/2017 10:30 PM, Eric Snowberg wrote:
>> This is not good. It will not work if you cross compile.
> 
> What error do you see on a cross compile?  I see __sparc__ used throughout 
> the code.

I agree. This should not cause any issues when cross-compiling, the 
cross-compiler
targeting __$ARCH__ always comes with the same compiler macros and definitions
as the native compiler.

>>> +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 = NULL, *path = NULL;
>>
>> I think that you do not need to initialize *p here.
> 
> I’ll remove the initialization. 

I think that change was suggested by me as the code wouldn't build with
-Werror without the initialization. -Werror is the default when building
GRUB, if I remember correctly.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2017-12-18 Thread Eric Snowberg

> On Dec 18, 2017, at 8:22 AM, Daniel Kiper  wrote:
> 
> On Wed, Dec 06, 2017 at 03:53:30PM -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 
>> ---
>> grub-core/osdep/linux/ofpath.c | 147 
>> -
>> 1 file changed, 144 insertions(+), 3 deletions(-)
>> 
>> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
>> index 3a8bc95a9..525a42ae6 100644
>> --- a/grub-core/osdep/linux/ofpath.c
>> +++ b/grub-core/osdep/linux/ofpath.c
>> @@ -38,6 +38,44 @@
>> #include 
>> #include 
>> 
>> +#ifdef __sparc__
> 
> This is not good. It will not work if you cross compile.

What error do you see on a cross compile?  I see __sparc__ used throughout the 
code.

> 
>> +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.  */
> 
> Extra space after ".”.

I’ll take care of all the extra space changes in V2.

> 
>> +  {0x30, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* SAS-1068E.  */
> 
> Ditto and below...
> 
>> +  {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}
>> +};
>> +#endif
>> +
>> #ifdef OFPATH_STANDALONE
>> #define xmalloc malloc
>> void
>> @@ -337,6 +375,66 @@ vendor_is_ATA(const char *path)
>>   return (memcmp(bufcont, "ATA", 3) == 0);
>> }
>> 
>> +#ifdef __sparc__
> 
> Ditto.
> 
>> +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 = NULL, *path = NULL;
> 
> I think that you do not need to initialize *p here.

I’ll remove the initialization. 

> 
>> +  char buf[8];
>> +  int fd;
>> +
>> +  if (!ed)
>> +return;
>> +
>> +  p = xstrdup (sysfs_path);
> 
> Why do you need to duplicate sysfs_path?
> I do not think it is needed. Just p = sysfs_path?

This is duplicated since the result of p is modified ...

> 
>> +  ed = strstr (p, "host");
>> +
>> +  if (!ed)
>> +goto out;
>> +
>> +  *ed = '\0’;

right here.  If I didn’t duplicate the string, it would corrupt the sysfs_path. 
Also, sysfs_path is defined as const char *.

>> +
>> +  path_size = (strlen (p) + sizeof ("vendor"));
>> +  path = xmalloc (path_size);
>> +
>> +  if (!path)
>> +goto out;
>> +
>> +  snprintf (path, path_size, "%svendor", p);
>> +  fd = open (path, O_RDONLY);
>> +
>> +  if (fd < 0)
>> +goto out;
>> +
>> +  memset (buf, 0, sizeof (buf));
>> +
>> +  if (read (fd, buf, sizeof (buf) - 1) < 0)
>> +goto out;
>> +
>> +  close (fd);
>> +  sscanf (buf, "%x", vendor);
> 
> Please add empty line here.
> 
>> +  snprintf (path, path_size, "%sdevice", p);
> 
> I have a feeling that p should be changed somehow here
> or to be precise a bit earlier... Should not it?

It is being changed above?  *ed is a pointer within it.

> 
>> +  fd = open (path, O_RDONLY);
>> +
>> +  if (fd < 0)
>> +goto out;
>> +
>> +  memset (buf, 0, sizeof (buf));
>> +
>> +  if (read (fd, buf, sizeof (buf) - 1) < 0)
>> +goto out;
>> +
>> +  close (fd);
>> +  sscanf (buf, "%x", device_id);
>> +
>> +out:
> 
> err:? And please add one extra space before the label.
> 
>> +  free (path);
>> +  free (p);
>> +}
>> +#endif
>> +
>> static void
>> check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
>> {
>> @@ -398,7 +496,7 @@ of_path_of_scsi(const char *sys_devname 
>> __attribute__((unused)), const char *dev
>> {
>>   const char *p, *digit_string, *disk_name;
>>   int host, bus, tgt, lun;
>> -  unsigned long int sas_address;
>> +  unsigned long int sas_address = 0;
>>   char *sysfs_path, disk[MAX_DISK_CAT - sizeof ("/fp@0,0")];
>>   char *of_path;
>> 
>> @@ -415,9 +513,11 @@ of_path_of_scsi(const char *sys_devname 
>> __attribute__((unused)), const char *dev
>> }
>> 
>>   of_path = find_obppath(sysfs_path);
>> -  free (sysfs_path);
>>   if (!of_path)
>> -return NULL;
> 
> goto err:
> 
>> +{
>> +  free (sysfs

Re: [PATCH] sparc64: fix OF path names for sun4v systems

2017-12-18 Thread Daniel Kiper
On Wed, Dec 06, 2017 at 03:53:30PM -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 
> ---
>  grub-core/osdep/linux/ofpath.c | 147 
> -
>  1 file changed, 144 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> index 3a8bc95a9..525a42ae6 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -38,6 +38,44 @@
>  #include 
>  #include 
>
> +#ifdef __sparc__

This is not good. It will not work if you cross compile.

> +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.  */

Extra space after ".".

> +  {0x30, GRUB_OFPATH_SPARC_TGT_LUN},
> +  /* SAS-1068E.  */

Ditto and below...

> +  {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}
> +};
> +#endif
> +
>  #ifdef OFPATH_STANDALONE
>  #define xmalloc malloc
>  void
> @@ -337,6 +375,66 @@ vendor_is_ATA(const char *path)
>return (memcmp(bufcont, "ATA", 3) == 0);
>  }
>
> +#ifdef __sparc__

Ditto.

> +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 = NULL, *path = NULL;

I think that you do not need to initialize *p here.

> +  char buf[8];
> +  int fd;
> +
> +  if (!ed)
> +return;
> +
> +  p = xstrdup (sysfs_path);

Why do you need to duplicate sysfs_path?
I do not think it is needed. Just p = sysfs_path?

> +  ed = strstr (p, "host");
> +
> +  if (!ed)
> +goto out;
> +
> +  *ed = '\0';
> +
> +  path_size = (strlen (p) + sizeof ("vendor"));
> +  path = xmalloc (path_size);
> +
> +  if (!path)
> +goto out;
> +
> +  snprintf (path, path_size, "%svendor", p);
> +  fd = open (path, O_RDONLY);
> +
> +  if (fd < 0)
> +goto out;
> +
> +  memset (buf, 0, sizeof (buf));
> +
> +  if (read (fd, buf, sizeof (buf) - 1) < 0)
> +goto out;
> +
> +  close (fd);
> +  sscanf (buf, "%x", vendor);

Please add empty line here.

> +  snprintf (path, path_size, "%sdevice", p);

I have a feeling that p should be changed somehow here
or to be precise a bit earlier... Should not it?

> +  fd = open (path, O_RDONLY);
> +
> +  if (fd < 0)
> +goto out;
> +
> +  memset (buf, 0, sizeof (buf));
> +
> +  if (read (fd, buf, sizeof (buf) - 1) < 0)
> +goto out;
> +
> +  close (fd);
> +  sscanf (buf, "%x", device_id);
> +
> +out:

err:? And please add one extra space before the label.

> +  free (path);
> +  free (p);
> +}
> +#endif
> +
>  static void
>  check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
>  {
> @@ -398,7 +496,7 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>  {
>const char *p, *digit_string, *disk_name;
>int host, bus, tgt, lun;
> -  unsigned long int sas_address;
> +  unsigned long int sas_address = 0;
>char *sysfs_path, disk[MAX_DISK_CAT - sizeof ("/fp@0,0")];
>char *of_path;
>
> @@ -415,9 +513,11 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>  }
>
>of_path = find_obppath(sysfs_path);
> -  free (sysfs_path);
>if (!of_path)
> -return NULL;

goto err:

> +{
> +  free (sysfs_path);
> +  return NULL;
> +}

Drop this change.

>
>if (strstr (of_path, "qlc"))
>  strcat (of_path, "/fp@0,0");
> @@ -446,6 +546,45 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>  }
>else
>  {
> +#ifdef __sparc__

Ditto.

> +  ofpath_sparc_addressing addressing = GRUB_OFPATH_SPARC_TGT_LUN;
> +  int vendor = 0, device_id = 0;
> +  char *optr = disk;
> +
> +  check_hba_identifiers (sysfs_path, &vendor, &device_id);
> +
> +  /* LSI Logic Vendor ID */
> +  if (vendor == 0x1000)

Could you define a constant?

> +{
> +  struct ofpath_sparc_hba *lsi_hba;
> +
> +

[PATCH] sparc64: fix OF path names for sun4v systems

2017-12-06 Thread Eric Snowberg
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 
---
 grub-core/osdep/linux/ofpath.c | 147 -
 1 file changed, 144 insertions(+), 3 deletions(-)

diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index 3a8bc95a9..525a42ae6 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -38,6 +38,44 @@
 #include 
 #include 
 
+#ifdef __sparc__
+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}
+};
+#endif
+
 #ifdef OFPATH_STANDALONE
 #define xmalloc malloc
 void
@@ -337,6 +375,66 @@ 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 = NULL, *path = NULL;
+  char buf[8];
+  int fd;
+
+  if (!ed)
+return;
+
+  p = xstrdup (sysfs_path);
+  ed = strstr (p, "host");
+
+  if (!ed)
+goto out;
+
+  *ed = '\0';
+
+  path_size = (strlen (p) + sizeof ("vendor"));
+  path = xmalloc (path_size);
+
+  if (!path)
+goto out;
+
+  snprintf (path, path_size, "%svendor", p);
+  fd = open (path, O_RDONLY);
+
+  if (fd < 0)
+goto out;
+
+  memset (buf, 0, sizeof (buf));
+
+  if (read (fd, buf, sizeof (buf) - 1) < 0)
+goto out;
+
+  close (fd);
+  sscanf (buf, "%x", vendor);
+  snprintf (path, path_size, "%sdevice", p);
+  fd = open (path, O_RDONLY);
+
+  if (fd < 0)
+goto out;
+
+  memset (buf, 0, sizeof (buf));
+
+  if (read (fd, buf, sizeof (buf) - 1) < 0)
+goto out;
+
+  close (fd);
+  sscanf (buf, "%x", device_id);
+
+out:
+  free (path);
+  free (p);
+}
+#endif
+
 static void
 check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
 {
@@ -398,7 +496,7 @@ of_path_of_scsi(const char *sys_devname 
__attribute__((unused)), const char *dev
 {
   const char *p, *digit_string, *disk_name;
   int host, bus, tgt, lun;
-  unsigned long int sas_address;
+  unsigned long int sas_address = 0;
   char *sysfs_path, disk[MAX_DISK_CAT - sizeof ("/fp@0,0")];
   char *of_path;
 
@@ -415,9 +513,11 @@ of_path_of_scsi(const char *sys_devname 
__attribute__((unused)), const char *dev
 }
 
   of_path = find_obppath(sysfs_path);
-  free (sysfs_path);
   if (!of_path)
-return NULL;
+{
+  free (sysfs_path);
+  return NULL;
+}
 
   if (strstr (of_path, "qlc"))
 strcat (of_path, "/fp@0,0");
@@ -446,6 +546,45 @@ of_path_of_scsi(const char *sys_devname 
__attribute__((unused)), const char *dev
 }
   else
 {
+#ifdef __sparc__
+  ofpath_sparc_addressing addressing = GRUB_OFPATH_SPARC_TGT_LUN;
+  int vendor = 0, device_id = 0;
+  char *optr = disk;
+
+  check_hba_identifiers (sysfs_path, &vendor, &device_id);
+
+  /* LSI Logic Vendor ID */
+  if (vendor == 0x1000)
+{
+  struct ofpath_sparc_hba *lsi_hba;
+
+  /* Over time different OF addressing schemes have been supported.
+ There is no generic addressing scheme that works across
+ every HBA. */
+  for (lsi_hba = sparc_lsi_hba; lsi_hba->device_id; lsi_hba++)
+if (lsi_hba->device_id == device_id)
+  {
+addressing = lsi_hba->addressing;
+break;
+  }
+}
+
+  if (addressing == GRUB_OFPATH_SPARC_WWN_ADDR)
+optr += snprintf (disk, sizeof (disk), "/%s@w%lx,%x", disk_name,
+  sas_address, lun);
+  else
+optr += snprintf (disk, sizeof (disk), "/%s@%x,%x", disk_name, tgt,
+  lun);
+
+  if (*digit_string != '\0')
+{
+  int part;
+
+  sscanf (digit_string, "%d", &part);
+  snprintf (optr, sizeof (disk) - (optr - disk - 1), ":%c", 'a'
+  

Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-17 Thread Eric Snowberg

> On Feb 16, 2016, at 11:45 PM, Seth Goldberg  wrote:
> 
> 
> 
>> On Feb 16, 2016, at 7:24 PM, Andrei Borzenkov  wrote:
>> 
>> 17.02.2016 05:02, Eric Snowberg пишет:
>>> 
 On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
 
 On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
  wrote:
> Fix the open firmware 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 
> ---
> grub-core/osdep/linux/ofpath.c |  192 
> +++-
> 1 files changed, 190 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/osdep/linux/ofpath.c 
> b/grub-core/osdep/linux/ofpath.c
> index a79682a..de51c57 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
 ...
> @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>   }
> else
>   {
> +#ifdef __sparc__
> +  int vendor = 0, device_id = 0;
> +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
> +
> +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
> +{
> +   /* Over time different OF addressing schemes have been 
> supported */
> +   /* There is no generic addressing scheme that works across */
> +   /* every HBA */
> +  switch (device_id)
> +{
> +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
> +case 0x50: /* SAS-1068E */
> +case 0x56: /* SAS-1064E */
> +case 0x58: /* Pandora SAS-1068E */
> +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
> +case 0x79: /* Niwot, SAS 2108   */
 
 That really does not scale. Can we enumerate device aliases and take
 diskN and cdromN? So that we do not need to duplicate OBP logic?
>>> 
>>> Do you have an idea in mind on how to do this differently to make it scale 
>>> better?  This patch will default to the old disk@N for unknown HBAs.  I 
>>> agree it doesn’t scale that well, but new HBAs for SPARC with OF support 
>>> don't come out that often. 
>>> 
>>> Before this patch, there isn’t a single SAS HBA that is enumerated 
>>> correctly on SPARC.  I went back 10 years and believe I have added every 
>>> HBA with OF support.  This isn’t duplicating OBP logic.  The final part of 
>>> the device path is vendor specific.  OBP does not control that part of the 
>>> path, it just uses it.  Unfortunately there isn’t a standard. This code 
>>> only gets run during setup/install (not boot) to return the path for OBP to 
>>> use.
>>> 
>>> You mentioned cdroms being enumerated with cdromN.  I believe that type of 
>>> enumeration stopped with IDE drives.  A USB cdrom would be enumerated 
>>> something like this:
>>> 
>>> /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
>>> 
>>> So the function that runs during boot: check_string_removable does not 
>>> identify the cdrom properly.
>>> 
>>> I’m open to making any changes to this patch if you have some ideas in 
>>> mind. I tried to base it off of what was already in ofpath.c and I have 
>>> some follow on patches coming the rely on this path being correct.
>> 
>> I mean to read device aliases created by OBP (those displayed by
>> devalias OBP command).
>> 
>> disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
> 
>  Devaliases are not guaranteed to be there for all devices.  It is highly 
> dependent on the system vendor to create them and we certainly have systems 
> with disk drives that have no aliases.
> 
>  —S

And we also have systems with pre-populated devaliases for disks that don’t 
exist (that could be added in the future).  This is part of the reason why it 
can take 20 - 30 minutes to get to the grub menu on some SPARC systems.  On 
boot, grub builds a tree from the devaliases and then tries to open and close 
disks that don’t exist.  The HBA finally times out. 

But we are getting ahead of things here, since the patch I submitted only 
starts to solve this problem.  This will be fixed in later patches.


> 
> 
> 
>> ...
>> 
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-17 Thread Eric Snowberg

> On Feb 16, 2016, at 11:21 PM, Andrei Borzenkov  wrote:
> 
> On Wed, Feb 17, 2016 at 6:24 AM, Andrei Borzenkov  wrote:
>> 17.02.2016 05:02, Eric Snowberg пишет:
>>> 
 On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
 
 On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
  wrote:
> Fix the open firmware 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 
> ---
> grub-core/osdep/linux/ofpath.c |  192 
> +++-
> 1 files changed, 190 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/osdep/linux/ofpath.c 
> b/grub-core/osdep/linux/ofpath.c
> index a79682a..de51c57 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
 ...
> @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>}
>  else
>{
> +#ifdef __sparc__
> +  int vendor = 0, device_id = 0;
> +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
> +
> +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
> +{
> +   /* Over time different OF addressing schemes have been 
> supported */
> +   /* There is no generic addressing scheme that works across */
> +   /* every HBA */
> +  switch (device_id)
> +{
> +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
> +case 0x50: /* SAS-1068E */
> +case 0x56: /* SAS-1064E */
> +case 0x58: /* Pandora SAS-1068E */
> +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
> +case 0x79: /* Niwot, SAS 2108   */
 
 That really does not scale. Can we enumerate device aliases and take
 diskN and cdromN? So that we do not need to duplicate OBP logic?
 
>>> 
>>> Do you have an idea in mind on how to do this differently to make it scale 
>>> better?  This patch will default to the old disk@N for unknown HBAs.  I 
>>> agree it doesn’t scale that well, but new HBAs for SPARC with OF support 
>>> don't come out that often.
>>> 
>>> Before this patch, there isn’t a single SAS HBA that is enumerated 
>>> correctly on SPARC.  I went back 10 years and believe I have added every 
>>> HBA with OF support.  This isn’t duplicating OBP logic.  The final part of 
>>> the device path is vendor specific.  OBP does not control that part of the 
>>> path, it just uses it.  Unfortunately there isn’t a standard. This code 
>>> only gets run during setup/install (not boot) to return the path for OBP to 
>>> use.
>>> 
>>> You mentioned cdroms being enumerated with cdromN.  I believe that type of 
>>> enumeration stopped with IDE drives.  A USB cdrom would be enumerated 
>>> something like this:
>>> 
>>> /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
>>> 
>>> So the function that runs during boot: check_string_removable does not 
>>> identify the cdrom properly.
>>> 
>>> I’m open to making any changes to this patch if you have some ideas in 
>>> mind. I tried to base it off of what was already in ofpath.c and I have 
>>> some follow on patches coming the rely on this path being correct.
>>> 
>> 
>> I mean to read device aliases created by OBP (those displayed by
>> devalias OBP command).
>> 
>> disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
>> ...
> 
> Here is example
> 
>Node 0xf022d638
>screen:  '/pci@400/pci@1/pci@0/pci@7/pci@0/display@0'
>mouse:
> '/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/device@4/mouse@1'
>disk7:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p3'
>disk6:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p2'
>disk5:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p1'
>disk4:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p0'
>scsi1:  '/pci@700/pci@1/pci@0/pci@0/@0'
>net3:  '/pci@600/pci@2/pci@0/pci@3/network@0,1'
>net2:  '/pci@600/pci@2/pci@0/pci@3/network@0'
>rcdrom:
> '/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/hub@3/storage@2/disk@0'
>rkeyboard:
> '/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/device@4/keyboard@0'
>rscreen:  '/pci@400/pci@1/pci@0/pci@7/pci@0/display@0:r1280x1024x60'
>net1:  '/pci@400/pci@1/pci@0/pci@2/network@0,1'
>net0:  '/pci@400/pci@1/pci@0/pci@2/network@0'
>net:  '/pci@400/pci@1/pci@0/pci@2/network@0'
>disk3:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p3'
>disk2:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p2'
>disk1:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p1'
>disk0:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p0'
>disk:  '/pci@400/pci@1/pci@0/pci@0/@0/dis

Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-17 Thread Toomas Soome

> On 17. veebr 2016, at 8:45, Seth Goldberg  wrote:
>> 
>> disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
> 
> Devaliases are not guaranteed to be there for all devices.  It is highly 
> dependent on the system vendor to create them and we certainly have systems 
> with disk drives that have no aliases.
> 

guest LDOMS do not have aliases set up, unless admin will create them.

rgds,
toomas


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-16 Thread Seth Goldberg


> On Feb 16, 2016, at 7:24 PM, Andrei Borzenkov  wrote:
> 
> 17.02.2016 05:02, Eric Snowberg пишет:
>> 
>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
>>> 
>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>>  wrote:
 Fix the open firmware 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 
 ---
 grub-core/osdep/linux/ofpath.c |  192 
 +++-
 1 files changed, 190 insertions(+), 2 deletions(-)
 
 diff --git a/grub-core/osdep/linux/ofpath.c 
 b/grub-core/osdep/linux/ofpath.c
 index a79682a..de51c57 100644
 --- a/grub-core/osdep/linux/ofpath.c
 +++ b/grub-core/osdep/linux/ofpath.c
>>> ...
 @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
 __attribute__((unused)), const char *dev
}
  else
{
 +#ifdef __sparc__
 +  int vendor = 0, device_id = 0;
 +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
 +
 +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
 +{
 +   /* Over time different OF addressing schemes have been 
 supported */
 +   /* There is no generic addressing scheme that works across */
 +   /* every HBA */
 +  switch (device_id)
 +{
 +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
 +case 0x50: /* SAS-1068E */
 +case 0x56: /* SAS-1064E */
 +case 0x58: /* Pandora SAS-1068E */
 +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
 +case 0x79: /* Niwot, SAS 2108   */
>>> 
>>> That really does not scale. Can we enumerate device aliases and take
>>> diskN and cdromN? So that we do not need to duplicate OBP logic?
>> 
>> Do you have an idea in mind on how to do this differently to make it scale 
>> better?  This patch will default to the old disk@N for unknown HBAs.  I 
>> agree it doesn’t scale that well, but new HBAs for SPARC with OF support 
>> don't come out that often. 
>> 
>> Before this patch, there isn’t a single SAS HBA that is enumerated correctly 
>> on SPARC.  I went back 10 years and believe I have added every HBA with OF 
>> support.  This isn’t duplicating OBP logic.  The final part of the device 
>> path is vendor specific.  OBP does not control that part of the path, it 
>> just uses it.  Unfortunately there isn’t a standard. This code only gets run 
>> during setup/install (not boot) to return the path for OBP to use.
>> 
>> You mentioned cdroms being enumerated with cdromN.  I believe that type of 
>> enumeration stopped with IDE drives.  A USB cdrom would be enumerated 
>> something like this:
>> 
>> /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
>> 
>> So the function that runs during boot: check_string_removable does not 
>> identify the cdrom properly.
>> 
>> I’m open to making any changes to this patch if you have some ideas in mind. 
>> I tried to base it off of what was already in ofpath.c and I have some 
>> follow on patches coming the rely on this path being correct.
> 
> I mean to read device aliases created by OBP (those displayed by
> devalias OBP command).
> 
> disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0

  Devaliases are not guaranteed to be there for all devices.  It is highly 
dependent on the system vendor to create them and we certainly have systems 
with disk drives that have no aliases.

  --S



> ...
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-16 Thread Andrei Borzenkov
On Wed, Feb 17, 2016 at 6:24 AM, Andrei Borzenkov  wrote:
> 17.02.2016 05:02, Eric Snowberg пишет:
>>
>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
>>>
>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>>  wrote:
 Fix the open firmware 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 
 ---
 grub-core/osdep/linux/ofpath.c |  192 
 +++-
 1 files changed, 190 insertions(+), 2 deletions(-)

 diff --git a/grub-core/osdep/linux/ofpath.c 
 b/grub-core/osdep/linux/ofpath.c
 index a79682a..de51c57 100644
 --- a/grub-core/osdep/linux/ofpath.c
 +++ b/grub-core/osdep/linux/ofpath.c
>>> ...
 @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
 __attribute__((unused)), const char *dev
 }
   else
 {
 +#ifdef __sparc__
 +  int vendor = 0, device_id = 0;
 +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
 +
 +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
 +{
 +   /* Over time different OF addressing schemes have been 
 supported */
 +   /* There is no generic addressing scheme that works across */
 +   /* every HBA */
 +  switch (device_id)
 +{
 +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
 +case 0x50: /* SAS-1068E */
 +case 0x56: /* SAS-1064E */
 +case 0x58: /* Pandora SAS-1068E */
 +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
 +case 0x79: /* Niwot, SAS 2108   */
>>>
>>> That really does not scale. Can we enumerate device aliases and take
>>> diskN and cdromN? So that we do not need to duplicate OBP logic?
>>>
>>
>> Do you have an idea in mind on how to do this differently to make it scale 
>> better?  This patch will default to the old disk@N for unknown HBAs.  I 
>> agree it doesn’t scale that well, but new HBAs for SPARC with OF support 
>> don't come out that often.
>>
>> Before this patch, there isn’t a single SAS HBA that is enumerated correctly 
>> on SPARC.  I went back 10 years and believe I have added every HBA with OF 
>> support.  This isn’t duplicating OBP logic.  The final part of the device 
>> path is vendor specific.  OBP does not control that part of the path, it 
>> just uses it.  Unfortunately there isn’t a standard. This code only gets run 
>> during setup/install (not boot) to return the path for OBP to use.
>>
>> You mentioned cdroms being enumerated with cdromN.  I believe that type of 
>> enumeration stopped with IDE drives.  A USB cdrom would be enumerated 
>> something like this:
>>
>> /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
>>
>> So the function that runs during boot: check_string_removable does not 
>> identify the cdrom properly.
>>
>> I’m open to making any changes to this patch if you have some ideas in mind. 
>> I tried to base it off of what was already in ofpath.c and I have some 
>> follow on patches coming the rely on this path being correct.
>>
>
> I mean to read device aliases created by OBP (those displayed by
> devalias OBP command).
>
> disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
> ...

Here is example

Node 0xf022d638
screen:  '/pci@400/pci@1/pci@0/pci@7/pci@0/display@0'
mouse:
'/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/device@4/mouse@1'
disk7:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p3'
disk6:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p2'
disk5:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p1'
disk4:  '/pci@700/pci@1/pci@0/pci@0/@0/disk@p0'
scsi1:  '/pci@700/pci@1/pci@0/pci@0/@0'
net3:  '/pci@600/pci@2/pci@0/pci@3/network@0,1'
net2:  '/pci@600/pci@2/pci@0/pci@3/network@0'
rcdrom:
'/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/hub@3/storage@2/disk@0'
rkeyboard:
'/pci@400/pci@1/pci@0/pci@8/pci@0/usb@0,2/hub@2/device@4/keyboard@0'
rscreen:  '/pci@400/pci@1/pci@0/pci@7/pci@0/display@0:r1280x1024x60'
net1:  '/pci@400/pci@1/pci@0/pci@2/network@0,1'
net0:  '/pci@400/pci@1/pci@0/pci@2/network@0'
net:  '/pci@400/pci@1/pci@0/pci@2/network@0'
disk3:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p3'
disk2:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p2'
disk1:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p1'
disk0:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p0'
disk:  '/pci@400/pci@1/pci@0/pci@0/@0/disk@p0'
scsi0:  '/pci@400/pci@1/pci@0/pci@0/@0'
scsi:  '/pci@400/pci@1/pci@0/pci@0/@0'
virtual-console:  '/virtual-devices@100/console@1'
name:  'aliases'

_

Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-16 Thread Andrei Borzenkov
17.02.2016 05:02, Eric Snowberg пишет:
> 
>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
>>
>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>  wrote:
>>> Fix the open firmware 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 
>>> ---
>>> grub-core/osdep/linux/ofpath.c |  192 
>>> +++-
>>> 1 files changed, 190 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
>>> index a79682a..de51c57 100644
>>> --- a/grub-core/osdep/linux/ofpath.c
>>> +++ b/grub-core/osdep/linux/ofpath.c
>> ...
>>> @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
>>> __attribute__((unused)), const char *dev
>>> }
>>>   else
>>> {
>>> +#ifdef __sparc__
>>> +  int vendor = 0, device_id = 0;
>>> +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
>>> +
>>> +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
>>> +{
>>> +   /* Over time different OF addressing schemes have been 
>>> supported */
>>> +   /* There is no generic addressing scheme that works across */
>>> +   /* every HBA */
>>> +  switch (device_id)
>>> +{
>>> +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
>>> +case 0x50: /* SAS-1068E */
>>> +case 0x56: /* SAS-1064E */
>>> +case 0x58: /* Pandora SAS-1068E */
>>> +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
>>> +case 0x79: /* Niwot, SAS 2108   */
>>
>> That really does not scale. Can we enumerate device aliases and take
>> diskN and cdromN? So that we do not need to duplicate OBP logic?
>>
> 
> Do you have an idea in mind on how to do this differently to make it scale 
> better?  This patch will default to the old disk@N for unknown HBAs.  I agree 
> it doesn’t scale that well, but new HBAs for SPARC with OF support don't come 
> out that often. 
> 
> Before this patch, there isn’t a single SAS HBA that is enumerated correctly 
> on SPARC.  I went back 10 years and believe I have added every HBA with OF 
> support.  This isn’t duplicating OBP logic.  The final part of the device 
> path is vendor specific.  OBP does not control that part of the path, it just 
> uses it.  Unfortunately there isn’t a standard. This code only gets run 
> during setup/install (not boot) to return the path for OBP to use.
> 
> You mentioned cdroms being enumerated with cdromN.  I believe that type of 
> enumeration stopped with IDE drives.  A USB cdrom would be enumerated 
> something like this:
> 
> /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
> 
> So the function that runs during boot: check_string_removable does not 
> identify the cdrom properly.
> 
> I’m open to making any changes to this patch if you have some ideas in mind. 
> I tried to base it off of what was already in ofpath.c and I have some follow 
> on patches coming the rely on this path being correct.
> 

I mean to read device aliases created by OBP (those displayed by
devalias OBP command).

disk0 /pci@308/pci@1/usb@0/hub@1/storage@1/disk@0
...

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-16 Thread Eric Snowberg

> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov  wrote:
> 
> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>  wrote:
>> Fix the open firmware 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 
>> ---
>> grub-core/osdep/linux/ofpath.c |  192 
>> +++-
>> 1 files changed, 190 insertions(+), 2 deletions(-)
>> 
>> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
>> index a79682a..de51c57 100644
>> --- a/grub-core/osdep/linux/ofpath.c
>> +++ b/grub-core/osdep/linux/ofpath.c
> ...
>> @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
>> __attribute__((unused)), const char *dev
>> }
>>   else
>> {
>> +#ifdef __sparc__
>> +  int vendor = 0, device_id = 0;
>> +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
>> +
>> +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
>> +{
>> +   /* Over time different OF addressing schemes have been supported 
>> */
>> +   /* There is no generic addressing scheme that works across */
>> +   /* every HBA */
>> +  switch (device_id)
>> +{
>> +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
>> +case 0x50: /* SAS-1068E */
>> +case 0x56: /* SAS-1064E */
>> +case 0x58: /* Pandora SAS-1068E */
>> +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
>> +case 0x79: /* Niwot, SAS 2108   */
> 
> That really does not scale. Can we enumerate device aliases and take
> diskN and cdromN? So that we do not need to duplicate OBP logic?
> 

Do you have an idea in mind on how to do this differently to make it scale 
better?  This patch will default to the old disk@N for unknown HBAs.  I agree 
it doesn’t scale that well, but new HBAs for SPARC with OF support don't come 
out that often. 

Before this patch, there isn’t a single SAS HBA that is enumerated correctly on 
SPARC.  I went back 10 years and believe I have added every HBA with OF 
support.  This isn’t duplicating OBP logic.  The final part of the device path 
is vendor specific.  OBP does not control that part of the path, it just uses 
it.  Unfortunately there isn’t a standard. This code only gets run during 
setup/install (not boot) to return the path for OBP to use.

You mentioned cdroms being enumerated with cdromN.  I believe that type of 
enumeration stopped with IDE drives.  A USB cdrom would be enumerated something 
like this:

/pci@308/pci@1/usb@0/hub@1/storage@1/disk@0

So the function that runs during boot: check_string_removable does not identify 
the cdrom properly.

I’m open to making any changes to this patch if you have some ideas in mind. I 
tried to base it off of what was already in ofpath.c and I have some follow on 
patches coming the rely on this path being correct.

> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] sparc64: fix OF path names for sun4v systems

2016-02-16 Thread Andrei Borzenkov
On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
 wrote:
> Fix the open firmware 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 
> ---
>  grub-core/osdep/linux/ofpath.c |  192 
> +++-
>  1 files changed, 190 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> index a79682a..de51c57 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
...
> @@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
> __attribute__((unused)), const char *dev
>  }
>else
>  {
> +#ifdef __sparc__
> +  int vendor = 0, device_id = 0;
> +  check_hba_identifiers(sysfs_path, &vendor, &device_id);
> +
> +  if (vendor == 0x1000) /* LSI Logic Vendor ID */
> +{
> +   /* Over time different OF addressing schemes have been supported 
> */
> +   /* There is no generic addressing scheme that works across */
> +   /* every HBA */
> +  switch (device_id)
> +{
> +case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
> +case 0x50: /* SAS-1068E */
> +case 0x56: /* SAS-1064E */
> +case 0x58: /* Pandora SAS-1068E */
> +case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
> +case 0x79: /* Niwot, SAS 2108   */

That really does not scale. Can we enumerate device aliases and take
diskN and cdromN? So that we do not need to duplicate OBP logic?

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] sparc64: fix OF path names for sun4v systems

2016-02-15 Thread Eric Snowberg
Fix the open firmware 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 
---
 grub-core/osdep/linux/ofpath.c |  192 +++-
 1 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index a79682a..de51c57 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -336,6 +336,85 @@ vendor_is_ATA(const char *path)
 }
 
 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)
+{
+  free (p);
+  return;
+}
+  *ed = '\0';
+
+  path_size = (strlen (p) + sizeof("vendor"));
+  path = xmalloc (path_size);
+
+  if (!path)
+{
+  free (p);
+  return;
+}
+
+  snprintf (path, path_size, "%svendor", p);
+  fd = open (path, O_RDONLY);
+
+  if (fd < 0)
+{
+  free (p);
+  free (path);
+  return;
+}
+
+  memset (buf, 0, sizeof (buf));
+
+  if (read (fd, buf, sizeof (buf) - 1) < 0)
+{
+  close (fd);
+  free (p);
+  free (path);
+  return;
+}
+
+  close (fd);
+  sscanf (buf, "%x", vendor);
+  snprintf (path, path_size, "%sdevice", p);
+  fd = open (path, O_RDONLY);
+
+  if (fd < 0)
+{
+  free (p);
+  free (path);
+  return;
+}
+
+  memset (buf, 0, sizeof (buf));
+
+  if (read (fd, buf, sizeof (buf) - 1) < 0)
+{
+  close (fd);
+  free (p);
+  free (path);
+  return;
+}
+
+  close (fd);
+  sscanf (buf, "%x", device_id);
+  free (path);
+  free (p);
+}
+
+static void
 check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
 {
   char *ed = strstr (sysfs_path, "end_device");
@@ -413,9 +492,11 @@ of_path_of_scsi(const char *sys_devname 
__attribute__((unused)), const char *dev
 }
 
   of_path = find_obppath(sysfs_path);
-  free (sysfs_path);
   if (!of_path)
-return NULL;
+{
+  free (sysfs_path);
+  return NULL;
+}
 
   if (strstr (of_path, "qlc"))
 strcat (of_path, "/fp@0,0");
@@ -444,6 +525,111 @@ of_path_of_scsi(const char *sys_devname 
__attribute__((unused)), const char *dev
 }
   else
 {
+#ifdef __sparc__
+  int vendor = 0, device_id = 0;
+  check_hba_identifiers(sysfs_path, &vendor, &device_id);
+
+  if (vendor == 0x1000) /* LSI Logic Vendor ID */
+{
+   /* Over time different OF addressing schemes have been supported */
+   /* There is no generic addressing scheme that works across */
+   /* every HBA */
+  switch (device_id)
+{
+case 0x30: /* Rhea, Jasper 320, LSI53C1020/1030 */
+case 0x50: /* SAS-1068E */
+case 0x56: /* SAS-1064E */
+case 0x58: /* Pandora SAS-1068E */
+case 0x5d: /* Aspen, Invader, LSI SAS-3108  */
+case 0x79: /* Niwot, SAS 2108   */
+  /* Use Target/lun OF path */
+  if (*digit_string == '\0')
+{
+  if ( lun ==  0 )
+{
+  snprintf(disk, sizeof (disk), "/%s@%x", disk_name, tgt);
+}
+  else
+{
+  snprintf(disk, sizeof (disk), "/%s@%x,%x",
+   disk_name, tgt, lun);
+}
+}
+  else
+{
+  int part;
+
+  sscanf(digit_string, "%d", &part);
+  if ( lun == 0 )
+{
+  snprintf(disk, sizeof (disk),
+   "/%s@%x:%c", disk_name, tgt, 'a' + (part - 1));
+}
+  else
+{
+  snprintf(disk, sizeof (disk),
+   "/%s@%x,%x:%c", disk_name, tgt,
+   lun, 'a' + (part - 1));
+}
+}
+  break;
+case 0x72:  /* Erie, Falcon, LSI SAS 2008   */
+case 0x7e:  /* LSI WarpDrive 6203   */
+case 0x87:  /* LSI SAS 2308 */
+case 0x97:  /* LSI SAS 3008 */
+default:
+  if (*digit_string == '\0')
+{
+  if ( lun == 0 )
+{
+  /* Use non-standard disk@p OF path */
+  snprintf(disk, sizeof (disk), "/%s@p%x", disk_name, tgt);
+}
+