All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 00/10] xen: better grant v2 support
@ 2017-09-22 11:41 Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 01/10] xen: add function for obtaining highest possible memory address Juergen Gross
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Currently Linux has no support for grant v2 as this would reduce the
maximum number of active grants by a factor of 2 compared to v1,
because the number of possible grants are limited by the allowed number
of grant frames and grant entries of v2 need twice as much bytes as
those of v1.

Unfortunately grant v2 is the only way to support either guests with
more than 16TB memory size or PV guests with memory above the 16TB
border, as grant v1 limits the frame number to be 32 bits wide.

In order to remove the disadvantage of grant v2 this patch series
adds support for setting per-domain values regarding grant limits.
Additionally the default limit of grant frames is doubled in case
of hosts with potential memory above the 16TB border.

Changes in V9:
- dropped the already committed patches 1-3
- merged former patches 4 and 5 (Jan Beulich)
- merged former patches 13 and 14
- patch 1: bump sysctl interface version (Julien Grall)
- patch 1: drop thin common shim of get_upper_mfn_bound() (Jan Beulich)
- patch 1: let get_upper_mfn_bound() return the highest MFN, not the one
  following it (Jan Beulich)
- patch 7: allocate initial grant frames from grant_table_init()
  (Jan Beulich)
- patch 8: correct and cleanup gnttab_init_arch() for ARM (Julien Grall)
- patch 10: make leaf pv-only (Jan Beulich)
- patch 10: use hex value for mask (Jan Beulich)
- patch 10: guest address width -> machine address width (Jan Beulich)

Changes in V8:
- dropped previous patch 1, as already committed
- patch 1: make gnttab_grow_table() static instead doing so in next
  patch (Jan Beulich)
- patch 3: remove stale #if 0, adjust comments (Paul Durrant)

Changes in V7:
- added patches 5, 6, 9, 16
- patch 2: only call gnttab_set_frame_gfn() if no error (Julien Grall)
- patch 10: don't use xc_maximum_ram_page() but max_possible_mfn from
   physinfo
- patch 13: re-add #include <asm/grant-table.h> in grant_table.h
  (Julien Grall)
- patch 15: add boot parameter documentation changes

Changes in V6:
- several new patches (1, 6, 7, 10, 12)
- order of patches re-arranged to support new hypercall now being
  mandatory
- lots of other small changes

Changes in V5:
- patch 6: add set_gnttab_limits to create_domain_common in xen.if
  (Daniel De Graaf)

Changes in V4:
- patch 3: make ret more local (Wei Liu)
- patch 7: use domid_t (Wei Liu)
- patch 8: rename configuration items to use max_ prefixes (Wei Liu)

Changes in V3:
- patch 1: update commit message
- patch 3: move call of grant_table_init() from gnttab_setup_table() to
  gnttab_grow_table() (Paul Durrant)
- patch 4: correct error message (Paul Durrant)
- patch 6: rename *gnttbl* to *gnttab* (Paul Durrant)

Changes in V2:
- add per-domain grant limits instead of different v1 and v2 limits
- double default limit for huge hosts

Juergen Gross (10):
  xen: add function for obtaining highest possible memory address
  libxc: add libxc support for setting grant table resource limits
  tools: set grant limits for xenstore stubdom
  libxl: add max possible mfn to libxl_physinfo
  xl: add global grant limit config items
  libxl: add libxl support for setting grant table resource limits
  xen: delay allocation of grant table sub structures
  xen/arm: move arch specific grant table bits into grant_table.c
  xen: make grant resource limits per domain
  xen: add new Xen cpuid node for max address width info

 docs/man/xl.cfg.pod.5.in             |  16 ++
 docs/man/xl.conf.pod.5               |  12 ++
 docs/misc/xen-command-line.markdown  |  25 ++--
 tools/helpers/init-xenstore-domain.c |  11 ++
 tools/libxc/include/xenctrl.h        |  13 ++
 tools/libxc/xc_domain.c              |  13 ++
 tools/libxl/libxl.c                  |   1 +
 tools/libxl/libxl.h                  |  15 ++
 tools/libxl/libxl_dm.c               |   3 +
 tools/libxl/libxl_dom.c              |   6 +
 tools/libxl/libxl_types.idl          |   4 +
 tools/xl/xl.c                        |  15 ++
 tools/xl/xl.h                        |   2 +
 tools/xl/xl_parse.c                  |   9 ++
 tools/xl/xl_sxp.c                    |   2 +
 xen/arch/arm/domain.c                |   2 -
 xen/arch/arm/domain_build.c          |   6 +-
 xen/arch/arm/mm.c                    |   6 +
 xen/arch/x86/mm.c                    |  11 ++
 xen/arch/x86/traps.c                 |   7 +
 xen/common/compat/grant_table.c      |  31 +---
 xen/common/grant_table.c             | 278 ++++++++++++++++++++---------------
 xen/common/sysctl.c                  |   1 +
 xen/include/asm-arm/domain.h         |   1 -
 xen/include/asm-arm/grant_table.h    |  39 ++++-
 xen/include/asm-x86/grant_table.h    |  19 ++-
 xen/include/public/arch-x86/cpuid.h  |  11 +-
 xen/include/public/sysctl.h          |   4 +-
 xen/include/xen/grant_table.h        |   3 +-
 xen/include/xen/mm.h                 |   3 +
 30 files changed, 388 insertions(+), 181 deletions(-)

-- 
2.12.3


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v9 01/10] xen: add function for obtaining highest possible memory address
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 14:38   ` Jan Beulich
  2017-09-22 11:41 ` [PATCH v9 02/10] libxc: add libxc support for setting grant table resource limits Juergen Gross
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Add a function for obtaining the highest possible physical memory
address of the system. This value is influenced by:

- hypervisor configuration (CONFIG_BIGMEM)
- processor capability (max. addressable physical memory)
- memory map at boot time
- memory hotplug capability

Add this value to xen_sysctl_physinfo in order to enable dom0 to do a
proper sizing of grant frame limits of guests.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V9:
- merge patch with following one (Jan Beulich)
- bump sysctl interface version (Julien Grall)
- drop thin common shim of get_upper_mfn_bound() (Jan Beulich)
- let get_upper_mfn_bound() return the highest MFN, not the one
  following it (Jan Beulich)
---
 xen/arch/arm/mm.c           |  6 ++++++
 xen/arch/x86/mm.c           | 11 +++++++++++
 xen/common/sysctl.c         |  1 +
 xen/include/public/sysctl.h |  4 +++-
 xen/include/xen/mm.h        |  3 +++
 5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f3834b3dab..9a37f29ce6 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1472,6 +1472,12 @@ void clear_and_clean_page(struct page_info *page)
     unmap_domain_page(p);
 }
 
+unsigned long get_upper_mfn_bound(void)
+{
+    /* No memory hotplug yet, so current memory limit is the final one. */
+    return max_page - 1;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 67f583e3a7..7af371d499 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6312,6 +6312,17 @@ int pv_ro_page_fault(unsigned long addr, struct cpu_user_regs *regs)
     return 0;
 }
 
+unsigned long get_upper_mfn_bound(void)
+{
+    unsigned long max_mfn;
+
+    max_mfn = mem_hotplug ? PFN_DOWN(mem_hotplug) : max_page;
+#ifndef CONFIG_BIGMEM
+    max_mfn = min(max_mfn, 1UL << 32);
+#endif
+    return min(max_mfn, 1UL << (paddr_bits - PAGE_SHIFT)) - 1;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 3480f582fa..08198b7150 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -266,6 +266,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         get_outstanding_claims(&pi->free_pages, &pi->outstanding_pages);
         pi->scrub_pages = 0;
         pi->cpu_khz = cpu_khz;
+        pi->max_mfn = get_upper_mfn_bound();
         arch_do_physinfo(pi);
 
         if ( copy_to_guest(u_sysctl, op, 1) )
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 4d32a87cca..4315881c12 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
 #include "physdev.h"
 #include "tmem.h"
 
-#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000F
+#define XEN_SYSCTL_INTERFACE_VERSION 0x00000010
 
 /*
  * Read console content from Xen buffer ring.
@@ -104,6 +104,8 @@ struct xen_sysctl_physinfo {
 
     /* XEN_SYSCTL_PHYSCAP_??? */
     uint32_t capabilities;
+
+    uint64_t max_mfn;     /* Largest possible MFN on this host */
 };
 
 /*
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index f8b6177c32..e813c07b22 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -599,6 +599,9 @@ int prepare_ring_for_helper(struct domain *d, unsigned long gmfn,
                             struct page_info **_page, void **_va);
 void destroy_ring_for_helper(void **_va, struct page_info *page);
 
+/* Return the upper bound of MFNs, including hotplug memory. */
+unsigned long get_upper_mfn_bound(void);
+
 #include <asm/flushtlb.h>
 
 static inline void accumulate_tlbflush(bool *need_tlbflush,
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 02/10] libxc: add libxc support for setting grant table resource limits
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 01/10] xen: add function for obtaining highest possible memory address Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 03/10] tools: set grant limits for xenstore stubdom Juergen Gross
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Add a new libxc function xc_domain_set_gnttbl_limits() setting the
limits for the maximum numbers of grant table frames and maptrack
frames of a domain.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
V4:
- use domid_t (Wei Liu)
---
 tools/libxc/include/xenctrl.h | 13 +++++++++++++
 tools/libxc/xc_domain.c       | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 073fbc9b08..182367e7f4 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1064,6 +1064,19 @@ int xc_domain_set_virq_handler(xc_interface *xch, uint32_t domid, int virq);
 int xc_domain_set_max_evtchn(xc_interface *xch, uint32_t domid,
                              uint32_t max_port);
 
+/**
+ * Set the maximum number of grant frames and maptrack frames a domain
+ * can have. Must be used at domain setup time and only then.
+ *
+ * @param xch a handle to an open hypervisor interface
+ * @param domid the domain id
+ * @param grant_frames max. number of grant frames
+ * @param maptrack_frames max. number of maptrack frames
+ */
+int xc_domain_set_gnttab_limits(xc_interface *xch, domid_t domid,
+                                uint32_t grant_frames,
+                                uint32_t maptrack_frames);
+
 /*
  * CPUPOOL MANAGEMENT FUNCTIONS
  */
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index f40dc4fd7b..4a7078008f 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2266,6 +2266,19 @@ int xc_domain_set_max_evtchn(xc_interface *xch, uint32_t domid,
     return do_domctl(xch, &domctl);
 }
 
