[PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Roger Pau Monne]

Hello,

The first two patches in the series fix a race with concurrent device
addition/removal and two bugs related to manipulation of the list of active
domains in the devd subcommand. The last patch is not a bugfix itself, but
it makes the code easier to understand.

IMHO they should be part of 4.9 because they are confined to devd code, and
without them devd is unusable (it's trivial to segfault it), so the risk is
low. Worse thing that could happen is that devd crashes, which is already the
case without them.

Thanks, Roger.


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

[PATCH v3 for-4.9 1/3] libxl/devd: fix a race with concurrent device addition/removal

[Roger Pau Monne]

Current code can free the libxl__device inside of the libxl__ddomain_device
before the addition has finished if a removal happens while an addition is
still in process:

  backend_watch_callback
            |
            v
       add_device
            |                 backend_watch_callback
    (async operation)                   |
            |                           v
            |                     remove_device
            |                           |
            |                           V
            |                    device_complete
            |                 (free libxl__device)
            v
     device_complete
  (deref libxl__device)

Fix this by creating a temporary copy of the libxl__device, that's tracked by
the GC of the nested async operation. This ensures that the libxl__device used
by the async operations cannot be freed while being used.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>

Changes since v2:
 - Add a comment at the top of libxl__device explaining why it can be copied by
   assignment.
---
 tools/libxl/libxl_device.c   | 32 +++++++++++++++++++-------------
 tools/libxl/libxl_internal.h |  4 ++++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 5e966762c6..cd4ad05a6f 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1415,9 +1415,6 @@ static void device_complete(libxl__egc *egc, libxl__ao_device *aodev)
                libxl__device_action_to_string(aodev->action),
                aodev->rc ? "failed" : "succeed");
 
-    if (aodev->action == LIBXL__DEVICE_ACTION_REMOVE)
-        free(aodev->dev);
-
     libxl__nested_ao_free(aodev->ao);
 }
 
@@ -1521,7 +1518,12 @@ static int add_device(libxl__egc *egc, libxl__ao *ao,
 
         GCNEW(aodev);
         libxl__prepare_ao_device(ao, aodev);
-        aodev->dev = dev;
+        /*
+         * Clone the libxl__device to avoid races if remove_device is called
+         * before the device addition has finished.
+         */
+        GCNEW(aodev->dev);
+        *aodev->dev = *dev;
         aodev->action = LIBXL__DEVICE_ACTION_ADD;
         aodev->callback = device_complete;
         libxl__wait_device_connection(egc, aodev);
@@ -1564,7 +1566,12 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
 
         GCNEW(aodev);
         libxl__prepare_ao_device(ao, aodev);
-        aodev->dev = dev;
+        /*
+         * Clone the libxl__device to avoid races if there's a add_device
+         * running in parallel.
+         */
+        GCNEW(aodev->dev);
+        *aodev->dev = *dev;
         aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
         aodev->callback = device_complete;
         libxl__initiate_device_generic_remove(egc, aodev);
@@ -1576,7 +1583,6 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
                 goto out;
         }
         libxl__device_destroy(gc, dev);
-        free(dev);
         /* Fall through to return > 0, no ao has been dispatched */
     default:
         rc = 1;
@@ -1597,7 +1603,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
     char *p, *path;
     const char *sstate, *sonline;
     int state, online, rc, num_devs;
-    libxl__device *dev = NULL;
+    libxl__device *dev;
     libxl__ddomain_device *ddev = NULL;
     libxl__ddomain_guest *dguest = NULL;
     bool free_ao = false;
@@ -1625,7 +1631,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
         goto skip;
     online = atoi(sonline);
 
-    dev = libxl__zalloc(NOGC, sizeof(*dev));
+    GCNEW(dev);
     rc = libxl__parse_backend_path(gc, path, dev);
     if (rc)
         goto skip;
@@ -1659,7 +1665,8 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
          * to the list of active devices for a given guest.
          */
         ddev = libxl__zalloc(NOGC, sizeof(*ddev));
-        ddev->dev = dev;
+        ddev->dev = libxl__zalloc(NOGC, sizeof(*ddev->dev));
+        *ddev->dev = *dev;
         LIBXL_SLIST_INSERT_HEAD(&dguest->devices, ddev, next);
         LOGD(DEBUG, dev->domid, "Added device %s to the list of active devices",
              path);
@@ -1670,9 +1677,6 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
         /*
          * Removal of an active device, remove it from the list and
          * free it's data structures if they are no longer needed.
-         *
-         * The free of the associated libxl__device is left to the
-         * helper remove_device function.
          */
         LIBXL_SLIST_REMOVE(&dguest->devices, ddev, libxl__ddomain_device,
                            next);
@@ -1682,6 +1686,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
         if (rc > 0)
             free_ao = true;
 
