[09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services
diff mbox series

Message ID 20190913145100.303433-10-anthony.perard@citrix.com
State New
Headers show
Series
  • OvmfPkg/XenBusDxe: Fix ExitBootServices handler to avoid allocation
Related show

Commit Message

Anthony PERARD Sept. 13, 2019, 2:50 p.m. UTC
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(-)

Comments

Laszlo Ersek Sept. 16, 2019, 5:36 p.m. UTC | #1
On 09/13/19 16:50, Anthony PERARD wrote:
> 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.

I think this approach -- a lower-level bus driver calling out to
dependent device drivers -- is quite unusual.

How about the following instead:

- introduce two XenBusIo protocol member functions, AddReference() and
RemoveReference(). RemoveReference() should take a BOOLEAN called
"HandOffToOs". The device drivers should call AddReference() just before
exiting DriverBindingStart() with success, and RemoveReference(FALSE) in
DriverBindingStop().

- these protocol member functions would increment / decrement a
reference counter in the underlying XenBus abstraction. Additionally,
RemoveReference() would store the HandOffToOs parameter to a bus-level
BOOLEAN too (regardless of previous value stored there -- a TRUE->FALSE
transition would never happen anyway; see below).

- both XenBusDxe and the Xen device drivers should register EBS
callbacks, per controller handle (in BindingStart()), and unregister
them (in BindingStop())

- the ordering between EBS notification functions (queued at the same
TPL) is unspecified. In the device driver notification functions, the
last action should be a call to XenBusIo->RemoveReference(TRUE) -- after
the device-specific "forget me" actions have been done.

- if RemoveReference() gets a TRUE parameter, then it should check the
resultant (post-decrement) value of the refcount. If it has gone to
zero, RemoveReference() should re-set the xenbus / xenstore connection.
If the parameter is FALSE, it shouldn't do anything particular after
decrementing the refcount.

- in the XenBus EBS handler, if the refcount is positive at the time of
the call, nothing should be done. Otherwise, if HandOffToOs is TRUE,
nothing should be done, similarly. Otherwise, the xenbus/xenstore
connection should be re-set.

The idea is that normal Start/Stop should manage the refcount as
expected. At ExitBootServices(), the XenBus level handler should only
clear the connection to the hypervisor if no RemoveReference() call has
done, or will do, it. (If the counter is positive, then a later
RemoveReference() call will do it; if it's zero but HandOffToOs is TRUE,
then it's been done already. If the counter is zero and the BOOLEAN is
FALSE, then all devices have been disconnected normally with Stop() --
or none have been connected at all --, before ExitBootServices(), so the
XenBus driver itself has to ask for being forgotten.)

Admittedly, this is more complicated (due to the unspecified ordering
between EBS notifications). I just feel it's more idiomatic to go
through normal protocol member functions in EBS notification functions,
rather than special callbacks.

(Side comment: the reference counting could normally be replaced by
gBS->OpenProtocolInformation(); however, that service itself allocates
memory, so we can't use it in EBS notification functions.)

Thanks
Laszlo
Andrew Fish Sept. 16, 2019, 6:36 p.m. UTC | #2
> On Sep 16, 2019, at 10:36 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 09/13/19 16:50, Anthony PERARD wrote:
>> 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.
> 
> I think this approach -- a lower-level bus driver calling out to
> dependent device drivers -- is quite unusual.
> 

Laszlo,

I agree given the timer event activity is stopped prior to calling any of the EXIT_BOOT_SERVICES events there is usually not a requirement to call the drivers Stop() function. Generally Exit Boot Services events just turn off DMA, or any other hardware that could touch memory that is being freed back for OS usage. Since the timer activity, and thus all event activity is stopped there is not a lot of ways for the drivers to ever have any EFI code run again. 

The only other exception I can think of is if the OS driver makes some kind of assumption about the state of the hardware.

Thanks,

Andrew Fish


> How about the following instead:
> 
> - introduce two XenBusIo protocol member functions, AddReference() and
> RemoveReference(). RemoveReference() should take a BOOLEAN called
> "HandOffToOs". The device drivers should call AddReference() just before
> exiting DriverBindingStart() with success, and RemoveReference(FALSE) in
> DriverBindingStop().
> 
> - these protocol member functions would increment / decrement a
> reference counter in the underlying XenBus abstraction. Additionally,
> RemoveReference() would store the HandOffToOs parameter to a bus-level
> BOOLEAN too (regardless of previous value stored there -- a TRUE->FALSE
> transition would never happen anyway; see below).
> 
> - both XenBusDxe and the Xen device drivers should register EBS
> callbacks, per controller handle (in BindingStart()), and unregister
> them (in BindingStop())
> 
> - the ordering between EBS notification functions (queued at the same
> TPL) is unspecified. In the device driver notification functions, the
> last action should be a call to XenBusIo->RemoveReference(TRUE) -- after
> the device-specific "forget me" actions have been done.
> 
> - if RemoveReference() gets a TRUE parameter, then it should check the
> resultant (post-decrement) value of the refcount. If it has gone to
> zero, RemoveReference() should re-set the xenbus / xenstore connection.
> If the parameter is FALSE, it shouldn't do anything particular after
> decrementing the refcount.
> 
> - in the XenBus EBS handler, if the refcount is positive at the time of
> the call, nothing should be done. Otherwise, if HandOffToOs is TRUE,
> nothing should be done, similarly. Otherwise, the xenbus/xenstore
> connection should be re-set.
> 
> The idea is that normal Start/Stop should manage the refcount as
> expected. At ExitBootServices(), the XenBus level handler should only
> clear the connection to the hypervisor if no RemoveReference() call has
> done, or will do, it. (If the counter is positive, then a later
> RemoveReference() call will do it; if it's zero but HandOffToOs is TRUE,
> then it's been done already. If the counter is zero and the BOOLEAN is
> FALSE, then all devices have been disconnected normally with Stop() --
> or none have been connected at all --, before ExitBootServices(), so the
> XenBus driver itself has to ask for being forgotten.)
> 
> Admittedly, this is more complicated (due to the unspecified ordering
> between EBS notifications). I just feel it's more idiomatic to go
> through normal protocol member functions in EBS notification functions,
> rather than special callbacks.
> 
> (Side comment: the reference counting could normally be replaced by
> gBS->OpenProtocolInformation(); however, that service itself allocates
> memory, so we can't use it in EBS notification functions.)
> 
> Thanks
> Laszlo
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#47292): https://edk2.groups.io/g/devel/message/47292
> Mute This Topic: https://groups.io/mt/34128015/1755084
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [afish@apple.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Laszlo Ersek Sept. 16, 2019, 7:31 p.m. UTC | #3
On 09/16/19 20:36, Andrew Fish wrote:
> 
> 
>> On Sep 16, 2019, at 10:36 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 09/13/19 16:50, Anthony PERARD wrote:
>>> 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.
>>
>> I think this approach -- a lower-level bus driver calling out to
>> dependent device drivers -- is quite unusual.
>>
> 
> Laszlo,
> 
> I agree given the timer event activity is stopped prior to calling any of the EXIT_BOOT_SERVICES events there is usually not a requirement to call the drivers Stop() function. Generally Exit Boot Services events just turn off DMA, or any other hardware that could touch memory that is being freed back for OS usage. Since the timer activity, and thus all event activity is stopped there is not a lot of ways for the drivers to ever have any EFI code run again. 
> 
> The only other exception I can think of is if the OS driver makes some kind of assumption about the state of the hardware.

The

    hardware that could touch memory that is being freed back for OS
    usage

is the part that applies here. Most paravirtual devices work by sharing
at least some memory between the host-side device model (emulation) and
guest-side device driver. When the guest is transitioning from firmware
to OS (and those of course have different guest drivers for the
paravirtual device), the host must be made forget the memory shared with
the guest firmware driver (as the guest OS boot loader or the guest OS
itself might load anything at all into that RAM area after
ExitBootServices()).

So what I found unusual in this patch wasn't the necessity of EBS
notification functions, but how they would be ordered / coordinated
between each other. There is a bus-like device that shares its own
resources (RAM) with the host, and installs protocol interfaces. And
there are dependent endpoint-like devices that consume those protocol
interfaces, and share their own stuff (RAM) with the host OS.

All of that shared memory needs to be cleared from the host's
book-keeping when we leave firmware land; the tricky part is that the
bus-like device can't request the host for its bus-level shared memory
to be forgotten before all of the dependent endpoints ask for their
shared ranges to be forgotten.

Thanks!
Laszlo


>> How about the following instead:
>>
>> - introduce two XenBusIo protocol member functions, AddReference() and
>> RemoveReference(). RemoveReference() should take a BOOLEAN called
>> "HandOffToOs". The device drivers should call AddReference() just before
>> exiting DriverBindingStart() with success, and RemoveReference(FALSE) in
>> DriverBindingStop().
>>
>> - these protocol member functions would increment / decrement a
>> reference counter in the underlying XenBus abstraction. Additionally,
>> RemoveReference() would store the HandOffToOs parameter to a bus-level
>> BOOLEAN too (regardless of previous value stored there -- a TRUE->FALSE
>> transition would never happen anyway; see below).
>>
>> - both XenBusDxe and the Xen device drivers should register EBS
>> callbacks, per controller handle (in BindingStart()), and unregister
>> them (in BindingStop())
>>
>> - the ordering between EBS notification functions (queued at the same
>> TPL) is unspecified. In the device driver notification functions, the
>> last action should be a call to XenBusIo->RemoveReference(TRUE) -- after
>> the device-specific "forget me" actions have been done.
>>
>> - if RemoveReference() gets a TRUE parameter, then it should check the
>> resultant (post-decrement) value of the refcount. If it has gone to
>> zero, RemoveReference() should re-set the xenbus / xenstore connection.
>> If the parameter is FALSE, it shouldn't do anything particular after
>> decrementing the refcount.
>>
>> - in the XenBus EBS handler, if the refcount is positive at the time of
>> the call, nothing should be done. Otherwise, if HandOffToOs is TRUE,
>> nothing should be done, similarly. Otherwise, the xenbus/xenstore
>> connection should be re-set.
>>
>> The idea is that normal Start/Stop should manage the refcount as
>> expected. At ExitBootServices(), the XenBus level handler should only
>> clear the connection to the hypervisor if no RemoveReference() call has
>> done, or will do, it. (If the counter is positive, then a later
>> RemoveReference() call will do it; if it's zero but HandOffToOs is TRUE,
>> then it's been done already. If the counter is zero and the BOOLEAN is
>> FALSE, then all devices have been disconnected normally with Stop() --
>> or none have been connected at all --, before ExitBootServices(), so the
>> XenBus driver itself has to ask for being forgotten.)
>>
>> Admittedly, this is more complicated (due to the unspecified ordering
>> between EBS notifications). I just feel it's more idiomatic to go
>> through normal protocol member functions in EBS notification functions,
>> rather than special callbacks.
>>
>> (Side comment: the reference counting could normally be replaced by
>> gBS->OpenProtocolInformation(); however, that service itself allocates
>> memory, so we can't use it in EBS notification functions.)
>>
>> Thanks
>> Laszlo
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Groups.io Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#47292): https://edk2.groups.io/g/devel/message/47292
>> Mute This Topic: https://groups.io/mt/34128015/1755084
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [afish@apple.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
>
Andrew Fish Sept. 16, 2019, 8:50 p.m. UTC | #4
> On Sep 16, 2019, at 12:31 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 09/16/19 20:36, Andrew Fish wrote:
>> 
>> 
>>> On Sep 16, 2019, at 10:36 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> 
>>> On 09/13/19 16:50, Anthony PERARD wrote:
>>>> 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.
>>> 
>>> I think this approach -- a lower-level bus driver calling out to
>>> dependent device drivers -- is quite unusual.
>>> 
>> 
>> Laszlo,
>> 
>> I agree given the timer event activity is stopped prior to calling any of the EXIT_BOOT_SERVICES events there is usually not a requirement to call the drivers Stop() function. Generally Exit Boot Services events just turn off DMA, or any other hardware that could touch memory that is being freed back for OS usage. Since the timer activity, and thus all event activity is stopped there is not a lot of ways for the drivers to ever have any EFI code run again. 
>> 
>> The only other exception I can think of is if the OS driver makes some kind of assumption about the state of the hardware.
> 
> The
> 
>    hardware that could touch memory that is being freed back for OS
>    usage
> 
> is the part that applies here. Most paravirtual devices work by sharing
> at least some memory between the host-side device model (emulation) and
> guest-side device driver. When the guest is transitioning from firmware
> to OS (and those of course have different guest drivers for the
> paravirtual device), the host must be made forget the memory shared with
> the guest firmware driver (as the guest OS boot loader or the guest OS
> itself might load anything at all into that RAM area after
> ExitBootServices()).
> 
> So what I found unusual in this patch wasn't the necessity of EBS
> notification functions, but how they would be ordered / coordinated
> between each other. There is a bus-like device that shares its own
> resources (RAM) with the host, and installs protocol interfaces. And
> there are dependent endpoint-like devices that consume those protocol
> interfaces, and share their own stuff (RAM) with the host OS.
> 
> All of that shared memory needs to be cleared from the host's
> book-keeping when we leave firmware land; the tricky part is that the
> bus-like device can't request the host for its bus-level shared memory
> to be forgotten before all of the dependent endpoints ask for their
> shared ranges to be forgotten.
> 

Laszlo,

In "real hardware" the bus driver can usually clear the memory usage by by turning off the DMA. I guess this would look like sending a reset to the virtual device. The buffers allocated by children of the "hardware" bus driver free all their buffers back to the OS as a side effect of the ExitBootServices, so it is typical for the children of the bus driver to not have an ExitBootServices event. 

I find it strange there is not a reset operation for the virtual bus device, but it seems like worst case the bus driver could  could track all the shared memory allocations and free the child allocations 1st? 

In general the ExitBootServices routines for hardware can be very simple since EFI is a cooperative event model and the events are shutdown as part of the ExitBootService processes. Most of the times I've seen complex code in ExitBootServices routines it was due to people thinking in terms of threading vs. events, and forgetting that just returning from ExitBootServices frees most of the the resources allocated by the driver. 

I'd also point out the order of the ExitBootServices events are not architectural, but a given implementation may do the same thing every time. For example the ExitBootServices events are probably just an insertion to a doubly linked list based on execution order of the drivers. But if a lot of the drivers are Driver Model drivers then they all have the same Depex so the order could depend on the order of those drivers in the FV. Some times the root bridge driver is a DXE driver for something like PCI, but a USB Host Bridge driver would be a Driver Model driver that Starts on a PCI IO Handle.  

Thanks,

Andrew Fish

> Thanks!
> Laszlo
> 
> 
>>> How about the following instead:
>>> 
>>> - introduce two XenBusIo protocol member functions, AddReference() and
>>> RemoveReference(). RemoveReference() should take a BOOLEAN called
>>> "HandOffToOs". The device drivers should call AddReference() just before
>>> exiting DriverBindingStart() with success, and RemoveReference(FALSE) in
>>> DriverBindingStop().
>>> 
>>> - these protocol member functions would increment / decrement a
>>> reference counter in the underlying XenBus abstraction. Additionally,
>>> RemoveReference() would store the HandOffToOs parameter to a bus-level
>>> BOOLEAN too (regardless of previous value stored there -- a TRUE->FALSE
>>> transition would never happen anyway; see below).
>>> 
>>> - both XenBusDxe and the Xen device drivers should register EBS
>>> callbacks, per controller handle (in BindingStart()), and unregister
>>> them (in BindingStop())
>>> 
>>> - the ordering between EBS notification functions (queued at the same
>>> TPL) is unspecified. In the device driver notification functions, the
>>> last action should be a call to XenBusIo->RemoveReference(TRUE) -- after
>>> the device-specific "forget me" actions have been done.
>>> 
>>> - if RemoveReference() gets a TRUE parameter, then it should check the
>>> resultant (post-decrement) value of the refcount. If it has gone to
>>> zero, RemoveReference() should re-set the xenbus / xenstore connection.
>>> If the parameter is FALSE, it shouldn't do anything particular after
>>> decrementing the refcount.
>>> 
>>> - in the XenBus EBS handler, if the refcount is positive at the time of
>>> the call, nothing should be done. Otherwise, if HandOffToOs is TRUE,
>>> nothing should be done, similarly. Otherwise, the xenbus/xenstore
>>> connection should be re-set.
>>> 
>>> The idea is that normal Start/Stop should manage the refcount as
>>> expected. At ExitBootServices(), the XenBus level handler should only
>>> clear the connection to the hypervisor if no RemoveReference() call has
>>> done, or will do, it. (If the counter is positive, then a later
>>> RemoveReference() call will do it; if it's zero but HandOffToOs is TRUE,
>>> then it's been done already. If the counter is zero and the BOOLEAN is
>>> FALSE, then all devices have been disconnected normally with Stop() --
>>> or none have been connected at all --, before ExitBootServices(), so the
>>> XenBus driver itself has to ask for being forgotten.)
>>> 
>>> Admittedly, this is more complicated (due to the unspecified ordering
>>> between EBS notifications). I just feel it's more idiomatic to go
>>> through normal protocol member functions in EBS notification functions,
>>> rather than special callbacks.
>>> 
>>> (Side comment: the reference counting could normally be replaced by
>>> gBS->OpenProtocolInformation(); however, that service itself allocates
>>> memory, so we can't use it in EBS notification functions.)
>>> 
>>> Thanks
>>> Laszlo
>>> 
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>> Groups.io Links: You receive all messages sent to this group.
>>> 
>>> View/Reply Online (#47292): https://edk2.groups.io/g/devel/message/47292
>>> Mute This Topic: https://groups.io/mt/34128015/1755084
>>> Group Owner: devel+owner@edk2.groups.io
>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [afish@apple.com]
>>> -=-=-=-=-=-=-=-=-=-=-=-

Patch
diff mbox series

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