[edk2,v3,9/9] ArmVirtPkg/VirtFdtDxe: remove Xenio handling and rename to VirtioFdtDxe

Message ID 1460534539-2169-10-git-send-email-ard.biesheuvel@linaro.org
State Superseded
Headers show

Commit Message

Ard Biesheuvel April 13, 2016, 8:02 a.m.
Now that we have moved the handling of the xen,xen DT node to XenioFdtDxe,
remove its handling from VirtFdtDxe. Since the only functionality that
remains is handling the virtio,mmio DT node, rename VirtFdtDxe to
VirtioFdtDxe to reflect that. Also update the platforms that use this
driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/{VirtFdtDxe/VirtFdtDxe.inf => VirtioFdtDxe/VirtioFdtDxe.inf} |  32 ++-
 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c                                      | 218 --------------------
 ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c                                  | 125 +++++++++++
 ArmVirtPkg/ArmVirtQemu.dsc                                              |   2 +-
 ArmVirtPkg/ArmVirtQemu.fdf                                              |   2 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc                                        |   2 +-
 ArmVirtPkg/ArmVirtQemuKernel.fdf                                        |   2 +-
 7 files changed, 143 insertions(+), 240 deletions(-)

-- 
2.5.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek April 13, 2016, 2:18 p.m. | #1
On 04/13/16 10:02, Ard Biesheuvel wrote:
> Now that we have moved the handling of the xen,xen DT node to XenioFdtDxe,

> remove its handling from VirtFdtDxe. Since the only functionality that

> remains is handling the virtio,mmio DT node, rename VirtFdtDxe to

> VirtioFdtDxe to reflect that. Also update the platforms that use this

> driver.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmVirtPkg/{VirtFdtDxe/VirtFdtDxe.inf => VirtioFdtDxe/VirtioFdtDxe.inf} |  32 ++-

>  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c                                      | 218 --------------------

>  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c                                  | 125 +++++++++++

>  ArmVirtPkg/ArmVirtQemu.dsc                                              |   2 +-

>  ArmVirtPkg/ArmVirtQemu.fdf                                              |   2 +-

>  ArmVirtPkg/ArmVirtQemuKernel.dsc                                        |   2 +-

>  ArmVirtPkg/ArmVirtQemuKernel.fdf                                        |   2 +-

>  7 files changed, 143 insertions(+), 240 deletions(-)

> 

> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

> similarity index 61%

> rename from ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> rename to ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

> index 4dd46cf87011..e4ac1a285bc8 100644

> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

> @@ -1,7 +1,7 @@

>  ## @file

> -#  Device tree enumeration DXE driver for ARM Virtual Machines

> +#  Virtio FDT client protocol driver for virtio,mmio DT node

>  #

> -#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>

> +#  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>

>  #

>  #  This program and the accompanying materials are

>  #  licensed and made available under the terms and conditions of the BSD License

> @@ -15,41 +15,37 @@

>  

>  [Defines]

>    INF_VERSION                    = 0x00010005

> -  BASE_NAME                      = VirtFdtDxe

> +  BASE_NAME                      = VirtioFdtDxe

>    FILE_GUID                      = 9AD7DCB4-E6EC-472E-96BF-81C219A3F77E


(1) Can you please generate a new GUID? The new role of the driver is
finally acknowledged by this patch (as reflected by the name change
too). I'd like a new GUID, if you don't mind.

> diff --git a/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c

> new file mode 100644

> index 000000000000..09c311410fda

> --- /dev/null

> +++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c

> @@ -0,0 +1,125 @@

> +/** @file

> +*  Virtio FDT client protocol driver for virtio,mmio DT node

> +*

> +*  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>

> +*

> +*  This program and the accompanying materials are

> +*  licensed and made available under the terms and conditions of the BSD License

> +*  which accompanies this distribution.  The full text of the license may be found at

> +*  http://opensource.org/licenses/bsd-license.php

> +*

> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

> +*

> +**/

> +

> +#include <Library/BaseLib.h>

> +#include <Library/BaseMemoryLib.h>

> +#include <Library/DebugLib.h>

> +#include <Library/DevicePathLib.h>

> +#include <Library/MemoryAllocationLib.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include <Library/UefiDriverEntryPoint.h>

> +#include <Library/VirtioMmioDeviceLib.h>

> +

> +#include <Guid/VirtioMmioTransport.h>

> +

> +#include <Protocol/FdtClient.h>

> +

> +#pragma pack (1)