+        free(ddev->dev);
         free(ddev);
         /* If this was the last device in the domain, remove it from the list */
         num_devs = dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks;
@@ -1703,7 +1708,8 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
 
 skip:
     libxl__nested_ao_free(nested_ao);
-    free(dev);
+    if (ddev)
+        free(ddev->dev);
     free(ddev);
     free(dguest);
     return;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5d082c5704..afe6652847 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -501,6 +501,10 @@ struct libxl__ctx {
     libxl_version_info version_info;
 };
 
+/*
+ * libxl__device is a transparent structure that doesn't contain private fields
+ * or external memory references, and as such can be copied by assignment.
+ */
 typedef struct {
     uint32_t backend_devid;
     uint32_t backend_domid;
-- 
2.11.0 (Apple Git-81)


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

[PATCH v3 for-4.9 2/3] libxl/devd: correctly manipulate the dguest list

[Roger Pau Monne]

Current code in backend_watch_callback has two issues when manipulating the
dguest list:

1. backend_watch_callback forgets to remove a libxl__ddomain_guest from the
list of tracked domains when the related data is freed, causing dereferences
later on when the list is traversed. Make sure that a domain is always removed
from the list when freed.

2. A spurious device state change can cause a dguest to be freed, with active
devices and without being removed from the list. Fix this by always checking if
a dguest has active devices before freeing and removing it.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Reinis Martinsons <admin@frp.lv>
Suggested-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>

Changes since v2:
 - Introduce check_and_maybe_remove_guest.
 - Add a comment explaining why it's safe to free structures with pending async
   ops.

Changes since v1:
 - Fix commit message
---
 tools/libxl/libxl_device.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index cd4ad05a6f..c82ac3cace 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1493,6 +1493,24 @@ static libxl__ddomain_device *search_for_device(libxl__ddomain_guest *dguest,
     return NULL;
 }
 
+static void check_and_maybe_remove_guest(libxl__gc *gc,
+                                         libxl__ddomain *ddomain,
+                                         libxl__ddomain_guest *dguest)
+{
+    assert(ddomain);
+
+    if (dguest != NULL &&
+        dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks == 0) {
+        LIBXL_SLIST_REMOVE(&ddomain->guests, dguest, libxl__ddomain_guest,
+                           next);
+        LOGD(DEBUG, dguest->domid, "Removed domain from the list of active guests");
+        /* Clear any leftovers in libxl/<domid> */
+        libxl__xs_rm_checked(gc, XBT_NULL,
+                             GCSPRINTF("libxl/%u", dguest->domid));
+        free(dguest);
+    }
+}
+
 /*
  * The following comment applies to both add_device and remove_device.
  *
@@ -1602,7 +1620,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
     STATE_AO_GC(nested_ao);
     char *p, *path;
     const char *sstate, *sonline;
-    int state, online, rc, num_devs;
+    int state, online, rc;
     libxl__device *dev;
     libxl__ddomain_device *ddev = NULL;
     libxl__ddomain_guest *dguest = NULL;
@@ -1677,6 +1695,10 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
         /*
          * Removal of an active device, remove it from the list and
          * free it's data structures if they are no longer needed.
+         *
+         * NB: the freeing is safe because all the async ops launched from
+         * backend_watch_callback make a copy of the data they use, so
+         * there's no risk of dereferencing.
          */
         LIBXL_SLIST_REMOVE(&dguest->devices, ddev, libxl__ddomain_device,
                            next);
@@ -1688,17 +1710,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
 
         free(ddev->dev);
         free(ddev);
-        /* If this was the last device in the domain, remove it from the list */
-        num_devs = dguest->num_vifs + dguest->num_vbds + dguest->num_qdisks;
-        if (num_devs == 0) {
-            LIBXL_SLIST_REMOVE(&ddomain->guests, dguest, libxl__ddomain_guest,
-                               next);
-            LOGD(DEBUG, dguest->domid, "Removed domain from the list of active guests");
-            /* Clear any leftovers in libxl/<domid> */
-            libxl__xs_rm_checked(gc, XBT_NULL,
-                                 GCSPRINTF("libxl/%u", dguest->domid));
-            free(dguest);
-        }
+        check_and_maybe_remove_guest(gc, ddomain, dguest);
     }
 
     if (free_ao)
@@ -1711,7 +1723,7 @@ skip:
     if (ddev)
         free(ddev->dev);
     free(ddev);
-    free(dguest);
+    check_and_maybe_remove_guest(gc, ddomain, dguest);
     return;
 }
 
-- 
2.11.0 (Apple Git-81)


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

[PATCH v3 for-4.9 3/3] libxl/devd: move the device allocation/removal code

[Roger Pau Monne]

Move the device addition/removal code to the {add/remove}_device functions.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
---
 tools/libxl/libxl_device.c | 61 +++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c82ac3cace..00356afea3 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1520,14 +1520,25 @@ static void check_and_maybe_remove_guest(libxl__gc *gc,
  */
 static int add_device(libxl__egc *egc, libxl__ao *ao,
                       libxl__ddomain_guest *dguest,
-                      libxl__ddomain_device *ddev)
+                      libxl__device *dev)
 {
     AO_GC;
-    libxl__device *dev = ddev->dev;
     libxl__ao_device *aodev;
+    libxl__ddomain_device *ddev;
     libxl__dm_spawn_state *dmss;
     int rc = 0;
 
+    /*
+     * New device addition, allocate a struct to hold it and add it
+     * to the list of active devices for a given guest.
+     */
+    ddev = libxl__zalloc(NOGC, sizeof(*ddev));
+    ddev->dev = libxl__zalloc(NOGC, sizeof(*ddev->dev));
+    *ddev->dev = *dev;
+    LIBXL_SLIST_INSERT_HEAD(&dguest->devices, ddev, next);
+    LOGD(DEBUG, dev->domid, "Added device %s to the list of active devices",
+         libxl__device_backend_path(gc, dev));
+
     switch(dev->backend_kind) {
     case LIBXL__DEVICE_KIND_VBD:
     case LIBXL__DEVICE_KIND_VIF:
@@ -1607,6 +1618,22 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
         break;
     }
 
+    /*
+     * Removal of an active device, remove it from the list and
+     * free it's data structures if they are no longer needed.
+     *
+     * NB: the freeing is safe because all the async ops launched
+     * above or from add_device make a copy of the data they use, so
+     * there's no risk of dereferencing.
+     */
+    LIBXL_SLIST_REMOVE(&dguest->devices, ddev, libxl__ddomain_device,
+                       next);
+    LOGD(DEBUG, dev->domid, "Removed device %s from the list of active devices",
+         libxl__device_backend_path(gc, dev));
+
+    free(ddev->dev);
+    free(ddev);
+
 out:
     return rc;
 }
@@ -1678,38 +1705,13 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
          */
         goto skip;
     } else if (ddev == NULL) {
-        /*
-         * New device addition, allocate a struct to hold it and add it
-         * to the list of active devices for a given guest.
-         */
-        ddev = libxl__zalloc(NOGC, sizeof(*ddev));
-        ddev->dev = libxl__zalloc(NOGC, sizeof(*ddev->dev));
-        *ddev->dev = *dev;
-        LIBXL_SLIST_INSERT_HEAD(&dguest->devices, ddev, next);
-        LOGD(DEBUG, dev->domid, "Added device %s to the list of active devices",
-             path);
-        rc = add_device(egc, nested_ao, dguest, ddev);
+        rc = add_device(egc, nested_ao, dguest, dev);
         if (rc > 0)
             free_ao = true;
     } else if (state == XenbusStateClosed && online == 0) {
-        /*
-         * Removal of an active device, remove it from the list and
-         * free it's data structures if they are no longer needed.
-         *
-         * NB: the freeing is safe because all the async ops launched from
-         * backend_watch_callback make a copy of the data they use, so
-         * there's no risk of dereferencing.
-         */
-        LIBXL_SLIST_REMOVE(&dguest->devices, ddev, libxl__ddomain_device,
-                           next);
-        LOGD(DEBUG, dev->domid, "Removed device %s from the list of active devices",
-             path);
         rc = remove_device(egc, nested_ao, dguest, ddev);
         if (rc > 0)
             free_ao = true;
-
-        free(ddev->dev);
-        free(ddev);
         check_and_maybe_remove_guest(gc, ddomain, dguest);
     }
 
@@ -1720,9 +1722,6 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
 
 skip:
     libxl__nested_ao_free(nested_ao);
-    if (ddev)
-        free(ddev->dev);
-    free(ddev);
     check_and_maybe_remove_guest(gc, ddomain, dguest);
     return;
 }
-- 
2.11.0 (Apple Git-81)


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

Re: [PATCH v3 for-4.9 3/3] libxl/devd: move the device allocation/removal code

[Wei Liu]

On Tue, May 16, 2017 at 08:59:25AM +0100, Roger Pau Monne wrote:
> Move the device addition/removal code to the {add/remove}_device functions.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Julien Grall]

On 16/05/17 08:59, Roger Pau Monne wrote:
> Hello,

Hi Roger,

>
> The first two patches in the series fix a race with concurrent device
> addition/removal and two bugs related to manipulation of the list of active
> domains in the devd subcommand. The last patch is not a bugfix itself, but
> it makes the code easier to understand.
>
> IMHO they should be part of 4.9 because they are confined to devd code, and
> without them devd is unusable (it's trivial to segfault it), so the risk is
> low. Worse thing that could happen is that devd crashes, which is already the
> case without them.

For the first 2 patches:

Release-acked-by: Julien Grall <julien.grall@arm.com>

For the last patch, at this stage of the release I would prefer to defer 
it for Xen 4.10.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Julien Grall]