+int xc_domain_set_gnttab_limits(xc_interface *xch, domid_t domid,
+                                uint32_t grant_frames,
+                                uint32_t maptrack_frames)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_set_gnttab_limits;
+    domctl.domain = domid;
+    domctl.u.set_gnttab_limits.grant_frames = grant_frames;
+    domctl.u.set_gnttab_limits.maptrack_frames = maptrack_frames;
+    return do_domctl(xch, &domctl);
+}
+
 /* Plumbing Xen with vNUMA topology */
 int xc_domain_setvnuma(xc_interface *xch,
                        uint32_t domid,
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 03/10] tools: set grant limits for xenstore stubdom
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 01/10] xen: add function for obtaining highest possible memory address Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 02/10] libxc: add libxc support for setting grant table resource limits Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo Juergen Gross
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

When creating a Xenstore stubdom set the grant limits: the stubdom
will need to setup a very limited amount of grants only, so 4 grant
frames are enough. For being able to support up to 32768 domains it
will need 128 maptrack frames (1 mapping per domain, 256 maptrack
entries per maptrack frame).

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/helpers/init-xenstore-domain.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 8a41ee7d3a..047ad0cb1d 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -105,6 +105,17 @@ static int build(xc_interface *xch)
         fprintf(stderr, "xc_domain_setmaxmem failed\n");
         goto err;
     }