> +typedef struct {

> +  VENDOR_DEVICE_PATH                  Vendor;

> +  UINT64                              PhysBase;

> +  EFI_DEVICE_PATH_PROTOCOL            End;

> +} VIRTIO_TRANSPORT_DEVICE_PATH;

> +#pragma pack ()

> +

> +EFI_STATUS

> +EFIAPI

> +InitializeVirtioFdtDxe (

> +  IN EFI_HANDLE           ImageHandle,

> +  IN EFI_SYSTEM_TABLE     *SystemTable

> +  )

> +{

> +  EFI_STATUS                     Status, FindNodeStatus;

> +  FDT_CLIENT_PROTOCOL            *FdtClient;

> +  INT32                          Node;

> +  CONST UINT64                   *Reg;

> +  UINT32                         RegSize;

> +  VIRTIO_TRANSPORT_DEVICE_PATH   *DevicePath;

> +  EFI_HANDLE                     Handle;

> +  UINT64                         RegBase;

> +

> +  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,

> +                  (VOID **)&FdtClient);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  for (FindNodeStatus = FdtClient->FindCompatibleNode (FdtClient,

> +                                     "virtio,mmio", &Node);

> +       FindNodeStatus == EFI_SUCCESS;


(2) This would be more idiomatic as

  !EFI_ERROR (FindNodeStatus)

> +       FindNodeStatus = FdtClient->FindNextCompatibleNode (FdtClient,

> +                                     "virtio,mmio", Node, &Node)) {

> +

> +    Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",

> +                          (CONST VOID **)&Reg, &RegSize);

> +    if (EFI_ERROR (Status)) {

> +      DEBUG ((EFI_D_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",

> +        __FUNCTION__, Status));

> +      continue;

> +    }

> +

> +    ASSERT (RegSize == 16);

> +

> +    //

> +    // Create a unique device path for this transport on the fly

> +    //

> +    RegBase = SwapBytes64 (*Reg);

> +    DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode (

> +                                  HARDWARE_DEVICE_PATH,

> +                                  HW_VENDOR_DP,

> +                                  sizeof (VIRTIO_TRANSPORT_DEVICE_PATH));

> +    if (DevicePath == NULL) {

> +      DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__));

> +      break;

> +    }


(3) It is almost useless for me to obsess about this :), but the "break"
statement above doesn't work identically to the original. In the
original, it terminated the case branch, and the loop continued; hence
the "break" enabled the installation of further virtio-mmio devices.
Whereas here we give up permanently.

I would prefer a "continue" here, rather than a "break", but not for
practical reasons -- just so we stay true to the original (because "this
is refactoring").

> +

> +    CopyGuid (&DevicePath->Vendor.Guid, &gVirtioMmioTransportGuid);


Nice :)

> +    DevicePath->PhysBase = RegBase;

> +    SetDevicePathNodeLength (&DevicePath->Vendor,

> +      sizeof (*DevicePath) - sizeof (DevicePath->End));

> +    SetDevicePathEndNode (&DevicePath->End);

> +

> +    Handle = NULL;

> +    Status = gBS->InstallProtocolInterface (&Handle,

> +                     &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,

> +                     DevicePath);

> +    if (EFI_ERROR (Status)) {

> +      DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH "

> +        "protocol on a new handle (Status == %r)\n",

> +        __FUNCTION__, Status));

> +      FreePool (DevicePath);

> +      break;


(4) This should be a "continue" too.

> +    }

> +

> +    Status = VirtioMmioInstallDevice (RegBase, Handle);

> +    if (EFI_ERROR (Status)) {

> +      DEBUG ((EFI_D_ERROR, "%a: Failed to install VirtIO transport @ 0x%Lx "

> +        "on handle %p (Status == %r)\n", __FUNCTION__, RegBase,

> +        Handle, Status));

> +

> +      Status = gBS->UninstallProtocolInterface (Handle,

> +                      &gEfiDevicePathProtocolGuid, DevicePath);

> +      ASSERT_EFI_ERROR (Status);

> +      FreePool (DevicePath);

> +      break;


(5) This should also be a "continue".

(Yes, we're at the end of the loop body anyway, but an explicit
"continue" is clearer, IMO, esp. if we'll have to append something later
to the loop body.)

> +    }

> +  }

> +

