在 8/9/2018 6:19 PM, Leif Lindholm 写道:
> On Thu, Aug 09, 2018 at 02:16:49PM +0800, Ming wrote:
>>>> +UINT16 crc_tab[256] = {
>>>
>>> CrcTable.
>>
>> Modify it in v2.
>>
>>> Hmm.
>>> This might be useful to add to a core library/driver/protocol. We
>>> don't appear to have it yet. This is another note to the universe.
>>
>> Do you suggest to move the CRC16 function to
>> MdePkg/Library/BaseLib/CheckSum.c ?
>> I think it's not enouth time to do this before Linaro 18.08 maybe.
>
> Not needed for 18.08, but I would like to see the improvement made
> afterwards.
>
>>>> +EFI_STATUS
>>>> +EFIAPI
>>>> +OemGetMac (
>>>> + IN OUT EFI_MAC_ADDRESS *Mac,
>>>> + IN UINTN Port
>>>> + )
>>>> +{
>>>> + EFI_STATUS Status;
>>>> +
>>>> + if (NULL == Mac) {
>>>
>>> No jeopardy tests. Turn the comparison around to the logical order.
>>>
>>>> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Mac buffer is null!\n",
>>>> + __FUNCTION__, __LINE__));
>>>> + return EFI_INVALID_PARAMETER;
>>>> + }
>>>> +
>>>> + Status = OemGetMacE2prom (Port, Mac->Addr);
>>>> + if ((EFI_ERROR (Status))) {
>>>
>>> Parantheses around EFI_ERROR are not needed.
>>>
>>>> + DEBUG ((DEBUG_ERROR,
>>>> + "[%a]:[%dL] Cannot get MAC from EEPROM, Status: %r; using default
>>>> MAC.\n",
>>>> + __FUNCTION__, __LINE__, Status));
>>>> +
>>>> + Mac->Addr[0] = 0x00;
>>>> + Mac->Addr[1] = 0x18;
>>>> + Mac->Addr[2] = 0x82;
>>>> + Mac->Addr[3] = 0x2F;
>>>> + Mac->Addr[4] = 0x02;
>>>> + Mac->Addr[5] = Port;
>>>
>>> I'm not super happy about this. This would wreak havoc on any real
>>> network.
>>> Arguably, a server platform should just fail hard at this point.
>>> I would certainly appreciate a Pcd making that an option.
>>>
>>> Otherwise, some sort of proper scheme would need to be implemented:
>>> using the 'locally administered range' of MAC addresses, and ensuring
>>> addresses are only allocated after checking for possible duplicates on
>>> the network.
>>
>> Do you suggest we should return EFI_NOT_FOUND here?
>
> Yes please.
> It would be good if we could have some (common) code to handle the
> fluke situation where you end up without your own MAC address.
> (So that the node can boot up and report that it is broken.)
> But it needs to be done in a reliable way, and that's too big a task
> for 18.08.
I found some modules which invoke OemGetMac() don't judge the Status of
OemGetMac, so it may cause some issue now if changing to EFI_NOT_FOUND.
How about change it while we handle the fluke situation after 18.08 ?
>
> /
> Leif
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