+    /*
+     * 1 grant frame is enough: we don't need many grants.
+     * Mini-OS doesn't like less than 4, though, so use 4.
+     * 128 maptrack frames: 256 entries per frame, enough for 32768 domains.
+     */
+    rv = xc_domain_set_gnttab_limits(xch, domid, 4, 128);
+    if ( rv )
+    {
+        fprintf(stderr, "xc_domain_set_gnttab_limits failed\n");
+        goto err;
+    }
     rv = xc_domain_set_memmap_limit(xch, domid, limit_kb);
     if ( rv )
     {
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (2 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 03/10] tools: set grant limits for xenstore stubdom Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 13:20   ` Wei Liu
  2017-09-22 11:41 ` [PATCH v9 05/10] xl: add global grant limit config items Juergen Gross
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Add the maximum possible mfn of the host to the libxl_physinfo
data structure.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl.c         | 1 +
 tools/libxl/libxl.h         | 9 +++++++++
 tools/libxl/libxl_types.idl | 1 +
 3 files changed, 11 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 247c56cf83..b41ade9fda 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -373,6 +373,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
     physinfo->free_pages = xcphysinfo.free_pages;
     physinfo->scrub_pages = xcphysinfo.scrub_pages;
     physinfo->outstanding_pages = xcphysinfo.outstanding_pages;
+    physinfo->max_possible_mfn = xcphysinfo.max_mfn;
     l = xc_sharing_freed_pages(ctx->xch);
     if (l < 0 && errno == ENOSYS) {
         l = 0;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 7d853ca924..fb960debee 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -644,6 +644,15 @@ typedef struct libxl__ctx libxl_ctx;
 #define LIBXL_HAVE_PHYSINFO_OUTSTANDING_PAGES 1
 
 /*
+ * LIBXL_HAVE_PHYSINFO_MAX_POSSIBLE_MFN
+ *
+ * If this is defined, libxl_physinfo structure will contain an uint64 field
+ * called max_possible_mfn, containing the highest possible mfn on this host,
+ * possibly taking memory hotplug into account.
+ */
+#define LIBXL_HAVE_PHYSINFO_MAX_POSSIBLE_MFN 1
+
+/*
  * LIBXL_HAVE_DOMINFO_OUTSTANDING_MEMKB 1
  *
  * If this is defined, libxl_dominfo will contain a MemKB type field called
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 756e120ad7..5d9e7aabba 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -895,6 +895,7 @@ libxl_physinfo = Struct("physinfo", [
     ("outstanding_pages", uint64),
     ("sharing_freed_pages", uint64),
     ("sharing_used_frames", uint64),
+    ("max_possible_mfn", uint64),
 
     ("nr_nodes", uint32),
     ("hw_cap", libxl_hwcap),
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 05/10] xl: add global grant limit config items
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (3 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 13:20   ` Wei Liu
  2017-09-22 11:41 ` [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits Juergen Gross
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Add xl.conf config items for default values of grant limits:

max_grant_frames will set the default for the maximum number of grant
frames for a domain which will take effect if the domain's config file
doesn't specify a value. If max_grant_frames isn't set in xl.conf it
will default to 32 for hosts with all memory below 16TB and to 64 for
hosts with memory above 16TB.

max_maptrack_frames will set the default for the maximum number of
maptrack frames for a domain. If max_maptrack_frames isn't set in
xl.conf it will default to 0, as normally only backend domains need
maptrack frames.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
V7:
- don't use xc_maximum_ram_page() but max_possible_mfn from physinfo
---
 docs/man/xl.conf.pod.5 | 12 ++++++++++++
 tools/xl/xl.c          | 15 +++++++++++++++
 tools/xl/xl.h          |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 88ab506609..fe2cf27ea4 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -77,6 +77,18 @@ operations (primarily domain creation).
 
 Default: C</var/lock/xl>
 
+=item B<max_grant_frames=NUMBER>
+
+Sets the default value for the C<max_grant_frames> domain config value.
+
+Default: C<32> on hosts up to 16TB of memory, C<64> on hosts larger than 16TB
+
+=item B<max_maptrack_frames=NUMBER>
+
+Sets the default value for the C<max_maptrack_frames> domain config value.
+
+Default: C<0>
+
 =item B<vif.default.script="PATH">
 
 Configures the default hotplug script used by virtual network devices.
diff --git a/tools/xl/xl.c b/tools/xl/xl.c
index 02179a6229..c1bbb4b939 100644
--- a/tools/xl/xl.c
+++ b/tools/xl/xl.c
@@ -45,6 +45,8 @@ char *default_colo_proxy_script = NULL;
 enum output_format default_output_format = OUTPUT_FORMAT_JSON;
 int claim_mode = 1;
 bool progress_use_cr = 0;
+int max_grant_frames = -1;
+int max_maptrack_frames = 0;
 
 xentoollog_level minmsglevel = minmsglevel_default;
 
@@ -88,6 +90,7 @@ static void parse_global_config(const char *configfile,
     XLU_Config *config;
     int e;
     const char *buf;
+    libxl_physinfo physinfo;
 
     config = xlu_cfg_init(stderr, configfile);
     if (!config) {
@@ -188,6 +191,18 @@ static void parse_global_config(const char *configfile,
     xlu_cfg_replace_string (config, "colo.default.proxyscript",
         &default_colo_proxy_script, 0);
 
+    if (!xlu_cfg_get_long (config, "max_grant_frames", &l, 0))
+        max_grant_frames = l;
+    else {
+        libxl_physinfo_init(&physinfo);
+        max_grant_frames = (libxl_get_physinfo(ctx, &physinfo) != 0 ||
+                            !(physinfo.max_possible_mfn >> 32))
+                           ? 32 : 64;
+        libxl_physinfo_dispose(&physinfo);
+    }
+    if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
+        max_maptrack_frames = l;
+
     xlu_cfg_destroy(config);
 }
 
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 31d660b89a..6b60d1db50 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -275,6 +275,8 @@ extern char *default_vifbackend;
 extern char *default_remus_netbufscript;
 extern char *default_colo_proxy_script;
 extern char *blkdev_start;
+extern int max_grant_frames;
+extern int max_maptrack_frames;
 
 enum output_format {
     OUTPUT_FORMAT_JSON,
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (4 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 05/10] xl: add global grant limit config items Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 13:20   ` Wei Liu
  2017-09-22 11:41 ` [PATCH v9 07/10] xen: delay allocation of grant table sub structures Juergen Gross
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Add new domain config items for setting the limits for the maximum
numbers of grant table frames and maptrack frames of a domain.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
V6:
- made set_gnttab_limits hypercall mandatory, taking defaults from
  xl.conf

V4:
- rename configuration items to use max_ prefixes (Wei Liu)
---
 docs/man/xl.cfg.pod.5.in    | 16 ++++++++++++++++
 tools/libxl/libxl.h         |  6 ++++++
 tools/libxl/libxl_dm.c      |  3 +++
 tools/libxl/libxl_dom.c     |  6 ++++++
 tools/libxl/libxl_types.idl |  3 +++
 tools/xl/xl_parse.c         |  9 +++++++++
 tools/xl/xl_sxp.c           |  2 ++
 7 files changed, 45 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 247ae99ca7..e7ab67395b 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -444,6 +444,20 @@ unpausing the domain. With a properly constructed security policy (such
 as nomigrate_t in the example policy), this can be used to build a
 domain whose memory is not accessible to the toolstack domain.
 
+=item B<max_grant_frames=NUMBER>
+
+Specify the maximum number of grant frames the domain is allowed to have.
+This value controls how many pages the domain is able to grant access to for
+other domains, needed e.g. for the operation of paravirtualized devices.
+The default is settable via L<xl.conf(5)>.
+
+=item B<max_maptrack_frames=NUMBER>
+
+Specify the maximum number of grant maptrack frames the domain is allowed
+to have. This value controls how many pages of foreign domains can be accessed
+via the grant mechanism by this domain. The default value is settable via
+L<xl.conf(5)>.
+
 =item B<nomigrate=BOOLEAN>
 
 Disable migration of this domain.  This enables certain other features
@@ -2252,6 +2266,8 @@ No MCA capabilities in above list are enabled.
 
 =item L<xl(1)>
 
+=item L<xl.conf(5)>
+
 =item L<xlcpupool.cfg(5)>
 
 =item L<xl-disk-configuration(5)>
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index fb960debee..c6f42945de 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -311,6 +311,12 @@
 #define LIBXL_HAVE_P9S 1
 
 /*
+ * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
+ * has the max_grant_frames and max_maptrack_frames fields.
+ */
+#define LIBXL_HAVE_BUILDINFO_GRANT_LIMITS 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 98f89a95ce..bf651006b4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1845,6 +1845,9 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
         guest_config->b_info.video_memkb;
     dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
 
+    dm_config->b_info.max_grant_frames = guest_config->b_info.max_grant_frames;
+    dm_config->b_info.max_maptrack_frames = 0;
+
     dm_config->b_info.u.pv.features = "";
 
     dm_config->b_info.device_model_version =
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f54fd49a73..b420738adf 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -322,6 +322,12 @@ 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
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5d9e7aabba..dc3544873c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -472,6 +472,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("blkdev_start",    string),
 
     ("vnuma_nodes", Array(libxl_vnode_info, "num_vnuma_nodes")),
+
+    ("max_grant_frames",    uint32),
+    ("max_maptrack_frames", uint32),
     
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 0678fbc1b0..5ba18e9a15 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -990,6 +990,15 @@ void parse_config_data(const char *config_source,
         !xlu_cfg_get_string (config, "cpus_soft", &buf, 0))
         parse_vcpu_affinity(b_info, cpus, buf, num_cpus, false);
 
+    if (!xlu_cfg_get_long (config, "max_grant_frames", &l, 0))
+        b_info->max_grant_frames = l;
+    else
+        b_info->max_grant_frames = max_grant_frames;
+    if (!xlu_cfg_get_long (config, "max_maptrack_frames", &l, 0))
+        b_info->max_maptrack_frames = l;
+    else
+        b_info->max_maptrack_frames = max_maptrack_frames;
+
     libxl_defbool_set(&b_info->claim_mode, claim_mode);
 
     if (xlu_cfg_get_string (config, "on_poweroff", &buf, 0))
diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
index e738bf2465..e264cf2023 100644
--- a/tools/xl/xl_sxp.c
+++ b/tools/xl/xl_sxp.c
@@ -64,6 +64,8 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
 
     fprintf(fh, "\t(build_info)\n");
     fprintf(fh, "\t(max_vcpus %d)\n", b_info->max_vcpus);
+    fprintf(fh, "\t(max_grant_frames %d)\n", b_info->max_grant_frames);
+    fprintf(fh, "\t(max_maptrack_frames %d)\n", b_info->max_maptrack_frames);
     fprintf(fh, "\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
     fprintf(fh, "\t(max_memkb %"PRId64")\n", b_info->max_memkb);
     fprintf(fh, "\t(target_memkb %"PRId64")\n", b_info->target_memkb);
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 07/10] xen: delay allocation of grant table sub structures
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (5 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 14:43   ` Jan Beulich
       [not found]   ` <59C53DB6020000780017EB92@suse.com>
  2017-09-22 11:41 ` [PATCH v9 08/10] xen/arm: move arch specific grant table bits into grant_table.c Juergen Gross
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Delay the allocation of the grant table sub structures in order to
allow modifying parameters needed for sizing of these structures at a
per domain basis. Allocate the structures and the table frames only
from grant_table_set_limits() (dom0: from grant_table_create()).

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
V9:
- allocate initial grant frames from grant_table_init() (Jan Beulich)

V6:
- move call of grant_table_init() for dom0 to grant_table_create()
  (Jan Beulich)
- move frame allocations to gnttab_grow_table() (Jan Beulich)
- several other changes due to new patch order

V4:
- make ret more local (Wei Liu)

V3:
- move call of grant_table_init() from gnttab_setup_table() to
  gnttab_grow_table() (Paul Durrant)
---
 xen/common/grant_table.c | 118 +++++++++++++++++++++++------------------------
 1 file changed, 58 insertions(+), 60 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 0f09891f59..3250db4c5a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1667,6 +1667,10 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
     struct grant_table *gt = d->grant_table;
     unsigned int i, j;
 
+    ASSERT(gt->active);
+
+    if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
+        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
     ASSERT(req_nr_frames <= max_grant_frames);
 
     gdprintk(XENLOG_INFO,
@@ -1723,6 +1727,48 @@ active_alloc_failed:
     return 0;
 }
 
+static int
+grant_table_init(struct domain *d, struct grant_table *gt)
+{
+    if ( gt->active )
+        return -EBUSY;
+
+    /* Active grant table. */
+    gt->active = xzalloc_array(struct active_grant_entry *,
+                               max_nr_active_grant_frames);
+    if ( gt->active == NULL )
+        goto no_mem;
+
+    /* Tracking of mapped foreign frames table */
+    gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
+    if ( gt->maptrack == NULL )
+        goto no_mem;
+
+    /* Shared grant table. */
+    gt->shared_raw = xzalloc_array(void *, max_grant_frames);
+    if ( gt->shared_raw == NULL )
+        goto no_mem;
+
+    /* Status pages for grant table - for version 2 */
+    gt->status = xzalloc_array(grant_status_t *,
+                               grant_to_status_frames(max_grant_frames));
+    if ( gt->status == NULL )
+        goto no_mem;
+
+    /* gnttab_grow_table() allocates a min number of frames, so 0 is okay. */
+    if ( gnttab_grow_table(d, 0) )
+        return 0;
+
+ no_mem:
+    xfree(gt->shared_raw);
+    gt->shared_raw = NULL;
+    vfree(gt->maptrack);
+    gt->maptrack = NULL;
+    xfree(gt->active);
+    gt->active = NULL;
+    return -ENOMEM;
+}
+
 static long
 gnttab_setup_table(
     XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count)
@@ -3383,75 +3429,26 @@ grant_table_create(
     struct domain *d)
 {
     struct grant_table *t;
-    unsigned int i, j;
+    int ret = 0;
 
     if ( (t = xzalloc(struct grant_table)) == NULL )
-        goto no_mem_0;
+        return -ENOMEM;
 
     /* Simple stuff. */
     percpu_rwlock_resource_init(&t->lock, grant_rwlock);
     spin_lock_init(&t->maptrack_lock);
-    t->nr_grant_frames = INITIAL_NR_GRANT_FRAMES;
-
-    /* Active grant table. */
-    if ( (t->active = xzalloc_array(struct active_grant_entry *,
-                                    max_nr_active_grant_frames)) == NULL )
-        goto no_mem_1;
-    for ( i = 0;
-          i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES); i++ )
-    {
-        if ( (t->active[i] = alloc_xenheap_page()) == NULL )
-            goto no_mem_2;
-        clear_page(t->active[i]);
-        for ( j = 0; j < ACGNT_PER_PAGE; j++ )
-            spin_lock_init(&t->active[i][j].lock);
-    }
 
-    /* Tracking of mapped foreign frames table */
-    t->maptrack = vzalloc(max_maptrack_frames * sizeof(*t->maptrack));
-    if ( t->maptrack == NULL )
-        goto no_mem_2;
+    /* Okay, install the structure. */
+    d->grant_table = t;
 
-    /* Shared grant table. */
-    if ( (t->shared_raw = xzalloc_array(void *, max_grant_frames)) == NULL )
-        goto no_mem_3;
-    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
+    if ( d->domain_id == 0 )
     {
-        if ( (t->shared_raw[i] = alloc_xenheap_page()) == NULL )
-            goto no_mem_4;
-        clear_page(t->shared_raw[i]);
+        grant_write_lock(t);
+        ret = grant_table_init(d, t);
+        grant_write_unlock(t);
     }
 
-    /* Status pages for grant table - for version 2 */
-    t->status = xzalloc_array(grant_status_t *,
-                              grant_to_status_frames(max_grant_frames));
-    if ( t->status == NULL )
-        goto no_mem_4;
-
-    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
-        gnttab_create_shared_page(d, t, i);
-
-    t->nr_status_frames = 0;
-
-    /* Okay, install the structure. */
-    d->grant_table = t;
-    return 0;
-
- no_mem_4:
-    for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
-        free_xenheap_page(t->shared_raw[i]);
-    xfree(t->shared_raw);
- no_mem_3:
-    vfree(t->maptrack);
- no_mem_2:
-    for ( i = 0;
-          i < num_act_frames_from_sha_frames(INITIAL_NR_GRANT_FRAMES); i++ )
-        free_xenheap_page(t->active[i]);
-    xfree(t->active);
- no_mem_1:
-    xfree(t);
- no_mem_0:
-    return -ENOMEM;
+    return ret;
 }
 
 void
@@ -3653,8 +3650,9 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
 
     grant_write_lock(gt);
 
-    ret = 0;
-    /* Set limits, alloc needed arrays. */
+    /* Set limits. */
+    if ( !gt->active )
+        ret = grant_table_init(d, gt);
 
     grant_write_unlock(gt);
 
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 08/10] xen/arm: move arch specific grant table bits into grant_table.c
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (6 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 07/10] xen: delay allocation of grant table sub structures Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 09/10] xen: make grant resource limits per domain Juergen Gross
  2017-09-22 11:41 ` [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info Juergen Gross
  9 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Instead of attaching the ARM specific grant table data to the domain
structure add it to struct grant_table. Add the needed arch functions
to the asm-*/grant_table.h includes.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [non-ARM parts]
---
V9:
- correct and cleanup gnttab_init_arch() for ARM (Julien Grall)

V7:
- re-add #include <asm/grant-table.h> in grant_table.h (Julien Grall)
---
 xen/arch/arm/domain.c             |  2 --
 xen/common/grant_table.c          | 27 ++++++++++++++++++++-------
 xen/include/asm-arm/domain.h      |  1 -
 xen/include/asm-arm/grant_table.h | 29 ++++++++++++++++++++++-------
 xen/include/asm-x86/grant_table.h | 12 +++++++-----
 xen/include/xen/grant_table.h     |  2 ++
 6 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 784ae392cf..e39a79885c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -486,13 +486,11 @@ struct domain *alloc_domain_struct(void)
         return NULL;
 
     clear_page(d);
-    d->arch.grant_table_gfn = xzalloc_array(gfn_t, max_grant_frames);
     return d;
 }
 
 void free_domain_struct(struct domain *d)
 {
-    xfree(d->arch.grant_table_gfn);
     free_xenheap_page(d);
 }
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 3250db4c5a..73ba915a30 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -72,6 +72,8 @@ struct grant_table {
     struct active_grant_entry **active;
     /* Mapping tracking table per vcpu. */
     struct grant_mapping **maptrack;
+
+    struct grant_table_arch arch;
 };
 
 #ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
@@ -1730,6 +1732,8 @@ active_alloc_failed:
 static int
 grant_table_init(struct domain *d, struct grant_table *gt)
 {
+    int ret = -ENOMEM;
+
     if ( gt->active )
         return -EBUSY;
 
@@ -1737,36 +1741,43 @@ grant_table_init(struct domain *d, struct grant_table *gt)
     gt->active = xzalloc_array(struct active_grant_entry *,
                                max_nr_active_grant_frames);
     if ( gt->active == NULL )
-        goto no_mem;
+        goto out;
 
     /* Tracking of mapped foreign frames table */
     gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
     if ( gt->maptrack == NULL )
-        goto no_mem;
+        goto out;
 
     /* Shared grant table. */
     gt->shared_raw = xzalloc_array(void *, max_grant_frames);
     if ( gt->shared_raw == NULL )
-        goto no_mem;
+        goto out;
 
     /* Status pages for grant table - for version 2 */
     gt->status = xzalloc_array(grant_status_t *,
                                grant_to_status_frames(max_grant_frames));
     if ( gt->status == NULL )
-        goto no_mem;
+        goto out;
+
+    ret = gnttab_init_arch(gt);
+    if ( ret )
+        goto out;
 
     /* gnttab_grow_table() allocates a min number of frames, so 0 is okay. */
     if ( gnttab_grow_table(d, 0) )
         return 0;
 
- no_mem:
+ out:
+    gnttab_destroy_arch(gt);
+    xfree(gt->status);
+    gt->status = NULL;
     xfree(gt->shared_raw);
     gt->shared_raw = NULL;
     vfree(gt->maptrack);
     gt->maptrack = NULL;
     xfree(gt->active);
     gt->active = NULL;
-    return -ENOMEM;
+    return ret;
 }
 
 static long
@@ -3612,6 +3623,8 @@ grant_table_destroy(
     if ( t == NULL )
         return;
 
+    gnttab_destroy_arch(t);
+
     for ( i = 0; i < nr_grant_frames(t); i++ )
         free_xenheap_page(t->shared_raw[i]);
     xfree(t->shared_raw);
@@ -3738,7 +3751,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
     }
 
     if ( !rc )
-        gnttab_set_frame_gfn(d, idx, gfn);
+        gnttab_set_frame_gfn(gt, idx, gfn);
 
     grant_write_unlock(gt);
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index b174c65080..ce9b6a4032 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -50,7 +50,6 @@ struct arch_domain
     struct p2m_domain p2m;
 
     struct hvm_domain hvm_domain;
-    gfn_t *grant_table_gfn;
 
     struct vmmio vmmio;
 
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 0a248a765a..30db2d1616 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -6,6 +6,10 @@
 
 #define INITIAL_NR_GRANT_FRAMES 4
 
+struct grant_table_arch {
+    gfn_t *gfn;
+};
+
 void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
 int create_grant_host_mapping(unsigned long gpaddr,
         unsigned long mfn, unsigned int flags, unsigned int
@@ -22,11 +26,22 @@ static inline int replace_grant_supported(void)
     return 1;
 }
 
-static inline void gnttab_set_frame_gfn(struct domain *d, unsigned long idx,
-                                        gfn_t gfn)
-{
-    d->arch.grant_table_gfn[idx] = gfn;
-}
+#define gnttab_init_arch(gt)                                             \
+({                                                                       \
+    (gt)->arch.gfn = xzalloc_array(gfn_t, max_grant_frames);             \
+    ( (gt)->arch.gfn ? 0 : -ENOMEM );                                    \
+})
+
+#define gnttab_destroy_arch(gt)                                          \
+    do {                                                                 \
+        xfree((gt)->arch.gfn);                                           \
+        (gt)->arch.gfn = NULL;                                           \
+    } while ( 0 )
+
+#define gnttab_set_frame_gfn(gt, idx, gfn)                               \
+    do {                                                                 \
+        (gt)->arch.gfn[idx] = gfn;                                       \
+    } while ( 0 )
 
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \
@@ -36,8 +51,8 @@ static inline void gnttab_set_frame_gfn(struct domain *d, unsigned long idx,
     } while ( 0 )
 
 #define gnttab_shared_gmfn(d, t, i)                                      \
-    ( ((i >= nr_grant_frames(d->grant_table)) &&                         \
-     (i < max_grant_frames)) ? 0 : gfn_x(d->arch.grant_table_gfn[i]))
+    ( ((i >= nr_grant_frames(t)) &&                                      \
+       (i < max_grant_frames)) ? 0 : gfn_x(t->arch.gfn[i]))
 
 #define gnttab_need_iommu_mapping(d)                    \
     (is_domain_direct_mapped(d) && need_iommu(d))
diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index c865999a33..1b93c5720d 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -14,6 +14,9 @@
 
 #define INITIAL_NR_GRANT_FRAMES 4
 
+struct grant_table_arch {
+};
+
 /*
  * Caller must own caller's BIGLOCK, is responsible for flushing the TLB, and
  * must hold a reference to the page.
@@ -36,6 +39,10 @@ static inline int replace_grant_host_mapping(uint64_t addr, unsigned long frame,
     return replace_grant_pv_mapping(addr, frame, new_addr, flags);
 }
 
+#define gnttab_init_arch(gt) 0
+#define gnttab_destroy_arch(gt) do {} while ( 0 )
+#define gnttab_set_frame_gfn(gt, idx, gfn) do {} while ( 0 )
+
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \
         share_xen_page_with_guest(                                       \
@@ -75,11 +82,6 @@ static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st)
     asm volatile ("lock btrw %w1,%0" : "=m" (*st) : "Ir" (nr), "m" (*st));
 }
 
-static inline void gnttab_set_frame_gfn(struct domain *d, unsigned long idx,
-                                        gfn_t gfn)
-{
-}
-
 /* Foreign mappings of HHVM-guest pages do not modify the type count. */
 #define gnttab_host_mapping_get_page_type(ro, ld, rd)   \
     (!(ro) && (((ld) == (rd)) || !paging_mode_external(rd)))
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index df11b31264..d2bd2416c4 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -29,6 +29,8 @@
 #include <asm/page.h>
 #include <asm/grant_table.h>
 
+struct grant_table;
+
 /* The maximum size of a grant table. */
 extern unsigned int max_grant_frames;
 
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 09/10] xen: make grant resource limits per domain
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (7 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 08/10] xen/arm: move arch specific grant table bits into grant_table.c Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 15:15   ` Jan Beulich
       [not found]   ` <59C54521020000780017EBE2@suse.com>
  2017-09-22 11:41 ` [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info Juergen Gross
  9 siblings, 2 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

Instead of using the same global resource limits of grant tables (max.
number of grant frames, max. number of maptrack frames) for all domains
make these limits per domain. Set those per-domain limits in
grant_table_set_limits(). The global settings are serving as an upper
boundary now which must not be exceeded by a per-domain value. The
default of max_grant_frames is set to the maximum default xl will use.

While updating the semantics of the boot parameters remove the
documentation of the no longer existing gnttab_max_nr_frames.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V9:
- add caps for per-domain limits (Jan Beulich)
- some error messages enhanced (Jan Beulich)
- adjusted some types (Jan Beulich)
- merge parts of (former) patch 14 into this one
- make parameters changeable at runtime
- limit size of dom0's grant table on ARM
- set default max_grant_frames to 64

V6:
- several changes due to new patch order

V3:
- correct error message (Paul Durrant)
---
 docs/misc/xen-command-line.markdown |  25 +++---
 xen/arch/arm/domain_build.c         |   6 +-
 xen/common/compat/grant_table.c     |  31 ++-----
 xen/common/grant_table.c            | 159 +++++++++++++++++++++---------------
 xen/include/asm-arm/grant_table.h   |  18 +++-
 xen/include/asm-x86/grant_table.h   |   7 +-
 xen/include/xen/grant_table.h       |   3 -
 7 files changed, 134 insertions(+), 115 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 9797c8db2d..9b6cd8e9d0 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -875,27 +875,28 @@ Specify which console gdbstub should use. See **console**.
 ### gnttab\_max\_frames
 > `= <integer>`
 
-> Default: `32`
+> Default: `64`
+
+> Can be modified at runtime
 
 Specify the maximum number of frames which any domain may use as part
-of its grant table.
+of its grant table. This value is an upper boundary of the per-domain
+value settable via Xen tools.
+
+Dom0 is using this value for sizing its grant table.
 
 ### gnttab\_max\_maptrack\_frames
 > `= <integer>`
 
-> Default: `8 * gnttab_max_frames`
-
-Specify the maximum number of frames to use as part of a domains
-maptrack array.
+> Default: `1024`
 
-### gnttab\_max\_nr\_frames
-> `= <integer>`
+> Can be modified at runtime
 
-*Deprecated*
-Use **gnttab\_max\_frames** and **gnttab\_max\_maptrack\_frames** instead.
+Specify the maximum number of frames to use as part of a domains
+maptrack array. This value is an upper boundary of the per-domain
+value settable via Xen tools.
 
-Specify the maximum number of frames per grant table operation and the
-maximum number of maptrack frames domain.
+Dom0 is using this value for sizing its maptrack table.
 
 ### guest\_loglvl
 > `= <level>[/<rate-limited level>]` where level is `none | error | warning | info | debug | all`
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c34238ec1b..4664f3906d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2095,11 +2095,7 @@ static void __init find_gnttab_region(struct domain *d,
      * enough space for a large grant table
      */
     kinfo->gnttab_start = __pa(_stext);
-    kinfo->gnttab_size = (_etext - _stext) & PAGE_MASK;
-
-    /* Make sure the grant table will fit in the region */
-    if ( (kinfo->gnttab_size >> PAGE_SHIFT) < max_grant_frames )
-        panic("Cannot find a space for the grant table region\n");
+    kinfo->gnttab_size = gnttab_dom0_max() << PAGE_SHIFT;
 
 #ifdef CONFIG_ARM_32
     /*
diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
index cce3ff0b9a..ff1d678f01 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -157,21 +157,14 @@ int compat_grant_table_op(unsigned int cmd,
                 unsigned int max_frame_list_size_in_page =
                     (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.setup)) /
                     sizeof(*nat.setup->frame_list.p);
-                if ( max_frame_list_size_in_page < max_grant_frames )
-                {
-                    gdprintk(XENLOG_WARNING,
-                             "max_grant_frames is too large (%u,%u)\n",
-                             max_grant_frames, max_frame_list_size_in_page);
-                    rc = -EINVAL;
-                }
-                else
-                {
+
 #define XLAT_gnttab_setup_table_HNDL_frame_list(_d_, _s_) \
-                    set_xen_guest_handle((_d_)->frame_list, (unsigned long *)(nat.setup + 1))
-                    XLAT_gnttab_setup_table(nat.setup, &cmp.setup);
+                set_xen_guest_handle((_d_)->frame_list, (unsigned long *)(nat.setup + 1))
+                XLAT_gnttab_setup_table(nat.setup, &cmp.setup);
 #undef XLAT_gnttab_setup_table_HNDL_frame_list
-                    rc = gnttab_setup_table(guest_handle_cast(nat.uop, gnttab_setup_table_t), 1);
-                }
+                rc = gnttab_setup_table(guest_handle_cast(nat.uop,
+                                                          gnttab_setup_table_t),
+                                        1, max_frame_list_size_in_page);
             }
             ASSERT(rc <= 0);
             if ( rc == 0 )
@@ -294,16 +287,6 @@ int compat_grant_table_op(unsigned int cmd,
                 rc = -EFAULT;
                 break;
             }
-            if ( max_frame_list_size_in_pages <
-                 grant_to_status_frames(max_grant_frames) )
-            {
-                gdprintk(XENLOG_WARNING,
-                         "grant_to_status_frames(max_grant_frames) is too large (%u,%u)\n",
-                         grant_to_status_frames(max_grant_frames),
-                         max_frame_list_size_in_pages);
-                rc = -EINVAL;
-                break;
-            }
 
 #define XLAT_gnttab_get_status_frames_HNDL_frame_list(_d_, _s_) \
             set_xen_guest_handle((_d_)->frame_list, (uint64_t *)(nat.get_status + 1))
@@ -312,7 +295,7 @@ int compat_grant_table_op(unsigned int cmd,
 
             rc = gnttab_get_status_frames(
                 guest_handle_cast(nat.uop, gnttab_get_status_frames_t),
-                count);
+                count, max_frame_list_size_in_pages);
             if ( rc >= 0 )
             {
 #define XLAT_gnttab_get_status_frames_HNDL_frame_list(_d_, _s_) \
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 73ba915a30..d835f5ab80 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -54,6 +54,9 @@ struct grant_table {
      * what version to use yet.
      */
     unsigned int          gt_version;
+    /* Resource limits of the domain. */
+    unsigned int          max_grant_frames;
+    unsigned int          max_maptrack_frames;
     /* Table size. Number of frames shared with guest */
     unsigned int          nr_grant_frames;
     /* Number of grant status frames shared with guest (for version 2) */
@@ -78,23 +81,18 @@ struct grant_table {
 
 #ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
 /* Default maximum size of a grant table. [POLICY] */
-#define DEFAULT_MAX_NR_GRANT_FRAMES   32
+#define DEFAULT_MAX_NR_GRANT_FRAMES   64
 #endif
 
-unsigned int __read_mostly max_grant_frames;
-integer_param("gnttab_max_frames", max_grant_frames);
+static unsigned int __read_mostly max_grant_frames =
+                                               DEFAULT_MAX_NR_GRANT_FRAMES;
+integer_runtime_param("gnttab_max_frames", max_grant_frames);
 
-/* The maximum number of grant mappings is defined as a multiplier of the
- * maximum number of grant table entries. This defines the multiplier used.
- * Pretty arbitrary. [POLICY]
- * As gnttab_max_nr_frames has been deprecated, this multiplier is deprecated too.
- * New options allow to set max_maptrack_frames and
- * map_grant_table_frames independently.
- */
 #define DEFAULT_MAX_MAPTRACK_FRAMES 1024
 
-static unsigned int __read_mostly max_maptrack_frames;
-integer_param("gnttab_max_maptrack_frames", max_maptrack_frames);
+static unsigned int __read_mostly max_maptrack_frames =
+                                               DEFAULT_MAX_MAPTRACK_FRAMES;
+integer_runtime_param("gnttab_max_maptrack_frames", max_maptrack_frames);
 
 /*
  * Note that the three values below are effectively part of the ABI, even if
@@ -290,8 +288,8 @@ num_act_frames_from_sha_frames(const unsigned int num)
     return DIV_ROUND_UP(num * sha_per_page, ACGNT_PER_PAGE);
 }
 
-#define max_nr_active_grant_frames \
-    num_act_frames_from_sha_frames(max_grant_frames)
+#define max_nr_active_grant_frames(gt) \
+    num_act_frames_from_sha_frames((gt)->max_grant_frames)
 
 static inline unsigned int
 nr_active_grant_frames(struct grant_table *gt)
@@ -530,7 +528,7 @@ get_maptrack_handle(
      * out of memory, try stealing an entry from another VCPU (in case the
      * guest isn't mapping across its VCPUs evenly).
      */
-    if ( nr_maptrack_frames(lgt) < max_maptrack_frames )
+    if ( nr_maptrack_frames(lgt) < lgt->max_maptrack_frames )
         new_mt = alloc_xenheap_page();
 
     if ( !new_mt )
@@ -1672,8 +1670,8 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
     ASSERT(gt->active);
 
     if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
-        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
-    ASSERT(req_nr_frames <= max_grant_frames);
+        req_nr_frames = min(INITIAL_NR_GRANT_FRAMES, gt->max_grant_frames);
+    ASSERT(req_nr_frames <= gt->max_grant_frames);
 
     gdprintk(XENLOG_INFO,
             "Expanding dom (%d) grant table from (%d) to (%d) frames.\n",
@@ -1739,23 +1737,26 @@ grant_table_init(struct domain *d, struct grant_table *gt)
 
     /* Active grant table. */
     gt->active = xzalloc_array(struct active_grant_entry *,
-                               max_nr_active_grant_frames);
+                               max_nr_active_grant_frames(gt));
     if ( gt->active == NULL )
         goto out;
 
     /* Tracking of mapped foreign frames table */
-    gt->maptrack = vzalloc(max_maptrack_frames * sizeof(*gt->maptrack));
-    if ( gt->maptrack == NULL )
-        goto out;
+    if ( gt->max_maptrack_frames )
+    {
+        gt->maptrack = vzalloc(gt->max_maptrack_frames * sizeof(*gt->maptrack));
+        if ( gt->maptrack == NULL )
+            goto out;
+    }
 
     /* Shared grant table. */
-    gt->shared_raw = xzalloc_array(void *, max_grant_frames);
+    gt->shared_raw = xzalloc_array(void *, gt->max_grant_frames);
     if ( gt->shared_raw == NULL )
         goto out;
 
     /* Status pages for grant table - for version 2 */
     gt->status = xzalloc_array(grant_status_t *,
-                               grant_to_status_frames(max_grant_frames));
+                               grant_to_status_frames(gt->max_grant_frames));
     if ( gt->status == NULL )
         goto out;
 
@@ -1782,13 +1783,15 @@ grant_table_init(struct domain *d, struct grant_table *gt)
 
 static long
 gnttab_setup_table(
-    XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count)
+    XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count,
+    unsigned int limit_max)
 {
     struct vcpu *curr = current;
     struct gnttab_setup_table op;
     struct domain *d = NULL;
     struct grant_table *gt;
     unsigned int i;
+    int ret = 0;
 
     if ( count != 1 )
         return -EINVAL;
@@ -1796,15 +1799,6 @@ gnttab_setup_table(
     if ( unlikely(copy_from_guest(&op, uop, 1)) )
         return -EFAULT;
 
-    if ( unlikely(op.nr_frames > max_grant_frames) )
-    {
-        gdprintk(XENLOG_INFO, "Xen only supports up to %d grant-table frames"
-                " per domain.\n",
-                max_grant_frames);
-        op.status = GNTST_general_error;
-        goto out;
-    }
-
     if ( !guest_handle_okay(op.frame_list, op.nr_frames) )
         return -EFAULT;
 
@@ -1824,6 +1818,21 @@ gnttab_setup_table(
     gt = d->grant_table;
     grant_write_lock(gt);
 
+    if ( unlikely(op.nr_frames > gt->max_grant_frames) )
+    {
+        gdprintk(XENLOG_INFO, "d%u is limited to %u grant-table frames.\n",
+                d->domain_id, gt->max_grant_frames);
+        op.status = GNTST_general_error;
+        goto unlock;
+    }
+    if ( unlikely(limit_max < op.nr_frames) )
+    {
+        gdprintk(XENLOG_WARNING, "nr_frames for d%u is too large (%u,%u)\n",
+                 d->domain_id, op.nr_frames, limit_max);
+        op.status = GNTST_general_error;
+        goto unlock;
+    }
+
     if ( gt->gt_version == 0 )
         gt->gt_version = 1;
 
@@ -1833,8 +1842,9 @@ gnttab_setup_table(
          !gnttab_grow_table(d, op.nr_frames) )
     {
         gdprintk(XENLOG_INFO,
-                 "Expand grant table to %u failed. Current: %u Max: %u\n",
-                 op.nr_frames, nr_grant_frames(gt), max_grant_frames);
+                 "Expand grant table of d%u to %u failed. Current: %u Max: %u\n",
+                 d->domain_id, op.nr_frames, nr_grant_frames(gt),
+                 gt->max_grant_frames);
         op.status = GNTST_general_error;
         goto unlock;
     }
@@ -1857,10 +1867,10 @@ gnttab_setup_table(
     if ( d )
         rcu_unlock_domain(d);
 
-    if ( unlikely(__copy_field_to_guest(uop, &op, status)) )
+    if ( !ret && unlikely(__copy_field_to_guest(uop, &op, status)) )
         return -EFAULT;
 
-    return 0;
+    return ret;
 }
 
 static long
@@ -1869,6 +1879,7 @@ gnttab_query_size(
 {
     struct gnttab_query_size op;
     struct domain *d;
+    struct grant_table *gt;
 
     if ( count != 1 )
         return -EINVAL;
@@ -1889,13 +1900,15 @@ gnttab_query_size(
         goto out;
     }
 
-    grant_read_lock(d->grant_table);
+    gt = d->grant_table;
+
+    grant_read_lock(gt);
 
-    op.nr_frames     = nr_grant_frames(d->grant_table);
-    op.max_nr_frames = max_grant_frames;
+    op.nr_frames     = nr_grant_frames(gt);
+    op.max_nr_frames = gt->max_grant_frames;
     op.status        = GNTST_okay;
 
-    grant_read_unlock(d->grant_table);
+    grant_read_unlock(gt);
 
  out:
     if ( d )
@@ -2970,14 +2983,14 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
 
 static long
 gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
-                         int count)
+                         unsigned int count, unsigned int limit_max)
 {
     gnttab_get_status_frames_t op;
     struct domain *d;
     struct grant_table *gt;
     uint64_t       gmfn;
     int i;
-    int rc;
+    int rc, ret = 0;
 
     if ( count != 1 )
         return -EINVAL;
@@ -3010,9 +3023,19 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
 
     if ( unlikely(op.nr_frames > nr_status_frames(gt)) )
     {
-        gdprintk(XENLOG_INFO, "Guest requested addresses for %d grant status "
-                 "frames, but only %d are available.\n",
-                 op.nr_frames, nr_status_frames(gt));
+        gdprintk(XENLOG_INFO, "Guest requested addresses of d%u for %u grant "
+                 "status frames, but only %u are available.\n",
+                 d->domain_id, op.nr_frames, nr_status_frames(gt));
+        op.status = GNTST_general_error;
+        goto unlock;
+    }
+
+    if ( unlikely(limit_max < grant_to_status_frames(op.nr_frames)) )
+    {
+        gdprintk(XENLOG_WARNING,
+                 "grant_to_status_frames(%u) for d%u is too large (%u,%u)\n",
+                 op.nr_frames, d->domain_id,
+                 grant_to_status_frames(op.nr_frames), limit_max);
         op.status = GNTST_general_error;
         goto unlock;
     }
@@ -3029,10 +3052,10 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
  out2:
     rcu_unlock_domain(d);
  out1:
-    if ( unlikely(__copy_field_to_guest(uop, &op, status)) )
+    if ( !ret && unlikely(__copy_field_to_guest(uop, &op, status)) )
         return -EFAULT;
 
-    return 0;
+    return ret;
 }
 
 static long
@@ -3325,7 +3348,7 @@ do_grant_table_op(
 
     case GNTTABOP_setup_table:
         rc = gnttab_setup_table(
-            guest_handle_cast(uop, gnttab_setup_table_t), count);
+            guest_handle_cast(uop, gnttab_setup_table_t), count, UINT_MAX);
         ASSERT(rc <= 0);
         break;
 
@@ -3374,7 +3397,8 @@ do_grant_table_op(
 
     case GNTTABOP_get_status_frames:
         rc = gnttab_get_status_frames(
-            guest_handle_cast(uop, gnttab_get_status_frames_t), count);
+            guest_handle_cast(uop, gnttab_get_status_frames_t), count,
+                              UINT_MAX);
         break;
 
     case GNTTABOP_get_version:
@@ -3454,6 +3478,8 @@ grant_table_create(
 
     if ( d->domain_id == 0 )
     {
+        t->max_grant_frames = gnttab_dom0_frames(max_grant_frames);
+        t->max_maptrack_frames = max_maptrack_frames;
         grant_write_lock(t);
         ret = grant_table_init(d, t);
         grant_write_unlock(t);
@@ -3658,6 +3684,9 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
     struct grant_table *gt = d->grant_table;
     int ret = -EBUSY;
 
+    if ( !grant_frames || grant_frames > max_grant_frames ||
+         maptrack_frames > max_maptrack_frames )
+        return -EINVAL;
     if ( !gt )
         return -ENOENT;
 
@@ -3665,7 +3694,11 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
 
     /* Set limits. */
     if ( !gt->active )
+    {
+        gt->max_grant_frames = grant_frames;
+        gt->max_maptrack_frames = maptrack_frames;
         ret = grant_table_init(d, gt);
+    }
 
     grant_write_unlock(gt);
 
@@ -3741,7 +3774,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
     }
     else
     {
-        if ( (idx >= nr_grant_frames(gt)) && (idx < max_grant_frames) )
+        if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
             gnttab_grow_table(d, idx + 1);
 
         if ( idx < nr_grant_frames(gt) )
@@ -3769,6 +3802,12 @@ static void gnttab_usage_print(struct domain *rd)
 
     grant_read_lock(gt);
 
+    printk("grant-table for remote domain:%5d (v%d)\n"
+           "  %d frames (%d max), %d maptrack frames (%d max)\n",
+           rd->domain_id, gt->gt_version,
+           nr_grant_frames(gt), gt->max_grant_frames,
+           nr_maptrack_frames(gt), gt->max_maptrack_frames);
+
     for ( ref = 0; ref != nr_grant_entries(gt); ref++ )
     {
         struct active_grant_entry *act;
@@ -3796,12 +3835,7 @@ static void gnttab_usage_print(struct domain *rd)
             status = status_entry(gt, ref);
         }
 
-        if ( first )
-        {
-            printk("grant-table for remote domain:%5d (v%d)\n",
-                   rd->domain_id, gt->gt_version);
-            first = 0;
-        }
+        first = 0;
 
         /*      [0xXXX]  ddddd 0xXXXXXX 0xXXXXXXXX      ddddd 0xXXXXXX 0xXX */
         printk("[0x%03x]  %5d 0x%06lx 0x%08x      %5d 0x%06"PRIx64" 0x%02x\n",
@@ -3813,8 +3847,7 @@ static void gnttab_usage_print(struct domain *rd)
     grant_read_unlock(gt);
 
     if ( first )
-        printk("grant-table for remote domain:%5d ... "
-               "no active grant table entries\n", rd->domain_id);
+        printk("no active grant table entries\n");
 }
 
 static void gnttab_usage_print_all(unsigned char key)
@@ -3828,14 +3861,6 @@ static void gnttab_usage_print_all(unsigned char key)
 
 static int __init gnttab_usage_init(void)
 {
-    BUILD_BUG_ON(DEFAULT_MAX_MAPTRACK_FRAMES < DEFAULT_MAX_NR_GRANT_FRAMES);
-
-    if ( !max_grant_frames )
-        max_grant_frames = DEFAULT_MAX_NR_GRANT_FRAMES;
-
-    if ( !max_maptrack_frames )
-        max_maptrack_frames = DEFAULT_MAX_MAPTRACK_FRAMES;
-
     register_keyhandler('g', gnttab_usage_print_all,
                         "print grant table usage", 1);
     return 0;
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 30db2d1616..44ebcfb6e6 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -2,9 +2,11 @@
 #define __ASM_GRANT_TABLE_H__
 
 #include <xen/grant_table.h>
+#include <xen/kernel.h>
+#include <xen/pfn.h>
 #include <xen/sched.h>
 
-#define INITIAL_NR_GRANT_FRAMES 4
+#define INITIAL_NR_GRANT_FRAMES 4U
 
 struct grant_table_arch {
     gfn_t *gfn;
@@ -26,9 +28,19 @@ static inline int replace_grant_supported(void)
     return 1;
 }
 
+static inline unsigned int gnttab_dom0_max(void)
+{
+    return PFN_DOWN(_etext - _stext);
+}
+
+static inline unsigned int gnttab_dom0_frames(unsigned int max_frames)
+{
+    return min(max_frames, gnttab_dom0_max());
+}
+
 #define gnttab_init_arch(gt)                                             \
 ({                                                                       \
-    (gt)->arch.gfn = xzalloc_array(gfn_t, max_grant_frames);             \
+    (gt)->arch.gfn = xzalloc_array(gfn_t, (gt)->max_grant_frames);       \
     ( (gt)->arch.gfn ? 0 : -ENOMEM );                                    \
 })
 
@@ -52,7 +64,7 @@ static inline int replace_grant_supported(void)
 
 #define gnttab_shared_gmfn(d, t, i)                                      \
     ( ((i >= nr_grant_frames(t)) &&                                      \
-       (i < max_grant_frames)) ? 0 : gfn_x(t->arch.gfn[i]))
+       (i < (t)->max_grant_frames))? 0 : gfn_x((t)->arch.gfn[i]))
 
 #define gnttab_need_iommu_mapping(d)                    \
     (is_domain_direct_mapped(d) && need_iommu(d))
diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index 1b93c5720d..f1aa9b3209 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -12,7 +12,7 @@
 #include <asm/hvm/grant_table.h>
 #include <asm/pv/grant_table.h>
 
-#define INITIAL_NR_GRANT_FRAMES 4
+#define INITIAL_NR_GRANT_FRAMES 4U
 
 struct grant_table_arch {
 };
@@ -39,6 +39,11 @@ static inline int replace_grant_host_mapping(uint64_t addr, unsigned long frame,
     return replace_grant_pv_mapping(addr, frame, new_addr, flags);
 }
 
+static inline unsigned int gnttab_dom0_frames(unsigned int max_frames)
+{
+    return max_frames;
+}
+
 #define gnttab_init_arch(gt) 0
 #define gnttab_destroy_arch(gt) do {} while ( 0 )
 #define gnttab_set_frame_gfn(gt, idx, gfn) do {} while ( 0 )
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index d2bd2416c4..2b194c3526 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -31,9 +31,6 @@
 
 struct grant_table;
 
-/* The maximum size of a grant table. */
-extern unsigned int max_grant_frames;
-
 /* Create/destroy per-domain grant table context. */
 int grant_table_create(
     struct domain *d);
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info
  2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
                   ` (8 preceding siblings ...)
  2017-09-22 11:41 ` [PATCH v9 09/10] xen: make grant resource limits per domain Juergen Gross
@ 2017-09-22 11:41 ` Juergen Gross
  2017-09-22 14:47   ` Jan Beulich
       [not found]   ` <59C53E91020000780017EB95@suse.com>
  9 siblings, 2 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 11:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich,
	dgdegra

On very large hosts a pv-guest needs to know whether it will have to
handle frame numbers larger than 32 bits in order to select the
appropriate grant interface version.

Add a new Xen specific CPUID node to contain the maximum machine address
width similar to the x86 CPUID node 0x80000008 containing the maximum
physical address width. The maximum frame width needs to take memory
hotplug into account.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V9:
- make leaf pv-only (Jan Beulich)
- use hex value for mask (Jan Beulich)
- guest address width -> machine address width (Jan Beulich)
---
 xen/arch/x86/traps.c                |  7 +++++++
 xen/include/public/arch-x86/cpuid.h | 11 ++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 6091f239ce..b4b4918955 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -929,6 +929,13 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
         res->b = v->vcpu_id;
         break;
 
+    case 5: /* PV-specific parameters */
+        if ( is_hvm_domain(d) || subleaf != 0 )
+            break;
+
+        res->a = generic_flsl(get_upper_mfn_bound()) + PAGE_SHIFT;
+        break;
+
     default:
         ASSERT_UNREACHABLE();
     }
diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h
index d709340f18..2a96de66b4 100644
--- a/xen/include/public/arch-x86/cpuid.h
+++ b/xen/include/public/arch-x86/cpuid.h
@@ -85,6 +85,15 @@
 #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
 #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present in EBX */
 
-#define XEN_CPUID_MAX_NUM_LEAVES 4
+/*
+ * Leaf 6 (0x40000x05)
+ * PV-specific parameters
+ * EAX: bits 0-7: max machine address width
+ */
+
+/* Max. address width in bits taking memory hotplug into account. */
+#define XEN_CPUID_MACHINE_ADDRESS_WIDTH_MASK (0xffu << 0)
+
+#define XEN_CPUID_MAX_NUM_LEAVES 5
 
 #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */
-- 
2.12.3


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo
  2017-09-22 11:41 ` [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo Juergen Gross
@ 2017-09-22 13:20   ` Wei Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Wei Liu @ 2017-09-22 13:20 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, jbeulich, xen-devel, dgdegra

On Fri, Sep 22, 2017 at 01:41:28PM +0200, Juergen Gross wrote:
> Add the maximum possible mfn of the host to the libxl_physinfo
> data structure.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Ian Jackson <ian.jackson@eu.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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 05/10] xl: add global grant limit config items
  2017-09-22 11:41 ` [PATCH v9 05/10] xl: add global grant limit config items Juergen Gross
@ 2017-09-22 13:20   ` Wei Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Wei Liu @ 2017-09-22 13:20 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, jbeulich, xen-devel, dgdegra

