[edk2,v3,8/9] ArmVirtPkg/ArmVirtXen: move from VirtFdtDxe to new XenioFdtDxe driver

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

Commit Message

Ard Biesheuvel April 13, 2016, 8:02 a.m.
Now that the only functionality that remains in VirtFdtDxe is enumerating
the respective virtual I/O buses, it no longer makes sense to have a driver
that is shared between Xen domU and QEMU. So move the Xen I/O DT node
handling to a new driver, and update ArmVirtXen to switch to it.

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

---
 ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf | 44 ++++++++++++++
 ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c   | 63 ++++++++++++++++++++
 ArmVirtPkg/ArmVirtXen.dsc              |  2 +-
 ArmVirtPkg/ArmVirtXen.fdf              |  2 +-
 4 files changed, 109 insertions(+), 2 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, 1:30 p.m. | #1
On 04/13/16 10:02, Ard Biesheuvel wrote:
> Now that the only functionality that remains in VirtFdtDxe is enumerating

> the respective virtual I/O buses, it no longer makes sense to have a driver

> that is shared between Xen domU and QEMU. So move the Xen I/O DT node

> handling to a new driver, and update ArmVirtXen to switch to it.


That's very good. For a second I was worried about XenIoMmioInstall()
occurring twice in a single system, but the DSC / FDF changes in this
patch prevent it nicely.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

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

> ---

>  ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf | 44 ++++++++++++++

>  ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c   | 63 ++++++++++++++++++++

>  ArmVirtPkg/ArmVirtXen.dsc              |  2 +-

>  ArmVirtPkg/ArmVirtXen.fdf              |  2 +-

>  4 files changed, 109 insertions(+), 2 deletions(-)

> 

> diff --git a/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf

> new file mode 100644

> index 000000000000..d75e4f45ae41

> --- /dev/null

> +++ b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf

> @@ -0,0 +1,44 @@

> +## @file

> +#  Xenio FDT client protocol driver for xen,xen DT node

> +#

> +#  Copyright (c) 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.

> +#

> +##

> +

> +[Defines]

> +  INF_VERSION                    = 0x00010005

> +  BASE_NAME                      = XenioFdtDxe

> +  FILE_GUID                      = 338695EA-CA84-4FA2-9DA8-5C4BB87905C6

> +  MODULE_TYPE                    = DXE_DRIVER

> +  VERSION_STRING                 = 1.0

> +

> +  ENTRY_POINT                    = InitializeXenioFdtDxe

> +

> +[Sources]

> +  XenioFdtDxe.c

> +

> +[Packages]

> +  ArmVirtPkg/ArmVirtPkg.dec

> +  MdePkg/MdePkg.dec

> +  OvmfPkg/OvmfPkg.dec

> +

> +[LibraryClasses]

> +  BaseLib

> +  DebugLib

> +  UefiBootServicesTableLib

> +  UefiDriverEntryPoint

> +  XenIoMmioLib

> +

> +[Protocols]

> +  gFdtClientProtocolGuid                                ## CONSUMES

> +

> +[Depex]

> +  gFdtClientProtocolGuid

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

> new file mode 100644

> index 000000000000..64cb5c3bfd9e

> --- /dev/null

> +++ b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c

> @@ -0,0 +1,63 @@

> +/** @file

> +*  Xenio FDT client protocol driver for xen,xen DT node

> +*

> +*  Copyright (c) 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/DebugLib.h>

> +#include <Library/UefiDriverEntryPoint.h>

> +#include <Library/UefiBootServicesTableLib.h>

> +#include <Library/XenIoMmioLib.h>

> +

> +#include <Protocol/FdtClient.h>

> +

> +EFI_STATUS

> +EFIAPI

> +InitializeXenioFdtDxe (

> +  IN EFI_HANDLE           ImageHandle,

> +  IN EFI_SYSTEM_TABLE     *SystemTable

> +  )

> +{

> +  EFI_STATUS            Status;

> +  FDT_CLIENT_PROTOCOL   *FdtClient;

> +  CONST UINT64          *Reg;

> +  UINT32                RegElemSize, RegSize;

> +  EFI_HANDLE            Handle;

> +  UINT64                RegBase;

> +

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

> +                  (VOID **)&FdtClient);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  Status = FdtClient->FindCompatibleNodeReg (FdtClient, "xen,xen",

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


Can you add error handling here? I think you can emit an error message,
and simply return EFI_ABORTED or EFI_UNSUPPORTED if this call fails.

With it addressed:

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


Thanks!
Laszlo

> +

> +  ASSERT (RegSize == 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 = SwapBytes64 (Reg[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));

> +    return Status;

> +  }

> +

> +  DEBUG ((EFI_D_INFO, "Found Xen node with Grant table @ 0x%Lx\n", RegBase));

> +

> +  return EFI_SUCCESS;

> +}

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

> index 112dc8ed33df..594ca6491b7a 100644

> --- a/ArmVirtPkg/ArmVirtXen.dsc

> +++ b/ArmVirtPkg/ArmVirtXen.dsc

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

>    #

>    # Platform Driver

>    #

> -  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf

> +  ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf

>    ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>  

>    #

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

> index 56182e719e06..13412f9e2455 100644

> --- a/ArmVirtPkg/ArmVirtXen.fdf

> +++ b/ArmVirtPkg/ArmVirtXen.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/XenioFdtDxe/XenioFdtDxe.inf

>    INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf

>  

>    #

> 


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

Patch

diff --git a/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf
new file mode 100644
index 000000000000..d75e4f45ae41
--- /dev/null
+++ b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf
@@ -0,0 +1,44 @@ 
+## @file
+#  Xenio FDT client protocol driver for xen,xen DT node
+#
+#  Copyright (c) 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = XenioFdtDxe
+  FILE_GUID                      = 338695EA-CA84-4FA2-9DA8-5C4BB87905C6
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = InitializeXenioFdtDxe
+
+[Sources]
+  XenioFdtDxe.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  XenIoMmioLib
+
+[Protocols]
+  gFdtClientProtocolGuid                                ## CONSUMES
+
+[Depex]
+  gFdtClientProtocolGuid
diff --git a/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c
new file mode 100644
index 000000000000..64cb5c3bfd9e
--- /dev/null
+++ b/ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.c
@@ -0,0 +1,63 @@ 
+/** @file
+*  Xenio FDT client protocol driver for xen,xen DT node
+*
+*  Copyright (c) 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/DebugLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/XenIoMmioLib.h>
+
+#include <Protocol/FdtClient.h>
+
+EFI_STATUS
+EFIAPI
+InitializeXenioFdtDxe (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  EFI_STATUS            Status;
+  FDT_CLIENT_PROTOCOL   *FdtClient;
+  CONST UINT64          *Reg;
+  UINT32                RegElemSize, RegSize;
+  EFI_HANDLE            Handle;
+  UINT64                RegBase;
+
+  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+                  (VOID **)&FdtClient);
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNodeReg (FdtClient, "xen,xen",
+                        (CONST VOID **)&Reg, &RegElemSize, &RegSize);
+
+  ASSERT (RegSize == 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 = SwapBytes64 (Reg[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));
+    return Status;
+  }
+
+  DEBUG ((EFI_D_INFO, "Found Xen node with Grant table @ 0x%Lx\n", RegBase));
+
+  return EFI_SUCCESS;
+}
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 112dc8ed33df..594ca6491b7a 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -195,7 +195,7 @@  [Components.common]
   #
   # Platform Driver
   #
-  ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
+  ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf
   ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
 
   #
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 56182e719e06..13412f9e2455 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.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/XenioFdtDxe/XenioFdtDxe.inf
   INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
 
   #