> +  if (EFI_ERROR (FindNodeStatus) && FindNodeStatus != EFI_NOT_FOUND) {

> +     DEBUG ((EFI_D_ERROR, "%a: Error occurred while iterating DT nodes "

> +       "(FindNodeStatus == %r)\n", __FUNCTION__, FindNodeStatus));

> +    return Status;


(6) I propose to drop the return statement: let the firmware proceed
with the virtio-mmio devices we managed to install.

Otherwise, if you wanted to exit with an error (--> unload the driver),
then you would have to rollback all the devpath allocations, devpath
installations, and VirtioMmioInstallDevice() calls, that did succeed
before the error.

(This is fully doable, as VirtioMmioUninstallDevice() exists, but it's
out of scope for this refactoring. So, let's just drop the return
statement.)

> +  }

> +

> +  return EFI_SUCCESS;

> +}

> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

> index 70419ff3f856..34323bf83d64 100644

> --- a/ArmVirtPkg/ArmVirtQemu.dsc

> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

> @@ -322,7 +322,7 @@ [Components.common]

>    #

>    # Platform Driver

>    #

> -  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>    ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>    ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>    OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf

> index a7204898f473..071eaef5dd80 100644

> --- a/ArmVirtPkg/ArmVirtQemu.fdf

> +++ b/ArmVirtPkg/ArmVirtQemu.fdf

> @@ -103,7 +103,7 @@ [FV.FvMain]

>  

>    INF MdeModulePkg/Core/Dxe/DxeMain.inf

>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

> -  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>    INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>  

> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc

> index 65991baaf680..e1b45ffaf8fd 100644

> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc

> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc

> @@ -300,7 +300,7 @@ [Components.common]

>    #

>    # Platform Driver

>    #

> -  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>    ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>    OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

>    OvmfPkg/VirtioScsiDxe/VirtioScsi.inf

> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf

> index b8f66d2f472c..7c478340f9e7 100644

> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf

> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf

> @@ -125,7 +125,7 @@ [FV.FvMain]

>  

>    INF MdeModulePkg/Core/Dxe/DxeMain.inf

>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

> -  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>  

>    #

> 


Going over my requests (1) through (6) again, they are all one-liner
changes. With them addressed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel April 13, 2016, 2:25 p.m. | #2
On 13 April 2016 at 16:18, Laszlo Ersek <lersek@redhat.com> wrote:
> On 04/13/16 10:02, Ard Biesheuvel wrote:

>> Now that we have moved the handling of the xen,xen DT node to XenioFdtDxe,

>> remove its handling from VirtFdtDxe. Since the only functionality that

>> remains is handling the virtio,mmio DT node, rename VirtFdtDxe to

>> VirtioFdtDxe to reflect that. Also update the platforms that use this

>> driver.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  ArmVirtPkg/{VirtFdtDxe/VirtFdtDxe.inf => VirtioFdtDxe/VirtioFdtDxe.inf} |  32 ++-

>>  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c                                      | 218 --------------------

>>  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c                                  | 125 +++++++++++

>>  ArmVirtPkg/ArmVirtQemu.dsc                                              |   2 +-

>>  ArmVirtPkg/ArmVirtQemu.fdf                                              |   2 +-

>>  ArmVirtPkg/ArmVirtQemuKernel.dsc                                        |   2 +-

>>  ArmVirtPkg/ArmVirtQemuKernel.fdf                                        |   2 +-

>>  7 files changed, 143 insertions(+), 240 deletions(-)

>>

>> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>> similarity index 61%

>> rename from ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> rename to ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>> index 4dd46cf87011..e4ac1a285bc8 100644

>> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> +++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>> @@ -1,7 +1,7 @@

>>  ## @file

>> -#  Device tree enumeration DXE driver for ARM Virtual Machines

>> +#  Virtio FDT client protocol driver for virtio,mmio DT node

>>  #

>> -#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>

>> +#  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>

>>  #

>>  #  This program and the accompanying materials are

>>  #  licensed and made available under the terms and conditions of the BSD License

>> @@ -15,41 +15,37 @@

>>

>>  [Defines]

>>    INF_VERSION                    = 0x00010005

>> -  BASE_NAME                      = VirtFdtDxe

>> +  BASE_NAME                      = VirtioFdtDxe

>>    FILE_GUID                      = 9AD7DCB4-E6EC-472E-96BF-81C219A3F77E

>

> (1) Can you please generate a new GUID? The new role of the driver is

> finally acknowledged by this patch (as reflected by the name change

> too). I'd like a new GUID, if you don't mind.

>


Fair enough

>> diff --git a/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c

>> new file mode 100644

>> index 000000000000..09c311410fda

>> --- /dev/null

>> +++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c

>> @@ -0,0 +1,125 @@

>> +/** @file

>> +*  Virtio FDT client protocol driver for virtio,mmio DT node

>> +*

>> +*  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>

>> +*

>> +*  This program and the accompanying materials are

>> +*  licensed and made available under the terms and conditions of the BSD License

>> +*  which accompanies this distribution.  The full text of the license may be found at

>> +*  http://opensource.org/licenses/bsd-license.php

>> +*

>> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,

>> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

>> +*

>> +**/

>> +

>> +#include <Library/BaseLib.h>

>> +#include <Library/BaseMemoryLib.h>

>> +#include <Library/DebugLib.h>

>> +#include <Library/DevicePathLib.h>

>> +#include <Library/MemoryAllocationLib.h>

>> +#include <Library/UefiBootServicesTableLib.h>

>> +#include <Library/UefiDriverEntryPoint.h>

>> +#include <Library/VirtioMmioDeviceLib.h>

>> +

>> +#include <Guid/VirtioMmioTransport.h>

>> +

>> +#include <Protocol/FdtClient.h>

>> +

>> +#pragma pack (1)

>> +typedef struct {

>> +  VENDOR_DEVICE_PATH                  Vendor;

>> +  UINT64                              PhysBase;

>> +  EFI_DEVICE_PATH_PROTOCOL            End;

>> +} VIRTIO_TRANSPORT_DEVICE_PATH;

>> +#pragma pack ()

>> +

>> +EFI_STATUS

>> +EFIAPI

>> +InitializeVirtioFdtDxe (

>> +  IN EFI_HANDLE           ImageHandle,

>> +  IN EFI_SYSTEM_TABLE     *SystemTable

>> +  )

>> +{

>> +  EFI_STATUS                     Status, FindNodeStatus;

>> +  FDT_CLIENT_PROTOCOL            *FdtClient;

>> +  INT32                          Node;

>> +  CONST UINT64                   *Reg;

>> +  UINT32                         RegSize;

>> +  VIRTIO_TRANSPORT_DEVICE_PATH   *DevicePath;

>> +  EFI_HANDLE                     Handle;

>> +  UINT64                         RegBase;

>> +

>> +  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,

>> +                  (VOID **)&FdtClient);

>> +  ASSERT_EFI_ERROR (Status);

>> +

>> +  for (FindNodeStatus = FdtClient->FindCompatibleNode (FdtClient,

>> +                                     "virtio,mmio", &Node);

>> +       FindNodeStatus == EFI_SUCCESS;

>

> (2) This would be more idiomatic as

>

>   !EFI_ERROR (FindNodeStatus)

>


Indeed

>> +       FindNodeStatus = FdtClient->FindNextCompatibleNode (FdtClient,

>> +                                     "virtio,mmio", Node, &Node)) {

>> +

>> +    Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",

>> +                          (CONST VOID **)&Reg, &RegSize);

>> +    if (EFI_ERROR (Status)) {

>> +      DEBUG ((EFI_D_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",

>> +        __FUNCTION__, Status));

>> +      continue;

>> +    }

>> +

>> +    ASSERT (RegSize == 16);

>> +

>> +    //

>> +    // Create a unique device path for this transport on the fly

>> +    //

>> +    RegBase = SwapBytes64 (*Reg);

>> +    DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode (

>> +                                  HARDWARE_DEVICE_PATH,

>> +                                  HW_VENDOR_DP,

>> +                                  sizeof (VIRTIO_TRANSPORT_DEVICE_PATH));

>> +    if (DevicePath == NULL) {

>> +      DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__));

>> +      break;

>> +    }

>

> (3) It is almost useless for me to obsess about this :), but the "break"

> statement above doesn't work identically to the original. In the

> original, it terminated the case branch, and the loop continued; hence

> the "break" enabled the installation of further virtio-mmio devices.

> Whereas here we give up permanently.

>

> I would prefer a "continue" here, rather than a "break", but not for

> practical reasons -- just so we stay true to the original (because "this

> is refactoring").

>


Your attention to detail is impressive. I will change it, of course.

>> +

>> +    CopyGuid (&DevicePath->Vendor.Guid, &gVirtioMmioTransportGuid);

>

> Nice :)

>

>> +    DevicePath->PhysBase = RegBase;

>> +    SetDevicePathNodeLength (&DevicePath->Vendor,

>> +      sizeof (*DevicePath) - sizeof (DevicePath->End));

>> +    SetDevicePathEndNode (&DevicePath->End);

>> +

>> +    Handle = NULL;

>> +    Status = gBS->InstallProtocolInterface (&Handle,

>> +                     &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,

>> +                     DevicePath);