On Fri, Sep 22, 2017 at 01:41:29PM +0200, Juergen Gross wrote:
> Add xl.conf config items for default values of grant limits:
> 
> max_grant_frames will set the default for the maximum number of grant
> frames for a domain which will take effect if the domain's config file
> doesn't specify a value. If max_grant_frames isn't set in xl.conf it
> will default to 32 for hosts with all memory below 16TB and to 64 for
> hosts with memory above 16TB.
> 
> max_maptrack_frames will set the default for the maximum number of
> maptrack frames for a domain. If max_maptrack_frames isn't set in
> xl.conf it will default to 0, as normally only backend domains need
> maptrack frames.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Ian Jackson <ian.jackson@eu.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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits
  2017-09-22 11:41 ` [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits Juergen Gross
@ 2017-09-22 13:20   ` Wei Liu
  0 siblings, 0 replies; 23+ messages in thread
From: Wei Liu @ 2017-09-22 13:20 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, jbeulich, xen-devel, dgdegra

On Fri, Sep 22, 2017 at 01:41:30PM +0200, Juergen Gross wrote:
> Add new domain config items for setting the limits for the maximum
> numbers of grant table frames and maptrack frames of a domain.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Ian Jackson <ian.jackson@eu.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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 01/10] xen: add function for obtaining highest possible memory address
  2017-09-22 11:41 ` [PATCH v9 01/10] xen: add function for obtaining highest possible memory address Juergen Gross
@ 2017-09-22 14:38   ` Jan Beulich
  0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-22 14:38 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
> @@ -104,6 +104,8 @@ struct xen_sysctl_physinfo {
>  
>      /* XEN_SYSCTL_PHYSCAP_??? */
>      uint32_t capabilities;
> +
> +    uint64_t max_mfn;     /* Largest possible MFN on this host */
>  };

I'm sorry for not having noticed this earlier - this needs to be
uint64_aligned_t and creates an unnamed padding hole (yes,
there already is another one, but anyway). So I think this also
wants moving up to avoid creating yet another hole. I think
this is doable while committing (and if I end up doing it I'll likely
take the opportunity and move "capabilities" up into the earlier
hole at the same time). With that adjustment
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 07/10] xen: delay allocation of grant table sub structures
  2017-09-22 11:41 ` [PATCH v9 07/10] xen: delay allocation of grant table sub structures Juergen Gross
@ 2017-09-22 14:43   ` Jan Beulich
       [not found]   ` <59C53DB6020000780017EB92@suse.com>
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-22 14:43 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
> Delay the allocation of the grant table sub structures in order to
> allow modifying parameters needed for sizing of these structures at a
> per domain basis. Allocate the structures and the table frames only
> from grant_table_set_limits() (dom0: from grant_table_create()).

I think this last part is stale now - it's uniformly grant_table_init()
where this happens.

> +    if ( d->domain_id == 0 )
>      {
> -        if ( (t->shared_raw[i] = alloc_xenheap_page()) == NULL )
> -            goto no_mem_4;
> -        clear_page(t->shared_raw[i]);
> +        grant_write_lock(t);
> +        ret = grant_table_init(d, t);
> +        grant_write_unlock(t);
>      }

With this and ...

> @@ -3653,8 +3650,9 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
>  
>      grant_write_lock(gt);
>  
> -    ret = 0;
> -    /* Set limits, alloc needed arrays. */
> +    /* Set limits. */
> +    if ( !gt->active )
> +        ret = grant_table_init(d, gt);
>  
>      grant_write_unlock(gt);

... this, wouldn't it be more natural now to acquire and release the
lock in grant_table_init()?

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info
  2017-09-22 11:41 ` [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info Juergen Gross
@ 2017-09-22 14:47   ` Jan Beulich
       [not found]   ` <59C53E91020000780017EB95@suse.com>
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-22 14:47 UTC (permalink / raw)
  To: andrew.cooper3, Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, tim, ian.jackson,
	julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -929,6 +929,13 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
>          res->b = v->vcpu_id;
>          break;
>  
> +    case 5: /* PV-specific parameters */
> +        if ( is_hvm_domain(d) || subleaf != 0 )
> +            break;
> +
> +        res->a = generic_flsl(get_upper_mfn_bound()) + PAGE_SHIFT;
> +        break;

The subleaf check here should be mirrored ...

> --- a/xen/include/public/arch-x86/cpuid.h
> +++ b/xen/include/public/arch-x86/cpuid.h
> @@ -85,6 +85,15 @@
>  #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
>  #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present in EBX 
> */
>  
> -#define XEN_CPUID_MAX_NUM_LEAVES 4
> +/*
> + * Leaf 6 (0x40000x05)
> + * PV-specific parameters
> + * EAX: bits 0-7: max machine address width
> + */

... in the comment here. This is easily doable while committing,
but I'd like to be certain Andrew in particular has no objections
to this new leaf before putting this in.

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 09/10] xen: make grant resource limits per domain
  2017-09-22 11:41 ` [PATCH v9 09/10] xen: make grant resource limits per domain Juergen Gross
@ 2017-09-22 15:15   ` Jan Beulich
       [not found]   ` <59C54521020000780017EBE2@suse.com>
  1 sibling, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-22 15:15 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
> Instead of using the same global resource limits of grant tables (max.
> number of grant frames, max. number of maptrack frames) for all domains
> make these limits per domain. Set those per-domain limits in
> grant_table_set_limits(). The global settings are serving as an upper
> boundary now which must not be exceeded by a per-domain value. The
> default of max_grant_frames is set to the maximum default xl will use.
> 
> While updating the semantics of the boot parameters remove the
> documentation of the no longer existing gnttab_max_nr_frames.

"... and correct the default gnttab_max_maptrack_frames uses" (or
some such).

> @@ -1672,8 +1670,8 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
>      ASSERT(gt->active);
>  
>      if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
> -        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
> -    ASSERT(req_nr_frames <= max_grant_frames);
> +        req_nr_frames = min(INITIAL_NR_GRANT_FRAMES, gt->max_grant_frames);

I'm not convinced of this: You effectively allowing a zero size grant
table this way. I'd prefer if the "initial" constant stayed the lower
bound. I'm open to lowering that initial value, though.

> @@ -1824,6 +1818,21 @@ gnttab_setup_table(
>      gt = d->grant_table;
>      grant_write_lock(gt);
>  
> +    if ( unlikely(op.nr_frames > gt->max_grant_frames) )
> +    {
> +        gdprintk(XENLOG_INFO, "d%u is limited to %u grant-table frames.\n",

You've switched to %u one too many times - domain IDs want
printing with %d (also below).

> @@ -2970,14 +2983,14 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
>  
>  static long
>  gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
> -                         int count)
> +                         unsigned int count, unsigned int limit_max)
> {
>       gnttab_get_status_frames_t op;
>      struct domain *d;
>      struct grant_table *gt;
>      uint64_t       gmfn;
>      int i;
> -    int rc;
> +    int rc, ret = 0;

This variable doesn't look to be necessary anymore (also in
gnttab_setup_table(), as I notice only now).

> @@ -3010,9 +3023,19 @@ 
> gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
>  
>      if ( unlikely(op.nr_frames > nr_status_frames(gt)) )
>      {
> -        gdprintk(XENLOG_INFO, "Guest requested addresses for %d grant status "
> -                 "frames, but only %d are available.\n",
> -                 op.nr_frames, nr_status_frames(gt));
> +        gdprintk(XENLOG_INFO, "Guest requested addresses of d%u for %u grant "
> +                 "status frames, but only %u are available.\n",

Drop "Guest" and make the end ", has only %u\n"?

> @@ -3665,7 +3694,11 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
>  
>      /* Set limits. */
>      if ( !gt->active )
> +    {
> +        gt->max_grant_frames = grant_frames;

As per above I think you want to silently apply a lower bound here.

> @@ -3769,6 +3802,12 @@ static void gnttab_usage_print(struct domain *rd)
>  
>      grant_read_lock(gt);
>  
> +    printk("grant-table for remote domain:%5d (v%d)\n"

"grant table for d%d (v%u)\n"?

> +           "  %d frames (%d max), %d maptrack frames (%d max)\n",

%u (four times)

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 07/10] xen: delay allocation of grant table sub structures
       [not found]   ` <59C53DB6020000780017EB92@suse.com>
