[PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Andrew Cooper]

A git tree version is available:

http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/dombuilder-gnt-v2

Changes in v2: Mainly a rebase over c/s 5b42c82f "tools/libxc: Fix domid
parameter types", and fixup from review comments.  See individual patches for
details

Andrew Cooper (5):
  tools/dombuilder: Drop more PVH v1 leftovers
  tools/dombuilder: Remove clear_page() from xc_dom_boot.c
  tools/dombuilder: Switch to using gfn terminology for console and
    xenstore rings
  tools/dombuilder: Fix asymmetry when setting up console and xenstore
    rings
  tools/dombuilder: Prevent failures of xc_dom_gnttab_init()

 tools/libxc/include/xc_dom.h      |  26 ++++++--
 tools/libxc/xc_dom_arm.c          |  17 ++---
 tools/libxc/xc_dom_boot.c         | 126 +++++++++++++++++++++++---------------
 tools/libxc/xc_dom_compat_linux.c |   6 +-
 tools/libxc/xc_dom_core.c         |   8 +++
 tools/libxc/xc_dom_x86.c          |  57 +++++++++--------
 tools/libxl/libxl_dom.c           |  51 ++++++---------
 tools/libxl/libxl_internal.h      |   1 -
 8 files changed, 169 insertions(+), 123 deletions(-)

-- 
2.1.4


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

[PATCH for-4.10 v2 1/5] tools/dombuilder: Drop more PVH v1 leftovers

[Andrew Cooper]

alloc_magic_pages() is renamed to alloc_magic_pages_pv() to mirror its
alloc_magic_pages_hvm() counterpart.  Delete a redundant comment, introduce
some newlines clarity, and remove a logically dead allocation of shared info.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Julien Grall <julien.grall@arm.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@arm.com>
---
 tools/libxc/xc_dom_x86.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index bac584f..47db218 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -534,24 +534,20 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom)
 
 /* ------------------------------------------------------------------------ */
 
-static int alloc_magic_pages(struct xc_dom_image *dom)
+static int alloc_magic_pages_pv(struct xc_dom_image *dom)
 {
-    /* allocate special pages */
     dom->start_info_pfn = xc_dom_alloc_page(dom, "start info");
     if ( dom->start_info_pfn == INVALID_PFN )
         return -1;
+
     dom->xenstore_pfn = xc_dom_alloc_page(dom, "xenstore");
     if ( dom->xenstore_pfn == INVALID_PFN )
         return -1;
+
     dom->console_pfn = xc_dom_alloc_page(dom, "console");
     if ( dom->console_pfn == INVALID_PFN )
         return -1;
-    if ( xc_dom_translated(dom) )
-    {
-        dom->shared_info_pfn = xc_dom_alloc_page(dom, "shared info");
-        if ( dom->shared_info_pfn == INVALID_PFN )
-            return -1;
-    }
+
     dom->alloc_bootstack = 1;
 
     return 0;
@@ -1756,7 +1752,7 @@ static struct xc_dom_arch xc_dom_32_pae = {
     .sizeof_pfn = 4,
     .p2m_base_supported = 0,
     .arch_private_size = sizeof(struct xc_dom_image_x86),
-    .alloc_magic_pages = alloc_magic_pages,
+    .alloc_magic_pages = alloc_magic_pages_pv,
     .alloc_pgtables = alloc_pgtables_x86_32_pae,
     .alloc_p2m_list = alloc_p2m_list_x86_32,
     .setup_pgtables = setup_pgtables_x86_32_pae,
@@ -1775,7 +1771,7 @@ static struct xc_dom_arch xc_dom_64 = {
     .sizeof_pfn = 8,
     .p2m_base_supported = 1,
     .arch_private_size = sizeof(struct xc_dom_image_x86),
-    .alloc_magic_pages = alloc_magic_pages,
+    .alloc_magic_pages = alloc_magic_pages_pv,
     .alloc_pgtables = alloc_pgtables_x86_64,
     .alloc_p2m_list = alloc_p2m_list_x86_64,
     .setup_pgtables = setup_pgtables_x86_64,
-- 
2.1.4


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

[PATCH for-4.10 v2 2/5] tools/dombuilder: Remove clear_page() from xc_dom_boot.c

[Andrew Cooper]

pfn 0 is a legitimate (albeit unlikely) frame to use, so skipping it is wrong.
This behaviour appears to exists simply to cover the fact that zero is the
default value of an uninitialised field in dom.

ARM already clears the frames at the point that the pfns are allocated,
meaning that the added clear_page() is wasteful.  Alter x86 to match ARM and
clear the page when it is allocated.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Julien Grall <julien.grall@arm.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@arm.com>
---
 tools/libxc/xc_dom_arm.c  |  3 ++-
 tools/libxc/xc_dom_boot.c | 26 --------------------------
 tools/libxc/xc_dom_x86.c  |  8 ++++++++
 3 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 7c4997a..fce151d 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -91,7 +91,8 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
     xc_clear_domain_page(dom->xch, dom->guest_domid, base + MEMACCESS_PFN_OFFSET);
-    xc_clear_domain_page(dom->xch, dom->guest_domid, base + VUART_PFN_OFFSET);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->vuart_gfn);
+
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN,
             dom->console_pfn);
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN,
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 40eb518..2e5681d 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -62,25 +62,6 @@ static int setup_hypercall_page(struct xc_dom_image *dom)
     return rc;
 }
 
-static int clear_page(struct xc_dom_image *dom, xen_pfn_t pfn)
-{
-    xen_pfn_t dst;
-    int rc;
-
-    if ( pfn == 0 )
-        return 0;
-
-    dst = xc_dom_p2m(dom, pfn);
-    DOMPRINTF("%s: pfn 0x%" PRIpfn ", mfn 0x%" PRIpfn "",
-              __FUNCTION__, pfn, dst);
-    rc = xc_clear_domain_page(dom->xch, dom->guest_domid, dst);
-    if ( rc != 0 )
-        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
-                     "%s: xc_clear_domain_page failed (pfn 0x%" PRIpfn
-                     ", rc=%d)", __FUNCTION__, pfn, rc);
-    return rc;
-}
-
 
 /* ------------------------------------------------------------------------ */
 
@@ -222,13 +203,6 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
         if ( (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
             return rc;
 
-    if ( (rc = clear_page(dom, dom->console_pfn)) != 0 )
-        return rc;
-    if ( (rc = clear_page(dom, dom->xenstore_pfn)) != 0 )
-        return rc;
-    if ( (rc = clear_page(dom, dom->vuart_gfn)) != 0 )
-        return rc;
-
     /* start info page */
     if ( dom->arch_hooks->start_info )
         dom->arch_hooks->start_info(dom);
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 47db218..bff68a0 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -543,10 +543,14 @@ static int alloc_magic_pages_pv(struct xc_dom_image *dom)
     dom->xenstore_pfn = xc_dom_alloc_page(dom, "xenstore");
     if ( dom->xenstore_pfn == INVALID_PFN )
         return -1;
+    xc_clear_domain_page(dom->xch, dom->guest_domid,
+                         xc_dom_p2m(dom, dom->xenstore_pfn));
 
     dom->console_pfn = xc_dom_alloc_page(dom, "console");
     if ( dom->console_pfn == INVALID_PFN )
         return -1;
+    xc_clear_domain_page(dom->xch, dom->guest_domid,
+                         xc_dom_p2m(dom, dom->console_pfn));
 
     dom->alloc_bootstack = 1;
 
@@ -696,7 +700,11 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
                      special_pfn(SPECIALPAGE_IDENT_PT) << PAGE_SHIFT);
 
     dom->console_pfn = special_pfn(SPECIALPAGE_CONSOLE);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
+
     dom->xenstore_pfn = special_pfn(SPECIALPAGE_XENSTORE);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
+
     dom->parms.virt_hypercall = -1;
 
     rc = 0;
-- 
2.1.4


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

[PATCH for-4.10 v2 3/5] tools/dombuilder: Switch to using gfn terminology for console and xenstore rings

[Andrew Cooper]

The sole use of xc_dom_translated() and xc_dom_p2m() outside of the domain
builder is for libxl_dom() to translate the console and xenstore pfns back
into useful values.  PV guest pfns are only interesting to the domain builder,
and gfns are the address space used by all other hypercalls.

Renaming the fields in xc_dom_image is deliberate, as it will cause
out-of-tree users of the dombuilder to notice the different semantics.

Correct the terminology throughout xc_dom_gnttab{_hvm,}_seed(), which are all
using gfns despite the existing variable names.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Julien Grall <julien.grall@arm.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@arm.com>

v2:
 * More style fixes
---
 tools/libxc/include/xc_dom.h      | 10 +++++++--
 tools/libxc/xc_dom_arm.c          | 12 +++++------
 tools/libxc/xc_dom_boot.c         | 45 ++++++++++++++++++---------------------
 tools/libxc/xc_dom_compat_linux.c |  4 ++--
 tools/libxc/xc_dom_x86.c          | 45 ++++++++++++++++++++-------------------
 tools/libxl/libxl_dom.c           | 11 +++-------
 6 files changed, 63 insertions(+), 64 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index cdcdd07..5907559 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -94,8 +94,6 @@ struct xc_dom_image {
     struct xc_dom_seg devicetree_seg;
     struct xc_dom_seg start_info_seg; /* HVMlite only */
     xen_pfn_t start_info_pfn;
-    xen_pfn_t console_pfn;
-    xen_pfn_t xenstore_pfn;
     xen_pfn_t shared_info_pfn;
     xen_pfn_t bootstack_pfn;
     xen_pfn_t pfn_alloc_end;
@@ -103,6 +101,14 @@ struct xc_dom_image {
     xen_vaddr_t bsd_symtab_start;
 
     /*
+     * Details for the toolstack-prepared rings.
+     *
+     * *_gfn fields are allocated by the domain builder.
+     */
+    xen_pfn_t console_gfn;
+    xen_pfn_t xenstore_gfn;
+
+    /*
      * initrd parameters as specified in start_info page
      * Depending on capabilities of the booted kernel this may be a virtual
      * address or a pfn. Type is neutral and large enough to hold a virtual
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index fce151d..2fe75cd 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -84,19 +84,19 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
     if ( rc < 0 )
         return rc;
 
-    dom->console_pfn = base + CONSOLE_PFN_OFFSET;
-    dom->xenstore_pfn = base + XENSTORE_PFN_OFFSET;
+    dom->console_gfn = base + CONSOLE_PFN_OFFSET;
+    dom->xenstore_gfn = base + XENSTORE_PFN_OFFSET;
     dom->vuart_gfn = base + VUART_PFN_OFFSET;
 
-    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
-    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_gfn);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_gfn);
     xc_clear_domain_page(dom->xch, dom->guest_domid, base + MEMACCESS_PFN_OFFSET);
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->vuart_gfn);
 
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN,
-            dom->console_pfn);
+            dom->console_gfn);
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN,
-            dom->xenstore_pfn);
+            dom->xenstore_gfn);
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_MONITOR_RING_PFN,
             base + MEMACCESS_PFN_OFFSET);
     /* allocated by toolstack */
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 2e5681d..bbf98b6 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -257,24 +257,23 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, uint32_t domid)
 }
 
 int xc_dom_gnttab_seed(xc_interface *xch, uint32_t domid,
-                       xen_pfn_t console_gmfn,
-                       xen_pfn_t xenstore_gmfn,
+                       xen_pfn_t console_gfn,
+                       xen_pfn_t xenstore_gfn,
                        uint32_t console_domid,
                        uint32_t xenstore_domid)
 {
-
-    xen_pfn_t gnttab_gmfn;
+    xen_pfn_t gnttab_gfn;
     grant_entry_v1_t *gnttab;
 
-    gnttab_gmfn = xc_dom_gnttab_setup(xch, domid);
-    if ( gnttab_gmfn == -1 )
+    gnttab_gfn = xc_dom_gnttab_setup(xch, domid);
+    if ( gnttab_gfn == -1 )
         return -1;
 
     gnttab = xc_map_foreign_range(xch,
                                   domid,
                                   PAGE_SIZE,
                                   PROT_READ|PROT_WRITE,
-                                  gnttab_gmfn);
+                                  gnttab_gfn);
     if ( gnttab == NULL )
     {
         xc_dom_panic(xch, XC_INTERNAL_ERROR,
@@ -284,17 +283,17 @@ int xc_dom_gnttab_seed(xc_interface *xch, uint32_t domid,
         return -1;
     }
 
-    if ( domid != console_domid  && console_gmfn != -1)
+    if ( domid != console_domid && console_gfn != -1 )
     {
         gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
         gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
-        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
+        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gfn;
     }
-    if ( domid != xenstore_domid && xenstore_gmfn != -1)
+    if ( domid != xenstore_domid && xenstore_gfn != -1 )
     {
         gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
         gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
-        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
+        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gfn;
     }
 
     if ( munmap(gnttab, PAGE_SIZE) == -1 )
@@ -308,19 +307,19 @@ int xc_dom_gnttab_seed(xc_interface *xch, uint32_t domid,
 
     /* Guest shouldn't really touch its grant table until it has
      * enabled its caches. But lets be nice. */
-    xc_domain_cacheflush(xch, domid, gnttab_gmfn, 1);
+    xc_domain_cacheflush(xch, domid, gnttab_gfn, 1);
 
     return 0;
 }
 
 int xc_dom_gnttab_hvm_seed(xc_interface *xch, uint32_t domid,
-                           xen_pfn_t console_gpfn,
-                           xen_pfn_t xenstore_gpfn,
+                           xen_pfn_t console_gfn,
+                           xen_pfn_t xenstore_gfn,
                            uint32_t console_domid,
                            uint32_t xenstore_domid)
 {
     int rc;
-    xen_pfn_t scratch_gpfn;
+    xen_pfn_t scratch_gfn;
     struct xen_add_to_physmap xatp = {
         .domid = domid,
         .space = XENMAPSPACE_grant_table,
@@ -330,7 +329,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, uint32_t domid,
         .domid = domid,
     };
 
-    rc = xc_core_arch_get_scratch_gpfn(xch, domid, &scratch_gpfn);
+    rc = xc_core_arch_get_scratch_gpfn(xch, domid, &scratch_gfn);
     if ( rc < 0 )
     {
         xc_dom_panic(xch, XC_INTERNAL_ERROR,
@@ -339,11 +338,10 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, uint32_t domid,
                      __FUNCTION__, errno);
         return -1;
     }
-    xatp.gpfn = scratch_gpfn;
-    xrfp.gpfn = scratch_gpfn;
+    xatp.gpfn = xrfp.gpfn = scratch_gfn;
 
-    xc_dom_printf(xch, "%s: called, pfn=0x%"PRI_xen_pfn, __FUNCTION__,
-                  scratch_gpfn);
+    xc_dom_printf(xch, "%s: called, scratch gfn=0x%"PRI_xen_pfn, __FUNCTION__,
+                  scratch_gfn);
 
 
     rc = do_memory_op(xch, XENMEM_add_to_physmap, &xatp, sizeof(xatp));
@@ -357,7 +355,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, uint32_t domid,
     }
 
     rc = xc_dom_gnttab_seed(xch, domid,
-                            console_gpfn, xenstore_gpfn,
+                            console_gfn, xenstore_gfn,
                             console_domid, xenstore_domid);
     if (rc != 0)
     {
@@ -385,12 +383,11 @@ int xc_dom_gnttab_init(struct xc_dom_image *dom)
 {
     if ( xc_dom_translated(dom) ) {
         return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
-                                      dom->console_pfn, dom->xenstore_pfn,
+                                      dom->console_gfn, dom->xenstore_gfn,
                                       dom->console_domid, dom->xenstore_domid);
     } else {
         return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
-                                  xc_dom_p2m(dom, dom->console_pfn),
-                                  xc_dom_p2m(dom, dom->xenstore_pfn),
+                                  dom->console_gfn, dom->xenstore_gfn,
                                   dom->console_domid, dom->xenstore_domid);
     }
 }
diff --git a/tools/libxc/xc_dom_compat_linux.c b/tools/libxc/xc_dom_compat_linux.c
index c922c61..6d27ec2 100644
--- a/tools/libxc/xc_dom_compat_linux.c
+++ b/tools/libxc/xc_dom_compat_linux.c
@@ -78,8 +78,8 @@ int xc_linux_build(xc_interface *xch, uint32_t domid,
     if ( (rc = xc_dom_gnttab_init(dom)) != 0)
         goto out;
 
-    *console_mfn = xc_dom_p2m(dom, dom->console_pfn);
-    *store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
+    *console_mfn = dom->console_gfn;
+    *store_mfn = dom->xenstore_gfn;
 
  out:
     xc_dom_release(dom);
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index bff68a0..c74fb96 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -536,21 +536,23 @@ static int alloc_p2m_list_x86_64(struct xc_dom_image *dom)
 
 static int alloc_magic_pages_pv(struct xc_dom_image *dom)
 {
+    xen_pfn_t pfn;
+
     dom->start_info_pfn = xc_dom_alloc_page(dom, "start info");
     if ( dom->start_info_pfn == INVALID_PFN )
         return -1;
 
-    dom->xenstore_pfn = xc_dom_alloc_page(dom, "xenstore");
-    if ( dom->xenstore_pfn == INVALID_PFN )
+    pfn = xc_dom_alloc_page(dom, "xenstore");
+    if ( pfn == INVALID_PFN )
         return -1;
-    xc_clear_domain_page(dom->xch, dom->guest_domid,
-                         xc_dom_p2m(dom, dom->xenstore_pfn));
+    dom->xenstore_gfn = xc_dom_p2m(dom, pfn);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_gfn);
 
-    dom->console_pfn = xc_dom_alloc_page(dom, "console");
-    if ( dom->console_pfn == INVALID_PFN )
+    pfn = xc_dom_alloc_page(dom, "console");
+    if ( pfn == INVALID_PFN )
         return -1;
-    xc_clear_domain_page(dom->xch, dom->guest_domid,
-                         xc_dom_p2m(dom, dom->console_pfn));
+    dom->console_gfn = xc_dom_p2m(dom, pfn);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_gfn);
 
     dom->alloc_bootstack = 1;
 
@@ -612,14 +614,19 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
                                X86_HVM_NR_SPECIAL_PAGES) )
             goto error_out;
 
-    xc_hvm_param_set(xch, domid, HVM_PARAM_STORE_PFN,
-                     special_pfn(SPECIALPAGE_XENSTORE));
+    dom->xenstore_gfn = special_pfn(SPECIALPAGE_XENSTORE);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_gfn);
+    xc_hvm_param_set(xch, domid, HVM_PARAM_STORE_PFN, dom->xenstore_gfn);
+
     xc_hvm_param_set(xch, domid, HVM_PARAM_BUFIOREQ_PFN,
                      special_pfn(SPECIALPAGE_BUFIOREQ));
     xc_hvm_param_set(xch, domid, HVM_PARAM_IOREQ_PFN,
                      special_pfn(SPECIALPAGE_IOREQ));
-    xc_hvm_param_set(xch, domid, HVM_PARAM_CONSOLE_PFN,
-                     special_pfn(SPECIALPAGE_CONSOLE));
+
+    dom->console_gfn = special_pfn(SPECIALPAGE_CONSOLE);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_gfn);
+    xc_hvm_param_set(xch, domid, HVM_PARAM_CONSOLE_PFN, dom->console_gfn);
+
     xc_hvm_param_set(xch, domid, HVM_PARAM_PAGING_RING_PFN,
                      special_pfn(SPECIALPAGE_PAGING));
     xc_hvm_param_set(xch, domid, HVM_PARAM_MONITOR_RING_PFN,
@@ -699,12 +706,6 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     xc_hvm_param_set(xch, domid, HVM_PARAM_IDENT_PT,
                      special_pfn(SPECIALPAGE_IDENT_PT) << PAGE_SHIFT);
 
-    dom->console_pfn = special_pfn(SPECIALPAGE_CONSOLE);
-    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
-
-    dom->xenstore_pfn = special_pfn(SPECIALPAGE_XENSTORE);
-    xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
-
     dom->parms.virt_hypercall = -1;
 
     rc = 0;
@@ -744,9 +745,9 @@ static int start_info_x86_32(struct xc_dom_image *dom)
     start_info->mfn_list = dom->p2m_seg.vstart;
 
     start_info->flags = dom->flags;
-    start_info->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
+    start_info->store_mfn = dom->xenstore_gfn;
     start_info->store_evtchn = dom->xenstore_evtchn;
-    start_info->console.domU.mfn = xc_dom_p2m(dom, dom->console_pfn);
+    start_info->console.domU.mfn = dom->console_gfn;
     start_info->console.domU.evtchn = dom->console_evtchn;
 
     if ( dom->ramdisk_blob )
@@ -795,9 +796,9 @@ static int start_info_x86_64(struct xc_dom_image *dom)
     }
 
     start_info->flags = dom->flags;
-    start_info->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
+    start_info->store_mfn = dom->xenstore_gfn;
     start_info->store_evtchn = dom->xenstore_evtchn;
-    start_info->console.domU.mfn = xc_dom_p2m(dom, dom->console_pfn);
+    start_info->console.domU.mfn = dom->console_gfn;
     start_info->console.domU.evtchn = dom->console_evtchn;
 
     if ( dom->ramdisk_blob )
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ef834e6..0389a06 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -851,14 +851,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
     if (ret != 0)
         goto out;
 
-    if (xc_dom_translated(dom)) {
-        state->console_mfn = dom->console_pfn;
-        state->store_mfn = dom->xenstore_pfn;
-        state->vuart_gfn = dom->vuart_gfn;
-    } else {
-        state->console_mfn = xc_dom_p2m(dom, dom->console_pfn);
-        state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn);
-    }
+    state->console_mfn = dom->console_gfn;
+    state->store_mfn = dom->xenstore_gfn;
+    state->vuart_gfn = dom->vuart_gfn;
 
     ret = 0;
 out:
-- 
2.1.4


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

[PATCH for-4.10 v2 4/5] tools/dombuilder: Fix asymmetry when setting up console and xenstore rings

[Andrew Cooper]

libxl always uses xc_dom_gnttab_init(), which internally calls
xc_dom_gnttab{_hvm,}_seed() to set up the grants point at the console and
xenstore rings.  For HVM guests, libxl then asks Xen for the information set
up previously, and calls xc_dom_gnttab_hvm_seed() a second time, which is
wasteful.  ARM construction expects libxl to have set up
dom->{console,xenstore}_evtchn earlier, so only actually functions because of
this second call.

Rationalise everything and make it consistent for all guests.

 1) Users of the domain builder are expected to provide
    dom->{console,xenstore}_{evtchn,domid} unconditionally.  This is checked
    by setting invalid values in xc_dom_allocate(), and checking in
    xc_dom_boot_image().

 2) For x86 HVM and ARM guests, the event channels are given to Xen at the
    same time as the ring gfns.  ARM already did this, but x86 is updated to
    match.  x86 PV already provides this information in the start_info page.

 3) Libxl is updated to drop all relevant functionality from
    hvm_build_set_params(), and behave consistently with PV guests when it
    comes to the handling of dom->{console,xenstore}_{evtchn,domid,gfn}.

This removes several redundant hypercalls (including a foreign mapping) from
the x86 HVM and ARM construction paths.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Julien Grall <julien.grall@arm.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@arm.com>

v2:
 * use ~0 for INVALID_DOMID to avoid
   [-Werror,-Wtautological-constant-out-of-range-compare] from Clang
 * Set errno to EINVAL in xc_dom_check_required_fields()
---
 tools/libxc/include/xc_dom.h      | 12 ++++++++----
 tools/libxc/xc_dom_arm.c          |  2 +-
 tools/libxc/xc_dom_boot.c         | 39 +++++++++++++++++++++++++++++++++++++++
 tools/libxc/xc_dom_compat_linux.c |  2 ++
 tools/libxc/xc_dom_core.c         |  5 +++++
 tools/libxc/xc_dom_x86.c          |  4 ++++
 tools/libxl/libxl_dom.c           | 28 ++++++++++------------------
 tools/libxl/libxl_internal.h      |  1 -
 8 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 5907559..a1c3de2 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -20,6 +20,8 @@
 #include <xenguest.h>
 
 #define INVALID_PFN ((xen_pfn_t)-1)
+#define INVALID_EVTCHN (~0u)
+#define INVALID_DOMID  (~0)
 #define X86_HVM_NR_SPECIAL_PAGES    8
 #define X86_HVM_END_SPECIAL_REGION  0xff000u
 
@@ -104,10 +106,16 @@ struct xc_dom_image {
      * Details for the toolstack-prepared rings.
      *
      * *_gfn fields are allocated by the domain builder.
+     * *_{evtchn,domid} fields must be provided by the caller.
      */
     xen_pfn_t console_gfn;
     xen_pfn_t xenstore_gfn;
 
+    unsigned int console_evtchn;
+    unsigned int xenstore_evtchn;
+    uint32_t console_domid;
+    uint32_t xenstore_domid;
+
     /*
      * initrd parameters as specified in start_info page
      * Depending on capabilities of the booted kernel this may be a virtual
@@ -165,10 +173,6 @@ struct xc_dom_image {
 
     /* misc xen domain config stuff */
     unsigned long flags;
-    unsigned int console_evtchn;
-    unsigned int xenstore_evtchn;
-    uint32_t console_domid;
-    uint32_t xenstore_domid;
     xen_pfn_t shared_info_mfn;
 
     xc_interface *xch;
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 2fe75cd..2134ce4 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -99,7 +99,7 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
             dom->xenstore_gfn);
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_MONITOR_RING_PFN,
             base + MEMACCESS_PFN_OFFSET);
-    /* allocated by toolstack */
+
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_EVTCHN,
             dom->console_evtchn);
     xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_STORE_EVTCHN,
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index bbf98b6..75836bd 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -163,6 +163,42 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
     return ptr;
 }
 
+static int xc_dom_check_required_fields(struct xc_dom_image *dom)
+{
+    int rc = 0;
+
+    if ( dom->console_evtchn == INVALID_EVTCHN )
+    {
+        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
+                     "%s: Caller didn't set dom->console_evtchn", __func__);
+        rc = -1;
+    }
+    if ( dom->console_domid == INVALID_DOMID )
+    {
+        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
+                     "%s: Caller didn't set dom->console_domid", __func__);
+        rc = -1;
+    }
+
+    if ( dom->xenstore_evtchn == INVALID_EVTCHN )
+    {
+        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
+                     "%s: Caller didn't set dom->xenstore_evtchn", __func__);
+        rc = -1;
+    }
+    if ( dom->xenstore_domid == INVALID_DOMID )
+    {
+        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
+                     "%s: Caller didn't set dom->xenstore_domid", __func__);
+        rc = -1;
+    }
+
+    if ( rc )
+        errno = EINVAL;
+
+    return rc;
+}
+
 int xc_dom_boot_image(struct xc_dom_image *dom)
 {
     xc_dominfo_t info;
@@ -170,6 +206,9 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
 
     DOMPRINTF_CALLED(dom->xch);
 
+    if ( (rc = xc_dom_check_required_fields(dom)) != 0 )
+        return rc;
+
     /* misc stuff*/
     if ( (rc = dom->arch_hooks->bootearly(dom)) != 0 )
         return rc;
diff --git a/tools/libxc/xc_dom_compat_linux.c b/tools/libxc/xc_dom_compat_linux.c
index 6d27ec2..2ad43e4 100644
--- a/tools/libxc/xc_dom_compat_linux.c
+++ b/tools/libxc/xc_dom_compat_linux.c
@@ -61,7 +61,9 @@ int xc_linux_build(xc_interface *xch, uint32_t domid,
 
     dom->flags |= flags;
     dom->console_evtchn = console_evtchn;
+    dom->console_domid = 0;
     dom->xenstore_evtchn = store_evtchn;
+    dom->xenstore_domid = 0;
 
     if ( (rc = xc_dom_boot_xen_init(dom, xch, domid)) != 0 )
         goto out;
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index b5f316a..7087c50 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -779,6 +779,11 @@ struct xc_dom_image *xc_dom_allocate(xc_interface *xch,
     dom->parms.elf_paddr_offset = UNSET_ADDR;
     dom->parms.p2m_base = UNSET_ADDR;
 
+    dom->console_evtchn = INVALID_EVTCHN;
+    dom->xenstore_evtchn = INVALID_EVTCHN;
+    dom->console_domid = INVALID_DOMID;
+    dom->xenstore_domid = INVALID_DOMID;
+
     dom->flags = SIF_VIRT_P2M_4TOOLS;
 
     dom->alloc_malloc += sizeof(*dom);
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index c74fb96..e26857d 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -617,6 +617,8 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     dom->xenstore_gfn = special_pfn(SPECIALPAGE_XENSTORE);
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_gfn);
     xc_hvm_param_set(xch, domid, HVM_PARAM_STORE_PFN, dom->xenstore_gfn);
+    xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_STORE_EVTCHN,
+                     dom->xenstore_evtchn);
 
     xc_hvm_param_set(xch, domid, HVM_PARAM_BUFIOREQ_PFN,
                      special_pfn(SPECIALPAGE_BUFIOREQ));
@@ -626,6 +628,8 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     dom->console_gfn = special_pfn(SPECIALPAGE_CONSOLE);
     xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_gfn);
     xc_hvm_param_set(xch, domid, HVM_PARAM_CONSOLE_PFN, dom->console_gfn);
+    xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_EVTCHN,
+                     dom->console_evtchn);
 
     xc_hvm_param_set(xch, domid, HVM_PARAM_PAGING_RING_PFN,
                      special_pfn(SPECIALPAGE_PAGING));
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0389a06..fcdeef0 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -862,14 +862,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
 }
 
 static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
-                                libxl_domain_build_info *info,
-                                int store_evtchn, unsigned long *store_mfn,
-                                int console_evtchn, unsigned long *console_mfn,
-                                domid_t store_domid, domid_t console_domid)
+                                libxl_domain_build_info *info)
 {
     struct hvm_info_table *va_hvm;
     uint8_t *va_map, sum;
-    uint64_t str_mfn, cons_mfn;
     int i;
 
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
@@ -890,15 +886,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
         munmap(va_map, XC_PAGE_SIZE);
     }
 
-    xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);
-    xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);
-    xc_hvm_param_set(handle, domid, HVM_PARAM_STORE_EVTCHN, store_evtchn);
-    xc_hvm_param_set(handle, domid, HVM_PARAM_CONSOLE_EVTCHN, console_evtchn);
-
-    *store_mfn = str_mfn;
-    *console_mfn = cons_mfn;
-
-    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, console_domid, store_domid);
     return 0;
 }
 
@@ -1159,6 +1146,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
 
     dom->container_type = XC_DOM_HVM_CONTAINER;
 
+    dom->console_evtchn = state->console_port;
+    dom->console_domid = state->console_domid;
+    dom->xenstore_evtchn = state->store_port;
+    dom->xenstore_domid = state->store_domid;
+
     /* The params from the configuration file are in Mb, which are then
      * multiplied by 1 Kb. This was then divided off when calling
      * the old xc_hvm_build_target_mem() which then turned them to bytes.
@@ -1263,10 +1255,7 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
-    rc = hvm_build_set_params(ctx->xch, domid, info, state->store_port,
-                               &state->store_mfn, state->console_port,
-                               &state->console_mfn, state->store_domid,
-                               state->console_domid);
+    rc = hvm_build_set_params(ctx->xch, domid, info);
     if (rc != 0) {
         LOG(ERROR, "hvm build set params failed");
         goto out;
@@ -1278,6 +1267,9 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
         goto out;
     }
 
+    state->console_mfn = dom->console_gfn;
+    state->store_mfn = dom->xenstore_gfn;
+
     xc_dom_release(dom);
     return 0;
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 45e6df6..06a7f32 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -117,7 +117,6 @@
 #define STUBDOM_SPECIAL_CONSOLES 3
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DOMID_XS_PATH "domid"
-#define INVALID_DOMID ~0
 
 /* Size macros. */
 #define __AC(X,Y)   (X##Y)
-- 
2.1.4


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

[PATCH for-4.10 v2 5/5] tools/dombuilder: Prevent failures of xc_dom_gnttab_init()

[Andrew Cooper]

Recent changes in grant table configuration have caused calls to
xc_dom_gnttab_init() to fail if not proceeded with a call to
xc_domain_set_gnttab_limits().  This is backwards from the point of view of
3rd party dombuilder users.

Add max_{grant,maptrack}_frames parameters to struct xc_dom_image, and require
them to be set by callers using xc_dom_gnttab_init().  Libxl, which uses
xc_dom_gnttab_init() itself is updated appropriately.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Julien Grall <julien.grall@arm.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Julien Grall <julien.grall@arm.com>

v2:
 * Set errno to EINVAL if max_{grant,maptrack}_frames are not provided
---
 tools/libxc/include/xc_dom.h |  4 ++++
 tools/libxc/xc_dom_boot.c    | 16 ++++++++++++++++
 tools/libxc/xc_dom_core.c    |  3 +++
 tools/libxl/libxl_dom.c      | 12 ++++++------
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index a1c3de2..5292424 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -116,6 +116,10 @@ struct xc_dom_image {
     uint32_t console_domid;
     uint32_t xenstore_domid;
 
+    /* Grant limit configuration; mandatory if calling xc_dom_gnttab_init(). */
+    unsigned int max_grant_frames;
+    unsigned int max_maptrack_frames;
+
     /*
      * initrd parameters as specified in start_info page
      * Depending on capabilities of the booted kernel this may be a virtual
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 75836bd..7c21fea 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -420,6 +420,22 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, uint32_t domid,
 
 int xc_dom_gnttab_init(struct xc_dom_image *dom)
 {
+    int rc;
+
+    if ( dom->max_grant_frames == -1 || dom->max_maptrack_frames == -1 )
+    {
+        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
+                     "%s: Caller didn't set grant limit information", __func__);
+        errno = EINVAL;
+
+        return -1;
+    }
+
+    if ( (rc = xc_domain_set_gnttab_limits(dom->xch, dom->guest_domid,
+                                           dom->max_grant_frames,
+                                           dom->max_maptrack_frames)) != 0 )
+        return rc;
+
     if ( xc_dom_translated(dom) ) {
         return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
                                       dom->console_gfn, dom->xenstore_gfn,
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 7087c50..d660651 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -784,6 +784,9 @@ struct xc_dom_image *xc_dom_allocate(xc_interface *xch,
     dom->console_domid = INVALID_DOMID;
     dom->xenstore_domid = INVALID_DOMID;
 
+    dom->max_grant_frames = -1;
+    dom->max_maptrack_frames = -1;
+
     dom->flags = SIF_VIRT_P2M_4TOOLS;
 
     dom->alloc_malloc += sizeof(*dom);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index fcdeef0..fa5319d 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -358,12 +358,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
-    if (xc_domain_set_gnttab_limits(ctx->xch, domid, info->max_grant_frames,
-                                    info->max_maptrack_frames) != 0) {
-        LOG(ERROR, "Couldn't set grant table limits");
-        return ERROR_FAIL;
-    }
-
     /*
      * Check if the domain has any CPU or node affinity already. If not, try
      * to build up the latter via automatic NUMA placement. In fact, in case
@@ -815,6 +809,9 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
     dom->xenstore_domid = state->store_domid;
     dom->claim_enabled = libxl_defbool_val(info->claim_mode);
 
+    dom->max_grant_frames    = info->max_grant_frames;
+    dom->max_maptrack_frames = info->max_maptrack_frames;
+
     if (info->num_vnuma_nodes != 0) {
         unsigned int i;
 
@@ -1151,6 +1148,9 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     dom->xenstore_evtchn = state->store_port;
     dom->xenstore_domid = state->store_domid;
 
+    dom->max_grant_frames    = info->max_grant_frames;
+    dom->max_maptrack_frames = info->max_maptrack_frames;
+
     /* The params from the configuration file are in Mb, which are then
      * multiplied by 1 Kb. This was then divided off when calling
      * the old xc_hvm_build_target_mem() which then turned them to bytes.
-- 
2.1.4


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

Re: [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Hi,

I think this is 4.10 material (particularly patch #5). Juergen, would it 
be possible get the some feedback on this series?

Cheers,

On 12/10/17 20:19, Andrew Cooper wrote:
> A git tree version is available:
> 
> http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/dombuilder-gnt-v2
> 
> Changes in v2: Mainly a rebase over c/s 5b42c82f "tools/libxc: Fix domid
> parameter types", and fixup from review comments.  See individual patches for
> details
> 
> Andrew Cooper (5):
>    tools/dombuilder: Drop more PVH v1 leftovers
>    tools/dombuilder: Remove clear_page() from xc_dom_boot.c
>    tools/dombuilder: Switch to using gfn terminology for console and
>      xenstore rings
>    tools/dombuilder: Fix asymmetry when setting up console and xenstore
>      rings
>    tools/dombuilder: Prevent failures of xc_dom_gnttab_init()
> 
>   tools/libxc/include/xc_dom.h      |  26 ++++++--
>   tools/libxc/xc_dom_arm.c          |  17 ++---
>   tools/libxc/xc_dom_boot.c         | 126 +++++++++++++++++++++++---------------
>   tools/libxc/xc_dom_compat_linux.c |   6 +-
>   tools/libxc/xc_dom_core.c         |   8 +++
>   tools/libxc/xc_dom_x86.c          |  57 +++++++++--------
>   tools/libxl/libxl_dom.c           |  51 ++++++---------
>   tools/libxl/libxl_internal.h      |   1 -
>   8 files changed, 169 insertions(+), 123 deletions(-)
> 

-- 
Julien Grall

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

Re: [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Juergen Gross]

On 24/10/17 18:06, Julien Grall wrote:
> Hi,
> 
> I think this is 4.10 material (particularly patch #5). Juergen, would it
> be possible get the some feedback on this series?

Patch 5: Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

> 
> Cheers,
> 
> On 12/10/17 20:19, Andrew Cooper wrote:
>> A git tree version is available:
>>
>> http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/dombuilder-gnt-v2
>>
>>
>> Changes in v2: Mainly a rebase over c/s 5b42c82f "tools/libxc: Fix domid
>> parameter types", and fixup from review comments.  See individual
>> patches for
>> details
>>
>> Andrew Cooper (5):
>>    tools/dombuilder: Drop more PVH v1 leftovers
>>    tools/dombuilder: Remove clear_page() from xc_dom_boot.c
>>    tools/dombuilder: Switch to using gfn terminology for console and
>>      xenstore rings
>>    tools/dombuilder: Fix asymmetry when setting up console and xenstore
>>      rings
>>    tools/dombuilder: Prevent failures of xc_dom_gnttab_init()
>>
>>   tools/libxc/include/xc_dom.h      |  26 ++++++--
>>   tools/libxc/xc_dom_arm.c          |  17 ++---
>>   tools/libxc/xc_dom_boot.c         | 126
>> +++++++++++++++++++++++---------------
>>   tools/libxc/xc_dom_compat_linux.c |   6 +-
>>   tools/libxc/xc_dom_core.c         |   8 +++
>>   tools/libxc/xc_dom_x86.c          |  57 +++++++++--------
>>   tools/libxl/libxl_dom.c           |  51 ++++++---------
>>   tools/libxl/libxl_internal.h      |   1 -
>>   8 files changed, 169 insertions(+), 123 deletions(-)
>>
> 


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

Re: [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Hi Juergen,

On 25/10/17 08:08, Juergen Gross wrote:
> On 24/10/17 18:06, Julien Grall wrote:
>> Hi,
>>
>> I think this is 4.10 material (particularly patch #5). Juergen, would it
>> be possible get the some feedback on this series?
> 
> Patch 5: Reviewed-by: Juergen Gross <jgross@suse.com>

Thank you!

For the series:

Release-acked-by: Julien Grall <julien.grall@linaro.org>

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Hi,

I noticed I forgot to CC Wei and Ian for committing this patch series.

On 27/10/17 14:31, Julien Grall wrote:
> Hi Juergen,
> 
> On 25/10/17 08:08, Juergen Gross wrote:
>> On 24/10/17 18:06, Julien Grall wrote:
>>> Hi,
>>>
>>> I think this is 4.10 material (particularly patch #5). Juergen, would it
>>> be possible get the some feedback on this series?
>>
>> Patch 5: Reviewed-by: Juergen Gross <jgross@suse.com>
> 
> Thank you!
> 
> For the series:
> 
> Release-acked-by: Julien Grall <julien.grall@linaro.org>
> 
> Cheers,
> 

-- 
Julien Grall

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

Re: [PATCH for-4.10 v2 0/5] tools/dombuilder: Fixes and improvements to grant handling

[Julien Grall]

Actually CC them...

On 02/11/17 13:44, Julien Grall wrote:
> Hi,
> 
> I noticed I forgot to CC Wei and Ian for committing this patch series.
> 
> On 27/10/17 14:31, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 25/10/17 08:08, Juergen Gross wrote:
>>> On 24/10/17 18:06, Julien Grall wrote:
>>>> Hi,
>>>>
>>>> I think this is 4.10 material (particularly patch #5). Juergen, 
>>>> would it
>>>> be possible get the some feedback on this series?
>>>
>>> Patch 5: Reviewed-by: Juergen Gross <jgross@suse.com>
>>
>> Thank you!
>>
>> For the series:
>>
>> Release-acked-by: Julien Grall <julien.grall@linaro.org>
>>
>> Cheers,
>>
> 

-- 
Julien Grall

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

Re: [PATCH for-4.10 v2 3/5] tools/dombuilder: Switch to using gfn terminology for console and xenstore rings

[Wei Liu]

On Thu, Oct 12, 2017 at 08:19:07PM +0100, Andrew Cooper wrote:
> The sole use of xc_dom_translated() and xc_dom_p2m() outside of the domain
> builder is for libxl_dom() to translate the console and xenstore pfns back
> into useful values.  PV guest pfns are only interesting to the domain builder,
> and gfns are the address space used by all other hypercalls.
> 
> Renaming the fields in xc_dom_image is deliberate, as it will cause
> out-of-tree users of the dombuilder to notice the different semantics.
> 
> Correct the terminology throughout xc_dom_gnttab{_hvm,}_seed(), which are all
> using gfns despite the existing variable names.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> Tested-by: Julien Grall <julien.grall@arm.com>
> ---
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> 
> v2:
>  * More style fixes
> ---
>  tools/libxc/include/xc_dom.h      | 10 +++++++--
>  tools/libxc/xc_dom_arm.c          | 12 +++++------
>  tools/libxc/xc_dom_boot.c         | 45 ++++++++++++++++++---------------------
>  tools/libxc/xc_dom_compat_linux.c |  4 ++--
>  tools/libxc/xc_dom_x86.c          | 45 ++++++++++++++++++++-------------------
>  tools/libxl/libxl_dom.c           | 11 +++-------
>  6 files changed, 63 insertions(+), 64 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index cdcdd07..5907559 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -94,8 +94,6 @@ struct xc_dom_image {
>      struct xc_dom_seg devicetree_seg;
>      struct xc_dom_seg start_info_seg; /* HVMlite only */
>      xen_pfn_t start_info_pfn;
> -    xen_pfn_t console_pfn;
> -    xen_pfn_t xenstore_pfn;
>      xen_pfn_t shared_info_pfn;
>      xen_pfn_t bootstack_pfn;
>      xen_pfn_t pfn_alloc_end;
> @@ -103,6 +101,14 @@ struct xc_dom_image {
>      xen_vaddr_t bsd_symtab_start;
>  
>      /*
> +     * Details for the toolstack-prepared rings.
> +     *
> +     * *_gfn fields are allocated by the domain builder.
> +     */
> +    xen_pfn_t console_gfn;
> +    xen_pfn_t xenstore_gfn;
> +
> +    /*

Stubdom build is broken by this change:

mini-os.c:756:23: note: in expansion of macro ‘BOOTSEC_LOCATION’
     mbi.drives_addr = BOOTSEC_LOCATION + (60 * 1024);
                       ^~~~~~~~~~~~~~~~
gcc -mno-red-zone -O1 -fno-omit-frame-pointer -O1 -fno-omit-frame-pointer  -m64 -mno-red-zone -fno-reorder-blocks -fno-asynchronous-unwind-tables -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
 -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -fno-stack-protector -fno-exceptions -fno-builtin -Wall -Werror -Wredundant-decls -Wno-for
mat -Wno-redundant-decls -Wformat -fno-stack-protector -fgnu89-inline -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline -g -D__INSIDE_MINIOS__ -m64 -mno-red-zone -fno-reorder-blocks -fno-
asynchronous-unwind-tables -DCONFIG_PARAVIRT -DCONFIG_SPARSE_BSS -DCONFIG_BLKFRONT -DCONFIG_TPMFRONT -DCONFIG_TPM_TIS -DCONFIG_TPMBACK -DCONFIG_XENBUS -D__XEN_INTERFACE_VERSION__=0x00030205 -isystem
/local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include -D__MINIOS__ -DHAVE_LIBC -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/posix -isystem /local/work/COMMITTER/
xen.git/stubdom/../tools/xenstore/include  -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/x86 -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/x86/x8
6_64 -U __linux__ -U __FreeBSD__ -U __sun__ -nostdinc -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/posix -isystem /local/work/COMMITTER/xen.git/stubdom/cross-root-x86_64/x
86_64-xen-elf/include -isystem /usr/lib/gcc/x86_64-linux-gnu/6/include -isystem /local/work/COMMITTER/xen.git/stubdom/lwip-x86_64/src/include -isystem /local/work/COMMITTER/xen.git/stubdom/lwip-x86_6
4/src/include/ipv4 -I/local/work/COMMITTER/xen.git/stubdom/include -I/local/work/COMMITTER/xen.git/stubdom/../xen/include -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include -D__
MINIOS__ -DHAVE_LIBC -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/posix -isystem /local/work/COMMITTER/xen.git/stubdom/../tools/xenstore/include  -isystem /local/work/COMM
ITTER/xen.git/stubdom/../extras/mini-os/include/x86 -isystem /local/work/COMMITTER/xen.git/stubdom/../extras/mini-os/include/x86/x86_64 -c gntmap.c -o /local/work/COMMITTER/xen.git/stubdom/mini-os-x8
6_64-vtpmmgr/gntmap.o
In file included from /local/work/COMMITTER/xen.git/stubdom/include/xen/xen.h:30:0,
                 from /local/work/COMMITTER/xen.git/stubdom/grub/../../tools/libxc/include/xenctrl.h:35,
                 from kexec.c:22:
/local/work/COMMITTER/xen.git/stubdom/include/xen/xen-compat.h:34:0: warning: "__XEN_INTERFACE_VERSION__" redefined
 #define __XEN_INTERFACE_VERSION__ __XEN_LATEST_INTERFACE_VERSION__
<command-line>:0:0: note: this is the location of the previous definition
In file included from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/mm.h:39:0,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/x86/x86_64/hypercall-x86_64.h:39,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/hypervisor.h:20,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/x86/os.h:16,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/console.h:39,
                 from kexec.c:26:
kexec.c: In function ‘kexec’:
kexec.c:221:43: warning: taking address of expression of type ‘void’
     xen_pfn_t boot_page_mfn = virt_to_mfn(&_boot_page);
                                           ^
kexec.c:315:25: error: ‘struct xc_dom_image’ has no member named ‘console_pfn’; did you mean ‘console_gfn’?
     do_exchange(dom, dom->console_pfn, start_info.console.domU.mfn);
                         ^~
kexec.c:316:25: error: ‘struct xc_dom_image’ has no member named ‘xenstore_pfn’; did you mean ‘xenstore_gfn’?
     do_exchange(dom, dom->xenstore_pfn, start_info.store_mfn);
                         ^~
In file included from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/mm.h:39:0,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/x86/x86_64/hypercall-x86_64.h:39,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/hypervisor.h:20,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/x86/os.h:16,
                 from /local/work/COMMITTER/xen.git/extras/mini-os-remote/include/console.h:39,
                 from kexec.c:26:
kexec.c:322:25: warning: taking address of expression of type ‘void’
             virt_to_mfn(&_boot_page));
                         ^
Makefile:80: recipe for target '/local/work/COMMITTER/xen.git/stubdom/grub-x86_64/kexec.o' failed
make[2]: *** [/local/work/COMMITTER/xen.git/stubdom/grub-x86_64/kexec.o] Error 1
make[2]: Leaving directory '/local/work/COMMITTER/xen.git/stubdom/grub'
Makefile:537: recipe for target 'grub' failed
make[1]: *** [grub] Error 2
make[1]: *** Waiting for unfinished jobs....

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