>> +    if (EFI_ERROR (Status)) {

>> +      DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH "

>> +        "protocol on a new handle (Status == %r)\n",

>> +        __FUNCTION__, Status));

>> +      FreePool (DevicePath);

>> +      break;

>

> (4) This should be a "continue" too.

>

>> +    }

>> +

>> +    Status = VirtioMmioInstallDevice (RegBase, Handle);

>> +    if (EFI_ERROR (Status)) {

>> +      DEBUG ((EFI_D_ERROR, "%a: Failed to install VirtIO transport @ 0x%Lx "

>> +        "on handle %p (Status == %r)\n", __FUNCTION__, RegBase,

>> +        Handle, Status));

>> +

>> +      Status = gBS->UninstallProtocolInterface (Handle,

>> +                      &gEfiDevicePathProtocolGuid, DevicePath);

>> +      ASSERT_EFI_ERROR (Status);

>> +      FreePool (DevicePath);

>> +      break;

>

> (5) This should also be a "continue".

>

> (Yes, we're at the end of the loop body anyway, but an explicit

> "continue" is clearer, IMO, esp. if we'll have to append something later

> to the loop body.)

>

>> +    }

>> +  }

>> +

>> +  if (EFI_ERROR (FindNodeStatus) && FindNodeStatus != EFI_NOT_FOUND) {

>> +     DEBUG ((EFI_D_ERROR, "%a: Error occurred while iterating DT nodes "

>> +       "(FindNodeStatus == %r)\n", __FUNCTION__, FindNodeStatus));

>> +    return Status;

>

> (6) I propose to drop the return statement: let the firmware proceed

> with the virtio-mmio devices we managed to install.

>

> Otherwise, if you wanted to exit with an error (--> unload the driver),

> then you would have to rollback all the devpath allocations, devpath

> installations, and VirtioMmioInstallDevice() calls, that did succeed

> before the error.

>

> (This is fully doable, as VirtioMmioUninstallDevice() exists, but it's

> out of scope for this refactoring. So, let's just drop the return

> statement.)

>


Unloading the driver would only make sense if the first
FindCompatibleNode () failed, but for the subsequent
FindNextCompatibleNode () ones, it doesn't. So I will remove it.

>> +  }

>> +

>> +  return EFI_SUCCESS;

>> +}

>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc

>> index 70419ff3f856..34323bf83d64 100644

>> --- a/ArmVirtPkg/ArmVirtQemu.dsc

>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc

>> @@ -322,7 +322,7 @@ [Components.common]

>>    #

>>    # Platform Driver

>>    #

>> -  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> +  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>>    ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>>    ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>>    OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

>> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf

>> index a7204898f473..071eaef5dd80 100644

>> --- a/ArmVirtPkg/ArmVirtQemu.fdf

>> +++ b/ArmVirtPkg/ArmVirtQemu.fdf

>> @@ -103,7 +103,7 @@ [FV.FvMain]

>>

>>    INF MdeModulePkg/Core/Dxe/DxeMain.inf

>>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

>> -  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> +  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>>    INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf

>>

>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc

>> index 65991baaf680..e1b45ffaf8fd 100644

>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc

>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc

>> @@ -300,7 +300,7 @@ [Components.common]

>>    #

>>    # Platform Driver

>>    #

>> -  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> +  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>>    ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>>    OvmfPkg/VirtioBlkDxe/VirtioBlk.inf

>>    OvmfPkg/VirtioScsiDxe/VirtioScsi.inf

>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> index b8f66d2f472c..7c478340f9e7 100644

>> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf

>> @@ -125,7 +125,7 @@ [FV.FvMain]

>>

>>    INF MdeModulePkg/Core/Dxe/DxeMain.inf

>>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf

>> -  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

>> +  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf

>>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>>

>>    #

>>

>

> Going over my requests (1) through (6) again, they are all one-liner

> changes. With them addressed:

>

> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

>


Thanks! Almost done ....
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
similarity index 61%
rename from ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
rename to ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
index 4dd46cf87011..e4ac1a285bc8 100644
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
@@ -1,7 +1,7 @@ 
 ## @file
-#  Device tree enumeration DXE driver for ARM Virtual Machines
+#  Virtio FDT client protocol driver for virtio,mmio DT node
 #
-#  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>
 #
 #  This program and the accompanying materials are
 #  licensed and made available under the terms and conditions of the BSD License
@@ -15,41 +15,37 @@ 
 
 [Defines]
   INF_VERSION                    = 0x00010005