@ 2017-09-22 15:15     ` Juergen Gross
  0 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 15:15 UTC (permalink / raw)
  To: Jan Beulich
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

On 22/09/17 16:43, Jan Beulich wrote:
>>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
>> Delay the allocation of the grant table sub structures in order to
>> allow modifying parameters needed for sizing of these structures at a
>> per domain basis. Allocate the structures and the table frames only
>> from grant_table_set_limits() (dom0: from grant_table_create()).
> 
> I think this last part is stale now - it's uniformly grant_table_init()
> where this happens.
> 
>> +    if ( d->domain_id == 0 )
>>      {
>> -        if ( (t->shared_raw[i] = alloc_xenheap_page()) == NULL )
>> -            goto no_mem_4;
>> -        clear_page(t->shared_raw[i]);
>> +        grant_write_lock(t);
>> +        ret = grant_table_init(d, t);
>> +        grant_write_unlock(t);
>>      }
> 
> With this and ...
> 
>> @@ -3653,8 +3650,9 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
>>  
>>      grant_write_lock(gt);
>>  
>> -    ret = 0;
>> -    /* Set limits, alloc needed arrays. */
>> +    /* Set limits. */
>> +    if ( !gt->active )
>> +        ret = grant_table_init(d, gt);
>>  
>>      grant_write_unlock(gt);
> 
> ... this, wouldn't it be more natural now to acquire and release the
> lock in grant_table_init()?

Right. The test for !gt->active can be dropped, as grant_table_init()
will return -EBUSY in this case anyway.


Juergen

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info
       [not found]   ` <59C53E91020000780017EB95@suse.com>
