Re: [PATCH 2/2] LegacyBbs: add boot entries for virtio-blk devices

["David Woodhouse" <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 6327 bytes --]

On Fri, 2013-01-25 at 22:04 +0100, Laszlo Ersek wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>


Hm. This no longer works as-is because...

> ---
>  .../Csm/LegacyBiosDxe/LegacyBbs.c                  |  134 +++++++++++++++++++-
>  1 files changed, 133 insertions(+), 1 deletions(-)
> 
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBbs.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBbs.c
> index 6ee43ad..964d7d2 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBbs.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBbs.c
> @@ -259,8 +259,140 @@ LegacyBiosBuildBbs (
>      }
>    }
>  
> -  return EFI_SUCCESS;
> +  //
> +  // add virtio-blk devices
> +  //
> +  {
> +    EFI_STATUS Status;
> +    UINTN      NumPciIoHandles;
> +    EFI_HANDLE *PciIoHandles;
> +
> +    BbsIndex = HddIndex * 2 + 1;
> +
> +    //
> +    // scan all handles supporting the PCI IO protocol
> +    //
> +    Status = gBS->LocateHandleBuffer (
> +                    ByProtocol,
> +                    &gEfiPciIoProtocolGuid,
> +                    NULL,
> +                    &NumPciIoHandles,
> +                    &PciIoHandles
> +                    );
> +
> +    if (Status == EFI_SUCCESS) {
> +      UINTN CurPciIo;
> +
> +      for (CurPciIo = 0; CurPciIo < NumPciIoHandles; ++CurPciIo) {
> +        EFI_OPEN_PROTOCOL_INFORMATION_ENTRY *References;
> +        UINTN                               NumReferences;
> +        UINTN                               CurRef;
> +
> +        //
> +        // on each handle supporting the PCI IO protocol, see which drivers
> +        // (agents) have a PCI IO protocol interface actually opened
> +        //
> +        Status = gBS->OpenProtocolInformation (
> +                        PciIoHandles[CurPciIo],
> +                        &gEfiPciIoProtocolGuid,
> +                        &References,
> +                        &NumReferences
> +                        );
> +        if (EFI_ERROR (Status)) {
> +          continue;
> +        }
> +
> +        for (CurRef = 0; CurRef < NumReferences; ++CurRef) {
> +          if (References[CurRef].Attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) {
> +            EFI_COMPONENT_NAME2_PROTOCOL *ComponentName;
> +            CHAR16                       *DriverName;
> +
> +            //
> +            // OpenProtocol() enforced non-NULL AgentHandle for this case
> +            //
> +            ASSERT (References[CurRef].AgentHandle != NULL);
> +
> +            //
> +            // Check if the agent owning the open protocol interface can
> +            // provide its name, and if so, whether it's VirtioBlkDxe. For this
> +            // check we don't want a persistent reference to the agent's
> +            // EFI_COMPONENT_NAME2_PROTOCOL instance, therefore we use
> +            // HandleProtocol() instead of OpenProtocol().
> +            //
> +            Status = gBS->HandleProtocol (
> +                            References[CurRef].AgentHandle,
> +                            &gEfiComponentName2ProtocolGuid,
> +                            (VOID **) &ComponentName
> +                            );
> +            if (EFI_ERROR (Status)) {
> +              continue;
> +            }
> +
> +            Status = ComponentName->GetDriverName (
> +                                      ComponentName,
> +                                      "en",
> +                                      &DriverName
> +                                      );
>  
> +            if (Status == EFI_SUCCESS &&
> +                StrCmp (DriverName, L"Virtio Block Driver") == 0) {
> +              break;



... this would need to match on 'Virtio PCI Driver' instead. Obviously
that's a trivial fix but it raises the question of whether this is the
right thing to be matching on at all.

It also doesn't catch NVMe drives. 

Should we just be iterating over all block devices or something like
that instead?

> +            }
> +          }
> +        }
> +
> +        if (CurRef < NumReferences) {
> +          EFI_PCI_IO_PROTOCOL *PciIo;
> +          UINTN               Segment;
> +          UINTN               Bus;
> +          UINTN               Device;
> +          UINTN               Function;
> +
> +          //
> +          // reference by VirtioBlkDxe found, produce boot entry for device
> +          //
> +          Status = gBS->HandleProtocol (
> +                          PciIoHandles[CurPciIo],
> +                          &gEfiPciIoProtocolGuid,
> +                          (VOID **) &PciIo
> +                          );
> +          ASSERT (Status == EFI_SUCCESS);
> +
> +          Status = PciIo->GetLocation (
> +                            PciIo,
> +                            &Segment,
> +                            &Bus,
> +                            &Device,
> +                            &Function
> +                            );
> +          ASSERT (Status == EFI_SUCCESS);
> +
> +          if (Segment == 0) {
> +            BbsTable[BbsIndex].Bus                      = (UINT32) Bus;
> +            BbsTable[BbsIndex].Device                   = (UINT32) Device;
> +            BbsTable[BbsIndex].Function                 = (UINT32) Function;
> +            BbsTable[BbsIndex].Class                    = 1;
> +            BbsTable[BbsIndex].SubClass                 = 0x80;
> +            BbsTable[BbsIndex].StatusFlags.OldPosition  = 0;
> +            BbsTable[BbsIndex].StatusFlags.Reserved1    = 0;
> +            BbsTable[BbsIndex].StatusFlags.Enabled      = 0;
> +            BbsTable[BbsIndex].StatusFlags.Failed       = 0;
> +            BbsTable[BbsIndex].StatusFlags.MediaPresent = 0;
> +            BbsTable[BbsIndex].StatusFlags.Reserved2    = 0;
> +            BbsTable[BbsIndex].DeviceType               = BBS_HARDDISK;
> +            BbsTable[BbsIndex].BootPriority             = BBS_UNPRIORITIZED_ENTRY;
> +            ++BbsIndex;
> +          }
> +        }
> +
> +        FreePool (References);
> +      }
> +
> +      FreePool (PciIoHandles);
> +    }
> +  }
> +
> +  return EFI_SUCCESS;
>  }
>  
>  


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]