Re: [PATCH 09/25] scsi: hisi_sas: add phy SAS ADDR initialization

[John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>]

On 14/10/2015 09:40, Hannes Reinecke wrote:
> On 10/13/2015 07:14 PM, John Garry wrote:
>> On 13/10/2015 07:12, Hannes Reinecke wrote:
>>> On 10/12/2015 05:20 PM, John Garry wrote:
>>>> This SAS ID is chosen as Huawei IEEE id: 001882
>>>>
>>>> Signed-off-by: John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    drivers/scsi/hisi_sas/hisi_sas_init.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_init.c
>>>> b/drivers/scsi/hisi_sas/hisi_sas_init.c
>>>> index 44fc524..c295c39 100644
>>>> --- a/drivers/scsi/hisi_sas/hisi_sas_init.c
>>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_init.c
>>>> @@ -283,6 +283,19 @@ err_out:
>>>>        return NULL;
>>>>    }
>>>>
>>>> +static void hisi_sas_init_add(struct hisi_hba *hisi_hba)
>>>> +{
>>>> +    u8 i;
>>>> +
>>>> +    /* Huawei IEEE id (001882) */
>>>> +    for (i = 0; i < hisi_hba->n_phy; i++)
>>>> +        hisi_hba->phy[i].dev_sas_addr =
>>>> +            cpu_to_be64(0x5001882016072015ULL);
>>>> +
>>> Ouch. Each phy has the same SAS address?
>>> For all boards? Ever?
>>>
>>> Not sure if that's a good idea, nor even valid.
>>> It'll confuse the hell out of any SAS array.
>>>
>>> Please provide a means of having individual SAS addresses for each
>>> HBA.
>>>
>>> Cheers,
>>>
>>> Hannes
>>>
>> Hello,
>>
>> Are you saying we should be getting the SAS address from fw with
>> sas_request_addr() or the like?
>>
>> Marvell solution seems to hardcode it.
>>
> Doesn't make it any better, nor even valid.
>
> Problem is that using a hardcoded SAS address makes it impossible
> do any resource allocation on a SAS target array.
> The SAS target array is allocating / mapping the LUNs based on the
> SAS address, so if HBAs from different machines coming in with the
> same SAS address the SAS array will assume that all ports belong to
> the same machine, thus allowing access to all of them.
>
> And don't try to _ever_ connect these HBAs to a SAS switch; that
> will be even more confused.
>
> So please, if you have a chance _DO_ get the SAS address from fw.
> If the firmware doesn't provide it you really should get in touch
> with the firmware team to get you one.
>
> To clarify the situation the SAS spec states:
>
>    Device names are worldwide unique names for devices within a
>    transport protocol (see SAM-3).
>
> and:
>    The VENDOR - SPECIFIC IDENTIFIER contains a 36-bit numeric value
>    that is uniquely assigned by the organization associated with the
>    company identifier in the IEEE COMPANY ID field.
>
> So as you are using the Huawei Vendor ID Huawei as a company needs
> to ensure uniqueness of the vendor-specific identifier (ie the last
> 36 bits of the SAS address). Which the above patch most patently
> fails to address.
>
> Cheers,
>
> Hannes
>
Hi,

OK, we can look at adding the ability to read the SAS HBA address from a 
FW image or EFI variables.

Thanks,
John

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html