From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Pete Batard <pete@akeo.ie>,
"Andrei Warkentin (awarkentin@vmware.com)"
<awarkentin@vmware.com>
Subject: Re: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol
Date: Thu, 18 Jun 2020 12:34:42 +0200 [thread overview]
Message-ID: <2f7dd937-82e5-3f6c-c41e-ef474acddac5@arm.com> (raw)
In-Reply-To: <DB7PR08MB3260256BCD949360EE20CE71909A0@DB7PR08MB3260.eurprd08.prod.outlook.com>
On 6/17/20 8:22 PM, Samer El-Haj-Mahmoud wrote:
> Tested on RPi4 w/ 4GB
>
> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>
Thanks
Pushed as 743f501b66db712d486347ddca781ffc705c1766
>> -----Original Message-----
>> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Sent: Wednesday, June 17, 2020 2:15 PM
>> To: devel@edk2.groups.io
>> Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Pete Batard
>> <pete@akeo.ie>; Andrei Warkentin (awarkentin@vmware.com)
>> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
>> Mahmoud@arm.com>
>> Subject: [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe:
>> implement media state adapter info protocol
>>
>> NetLibDetectMedia () in DxeNetLib is used as a fallback on implementations
>> of the SNP protocol that do not also carry an implementation of the adapter
>> info protocol to provide media state information, and it does all kinds of
>> terrible things to the network interface (stopping and restarting multiple
>> times, reprogramming the multicast filters etc etc) to workaround some
>> alleged UNDI shortcoming.
>>
>> Although our GENET code should be bullet proof and therefore able to take
>> this kind of abuse, it is better to avoid it, and provide an implementation
>> of the adapter info protocol that returns the media state directly, without
>> the need to mistreat the SNP layer.
>>
>>
>> Cc: Pete Batard <pete@akeo.ie>
>>
>> Cc: Andrei Warkentin (awarkentin@vmware.com)
>> <awarkentin@vmware.com>
>>
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>
>> v2:
>>
>> - use the correct accessor to retrieve the GENET_PRIVATE_DATA pointer
>>
>> - call into the PHY routines to get the state, to ensure it is not stale
>>
>>
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf | 3 +
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h | 5 +
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c | 106
>> ++++++++++++++++++++
>> Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c | 12 ++-
>> 4 files changed, 121 insertions(+), 5 deletions(-)
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> index 28f3e66ebaf0..89dee9f10c83 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf
>> @@ -19,6 +19,7 @@ [Defines]
>> UNLOAD_IMAGE = GenetUnload
>>
>>
>>
>> [Sources]
>>
>> + AdapterInfo.c
>>
>> BcmGenetDxe.h
>>
>> ComponentName.c
>>
>> DriverBinding.c
>>
>> @@ -49,10 +50,12 @@ [LibraryClasses]
>>
>>
>> [Protocols]
>>
>> gBcmGenetPlatformDeviceProtocolGuid ## TO_START
>>
>> + gEfiAdapterInformationProtocolGuid ## BY_START
>>
>> gEfiDevicePathProtocolGuid ## BY_START
>>
>> gEfiSimpleNetworkProtocolGuid ## BY_START
>>
>>
>>
>> [Guids]
>>
>> + gEfiAdapterInfoMediaStateGuid
>>
>> gEfiEventExitBootServicesGuid
>>
>>
>>
>> [FixedPcd]
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> index 0af9d5209cf2..1a117b25f87f 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h
>> @@ -14,6 +14,7 @@
>> #include <Uefi.h>
>>
>> #include <Library/UefiLib.h>
>>
>> #include <Protocol/BcmGenetPlatformDevice.h>
>>
>> +#include <Protocol/AdapterInformation.h>
>>
>> #include <Protocol/ComponentName.h>
>>
>> #include <Protocol/ComponentName2.h>
>>
>> #include <Protocol/SimpleNetwork.h>
>>
>> @@ -209,6 +210,8 @@ typedef struct {
>> EFI_SIMPLE_NETWORK_PROTOCOL Snp;
>>
>> EFI_SIMPLE_NETWORK_MODE SnpMode;
>>
>>
>>
>> + EFI_ADAPTER_INFORMATION_PROTOCOL Aip;
>>
>> +
>>
>> BCM_GENET_PLATFORM_DEVICE_PROTOCOL *Dev;
>>
>>
>>
>> GENERIC_PHY_PRIVATE_DATA Phy;
>>
>> @@ -234,9 +237,11 @@ extern EFI_COMPONENT_NAME_PROTOCOL
>> gGenetComponentName;
>> extern EFI_COMPONENT_NAME2_PROTOCOL
>> gGenetComponentName2;
>>
>>
>>
>> extern CONST EFI_SIMPLE_NETWORK_PROTOCOL
>> gGenetSimpleNetworkTemplate;
>>
>> +extern CONST EFI_ADAPTER_INFORMATION_PROTOCOL
>> gGenetAdapterInfoTemplate;
>>
>>
>>
>> #define GENET_DRIVER_SIGNATURE SIGNATURE_32('G', 'N', 'E', 'T')
>>
>> #define GENET_PRIVATE_DATA_FROM_SNP_THIS(a) CR(a,
>> GENET_PRIVATE_DATA, Snp, GENET_DRIVER_SIGNATURE)
>>
>> +#define GENET_PRIVATE_DATA_FROM_AIP_THIS(a) CR(a,
>> GENET_PRIVATE_DATA, Aip, GENET_DRIVER_SIGNATURE)
>>
>>
>>
>> #define GENET_RX_BUFFER(g, idx) ((UINT8 *)(UINTN)(g)->RxBuffer +
>> GENET_MAX_PACKET_SIZE * (idx))
>>
>>
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> new file mode 100644
>> index 000000000000..4b42b2bba67a
>> --- /dev/null
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
>> @@ -0,0 +1,106 @@
>> +/** @file
>>
>> +
>>
>> + Copyright (c) 2020 Arm, Limited. All rights reserved.
>>
>> +
>>
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>> +
>>
>> +**/
>>
>> +
>>
>> +#include <Uefi.h>
>>
>> +#include <Library/BaseMemoryLib.h>
>>
>> +#include <Library/DebugLib.h>
>>
>> +#include <Library/MemoryAllocationLib.h>
>>
>> +
>>
>> +#include "BcmGenetDxe.h"
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipGetInformation (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + IN EFI_GUID *InformationType,
>>
>> + OUT VOID **InformationBlock,
>>
>> + OUT UINTN *InformationBlockSize
>>
>> + )
>>
>> +{
>>
>> + EFI_ADAPTER_INFO_MEDIA_STATE *AdapterInfo;
>>
>> + GENET_PRIVATE_DATA *Genet;
>>
>> +
>>
>> + if (This == NULL || InformationBlock == NULL ||
>>
>> + InformationBlockSize == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + if (!CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>>
>> + return EFI_UNSUPPORTED;
>>
>> + }
>>
>> +
>>
>> + AdapterInfo = AllocateZeroPool (sizeof
>> (EFI_ADAPTER_INFO_MEDIA_STATE));
>>
>> + if (AdapterInfo == NULL) {
>>
>> + return EFI_OUT_OF_RESOURCES;
>>
>> + }
>>
>> +
>>
>> + *InformationBlock = AdapterInfo;
>>
>> + *InformationBlockSize = sizeof (EFI_ADAPTER_INFO_MEDIA_STATE);
>>
>> +
>>
>> + Genet = GENET_PRIVATE_DATA_FROM_AIP_THIS (This);
>>
>> + AdapterInfo->MediaState = GenericPhyUpdateConfig (&Genet->Phy);
>>
>> +
>>
>> + return EFI_SUCCESS;
>>
>> +}
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipSetInformation (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + IN EFI_GUID *InformationType,
>>
>> + IN VOID *InformationBlock,
>>
>> + IN UINTN InformationBlockSize
>>
>> + )
>>
>> +{
>>
>> + if (This == NULL || InformationBlock == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + if (CompareGuid (InformationType, &gEfiAdapterInfoMediaStateGuid)) {
>>
>> + return EFI_WRITE_PROTECTED;
>>
>> + }
>>
>> +
>>
>> + return EFI_UNSUPPORTED;
>>
>> +}
>>
>> +
>>
>> +STATIC
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +GenetAipGetSupportedTypes (
>>
>> + IN EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>>
>> + OUT EFI_GUID **InfoTypesBuffer,
>>
>> + OUT UINTN *InfoTypesBufferCount
>>
>> + )
>>
>> +{
>>
>> + EFI_GUID *Guid;
>>
>> +
>>
>> + if (This == NULL || InfoTypesBuffer == NULL ||
>>
>> + InfoTypesBufferCount == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>> +
>>
>> + Guid = AllocatePool (sizeof *Guid);
>>
>> + if (Guid == NULL) {
>>
>> + return EFI_OUT_OF_RESOURCES;
>>
>> + }
>>
>> +
>>
>> + CopyGuid (Guid, &gEfiAdapterInfoMediaStateGuid);
>>
>> +
>>
>> + *InfoTypesBuffer = Guid;
>>
>> + *InfoTypesBufferCount = 1;
>>
>> +
>>
>> + return EFI_SUCCESS;
>>
>> +}
>>
>> +
>>
>> +CONST EFI_ADAPTER_INFORMATION_PROTOCOL
>> gGenetAdapterInfoTemplate = {
>>
>> + GenetAipGetInformation,
>>
>> + GenetAipSetInformation,
>>
>> + GenetAipGetSupportedTypes,
>>
>> +};
>>
>> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> index 7f93c68cd608..f9aa006dc799 100644
>> --- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c
>> @@ -163,6 +163,7 @@ GenetDriverBindingStart (
>>
>>
>> EfiInitializeLock (&Genet->Lock, TPL_CALLBACK);
>>
>> CopyMem (&Genet->Snp, &gGenetSimpleNetworkTemplate, sizeof Genet-
>>> Snp);
>>
>> + CopyMem (&Genet->Aip, &gGenetAdapterInfoTemplate, sizeof Genet-
>>> Aip);
>>
>>
>>
>> Genet->Snp.Mode = &Genet->SnpMode;
>>
>> Genet->SnpMode.State = EfiSimpleNetworkStopped;
>>
>> @@ -203,7 +204,8 @@ GenetDriverBindingStart (
>> }
>>
>>
>>
>> Status = gBS->InstallMultipleProtocolInterfaces (&ControllerHandle,
>>
>> - &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>>
>> NULL);
>>
>>
>>
>> if (EFI_ERROR (Status)) {
>>
>> @@ -273,10 +275,10 @@ GenetDriverBindingStop (
>>
>>
>> ASSERT (Genet->ControllerHandle == ControllerHandle);
>>
>>
>>
>> - Status = gBS->UninstallProtocolInterface (ControllerHandle,
>>
>> - &gEfiSimpleNetworkProtocolGuid,
>>
>> - &Genet->Snp
>>
>> - );
>>
>> + Status = gBS->UninstallMultipleProtocolInterfaces (ControllerHandle,
>>
>> + &gEfiSimpleNetworkProtocolGuid, &Genet->Snp,
>>
>> + &gEfiAdapterInformationProtocolGuid, &Genet->Aip,
>>
>> + NULL);
>>
>> if (EFI_ERROR (Status)) {
>>
>> return Status;
>>
>> }
>>
>> --
>> 2.27.0
>
prev parent reply other threads:[~2020-06-18 10:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-17 18:15 [PATCH edk2-platforms v2] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol Ard Biesheuvel
2020-06-17 18:22 ` Samer El-Haj-Mahmoud
2020-06-18 10:34 ` Ard Biesheuvel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2f7dd937-82e5-3f6c-c41e-ef474acddac5@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox