Re: [PATCH v2 08/13] libxc: Check xc_domain_maximum_gpfn for negative return values

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> > All of them are 
> > 
> >  git://xenbits.xen.org/people/konradwilk/xen.git xc_cleanup.v4

..snip..
> Looks good, only one minor comment:
> [...]
> 
> > -    *gpfn = (xen_pfn_t)rc + 1;
> 
> Perhaps the new parameter to xc_domain_maximum_gpfn should be a
> xen_pfn_t?
> 

Here you go. They are at:

 git://xenbits.xen.org/people/konradwilk/xen.git xc_cleanup.v4.1

>From 45bd7cd377b0b8364757cc2bc0bd8d6a13523a97 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 13 Mar 2015 14:57:44 -0400
Subject: [PATCH] libxc: Check xc_domain_maximum_gpfn for negative return
 values

Instead of assuming everything is always OK. We stash
the gpfns value as an parameter. Since we use it in three
of places we might as well update xc_domain_maximum_gpfn
to do the right thing.

Suggested-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 tools/libxc/include/xenctrl.h |  2 +-
 tools/libxc/xc_core_arm.c     | 11 ++++-------
 tools/libxc/xc_core_x86.c     | 29 +++++++++++------------------
 tools/libxc/xc_domain.c       | 11 +++++++++--
 tools/libxc/xc_domain_save.c  |  6 +++++-
 5 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index df18292..7853fdb 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1315,7 +1315,7 @@ int xc_domain_get_tsc_info(xc_interface *xch,
 
 int xc_domain_disable_migrate(xc_interface *xch, uint32_t domid);
 
-int xc_domain_maximum_gpfn(xc_interface *xch, domid_t domid);
+int xc_domain_maximum_gpfn(xc_interface *xch, domid_t domid, xen_pfn_t *gpfns);
 
 int xc_domain_increase_reservation(xc_interface *xch,
                                    uint32_t domid,
diff --git a/tools/libxc/xc_core_arm.c b/tools/libxc/xc_core_arm.c
index 16508e7..c3f5868 100644
--- a/tools/libxc/xc_core_arm.c
+++ b/tools/libxc/xc_core_arm.c
@@ -30,12 +30,6 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context *arch_ctxt,
     return 0;
 }
 
-
-static int nr_gpfns(xc_interface *xch, domid_t domid)
-{
-    return xc_domain_maximum_gpfn(xch, domid) + 1;
-}
-
 int
 xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
 {
@@ -48,9 +42,12 @@ xc_core_arch_memory_map_get(xc_interface *xch, struct xc_core_arch_context *unus
                             xc_core_memory_map_t **mapp,
                             unsigned int *nr_entries)
 {
-    unsigned long p2m_size = nr_gpfns(xch, info->domid);
+    xen_pfn_t p2m_size = 0;
     xc_core_memory_map_t *map;
 
+    if ( xc_domain_maximum_gpfn(xch, info->domid, &p2m_size) < 0 )
+        return -1;
+
     map = malloc(sizeof(*map));
     if ( map == NULL )
     {
diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
index d8846f1..4552e43 100644
--- a/tools/libxc/xc_core_x86.c
+++ b/tools/libxc/xc_core_x86.c
@@ -35,12 +35,6 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context *arch_ctxt,
     return 1;
 }
 
-
-static int nr_gpfns(xc_interface *xch, domid_t domid)
-{
-    return xc_domain_maximum_gpfn(xch, domid) + 1;
-}
-
 int
 xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
 {
@@ -53,9 +47,12 @@ xc_core_arch_memory_map_get(xc_interface *xch, struct xc_core_arch_context *unus
                             xc_core_memory_map_t **mapp,
                             unsigned int *nr_entries)
 {
-    unsigned long p2m_size = nr_gpfns(xch, info->domid);
+    xen_pfn_t p2m_size = 0;
     xc_core_memory_map_t *map;
 
+    if ( xc_domain_maximum_gpfn(xch, info->domid, &p2m_size) < 0 )
+        return -1;
+
     map = malloc(sizeof(*map));
     if ( map == NULL )
     {
@@ -88,7 +85,12 @@ xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc
     int err;
     int i;
 
-    dinfo->p2m_size = nr_gpfns(xch, info->domid);
+    if ( xc_domain_maximum_gpfn(xch, info->domid, &dinfo->p2m_size) < 0 )
+    {
+        ERROR("Could not get maximum GPFN!");
+        goto out;
+    }
+
     if ( dinfo->p2m_size < info->nr_pages  )
     {
         ERROR("p2m_size < nr_pages -1 (%lx < %lx", dinfo->p2m_size, info->nr_pages - 1);
@@ -210,16 +212,7 @@ int
 xc_core_arch_get_scratch_gpfn(xc_interface *xch, domid_t domid,
                               xen_pfn_t *gpfn)
 {
-    int rc;
-
-    rc = xc_domain_maximum_gpfn(xch, domid);
-
-    if ( rc < 0 )
-        return rc;
-
-    *gpfn = (xen_pfn_t)rc + 1;
-
-    return 0;
+    return xc_domain_maximum_gpfn(xch, domid, gpfn);
 }
 
 /*
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 2fed727..da88e08 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -789,9 +789,16 @@ int xc_domain_get_tsc_info(xc_interface *xch,
 }
 
 
-int xc_domain_maximum_gpfn(xc_interface *xch, domid_t domid)
+int xc_domain_maximum_gpfn(xc_interface *xch, domid_t domid, xen_pfn_t *gpfns)
 {
-    return do_memory_op(xch, XENMEM_maximum_gpfn, &domid, sizeof(domid));
+    int rc = do_memory_op(xch, XENMEM_maximum_gpfn, &domid, sizeof(domid));
+
+    if ( rc >= 0 )
+    {
+        *gpfns = rc + 1;
+        rc = 0;
+    }
+    return rc;
 }
 
 int xc_domain_increase_reservation(xc_interface *xch,
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index 254fdb3..b611c07 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -939,7 +939,11 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
     }
 
     /* Get the size of the P2M table */
-    dinfo->p2m_size = xc_domain_maximum_gpfn(xch, dom) + 1;
+    if ( xc_domain_maximum_gpfn(xch, dom, &dinfo->p2m_size) < 0 )
+    {
+        ERROR("Could not get maximum GPFN!");
+        goto out;
+    }
 
     if ( dinfo->p2m_size > ~XEN_DOMCTL_PFINFO_LTAB_MASK )
     {
-- 
2.1.0

>