Re: [PATCH edk2-platforms] Silicon/Broadcom/BcmGenetDxe: implement media state adapter info protocol

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/16/20 8:17 PM, Ard Biesheuvel wrote:
> 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>
> ---

Note that this code was build tested only, and turns out not to work as 
expected. Samer and I are collaborating off-list, I will follow up with 
a v2 once we have code that actually works.


>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf |   3 +
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.h   |   4 +
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c   | 109 ++++++++++++++++++++
>   Silicon/Broadcom/Drivers/Net/BcmGenetDxe/DriverBinding.c |  12 ++-
>   4 files changed, 123 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..48bb8550426f 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,6 +237,7 @@ 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)
> diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
> new file mode 100644
> index 000000000000..cb7733bbba76
> --- /dev/null
> +++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/AdapterInfo.c
> @@ -0,0 +1,109 @@
> +/** @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_SNP_THIS (This);
> +  if (Genet->Snp.Mode->MediaPresent) {
> +    AdapterInfo->MediaState = EFI_SUCCESS;
> +  } else {
> +    AdapterInfo->MediaState = EFI_NOT_READY;
> +  }
> +  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;
>     }
>