(CC Ian and Wei)

On 17/05/17 15:02, Julien Grall wrote:
> On 16/05/17 08:59, Roger Pau Monne wrote:
>> Hello,
>
> Hi Roger,
>
>>
>> The first two patches in the series fix a race with concurrent device
>> addition/removal and two bugs related to manipulation of the list of
>> active
>> domains in the devd subcommand. The last patch is not a bugfix itself,
>> but
>> it makes the code easier to understand.
>>
>> IMHO they should be part of 4.9 because they are confined to devd
>> code, and
>> without them devd is unusable (it's trivial to segfault it), so the
>> risk is
>> low. Worse thing that could happen is that devd crashes, which is
>> already the
>> case without them.
>
> For the first 2 patches:
>
> Release-acked-by: Julien Grall <julien.grall@arm.com>
>
> For the last patch, at this stage of the release I would prefer to defer
> it for Xen 4.10.
>
> Cheers,
>

-- 
Julien Grall

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

Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Wei Liu]

On Thu, May 18, 2017 at 04:06:33PM +0100, Julien Grall wrote:
> (CC Ian and Wei)
> 

I did see this. Ian said he wanted to review this in detail. That's why
I haven't committed them.

> On 17/05/17 15:02, Julien Grall wrote:
> > On 16/05/17 08:59, Roger Pau Monne wrote:
> > > Hello,
> > 
> > Hi Roger,
> > 
> > > 
> > > The first two patches in the series fix a race with concurrent device
> > > addition/removal and two bugs related to manipulation of the list of
> > > active
> > > domains in the devd subcommand. The last patch is not a bugfix itself,
> > > but
> > > it makes the code easier to understand.
> > > 
> > > IMHO they should be part of 4.9 because they are confined to devd
> > > code, and
> > > without them devd is unusable (it's trivial to segfault it), so the
> > > risk is
> > > low. Worse thing that could happen is that devd crashes, which is
> > > already the
> > > case without them.
> > 
> > For the first 2 patches:
> > 
> > Release-acked-by: Julien Grall <julien.grall@arm.com>
> > 
> > For the last patch, at this stage of the release I would prefer to defer
> > it for Xen 4.10.
> > 
> > Cheers,
> > 
> 
> -- 
> Julien Grall
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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

Re: [PATCH v3 for-4.9 2/3] libxl/devd: correctly manipulate the dguest list

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 for-4.9 2/3] libxl/devd: correctly manipulate the dguest list"):
> Current code in backend_watch_callback has two issues when manipulating the
> dguest list:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v3 for-4.9 1/3] libxl/devd: fix a race with concurrent device addition/removal

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 for-4.9 1/3] libxl/devd: fix a race with concurrent device addition/removal"):
> Current code can free the libxl__device inside of the libxl__ddomain_device
> before the addition has finished if a removal happens while an addition is
> still in process:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v3 for-4.9 3/3] libxl/devd: move the device allocation/removal code

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 for-4.9 3/3] libxl/devd: move the device allocation/removal code"):
> Move the device addition/removal code to the {add/remove}_device functions.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Ian Jackson]

Julien Grall writes ("Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes"):
> On 17/05/17 15:02, Julien Grall wrote:
> > For the last patch, at this stage of the release I would prefer to defer
> > it for Xen 4.10.

After reviewing these, I'd like to make a case for the third patch for
4.9:

I haven't quite managed to prove to myself that the 3rd patch is a
no-op.  But this is because I haven't quite proved to myself that the
code _before_ the 3rd patch is correct.

The code _after_ the 3rd patch seems more obviously correct to me.  Ie
I think the risk of bugs is lower with the 3rd patch than without
(even after the first two patches).

Ian.

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

Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes

[Julien Grall]

Hi Ian,

On 18/05/17 19:11, Ian Jackson wrote:
> Julien Grall writes ("Re: [PATCH v3 for-4.9 0/3] libxl/devd: bugfixes"):
>> On 17/05/17 15:02, Julien Grall wrote:
>>> For the last patch, at this stage of the release I would prefer to defer
>>> it for Xen 4.10.
>
> After reviewing these, I'd like to make a case for the third patch for
> 4.9:
>
> I haven't quite managed to prove to myself that the 3rd patch is a
> no-op.  But this is because I haven't quite proved to myself that the
> code _before_ the 3rd patch is correct.
>
> The code _after_ the 3rd patch seems more obviously correct to me.  Ie
> I think the risk of bugs is lower with the 3rd patch than without
> (even after the first two patches).

Thank you for the explanation. It was not obvious from Roger's e-mail 
that it may fix something.

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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