-  BASE_NAME                      = VirtFdtDxe
+  BASE_NAME                      = VirtioFdtDxe
   FILE_GUID                      = 9AD7DCB4-E6EC-472E-96BF-81C219A3F77E
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
 
-  ENTRY_POINT                    = InitializeVirtFdtDxe
+  ENTRY_POINT                    = InitializeVirtioFdtDxe
 
 [Sources]
-  VirtFdtDxe.c
+  VirtioFdtDxe.c
 
 [Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
-  ArmPkg/ArmPkg.dec
-  ArmPlatformPkg/ArmPlatformPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseLib
-  PcdLib
+  BaseMemoryLib
+  DebugLib
+  DevicePathLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
   UefiDriverEntryPoint
-  DxeServicesLib
-  FdtLib
   VirtioMmioDeviceLib
-  HobLib
-  XenIoMmioLib
 
 [Guids]
   gVirtioMmioTransportGuid
-  gFdtHobGuid
 
 [Protocols]
-  gEfiDevicePathProtocolGuid
+  gEfiDevicePathProtocolGuid                            ## PRODUCES
+  gFdtClientProtocolGuid                                ## CONSUMES
 
 [Depex]
-  TRUE
+  gFdtClientProtocolGuid
diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
deleted file mode 100644
index cebd4aa91fd9..000000000000
--- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c
+++ /dev/null
@@ -1,218 +0,0 @@ 
-/** @file
-*  Device tree enumeration DXE driver for ARM Virtual Machines
-*
-*  Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
-*
-*  This program and the accompanying materials are
-*  licensed and made available under the terms and conditions of the BSD License
-*  which accompanies this distribution.  The full text of the license may be found at
-*  http://opensource.org/licenses/bsd-license.php
-*
-*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-*
-**/
-
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-#include <Library/UefiLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/UefiDriverEntryPoint.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/VirtioMmioDeviceLib.h>
-#include <Library/DevicePathLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DxeServicesLib.h>
-#include <Library/HobLib.h>
-#include <libfdt.h>
-#include <Library/XenIoMmioLib.h>
-
-#include <Guid/VirtioMmioTransport.h>
-#include <Guid/FdtHob.h>
-
-#pragma pack (1)
-typedef struct {
-  VENDOR_DEVICE_PATH                  Vendor;
-  UINT64                              PhysBase;
-  EFI_DEVICE_PATH_PROTOCOL            End;
-} VIRTIO_TRANSPORT_DEVICE_PATH;
-#pragma pack ()
-
-typedef enum {
-  PropertyTypeUnknown,
-  PropertyTypeVirtio,
-  PropertyTypeXen,
-} PROPERTY_TYPE;
-
-typedef struct {
-  PROPERTY_TYPE Type;
-  CHAR8         Compatible[32];
-} PROPERTY;
-
-STATIC CONST PROPERTY CompatibleProperties[] = {
-  { PropertyTypeVirtio,  "virtio,mmio"           },
-  { PropertyTypeXen,     "xen,xen"               },
-  { PropertyTypeUnknown, ""                      }
-};
-
-STATIC
-PROPERTY_TYPE
-GetTypeFromNode (
-  IN CONST CHAR8 *NodeType,
-  IN UINTN       Size
-  )
-{
-  CONST CHAR8    *Compatible;
-  CONST PROPERTY *CompatibleProperty;
-
-  //
-  // A 'compatible' node may contain a sequence of NULL terminated
-  // compatible strings so check each one
-  //
-  for (Compatible = NodeType; Compatible < NodeType + Size && *Compatible;
-       Compatible += 1 + AsciiStrLen (Compatible)) {
-    for (CompatibleProperty = CompatibleProperties; CompatibleProperty->Compatible[0]; CompatibleProperty++) {
-      if (AsciiStrCmp (CompatibleProperty->Compatible, Compatible) == 0) {
-        return CompatibleProperty->Type;
-      }
-    }
-  }
-  return PropertyTypeUnknown;
-}
-
-EFI_STATUS
-EFIAPI
-InitializeVirtFdtDxe (
-  IN EFI_HANDLE           ImageHandle,
-  IN EFI_SYSTEM_TABLE     *SystemTable
-  )
-{
-  VOID                           *Hob;
-  VOID                           *DeviceTreeBase;
-  INT32                          Node, Prev;
-  EFI_STATUS                     Status;
-  CONST CHAR8                    *Type;
-  INT32                          Len;
-  PROPERTY_TYPE                  PropType;
-  CONST VOID                     *RegProp;
-  VIRTIO_TRANSPORT_DEVICE_PATH   *DevicePath;
-  EFI_HANDLE                     Handle;
-  UINT64                         RegBase;
-
-  Hob = GetFirstGuidHob(&gFdtHobGuid);
-  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64)) {
-    return EFI_NOT_FOUND;
-  }
-  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
-
-  if (fdt_check_header (DeviceTreeBase) != 0) {
-    DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
-    return EFI_NOT_FOUND;
-  }
-
-  DEBUG ((EFI_D_INFO, "%a: DTB @ 0x%p\n", __FUNCTION__, DeviceTreeBase));
-
-  //
-  // Now enumerate the nodes and install peripherals that we are interested in,
-  // i.e., GIC, RTC and virtio MMIO nodes
-  //
-  for (Prev = 0;; Prev = Node) {
-    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
-    if (Node < 0) {
-      break;
-    }
-
-    Type = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
-    if (Type == NULL) {
-      continue;
-    }
-
-    PropType = GetTypeFromNode (Type, Len);
-    if (PropType == PropertyTypeUnknown) {
-      continue;
-    }
-
-    //
-    // Get the 'reg' property of this node. For now, we will assume
-    // 8 byte quantities for base and size, respectively.
-    // TODO use #cells root properties instead
-    //
-    RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
-    ASSERT (RegProp != NULL);
-
-    switch (PropType) {
-    case PropertyTypeVirtio:
-      ASSERT (Len == 16);
-      //
-      // Create a unique device path for this transport on the fly
-      //
-      RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
-      DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode (
-                                    HARDWARE_DEVICE_PATH,
-                                    HW_VENDOR_DP,
-                                    sizeof (VIRTIO_TRANSPORT_DEVICE_PATH));
-      if (DevicePath == NULL) {
-        DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__));
-        break;
-      }
-
-      CopyMem (&DevicePath->Vendor.Guid, &gVirtioMmioTransportGuid,
-        sizeof (EFI_GUID));
-      DevicePath->PhysBase = RegBase;
-      SetDevicePathNodeLength (&DevicePath->Vendor,
-                               sizeof (*DevicePath) - sizeof (DevicePath->End));
-      SetDevicePathEndNode (&DevicePath->End);
-
-      Handle = NULL;
-      Status = gBS->InstallProtocolInterface (&Handle,
-                     &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
-                     DevicePath);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH "
-          "protocol on a new handle (Status == %r)\n",
-          __FUNCTION__, Status));
-        FreePool (DevicePath);
-        break;
-      }
-
-      Status = VirtioMmioInstallDevice (RegBase, Handle);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((EFI_D_ERROR, "%a: Failed to install VirtIO transport @ 0x%Lx "
-          "on handle %p (Status == %r)\n", __FUNCTION__, RegBase,
-          Handle, Status));
-
-        Status = gBS->UninstallProtocolInterface (Handle,
-                        &gEfiDevicePathProtocolGuid, DevicePath);
-        ASSERT_EFI_ERROR (Status);
-        FreePool (DevicePath);
-      }
-      break;
-
-    case PropertyTypeXen:
-      ASSERT (Len == 16);
-
-      //
-      // Retrieve the reg base from this node and wire it up to the
-      // MMIO flavor of the XenBus root device I/O protocol
-      //
-      RegBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
-      Handle = NULL;
-      Status = XenIoMmioInstall (&Handle, RegBase);
-      if (EFI_ERROR (Status)) {
-        DEBUG ((EFI_D_ERROR, "%a: XenIoMmioInstall () failed on a new handle "
-          "(Status == %r)\n", __FUNCTION__, Status));
-        break;
-      }
-
-      DEBUG ((EFI_D_INFO, "Found Xen node with Grant table @ 0x%Lx\n", RegBase));
-
-      break;
-
-    default:
-      break;
-    }
-  }
-
-  return EFI_SUCCESS;
-}
diff --git a/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c
new file mode 100644
index 000000000000..09c311410fda
--- /dev/null
+++ b/ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.c
@@ -0,0 +1,125 @@ 
+/** @file
+*  Virtio FDT client protocol driver for virtio,mmio DT node
+*
+*  Copyright (c) 2014 - 2016, Linaro Ltd. All rights reserved.<BR>
+*
+*  This program and the accompanying materials are
+*  licensed and made available under the terms and conditions of the BSD License
+*  which accompanies this distribution.  The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/VirtioMmioDeviceLib.h>
+
+#include <Guid/VirtioMmioTransport.h>
+
+#include <Protocol/FdtClient.h>
+
+#pragma pack (1)
+typedef struct {
+  VENDOR_DEVICE_PATH                  Vendor;
+  UINT64                              PhysBase;
+  EFI_DEVICE_PATH_PROTOCOL            End;
+} VIRTIO_TRANSPORT_DEVICE_PATH;
+#pragma pack ()
+
+EFI_STATUS
+EFIAPI
+InitializeVirtioFdtDxe (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  EFI_STATUS                     Status, FindNodeStatus;
+  FDT_CLIENT_PROTOCOL            *FdtClient;
+  INT32                          Node;
+  CONST UINT64                   *Reg;
+  UINT32                         RegSize;
+  VIRTIO_TRANSPORT_DEVICE_PATH   *DevicePath;
+  EFI_HANDLE                     Handle;
+  UINT64                         RegBase;
+
+  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+                  (VOID **)&FdtClient);
+  ASSERT_EFI_ERROR (Status);
+
+  for (FindNodeStatus = FdtClient->FindCompatibleNode (FdtClient,
+                                     "virtio,mmio", &Node);
+       FindNodeStatus == EFI_SUCCESS;
+       FindNodeStatus = FdtClient->FindNextCompatibleNode (FdtClient,
+                                     "virtio,mmio", Node, &Node)) {
+
+    Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",
+                          (CONST VOID **)&Reg, &RegSize);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_ERROR, "%a: GetNodeProperty () failed (Status == %r)\n",
+        __FUNCTION__, Status));
+      continue;
+    }
+
+    ASSERT (RegSize == 16);
+
+    //
+    // Create a unique device path for this transport on the fly
+    //
+    RegBase = SwapBytes64 (*Reg);
+    DevicePath = (VIRTIO_TRANSPORT_DEVICE_PATH *)CreateDeviceNode (
+                                  HARDWARE_DEVICE_PATH,
+                                  HW_VENDOR_DP,
+                                  sizeof (VIRTIO_TRANSPORT_DEVICE_PATH));
+    if (DevicePath == NULL) {
+      DEBUG ((EFI_D_ERROR, "%a: Out of memory\n", __FUNCTION__));
+      break;
+    }
+
+    CopyGuid (&DevicePath->Vendor.Guid, &gVirtioMmioTransportGuid);
+    DevicePath->PhysBase = RegBase;
+    SetDevicePathNodeLength (&DevicePath->Vendor,
+      sizeof (*DevicePath) - sizeof (DevicePath->End));
+    SetDevicePathEndNode (&DevicePath->End);
+
+    Handle = NULL;
+    Status = gBS->InstallProtocolInterface (&Handle,
+                     &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
+                     DevicePath);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_ERROR, "%a: Failed to install the EFI_DEVICE_PATH "
+        "protocol on a new handle (Status == %r)\n",
+        __FUNCTION__, Status));
+      FreePool (DevicePath);
+      break;
+    }
+
+    Status = VirtioMmioInstallDevice (RegBase, Handle);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_ERROR, "%a: Failed to install VirtIO transport @ 0x%Lx "
+        "on handle %p (Status == %r)\n", __FUNCTION__, RegBase,
+        Handle, Status));
+
+      Status = gBS->UninstallProtocolInterface (Handle,
+                      &gEfiDevicePathProtocolGuid, DevicePath);
+      ASSERT_EFI_ERROR (Status);
+      FreePool (DevicePath);
+      break;
+    }
+  }
+
+  if (EFI_ERROR (FindNodeStatus) && FindNodeStatus != EFI_NOT_FOUND) {
+     DEBUG ((EFI_D_ERROR, "%a: Error occurred while iterating DT nodes "
+       "(FindNodeStatus == %r)\n", __FUNCTION__, FindNodeStatus));
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 70419ff3f856..34323bf83d64 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -322,7 +322,7 @@  [Components.common]
   #
   # Platform Driver
   #
-  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
   ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
   ArmVirtPkg/HighMemDxe/HighMemDxe.inf
   OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index a7204898f473..071eaef5dd80 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -103,7 +103,7 @@  [FV.FvMain]
 
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
-  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
   INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
   INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 65991baaf680..e1b45ffaf8fd 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -300,7 +300,7 @@  [Components.common]
   #
   # Platform Driver
   #
-  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+  ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
   ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
   OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
   OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf
index b8f66d2f472c..7c478340f9e7 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
+++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
@@ -125,7 +125,7 @@  [FV.FvMain]
 
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
-  INF ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+  INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
   INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
 
   #