@ 2017-09-22 16:27     ` Juergen Gross
  2017-09-25  7:06       ` Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 16:27 UTC (permalink / raw)
  To: Jan Beulich, andrew.cooper3
  Cc: sstabellini, wei.liu2, George.Dunlap, tim, ian.jackson,
	julien.grall, xen-devel, dgdegra

On 22/09/17 16:47, Jan Beulich wrote:
>>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -929,6 +929,13 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
>>          res->b = v->vcpu_id;
>>          break;
>>  
>> +    case 5: /* PV-specific parameters */
>> +        if ( is_hvm_domain(d) || subleaf != 0 )
>> +            break;
>> +
>> +        res->a = generic_flsl(get_upper_mfn_bound()) + PAGE_SHIFT;
>> +        break;
> 
> The subleaf check here should be mirrored ...
> 
>> --- a/xen/include/public/arch-x86/cpuid.h
>> +++ b/xen/include/public/arch-x86/cpuid.h
>> @@ -85,6 +85,15 @@
>>  #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
>>  #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present in EBX 
>> */
>>  
>> -#define XEN_CPUID_MAX_NUM_LEAVES 4
>> +/*
>> + * Leaf 6 (0x40000x05)
>> + * PV-specific parameters
>> + * EAX: bits 0-7: max machine address width
>> + */
> 
> ... in the comment here. This is easily doable while committing,

