Re: [PATCH v2] ofdisk: add sas disks to the device list

[Paulo Flabiano Smorigo]

On Mon, Nov 9, 2015 at 10:01 AM, Vladimir 'phcoder' Serbinenko
<address@hidden> wrote:
>
> Le 9 nov. 2015 8:39 AM, "Paulo Flabiano Smorigo"
> <address@hidden> a écrit :
>>
>> All SAS disks attached will be added to the device list. This is the
>> second
>> version of a patch that I send a while ago [1].
>>
>> [1] https://lists.gnu.org/archive/html/grub-devel/2015-08/msg00000.html
>>
>> ---
>>  grub-core/disk/ieee1275/ofdisk.c | 76
>> ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>
>> diff --git a/grub-core/disk/ieee1275/ofdisk.c
>> b/grub-core/disk/ieee1275/ofdisk.c
>> index 297f058..afec9ba 100644
>> --- a/grub-core/disk/ieee1275/ofdisk.c
>> +++ b/grub-core/disk/ieee1275/ofdisk.c
>> @@ -260,6 +260,82 @@ dev_iterate (const struct grub_ieee1275_devalias
>> *alias)
>>        grub_free (buf);
>>        return;
>>      }
>> +  else if (grub_strcmp (alias->type, "sas_ioa") == 0)
>> +    {
>> +      /* The method returns the number of disks and a table where
>> +       * each ID is 64-bit long. Example of sas paths:
>> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden
>> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden
>> +       *  /address@hidden/pci1014,address@hidden/sas/address@hidden */
>> +
>> +      struct sas_children
>> +        {
>> +          struct grub_ieee1275_common_hdr common;
>> +          grub_ieee1275_cell_t method;
>> +          grub_ieee1275_cell_t ihandle;
>> +          grub_ieee1275_cell_t max;
>> +          grub_ieee1275_cell_t table;
>> +          grub_ieee1275_cell_t catch_result;
>> +          grub_ieee1275_cell_t nentries;
>> +        }
>> +      args;
>> +      char *buf, *bufptr, *table;
>> +      unsigned i;
>> +      grub_uint32_t table_size;
>> +      grub_ieee1275_ihandle_t ihandle;
>> +
>> +      buf = grub_malloc (grub_strlen (alias->path)
>> +                         + sizeof ("/address@hidden"));
>> +      if (!buf)
>> +        return;
>> +
>> +      /* 64 entries should be enough */
> Why? Sounds like an arbitrary limit. What does OF call return when it's not
> true? Does it set nentries to total or to how much it has really provided?

Asked the Open firmware development team...

The maximum value for max is 0xffff but this is a very high number IMHO.

About the limit, if there are more entries than will fit in the buffer
provided, you will only get up to that number. The return value is the
number in the buffer.

I tested here using max = 4 and nentries was 4. The table only had 4
entries as well.

>> +      table_size = sizeof (grub_uint64_t) * 64;
>> +      table = grub_malloc (table_size);
>> +
>> +      if (!table)
>> +        {
>> +          grub_free (buf);
>> +          return;
>> +        }
>> +
>> +      bufptr = grub_stpcpy (buf, alias->path);
>> +
>> +      if (grub_ieee1275_open (alias->path, &ihandle))
>> +        {
>> +          grub_free (buf);
>> +          grub_free (table);
>> +          return;
>> +        }
>> +
>> +      INIT_IEEE1275_COMMON (&args.common, "call-method", 4, 2);
>> +      args.method = (grub_ieee1275_cell_t) "get-sas-children";
>> +      args.ihandle = ihandle;
>> +      args.max = table_size;
> Please double check that this field is in bytes.

My fault. it's entries, not bytes. Will fix it.

>> +      args.table = (grub_ieee1275_cell_t) table;
>> +      args.catch_result = 0;
>> +      args.nentries = 0;
>> +
>> +      if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>> +        {
>> +          grub_ieee1275_close (ihandle);
>> +          grub_free (table);
>> +          grub_free (buf);
>> +          return;
>> +        }
>> +
>> +      grub_uint64_t *ptr;
>> +      for (i = 0; i < args.nentries; i++)
>> +        {
>> +          ptr = (grub_uint64_t *) (table + sizeof (grub_uint64_t) * i);
>> +          grub_snprintf (bufptr, 32, "/address@hidden" PRIxGRUB_UINT64_T, 
>> *ptr);
> This 32 doesn't match the above allocation.

Ok.

>> +          dev_iterate_real (buf, buf);
>> +        }
>> +
>> +      grub_ieee1275_close (ihandle);
>> +      grub_free (table);
>> +      grub_free (buf);
>> +    }
>>
>>    if (!grub_ieee1275_test_flag
>> (GRUB_IEEE1275_FLAG_NO_TREE_SCANNING_FOR_DISKS)
>>        && grub_strcmp (alias->type, "block") == 0)
>> --
>> 2.1.0
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>