[U-Boot] [PATCH 3/4] sun8i: On H3 Use word 1 instead of word 3 from the SID

[Hans de Goede]

Hi,

On 28-07-16 05:13, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Jul 28, 2016 at 3:14 AM, Siarhei Siamashka
> <siarhei.siamashka at gmail.com> wrote:
>> Hello Hans,
>>
>> On Wed, 27 Jul 2016 18:10:34 +0200
>> Hans de Goede <hdegoede at redhat.com> wrote:
>>
>>> It seems that bytes 13-14 of the SID / bytes 1-2 from word 3 of the SID
>>> are always 0 on H3 making it a poor candidate to use as source for the
>>> serialnr / mac-address, switch to word1 which seems to be more random.
>>>
>>> Cc: Chen-Yu Tsai <wens at csie.org>
>>> Cc: Corentin LABBE <clabbe.montjoie at gmail.com>
>>> Cc: Amit Singh Tomar <amittomer25 at gmail.com>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> ---
>>>  board/sunxi/board.c | 23 ++++++++++++++---------
>>>  1 file changed, 14 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>>> index ef3fe26..bbe5340 100644
>>> --- a/board/sunxi/board.c
>>> +++ b/board/sunxi/board.c
>>> @@ -620,12 +620,17 @@ static void setup_environment(const void *fdt)
>>>       uint8_t mac_addr[6];
>>>       char ethaddr[16];
>>>       int i, ret;
>>> +#ifdef CONFIG_MACH_SUN8I_H3
>>> +     const int idx0 = 0, idx1 = 1;
>>> +#else
>>> +     const int idx0 = 0, idx1 = 3;
>>> +#endif
>>>
>>>       ret = sunxi_get_sid(sid);
>>> -     if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
>>> +     if (ret == 0 && sid[idx0] != 0 && sid[idx1] != 0) {
>>>               /* Ensure the NIC specific bytes of the mac are not all 0 */
>>> -             if ((sid[3] & 0xffffff) == 0)
>>> -                     sid[3] |= 0x800000;
>>> +             if ((sid[idx1] & 0xffffff) == 0)
>>> +                     sid[idx1] |= 0x800000;
>>>
>>>               for (i = 0; i < 4; i++) {
>>>                       sprintf(ethaddr, "ethernet%d", i);
>>> @@ -642,18 +647,18 @@ static void setup_environment(const void *fdt)
>>>
>>>                       /* Non OUI / registered MAC address */
>>>                       mac_addr[0] = (i << 4) | 0x02;
>>> -                     mac_addr[1] = (sid[0] >>  0) & 0xff;
>>> -                     mac_addr[2] = (sid[3] >> 24) & 0xff;
>>> -                     mac_addr[3] = (sid[3] >> 16) & 0xff;
>>> -                     mac_addr[4] = (sid[3] >>  8) & 0xff;
>>> -                     mac_addr[5] = (sid[3] >>  0) & 0xff;
>>> +                     mac_addr[1] = (sid[idx0] >>  0) & 0xff;
>>> +                     mac_addr[2] = (sid[idx1] >> 24) & 0xff;
>>> +                     mac_addr[3] = (sid[idx1] >> 16) & 0xff;
>>> +                     mac_addr[4] = (sid[idx1] >>  8) & 0xff;
>>> +                     mac_addr[5] = (sid[idx1] >>  0) & 0xff;
>>>
>>>                       eth_setenv_enetaddr(ethaddr, mac_addr);
>>>               }
>>>
>>>               if (!getenv("serial#")) {
>>>                       snprintf(serial_string, sizeof(serial_string),
>>> -                             "%08x%08x", sid[0], sid[3]);
>>> +                             "%08x%08x", sid[idx0], sid[idx1]);
>>>
>>>                       setenv("serial#", serial_string);
>>>               }
>>
>> Is it really a good idea trying to guess which parts of the SID are
>> sufficiently unique, depending on the SoC generation?

Probably not, but that is what we've been doing so far.

>> We can calculate some kind of hash (CRC32? SHA256?) over the whole
>> SID. This will ensure that all the SID bits are accounted for.

That seems like a good idea.

I'm thinking about doing the following (for H3 at least, and probably
also for other new SoCs):

	sid[3] ^= sid[1] ^ sid[2];

No need to use a crpytho graphically secure hash, and AFAIK we
don't have enough data for that anyways (without using some seed).

Before using sid[3] to get the low 4 bytes of the mac / serial.

I believe that using sid[0] as the first 4 bytes of the serial
is still a good idea, as it gives us info on which SoC is being used.


>>
>> Also changing the MAC address generation algorithm is an invasive
>> change, which is affecting the end users. So IMHO it's best to have
>> it implemented properly right from the start, rather than evolving
>> via a trial and error method. What if it turns out that word1
>> from the SID is actually not sufficiently random on H3? How large
>> was your sample set?
>
> I've added the SID values from whatever H3 boards I have to:
>
>     http://linux-sunxi.org/SID_Register_Guide#Currently_known_SID.27s

I had done the same for my boards yesterday, but apparently never saved
the preview :|

I've just done this a second time.

> And it seems word1 is more like a batch number. Note the last 3 boards,
> which have identical word1's. These were given to me by Steven at the
> same time. word2 seems to follow the pattern 5xxxxxxe.
>
> In short there are quite a few bits that are the same.

Ack, so nack to my own patch as using word1 is worse then using word3,
I suggest we simply ex-or word1-3 together and use that, any comments
on that ?

Regards,

Hans