Up to now there is no example for this: the time leaf isn't documented
and the HVM leaf from which I copied the subleaf check doesn't have
anything related to it in the comments.

Do you have any special format recommendations?

And should I add related comments to the HVM leaf section?

> but I'd like to be certain Andrew in particular has no objections
> to this new leaf before putting this in.

Sure.


Juergen


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 09/10] xen: make grant resource limits per domain
       [not found]   ` <59C54521020000780017EBE2@suse.com>
@ 2017-09-22 17:50     ` Juergen Gross
  2017-09-25  7:01       ` Jan Beulich
  0 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2017-09-22 17:50 UTC (permalink / raw)
  To: Jan Beulich
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

On 22/09/17 17:15, Jan Beulich wrote:
>>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
>> Instead of using the same global resource limits of grant tables (max.
>> number of grant frames, max. number of maptrack frames) for all domains
>> make these limits per domain. Set those per-domain limits in
>> grant_table_set_limits(). The global settings are serving as an upper
>> boundary now which must not be exceeded by a per-domain value. The
>> default of max_grant_frames is set to the maximum default xl will use.
>>
>> While updating the semantics of the boot parameters remove the
>> documentation of the no longer existing gnttab_max_nr_frames.
> 
> "... and correct the default gnttab_max_maptrack_frames uses" (or
> some such).

Okay.

> 
>> @@ -1672,8 +1670,8 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
>>      ASSERT(gt->active);
>>  
>>      if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
>> -        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
>> -    ASSERT(req_nr_frames <= max_grant_frames);
>> +        req_nr_frames = min(INITIAL_NR_GRANT_FRAMES, gt->max_grant_frames);
> 
> I'm not convinced of this: You effectively allowing a zero size grant
> table this way. I'd prefer if the "initial" constant stayed the lower
> bound. I'm open to lowering that initial value, though.

Okay. What about the value "1" for it? Should be enough for e.g.
stubdoms, dom0, ...

> 
>> @@ -1824,6 +1818,21 @@ gnttab_setup_table(
>>      gt = d->grant_table;
>>      grant_write_lock(gt);
>>  
>> +    if ( unlikely(op.nr_frames > gt->max_grant_frames) )
>> +    {
>> +        gdprintk(XENLOG_INFO, "d%u is limited to %u grant-table frames.\n",
> 
> You've switched to %u one too many times - domain IDs want
> printing with %d (also below).

Okay.

> 
>> @@ -2970,14 +2983,14 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARAM(gnttab_set_version_t) uop)
>>  
>>  static long
>>  gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
>> -                         int count)
>> +                         unsigned int count, unsigned int limit_max)
>> {
>>       gnttab_get_status_frames_t op;
>>      struct domain *d;
>>      struct grant_table *gt;
>>      uint64_t       gmfn;
>>      int i;
>> -    int rc;
>> +    int rc, ret = 0;
> 
> This variable doesn't look to be necessary anymore (also in
> gnttab_setup_table(), as I notice only now).

Indeed.

> 
>> @@ -3010,9 +3023,19 @@ 
>> gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
>>  
>>      if ( unlikely(op.nr_frames > nr_status_frames(gt)) )
>>      {
>> -        gdprintk(XENLOG_INFO, "Guest requested addresses for %d grant status "
>> -                 "frames, but only %d are available.\n",
>> -                 op.nr_frames, nr_status_frames(gt));
>> +        gdprintk(XENLOG_INFO, "Guest requested addresses of d%u for %u grant "
>> +                 "status frames, but only %u are available.\n",
> 
> Drop "Guest" and make the end ", has only %u\n"?

Okay.

> 
>> @@ -3665,7 +3694,11 @@ int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
>>  
>>      /* Set limits. */
>>      if ( !gt->active )
>> +    {
>> +        gt->max_grant_frames = grant_frames;
> 
> As per above I think you want to silently apply a lower bound here.

I already have. It is 1 (note the test for !grant_frames some lines
higher).

> 
>> @@ -3769,6 +3802,12 @@ static void gnttab_usage_print(struct domain *rd)
>>  
>>      grant_read_lock(gt);
>>  
>> +    printk("grant-table for remote domain:%5d (v%d)\n"
> 
> "grant table for d%d (v%u)\n"?

Okay.

> 
>> +           "  %d frames (%d max), %d maptrack frames (%d max)\n",
> 
> %u (four times)

Okay.

One final note: I have detected another problem  in the ARM part of
this patch: gnttab_size is set wrong. Will correct this.


Juergen

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 09/10] xen: make grant resource limits per domain
  2017-09-22 17:50     ` Juergen Gross
@ 2017-09-25  7:01       ` Jan Beulich
  0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-25  7:01 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 19:50, <jgross@suse.com> wrote:
> On 22/09/17 17:15, Jan Beulich wrote:
>>>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
>>> @@ -1672,8 +1670,8 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
>>>      ASSERT(gt->active);
>>>  
>>>      if ( req_nr_frames < INITIAL_NR_GRANT_FRAMES )
>>> -        req_nr_frames = INITIAL_NR_GRANT_FRAMES;
>>> -    ASSERT(req_nr_frames <= max_grant_frames);
>>> +        req_nr_frames = min(INITIAL_NR_GRANT_FRAMES, gt->max_grant_frames);
>> 
>> I'm not convinced of this: You effectively allowing a zero size grant
>> table this way. I'd prefer if the "initial" constant stayed the lower
>> bound. I'm open to lowering that initial value, though.
> 
> Okay. What about the value "1" for it? Should be enough for e.g.
> stubdoms, dom0, ...

Yes, 1 is fine with me.

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info
  2017-09-22 16:27     ` Juergen Gross
@ 2017-09-25  7:06       ` Jan Beulich
  0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2017-09-25  7:06 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
	ian.jackson, tim, julien.grall, xen-devel, dgdegra

>>> On 22.09.17 at 18:27, <jgross@suse.com> wrote:
> On 22/09/17 16:47, Jan Beulich wrote:
>>>>> On 22.09.17 at 13:41, <jgross@suse.com> wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -929,6 +929,13 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
> uint32_t leaf,
>>>          res->b = v->vcpu_id;
>>>          break;
>>>  
>>> +    case 5: /* PV-specific parameters */
>>> +        if ( is_hvm_domain(d) || subleaf != 0 )
>>> +            break;
>>> +
>>> +        res->a = generic_flsl(get_upper_mfn_bound()) + PAGE_SHIFT;
>>> +        break;
>> 
>> The subleaf check here should be mirrored ...
>> 
>>> --- a/xen/include/public/arch-x86/cpuid.h
>>> +++ b/xen/include/public/arch-x86/cpuid.h
>>> @@ -85,6 +85,15 @@
>>>  #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
>>>  #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present in EBX 
> 
>>> */
>>>  
>>> -#define XEN_CPUID_MAX_NUM_LEAVES 4
>>> +/*
>>> + * Leaf 6 (0x40000x05)
>>> + * PV-specific parameters
>>> + * EAX: bits 0-7: max machine address width
>>> + */
>> 
>> ... in the comment here. This is easily doable while committing,
> 
> Up to now there is no example for this: the time leaf isn't documented
> and the HVM leaf from which I copied the subleaf check doesn't have
> anything related to it in the comments.

As (almost) always, omissions in the past shouldn't be a reason to
spread the badness.

> Do you have any special format recommendations?

 * Sub-leaf 0: EAX: bits 0-7: max machine address width

> And should I add related comments to the HVM leaf section?

Well, if you did so in a separate patch, this would certainly be
nice.

Jan


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-09-25  7:06 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 11:41 [PATCH v9 00/10] xen: better grant v2 support Juergen Gross
2017-09-22 11:41 ` [PATCH v9 01/10] xen: add function for obtaining highest possible memory address Juergen Gross
2017-09-22 14:38   ` Jan Beulich
2017-09-22 11:41 ` [PATCH v9 02/10] libxc: add libxc support for setting grant table resource limits Juergen Gross
2017-09-22 11:41 ` [PATCH v9 03/10] tools: set grant limits for xenstore stubdom Juergen Gross
2017-09-22 11:41 ` [PATCH v9 04/10] libxl: add max possible mfn to libxl_physinfo Juergen Gross
2017-09-22 13:20   ` Wei Liu
2017-09-22 11:41 ` [PATCH v9 05/10] xl: add global grant limit config items Juergen Gross
2017-09-22 13:20   ` Wei Liu
2017-09-22 11:41 ` [PATCH v9 06/10] libxl: add libxl support for setting grant table resource limits Juergen Gross
2017-09-22 13:20   ` Wei Liu
2017-09-22 11:41 ` [PATCH v9 07/10] xen: delay allocation of grant table sub structures Juergen Gross
2017-09-22 14:43   ` Jan Beulich
     [not found]   ` <59C53DB6020000780017EB92@suse.com>
2017-09-22 15:15     ` Juergen Gross
2017-09-22 11:41 ` [PATCH v9 08/10] xen/arm: move arch specific grant table bits into grant_table.c Juergen Gross
2017-09-22 11:41 ` [PATCH v9 09/10] xen: make grant resource limits per domain Juergen Gross
2017-09-22 15:15   ` Jan Beulich
     [not found]   ` <59C54521020000780017EBE2@suse.com>
2017-09-22 17:50     ` Juergen Gross
2017-09-25  7:01       ` Jan Beulich
2017-09-22 11:41 ` [PATCH v9 10/10] xen: add new Xen cpuid node for max address width info Juergen Gross
2017-09-22 14:47   ` Jan Beulich
     [not found]   ` <59C53E91020000780017EB95@suse.com>
2017-09-22 16:27     ` Juergen Gross
2017-09-25  7:06       ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.