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

[Eric Snowberg]

> On Feb 7, 2018, at 4:53 AM, Daniel Kiper <address@hidden> wrote:
> 
> On Tue, Jan 30, 2018 at 08:49:48PM -0800, Eric Snowberg wrote:
>> Fix the Open Firmware (OF) path property for sun4v SPARC systems.
>> These platforms do not have a /sas/ within their path.  Over time
>> different OF addressing schemes have been supported. There
>> is no generic addressing scheme that works across every HBA.
>> 
>> Signed-off-by: Eric Snowberg <address@hidden>
> 
> In general Reviewed-by: Daniel Kiper <address@hidden>...
> 
> Just a few comments below...
> 
>> ---
>> V2:
>>  - Requested coding style changes
>> ---
>> grub-core/osdep/linux/ofpath.c |  148 
>> +++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 145 insertions(+), 3 deletions(-)
>> 
>> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
>> index dce4e59..5369508 100644
>> --- a/grub-core/osdep/linux/ofpath.c
>> +++ b/grub-core/osdep/linux/ofpath.c
>> @@ -38,6 +38,46 @@
>> #include <errno.h>
>> #include <ctype.h>
>> 
>> +#ifdef __sparc__
> 
> I have discussed this with Vladimir. It looks that this will not
> work if you try to cross-install SPARC GRUB2 binary using e.g.
> x86 grub-install. By default it should work. However, we will also
> have other issues here, like lack of access to OF firmware/paths,
> which make such configs unusable anyway. So, I would leave this patch
> as is for time being. If somebody cares then he/she should fix it.
> 
> Anyway, I will add something like that into the commit message before 
> committing.

That is fine with me.

> 
>> +typedef enum
>> +  {
>> +    GRUB_OFPATH_SPARC_WWN_ADDR = 1,
>> +    GRUB_OFPATH_SPARC_TGT_LUN,
>> +  } ofpath_sparc_addressing;
>> +
>> +struct ofpath_sparc_hba
>> +{
>> +  grub_uint32_t device_id;
>> +  ofpath_sparc_addressing addressing;
>> +};
>> +
>> +static struct ofpath_sparc_hba sparc_lsi_hba[] = {
>> +  /* Rhea, Jasper 320, LSI53C1020/1030. */
>> +  {0x30, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* SAS-1068E. */
>> +  {0x50, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* SAS-1064E. */
>> +  {0x56, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* Pandora SAS-1068E. */
>> +  {0x58, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* Aspen, Invader, LSI SAS-3108. */
>> +  {0x5d, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* Niwot, SAS 2108. */
>> +  {0x79, GRUB_OFPATH_SPARC_TGT_LUN},
>> +  /* Erie, Falcon, LSI SAS 2008. */
>> +  {0x72, GRUB_OFPATH_SPARC_WWN_ADDR},
>> +  /* LSI WarpDrive 6203. */
>> +  {0x7e, GRUB_OFPATH_SPARC_WWN_ADDR},
>> +  /* LSI SAS 2308. */
>> +  {0x87, GRUB_OFPATH_SPARC_WWN_ADDR},
>> +  /* LSI SAS 3008. */
>> +  {0x97, GRUB_OFPATH_SPARC_WWN_ADDR},
>> +  {0, 0}
>> +};
>> +
>> +static const int LSI_VENDOR_ID = 0x1000;
>> +#endif
>> +
>> #ifdef OFPATH_STANDALONE
>> #define xmalloc malloc
>> void
>> @@ -338,6 +378,67 @@ vendor_is_ATA(const char *path)
>>   return (memcmp(bufcont, "ATA", 3) == 0);
>> }
>> 
>> +#ifdef __sparc__
>> +static void
>> +check_hba_identifiers (const char *sysfs_path, int *vendor, int *device_id)
>> +{
>> +  char *ed = strstr (sysfs_path, "host");
>> +  size_t path_size;
>> +  char *p, *path;
>> +  char buf[8];
>> +  int fd;
>> +
>> +  if (!ed)
>> +    return;
>> +
>> +  p = xstrdup (sysfs_path);
>> +  ed = strstr (p, "host");
>> +
>> +  if (!ed)
>> +    goto out;
> 
> In normal circumstances this will never ever fail.
> If you are OK I will drop this "if" before committing.

I put that in for the case if the xstrdup above can not allocate memory.  Then 
p will be null, the strstr will fail and then we should jump to out.


> 
> If there are no objections I will commit this patch in a week or so.

Thanks…

Is there some reason this patch is being held up too?

http://lists.gnu.org/archive/html/grub-devel/2017-05/msg00045.html

This is where things last left off:

http://lists.gnu.org/archive/html/grub-devel/2017-10/msg00046.html

We really need minimal GPT support in there for many of the follow on patches 
in my queue.