* [PATCH] sparc64: fix OF path names for sun4v systems
@ 2016-02-15 19:11 Eric Snowberg
2016-02-16 8:16 ` Andrei Borzenkov
0 siblings, 1 reply; 16+ messages in thread
From: Eric Snowberg @ 2016-02-15 19:11 UTC (permalink / raw)
To: grub-devel; +Cc: Eric Snowberg, daniel.kiper
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 <eric.snowberg@oracle.com>
---
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<PHY> OF path */
+ snprintf(disk, sizeof (disk), "/%s@p%x", disk_name, tgt);
+ }
+ else
+ {
+ /* Use WWN OF path */
+ snprintf(disk, sizeof (disk), "/%s@w%lx", disk_name,
+ sas_address);
+ }
+ }
+ else
+ {
+ int part;
+
+ sscanf(digit_string, "%d", &part);
+ if ( lun == 0)
+ {
+ /* Use non-standard disk@p<PHY> OF path */
+ snprintf(disk, sizeof (disk), "/%s@p%x:%c",
+ disk_name, tgt, 'a' + (part - 1));
+ }
+ else
+ {
+ /* Use WWN OF path */
+ snprintf(disk, sizeof (disk), "/%s@w%lx:%c",
+ disk_name, sas_address, 'a' + (part - 1));
+ }
+ }
+ break;
+ }
+ }
+ else
+ {
+ /* Use Target OF path */
+ if (*digit_string == '\0')
+ {
+ snprintf(disk, sizeof (disk), "/%s@%x", disk_name, tgt);
+ }
+ else
+ {
+ int part;
+
+ sscanf(digit_string, "%d", &part);
+ snprintf(disk, sizeof (disk),
+ "/%s@%x:%c", disk_name, tgt, 'a' + (part - 1));
+ }
+ }
+#else
if (lun == 0)
{
int sas_id = 0;
@@ -491,7 +677,9 @@ of_path_of_scsi(const char *sys_devname __attribute__((unused)), const char *dev
}
free (lunstr);
}
+#endif
}
+ free (sysfs_path);
strcat(of_path, disk);
return of_path;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-15 19:11 [PATCH] sparc64: fix OF path names for sun4v systems Eric Snowberg
@ 2016-02-16 8:16 ` Andrei Borzenkov
2016-02-17 2:02 ` Eric Snowberg
0 siblings, 1 reply; 16+ messages in thread
From: Andrei Borzenkov @ 2016-02-16 8:16 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Eric Snowberg, Daniel Kiper
On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
<eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
> ---
> 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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-16 8:16 ` Andrei Borzenkov
@ 2016-02-17 2:02 ` Eric Snowberg
2016-02-17 3:24 ` Andrei Borzenkov
0 siblings, 1 reply; 16+ messages in thread
From: Eric Snowberg @ 2016-02-17 2:02 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Daniel Kiper
> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 2:02 ` Eric Snowberg
@ 2016-02-17 3:24 ` Andrei Borzenkov
2016-02-17 6:21 ` Andrei Borzenkov
2016-02-17 6:45 ` Seth Goldberg
0 siblings, 2 replies; 16+ messages in thread
From: Andrei Borzenkov @ 2016-02-17 3:24 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Daniel Kiper
17.02.2016 05:02, Eric Snowberg пишет:
>
>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>
>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>>> ---
>>> 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
...
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 3:24 ` Andrei Borzenkov
@ 2016-02-17 6:21 ` Andrei Borzenkov
2016-02-17 18:05 ` Eric Snowberg
2016-02-17 6:45 ` Seth Goldberg
1 sibling, 1 reply; 16+ messages in thread
From: Andrei Borzenkov @ 2016-02-17 6:21 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Daniel Kiper
On Wed, Feb 17, 2016 at 6:24 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 17.02.2016 05:02, Eric Snowberg пишет:
>>
>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>>
>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>>>> ---
>>>> 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'
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 3:24 ` Andrei Borzenkov
2016-02-17 6:21 ` Andrei Borzenkov
@ 2016-02-17 6:45 ` Seth Goldberg
2016-02-17 9:50 ` Toomas Soome
2016-02-17 18:11 ` Eric Snowberg
1 sibling, 2 replies; 16+ messages in thread
From: Seth Goldberg @ 2016-02-17 6:45 UTC (permalink / raw)
To: The development of GNU GRUB
> On Feb 16, 2016, at 7:24 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> 17.02.2016 05:02, Eric Snowberg пишет:
>>
>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>>
>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>>>> ---
>>>> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 6:45 ` Seth Goldberg
@ 2016-02-17 9:50 ` Toomas Soome
2016-02-17 18:11 ` Eric Snowberg
1 sibling, 0 replies; 16+ messages in thread
From: Toomas Soome @ 2016-02-17 9:50 UTC (permalink / raw)
To: The development of GNU GRUB
> On 17. veebr 2016, at 8:45, Seth Goldberg <seth.goldberg@oracle.com> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 6:21 ` Andrei Borzenkov
@ 2016-02-17 18:05 ` Eric Snowberg
0 siblings, 0 replies; 16+ messages in thread
From: Eric Snowberg @ 2016-02-17 18:05 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Daniel Kiper
> On Feb 16, 2016, at 11:21 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>
> On Wed, Feb 17, 2016 at 6:24 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>> 17.02.2016 05:02, Eric Snowberg пишет:
>>>
>>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>>>
>>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>>> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>>>>> ---
>>>>> 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'
>
With the example provided, the current code in ofpath.c, will not correctly enumerate any of those drives.
To prove this, simply run grub-ofpathname /dev/sd<n> on your machine.
Then look at what follows disk@. I’m sure you will not see the “p”. Therefore the wrong path is placed in grub.cfg and in core.img.
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2016-02-17 6:45 ` Seth Goldberg
2016-02-17 9:50 ` Toomas Soome
@ 2016-02-17 18:11 ` Eric Snowberg
1 sibling, 0 replies; 16+ messages in thread
From: Eric Snowberg @ 2016-02-17 18:11 UTC (permalink / raw)
To: The development of GNU GRUB
> On Feb 16, 2016, at 11:45 PM, Seth Goldberg <seth.goldberg@oracle.com> wrote:
>
>
>
>> On Feb 16, 2016, at 7:24 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>
>> 17.02.2016 05:02, Eric Snowberg пишет:
>>>
>>>> On Feb 16, 2016, at 1:16 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
>>>>
>>>> On Mon, Feb 15, 2016 at 10:11 PM, Eric Snowberg
>>>> <eric.snowberg@oracle.com> 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 <eric.snowberg@oracle.com>
>>>>> ---
>>>>> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] sparc64: fix OF path names for sun4v systems
@ 2017-12-06 23:53 Eric Snowberg
2017-12-18 15:22 ` Daniel Kiper
0 siblings, 1 reply; 16+ messages in thread
From: Eric Snowberg @ 2017-12-06 23:53 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, 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 <eric.snowberg@oracle.com>
---
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 <errno.h>
#include <ctype.h>
+#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'
+ + (part - 1));
+ }
+#else
if (lun == 0)
{
int sas_id = 0;
@@ -493,7 +632,9 @@ of_path_of_scsi(const char *sys_devname __attribute__((unused)), const char *dev
}
free (lunstr);
}
+#endif
}
+ free (sysfs_path);
strcat(of_path, disk);
return of_path;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-06 23:53 Eric Snowberg
@ 2017-12-18 15:22 ` Daniel Kiper
2017-12-18 21:30 ` Eric Snowberg
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Kiper @ 2017-12-18 15:22 UTC (permalink / raw)
To: Eric Snowberg; +Cc: grub-devel
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 <eric.snowberg@oracle.com>
> ---
> 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 <errno.h>
> #include <ctype.h>
>
> +#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;
> +
> + /* 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'
> + + (part - 1));
> + }
> +#else
> if (lun == 0)
> {
> int sas_id = 0;
> @@ -493,7 +632,9 @@ of_path_of_scsi(const char *sys_devname __attribute__((unused)), const char *dev
> }
> free (lunstr);
> }
> +#endif
> }
> + free (sysfs_path);
Drop this change.
> strcat(of_path, disk);
err:
free (sysfs_path);
> return of_path;
> }
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.
Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-18 15:22 ` Daniel Kiper
@ 2017-12-18 21:30 ` Eric Snowberg
2017-12-18 22:58 ` John Paul Adrian Glaubitz
2017-12-19 20:56 ` Daniel Kiper
0 siblings, 2 replies; 16+ messages in thread
From: Eric Snowberg @ 2017-12-18 21:30 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Daniel Kiper
> On Dec 18, 2017, at 8:22 AM, Daniel Kiper <daniel.kiper@oracle.com> 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 <eric.snowberg@oracle.com>
>> ---
>> 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 <errno.h>
>> #include <ctype.h>
>>
>> +#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_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;
>> +
>> + /* 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'
>> + + (part - 1));
>> + }
>> +#else
>> if (lun == 0)
>> {
>> int sas_id = 0;
>> @@ -493,7 +632,9 @@ of_path_of_scsi(const char *sys_devname __attribute__((unused)), const char *dev
>> }
>> free (lunstr);
>> }
>> +#endif
>> }
>> + free (sysfs_path);
>
> Drop this change.
>
>> strcat(of_path, disk);
>
> err:
> free (sysfs_path);
I’ll make these goto changes.
>
>> return of_path;
>> }
>
> 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-18 21:30 ` Eric Snowberg
@ 2017-12-18 22:58 ` John Paul Adrian Glaubitz
2017-12-19 21:36 ` Daniel Kiper
2017-12-19 20:56 ` Daniel Kiper
1 sibling, 1 reply; 16+ messages in thread
From: John Paul Adrian Glaubitz @ 2017-12-18 22:58 UTC (permalink / raw)
To: The development of GNU GRUB, Eric Snowberg; +Cc: Daniel Kiper
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 - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-18 21:30 ` Eric Snowberg
2017-12-18 22:58 ` John Paul Adrian Glaubitz
@ 2017-12-19 20:56 ` Daniel Kiper
1 sibling, 0 replies; 16+ messages in thread
From: Daniel Kiper @ 2017-12-19 20:56 UTC (permalink / raw)
To: Eric Snowberg; +Cc: grub-devel, phcoder
On Mon, Dec 18, 2017 at 02:30:51PM -0700, Eric Snowberg wrote:
> > On Dec 18, 2017, at 8:22 AM, Daniel Kiper <daniel.kiper@oracle.com> 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 <eric.snowberg@oracle.com>
> >> ---
> >> 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 <errno.h>
> >> #include <ctype.h>
> >>
> >> +#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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-18 22:58 ` John Paul Adrian Glaubitz
@ 2017-12-19 21:36 ` Daniel Kiper
2018-01-29 12:20 ` John Paul Adrian Glaubitz
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Kiper @ 2017-12-19 21:36 UTC (permalink / raw)
To: John Paul Adrian Glaubitz; +Cc: The development of GNU GRUB, Eric Snowberg
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] sparc64: fix OF path names for sun4v systems
2017-12-19 21:36 ` Daniel Kiper
@ 2018-01-29 12:20 ` John Paul Adrian Glaubitz
0 siblings, 0 replies; 16+ messages in thread
From: John Paul Adrian Glaubitz @ 2018-01-29 12:20 UTC (permalink / raw)
To: The development of GNU GRUB, Daniel Kiper; +Cc: Eric Snowberg
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 - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-01-29 12:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-15 19:11 [PATCH] sparc64: fix OF path names for sun4v systems Eric Snowberg
2016-02-16 8:16 ` Andrei Borzenkov
2016-02-17 2:02 ` Eric Snowberg
2016-02-17 3:24 ` Andrei Borzenkov
2016-02-17 6:21 ` Andrei Borzenkov
2016-02-17 18:05 ` Eric Snowberg
2016-02-17 6:45 ` Seth Goldberg
2016-02-17 9:50 ` Toomas Soome
2016-02-17 18:11 ` Eric Snowberg
-- strict thread matches above, loose matches on Subject: below --
2017-12-06 23:53 Eric Snowberg
2017-12-18 15:22 ` Daniel Kiper
2017-12-18 21:30 ` Eric Snowberg
2017-12-18 22:58 ` John Paul Adrian Glaubitz
2017-12-19 21:36 ` Daniel Kiper
2018-01-29 12:20 ` John Paul Adrian Glaubitz
2017-12-19 20:56 ` Daniel Kiper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.