[Xen-devel] [PATCH 09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services

[Anthony PERARD <anthony.perard@citrix.com>]

This patch fix the EVT_SIGNAL_EXIT_BOOT_SERVICES handler to avoid
using the Memory Allocation Services.

This comes with a new interface named RegisterExitCallback so that PV
drivers can disconnect from the backend before XenBusDxe is teared
down.

Instead of using Disconnect() to tear down the XenBus driver and the
children drivers, we are going to ask every driver using
XENBUS_PROTOCOL to disconnect from the hardware via the callback set
with RegisterExitCallback, then reset the xenstore shared ring and
the grant table.

Since there are no driver using RegisterExitCallback yet, no driver are
going to be disconnected. Linux can deal with that. And that will be
fixed by the next patch with a change for XenPvBlkDxe.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 OvmfPkg/Include/Protocol/XenBus.h | 35 +++++++++++++++++++++++++++++++
 OvmfPkg/XenBusDxe/XenBus.c        | 18 ++++++++++++++++
 OvmfPkg/XenBusDxe/XenBusDxe.c     | 27 +++++++++++++++++++++---
 OvmfPkg/XenBusDxe/XenBusDxe.h     |  2 ++
 OvmfPkg/XenBusDxe/XenStore.c      | 22 ++++++++++++++++++-
 OvmfPkg/XenBusDxe/XenStore.h      | 21 +++++++++++++++++++
 6 files changed, 121 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Include/Protocol/XenBus.h b/OvmfPkg/Include/Protocol/XenBus.h
index c22bdfb368..04986747c8 100644
--- a/OvmfPkg/Include/Protocol/XenBus.h
+++ b/OvmfPkg/Include/Protocol/XenBus.h
@@ -373,6 +373,38 @@ XENSTORE_STATUS
   IN VOID             *Token
   );
 
+typedef
+VOID
+(EFIAPI *XENBUS_EXIT_CALLBACK)(
+  IN  VOID                     *Context
+  );
+
+/**
+  Register a function to be called during the ExitBootServices event.
+
+  NotifyFunction will be called when XenBusDxe is notified of
+  EVT_SIGNAL_EXIT_BOOT_SERVICES. The function should follow the same
+  requirements as if it as registered an event on
+  EVT_SIGNAL_EXIT_BOOT_SERVICES, i.e. no use of the Memory Allocation
+  Services.
+
+  To unregister the function, call RegisterExitCallback with
+  NotifyFunction=NULL.
+
+  @note There can only be one callback per driver.
+
+  @param This             A pointer to the XENBUS_PROTOCOL.
+  @param NotifyFunction   The function to be called.
+  @param NotifyContext    A context.
+**/
+typedef
+VOID
+(EFIAPI *XENBUS_SET_EXIT_CALLBACK) (
+  IN XENBUS_PROTOCOL       *This,
+  IN XENBUS_EXIT_CALLBACK  NotifyFunction,
+  IN VOID                  *NotifyContext
+  );
+
 
 ///
 /// Protocol structure
@@ -400,6 +432,9 @@ struct _XENBUS_PROTOCOL {
   XENBUS_REGISTER_WATCH_BACKEND RegisterWatchBackend;
   XENBUS_UNREGISTER_WATCH       UnregisterWatch;
   XENBUS_WAIT_FOR_WATCH         WaitForWatch;
+
+  XENBUS_SET_EXIT_CALLBACK      RegisterExitCallback;
+
   //
   // Protocol data fields
   //
diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
index 78835ec7b3..54166505d2 100644
--- a/OvmfPkg/XenBusDxe/XenBus.c
+++ b/OvmfPkg/XenBusDxe/XenBus.c
@@ -343,6 +343,23 @@ XenBusSetState (
   return Status;
 }
 
+STATIC
+VOID
+EFIAPI
+XenBusRegisterExitCallback (
+  IN XENBUS_PROTOCOL       *This,
+  IN XENBUS_EXIT_CALLBACK  NotifyFunction,
+  IN VOID                  *NotifyContext
+  )
+{
+  XENBUS_PRIVATE_DATA *Dev;
+
+  Dev = XENBUS_PRIVATE_DATA_FROM_THIS (This);
+
+  Dev->ExitCallback = NotifyFunction;
+  Dev->ExitCallbackContext = NotifyContext;
+}
+
 STATIC XENBUS_PRIVATE_DATA gXenBusPrivateData = {
   XENBUS_PRIVATE_DATA_SIGNATURE,    // Signature
   { NULL, NULL },                   // Link
@@ -364,6 +381,7 @@ STATIC XENBUS_PRIVATE_DATA gXenBusPrivateData = {
     XenBusRegisterWatchBackend,     // XenBusIo.RegisterWatchBackend
     XenBusUnregisterWatch,          // XenBusIo.UnregisterWatch
     XenBusWaitForWatch,             // XenBusIo.WaitForWatch
+    XenBusRegisterExitCallback,     // XenBusIo.RegisterExitCallback
 
     NULL,                           // XenBusIo.Type
     0,                              // XenBusIo.DeviceId
diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c
index 634c7b71eb..c71966a666 100644
--- a/OvmfPkg/XenBusDxe/XenBusDxe.c
+++ b/OvmfPkg/XenBusDxe/XenBusDxe.c
@@ -258,10 +258,31 @@ NotifyExitBoot (
   IN VOID *Context
   )
 {
-  XENBUS_DEVICE *Dev = Context;
+  XENBUS_DEVICE       *Dev;
+  LIST_ENTRY          *Entry;
+  XENBUS_PRIVATE_DATA *Child;
 
-  gBS->DisconnectController(Dev->ControllerHandle,
-                            Dev->This->DriverBindingHandle, NULL);
+  Dev = Context;
+
+  //
+  // First, ask every driver using xenbus to disconnect without
+  // deallocating memory.
+  //
+  for (Entry = GetFirstNode (&Dev->ChildList);
+       !IsNodeAtEnd (&Dev->ChildList, Entry);
+       Entry = GetNextNode (&Dev->ChildList, Entry)) {
+    Child = XENBUS_PRIVATE_DATA_FROM_LINK (Entry);
+    if (Child->ExitCallback != NULL) {
+      Child->ExitCallback (Child->ExitCallbackContext);
+    }
+  }
+
+  //
+  // Now, we can reset the devices used by XenBusDxe.
+  //
+  XenStoreResetWatches ();
+  XenStoreResetRing (Dev);
+  XenGrantTableDeinit (Dev);
 }
 
 /**
diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.h b/OvmfPkg/XenBusDxe/XenBusDxe.h
index b1dcc3549c..0e91c24338 100644
--- a/OvmfPkg/XenBusDxe/XenBusDxe.h
+++ b/OvmfPkg/XenBusDxe/XenBusDxe.h
@@ -94,6 +94,8 @@ typedef struct {
     XENBUS_PROTOCOL XenBusIo;
     XENBUS_DEVICE *Dev;
     XENBUS_DEVICE_PATH *DevicePath;
+    XENBUS_EXIT_CALLBACK ExitCallback;
+    VOID *ExitCallbackContext;
 } XENBUS_PRIVATE_DATA;
 
 #define XENBUS_PRIVATE_DATA_FROM_THIS(a) \
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index cb2d9e1215..4026c8a829 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -1054,8 +1054,18 @@ XenStoreDeinit (
     }
   }
 
+  XenStoreResetRing (Dev);
+
   gBS->CloseEvent (xs.EventChannelEvent);
 
+  xs.XenStore = NULL;
+}
+
+VOID
+XenStoreResetRing (
+  IN XENBUS_DEVICE *Dev
+  )
+{
   if (xs.XenStore->server_features & XENSTORE_SERVER_FEATURE_RECONNECTION) {
     xs.XenStore->connection = XENSTORE_RECONNECT;
     XenEventChannelNotify (xs.Dev, xs.EventChannel);
@@ -1072,7 +1082,17 @@ XenStoreDeinit (
     xs.XenStore->req_cons = xs.XenStore->req_prod = 0;
     xs.XenStore->rsp_cons = xs.XenStore->rsp_prod = 0;
   }
-  xs.XenStore = NULL;
+}
+
+VOID
+XenStoreResetWatches (
+  VOID
+  )
+{
+  XENSTORE_STATUS     Status;
+
+  Status = XenStoreSingle (XST_NIL, XS_RESET_WATCHES, "", NULL, NULL, NULL);
+  ASSERT (Status == XENSTORE_STATUS_SUCCESS);
 }
 
 //
diff --git a/OvmfPkg/XenBusDxe/XenStore.h b/OvmfPkg/XenBusDxe/XenStore.h
index ca8c080433..62cd5e97bf 100644
--- a/OvmfPkg/XenBusDxe/XenStore.h
+++ b/OvmfPkg/XenBusDxe/XenStore.h
@@ -271,6 +271,27 @@ XenStoreDeinit (
   IN XENBUS_DEVICE *Dev
   );
 
+/**
+  Effectively reset all watches registered with xenstore.
+
+  To be used by ExitBootServices
+**/
+VOID
+XenStoreResetWatches (
+  VOID
+  );
+
+/**
+  Reset the xenstore shared ring before handing it over to the next
+  operating system.
+
+  To be used by ExitBootServices
+**/
+VOID
+XenStoreResetRing (
+  IN XENBUS_DEVICE *Dev
+  );
+
 
 //
 // XENBUS protocol
-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel