* [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains
@ 2019-12-17 20:15 Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 1/4] tools/dombuilder: xc_dom_x86 cleanup Andrew Cooper
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Andrew Cooper @ 2019-12-17 20:15 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Varad Gautam, Ian Jackson, Volodymyr Babchuk
Vastly drop xl's memory usage for HVM (x86 and ARM) guest construction.
See
https://lore.kernel.org/xen-devel/1562159202-11316-1-git-send-email-vrd@amazon.de/T/#u
for the origins of this work, but ultimately I think this is a far cleaner
solution to the problem.
Andrew Cooper (4):
tools/dombuilder: xc_dom_x86 cleanup
tools/dombuilder: Remove PV-only, mandatory hooks
tools/dombuilder: Remove p2m_guest from the common interface
tools/dombuilder: Don't allocate dom->p2m_host[] for translated
domains
stubdom/grub/kexec.c | 36 ++++------
tools/libxc/include/xc_dom.h | 24 +++----
tools/libxc/xc_dom_arm.c | 30 --------
tools/libxc/xc_dom_boot.c | 6 +-
tools/libxc/xc_dom_core.c | 43 +----------
tools/libxc/xc_dom_x86.c | 166 ++++++++++++++++++++++---------------------
6 files changed, 114 insertions(+), 191 deletions(-)
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Xen-devel] [PATCH 1/4] tools/dombuilder: xc_dom_x86 cleanup
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
@ 2019-12-17 20:15 ` Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks Andrew Cooper
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2019-12-17 20:15 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Varad Gautam, Ian Jackson, Volodymyr Babchuk
The two xc_dom_params structures for PV pagetables are never modified and can
live in .rodata. Reduce their scope to the alloc_pgtable_*() functions which
construct xc_dom_image_x86 appropriately.
Rename {alloc,setup}_pgtables() to {alloc,setup}_pgtables_pv() to highlight
that they are PV only, and drop some _x86() suffixes from static helpers.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Varad Gautam <vrd@amazon.de>
---
tools/libxc/xc_dom_x86.c | 60 ++++++++++++++++++++++--------------------------
1 file changed, 28 insertions(+), 32 deletions(-)
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 9e279d6768..1ce3c798ef 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -103,7 +103,7 @@ struct xc_dom_image_x86 {
unsigned n_mappings;
#define MAPPING_MAX 2
struct xc_dom_x86_mapping maps[MAPPING_MAX];
- struct xc_dom_params *params;
+ const struct xc_dom_params *params;
};
/* get guest IO ABI protocol */
@@ -235,7 +235,7 @@ static int count_pgtables(struct xc_dom_image *dom, xen_vaddr_t from,
return 0;
}
-static int alloc_pgtables(struct xc_dom_image *dom)
+static int alloc_pgtables_pv(struct xc_dom_image *dom)
{
int pages, extra_pages;
xen_vaddr_t try_virt_end;
@@ -268,20 +268,20 @@ static int alloc_pgtables(struct xc_dom_image *dom)
/* ------------------------------------------------------------------------ */
/* i386 pagetables */
-static struct xc_dom_params x86_32_params = {
- .levels = PGTBL_LEVELS_I386,
- .vaddr_mask = bits_to_mask(VIRT_BITS_I386),
- .lvl_prot[0] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED,
- .lvl_prot[1] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
- .lvl_prot[2] = _PAGE_PRESENT,
-};
-
static int alloc_pgtables_x86_32_pae(struct xc_dom_image *dom)
{
+ static const struct xc_dom_params x86_32_params = {
+ .levels = PGTBL_LEVELS_I386,
+ .vaddr_mask = bits_to_mask(VIRT_BITS_I386),
+ .lvl_prot[0] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED,
+ .lvl_prot[1] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
+ .lvl_prot[2] = _PAGE_PRESENT,
+ };
struct xc_dom_image_x86 *domx86 = dom->arch_private;
domx86->params = &x86_32_params;
- return alloc_pgtables(dom);
+
+ return alloc_pgtables_pv(dom);
}
#define pfn_to_paddr(pfn) ((xen_paddr_t)(pfn) << PAGE_SHIFT_X86)
@@ -355,7 +355,7 @@ static xen_pfn_t move_l3_below_4G(struct xc_dom_image *dom,
return l3mfn;
}
-static x86_pgentry_t *get_pg_table_x86(struct xc_dom_image *dom, int m, int l)
+static x86_pgentry_t *get_pg_table(struct xc_dom_image *dom, int m, int l)
{
struct xc_dom_image_x86 *domx86 = dom->arch_private;
struct xc_dom_x86_mapping *map;
@@ -371,8 +371,7 @@ static x86_pgentry_t *get_pg_table_x86(struct xc_dom_image *dom, int m, int l)
return NULL;
}
-static x86_pgentry_t get_pg_prot_x86(struct xc_dom_image *dom, int l,
- xen_pfn_t pfn)
+static x86_pgentry_t get_pg_prot(struct xc_dom_image *dom, int l, xen_pfn_t pfn)
{
struct xc_dom_image_x86 *domx86 = dom->arch_private;
struct xc_dom_x86_mapping *map;
@@ -396,7 +395,7 @@ static x86_pgentry_t get_pg_prot_x86(struct xc_dom_image *dom, int l,
return prot;
}
-static int setup_pgtables_x86(struct xc_dom_image *dom)
+static int setup_pgtables_pv(struct xc_dom_image *dom)
{
struct xc_dom_image_x86 *domx86 = dom->arch_private;
struct xc_dom_x86_mapping *map1, *map2;
@@ -413,7 +412,7 @@ static int setup_pgtables_x86(struct xc_dom_image *dom)
map1 = domx86->maps + m1;
from = map1->lvls[l].from;
to = map1->lvls[l].to;
- pg = get_pg_table_x86(dom, m1, l);
+ pg = get_pg_table(dom, m1, l);
if ( !pg )
return -1;
for ( m2 = 0; m2 < domx86->n_mappings; m2++ )
@@ -433,7 +432,7 @@ static int setup_pgtables_x86(struct xc_dom_image *dom)
for ( p = p_s; p <= p_e; p++ )
{
pg[p] = pfn_to_paddr(xc_dom_p2m(dom, pfn)) |
- get_pg_prot_x86(dom, l, pfn);
+ get_pg_prot(dom, l, pfn);
pfn++;
}
}
@@ -464,32 +463,32 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
}
}
- return setup_pgtables_x86(dom);
+ return setup_pgtables_pv(dom);
}
/* ------------------------------------------------------------------------ */
/* x86_64 pagetables */
-static struct xc_dom_params x86_64_params = {
- .levels = PGTBL_LEVELS_X86_64,
- .vaddr_mask = bits_to_mask(VIRT_BITS_X86_64),
- .lvl_prot[0] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED,
- .lvl_prot[1] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
- .lvl_prot[2] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
- .lvl_prot[3] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
-};
-
static int alloc_pgtables_x86_64(struct xc_dom_image *dom)
{
+ const static struct xc_dom_params x86_64_params = {
+ .levels = PGTBL_LEVELS_X86_64,
+ .vaddr_mask = bits_to_mask(VIRT_BITS_X86_64),
+ .lvl_prot[0] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED,
+ .lvl_prot[1] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
+ .lvl_prot[2] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
+ .lvl_prot[3] = _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY|_PAGE_USER,
+ };
struct xc_dom_image_x86 *domx86 = dom->arch_private;
domx86->params = &x86_64_params;
- return alloc_pgtables(dom);
+
+ return alloc_pgtables_pv(dom);
}
static int setup_pgtables_x86_64(struct xc_dom_image *dom)
{
- return setup_pgtables_x86(dom);
+ return setup_pgtables_pv(dom);
}
/* ------------------------------------------------------------------------ */
@@ -1908,9 +1907,6 @@ static struct xc_dom_arch xc_hvm_32 = {
.sizeof_pfn = 4,
.alloc_magic_pages = alloc_magic_pages_hvm,
.alloc_pgtables = alloc_pgtables_hvm,
- .setup_pgtables = NULL,
- .start_info = NULL,
- .shared_info = NULL,
.vcpu = vcpu_hvm,
.meminit = meminit_hvm,
.bootearly = bootearly,
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 1/4] tools/dombuilder: xc_dom_x86 cleanup Andrew Cooper
@ 2019-12-17 20:15 ` Andrew Cooper
2019-12-23 18:12 ` Julien Grall
2019-12-17 20:15 ` [Xen-devel] [PATCH 3/4] tools/dombuilder: Remove p2m_guest from the common interface Andrew Cooper
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2019-12-17 20:15 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Varad Gautam, Ian Jackson, Volodymyr Babchuk
Currently, the setup_pgtable() hook is optional, but alloc_pgtable() hook is
not. Both are specific to x86 PV guests, and stubbed in various ways by the
dombuilders for translated guests (x86 HVM, ARM).
Make alloc_pgtables() optional, and drop all the stubs for translated guest
types.
No change in the constructed guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Varad Gautam <vrd@amazon.de>
---
tools/libxc/include/xc_dom.h | 3 ++-
tools/libxc/xc_dom_arm.c | 21 ---------------------
tools/libxc/xc_dom_boot.c | 6 +++---
tools/libxc/xc_dom_core.c | 3 ++-
tools/libxc/xc_dom_x86.c | 7 -------
5 files changed, 7 insertions(+), 33 deletions(-)
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 5900bbe8fa..9ff1cb8b07 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -253,8 +253,9 @@ void xc_dom_register_loader(struct xc_dom_loader *loader);
/* --- arch specific hooks ----------------------------------------- */
struct xc_dom_arch {
- /* pagetable setup */
int (*alloc_magic_pages) (struct xc_dom_image * dom);
+
+ /* pagetable setup - x86 PV only */
int (*alloc_pgtables) (struct xc_dom_image * dom);
int (*alloc_p2m_list) (struct xc_dom_image * dom);
int (*setup_pgtables) (struct xc_dom_image * dom);
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 5b9eca6087..7e0fb9169f 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -47,23 +47,6 @@ const char *xc_domain_get_native_protocol(xc_interface *xch,
}
/* ------------------------------------------------------------------------ */
-/*
- * arm guests are hybrid and start off with paging disabled, therefore no
- * pagetables and nothing to do here.
- */
-static int alloc_pgtables_arm(struct xc_dom_image *dom)
-{
- DOMPRINTF_CALLED(dom->xch);
- return 0;
-}
-
-static int setup_pgtables_arm(struct xc_dom_image *dom)
-{
- DOMPRINTF_CALLED(dom->xch);
- return 0;
-}
-
-/* ------------------------------------------------------------------------ */
static int alloc_magic_pages(struct xc_dom_image *dom)
{
@@ -539,8 +522,6 @@ static struct xc_dom_arch xc_dom_32 = {
.page_shift = PAGE_SHIFT_ARM,
.sizeof_pfn = 8,
.alloc_magic_pages = alloc_magic_pages,
- .alloc_pgtables = alloc_pgtables_arm,
- .setup_pgtables = setup_pgtables_arm,
.start_info = start_info_arm,
.shared_info = shared_info_arm,
.vcpu = vcpu_arm32,
@@ -555,8 +536,6 @@ static struct xc_dom_arch xc_dom_64 = {
.page_shift = PAGE_SHIFT_ARM,
.sizeof_pfn = 8,
.alloc_magic_pages = alloc_magic_pages,
- .alloc_pgtables = alloc_pgtables_arm,
- .setup_pgtables = setup_pgtables_arm,
.start_info = start_info_arm,
.shared_info = shared_info_arm,
.vcpu = vcpu_arm64,
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 918ee4d045..79dbbf6571 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -199,9 +199,9 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
/* initial mm setup */
if ( (rc = xc_dom_update_guest_p2m(dom)) != 0 )
return rc;
- if ( dom->arch_hooks->setup_pgtables )
- if ( (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
- return rc;
+ if ( dom->arch_hooks->setup_pgtables &&
+ (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
+ return rc;
/* start info page */
if ( dom->arch_hooks->start_info )
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 9bd04cb2d5..fc77804a7e 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -1247,7 +1247,8 @@ int xc_dom_build_image(struct xc_dom_image *dom)
goto err;
if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 )
goto err;
- if ( dom->arch_hooks->alloc_pgtables(dom) != 0 )
+ if ( dom->arch_hooks->alloc_pgtables &&
+ dom->arch_hooks->alloc_pgtables(dom) != 0 )
goto err;
if ( dom->alloc_bootstack )
{
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 1ce3c798ef..d2acff1061 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1690,12 +1690,6 @@ static int bootlate_pv(struct xc_dom_image *dom)
return 0;
}
-static int alloc_pgtables_hvm(struct xc_dom_image *dom)
-{
- DOMPRINTF("%s: doing nothing", __func__);
- return 0;
-}
-
/*
* The memory layout of the start_info page and the modules, and where the
* addresses are stored:
@@ -1906,7 +1900,6 @@ static struct xc_dom_arch xc_hvm_32 = {
.page_shift = PAGE_SHIFT_X86,
.sizeof_pfn = 4,
.alloc_magic_pages = alloc_magic_pages_hvm,
- .alloc_pgtables = alloc_pgtables_hvm,
.vcpu = vcpu_hvm,
.meminit = meminit_hvm,
.bootearly = bootearly,
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Xen-devel] [PATCH 3/4] tools/dombuilder: Remove p2m_guest from the common interface
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 1/4] tools/dombuilder: xc_dom_x86 cleanup Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks Andrew Cooper
@ 2019-12-17 20:15 ` Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-31 16:37 ` [Xen-devel] [PATCH 0/4] " Wei Liu
4 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2019-12-17 20:15 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Varad Gautam, Ian Jackson, Volodymyr Babchuk
In-guest p2m's are a concept specific to x86 PV guests. alloc_p2m_list() is
the only hook which initialises dom->p2m_guest, making
xc_dom_update_guest_p2m() a nop for non-PV guests.
Move p2m_guest into xc_dom_image_x86 and adjust alloc_p2m_list() to match.
Drop xc_dom_update_guest_p2m() entirely.
One caller, move_l3_below_4G(), only uses it to modify a single entry, so
rewriting the whole guest p2m is wasteful - opencode the single update
instead. The other caller is common code. Instead, move the logic into the
setup_pgtables() hooks, which know their own sizeof_pfn and can do away with
the switch statement.
No change in the constructed guests.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Varad Gautam <vrd@amazon.de>
---
stubdom/grub/kexec.c | 8 --------
tools/libxc/include/xc_dom.h | 2 --
tools/libxc/xc_dom_boot.c | 2 --
tools/libxc/xc_dom_core.c | 40 ----------------------------------------
tools/libxc/xc_dom_x86.c | 41 +++++++++++++++++++++++++++++++++++------
5 files changed, 35 insertions(+), 58 deletions(-)
diff --git a/stubdom/grub/kexec.c b/stubdom/grub/kexec.c
index 61ca082d42..10891eabcc 100644
--- a/stubdom/grub/kexec.c
+++ b/stubdom/grub/kexec.c
@@ -320,14 +320,6 @@ void kexec(void *kernel, long kernel_size, void *module, long module_size, char
do_exchange(dom, PHYS_PFN(_boot_target - dom->parms.virt_base),
virt_to_mfn(&_boot_page));
- /* Make sure the bootstrap page table does not RW-map any of our current
- * page table frames */
- if ( (rc = xc_dom_update_guest_p2m(dom))) {
- printk("xc_dom_update_guest_p2m returned %d\n", rc);
- errnum = ERR_BOOT_FAILURE;
- goto out;
- }
-
if ( dom->arch_hooks->setup_pgtables )
if ( (rc = dom->arch_hooks->setup_pgtables(dom))) {
printk("setup_pgtables returned %d\n", rc);
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 9ff1cb8b07..b7d0faf7e1 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -133,7 +133,6 @@ struct xc_dom_image {
* Note that the input is offset by rambase.
*/
xen_pfn_t *p2m_host;
- void *p2m_guest;
/* physical memory
*
@@ -331,7 +330,6 @@ int xc_dom_devicetree_mem(struct xc_dom_image *dom, const void *mem,
int xc_dom_parse_image(struct xc_dom_image *dom);
int xc_dom_set_arch_hooks(struct xc_dom_image *dom);
int xc_dom_build_image(struct xc_dom_image *dom);
-int xc_dom_update_guest_p2m(struct xc_dom_image *dom);
int xc_dom_boot_xen_init(struct xc_dom_image *dom, xc_interface *xch,
uint32_t domid);
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index 79dbbf6571..bb599b33ba 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -197,8 +197,6 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
return -1;
/* initial mm setup */
- if ( (rc = xc_dom_update_guest_p2m(dom)) != 0 )
- return rc;
if ( dom->arch_hooks->setup_pgtables &&
(rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
return rc;
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index fc77804a7e..f30c73b5e8 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -969,46 +969,6 @@ int xc_dom_mem_init(struct xc_dom_image *dom, unsigned int mem_mb)
return 0;
}
-int xc_dom_update_guest_p2m(struct xc_dom_image *dom)
-{
- uint32_t *p2m_32;
- uint64_t *p2m_64;
- xen_pfn_t i;
-
- if ( !dom->p2m_guest )
- return 0;
-
- switch ( dom->arch_hooks->sizeof_pfn )
- {
- case 4:
- DOMPRINTF("%s: dst 32bit, pages 0x%" PRIpfn "",
- __FUNCTION__, dom->p2m_size);
- p2m_32 = dom->p2m_guest;
- for ( i = 0; i < dom->p2m_size; i++ )
- if ( dom->p2m_host[i] != INVALID_PFN )
- p2m_32[i] = dom->p2m_host[i];
- else
- p2m_32[i] = (uint32_t) - 1;
- break;
- case 8:
- DOMPRINTF("%s: dst 64bit, pages 0x%" PRIpfn "",
- __FUNCTION__, dom->p2m_size);
- p2m_64 = dom->p2m_guest;
- for ( i = 0; i < dom->p2m_size; i++ )
- if ( dom->p2m_host[i] != INVALID_PFN )
- p2m_64[i] = dom->p2m_host[i];
- else
- p2m_64[i] = (uint64_t) - 1;
- break;
- default:
- xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
- "sizeof_pfn is invalid (is %d, can be 4 or 8)",
- dom->arch_hooks->sizeof_pfn);
- return -1;
- }
- return 0;
-}
-
static int xc_dom_build_module(struct xc_dom_image *dom, unsigned int mod)
{
size_t unziplen, modulelen;
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index d2acff1061..f21662c8b9 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -104,6 +104,9 @@ struct xc_dom_image_x86 {
#define MAPPING_MAX 2
struct xc_dom_x86_mapping maps[MAPPING_MAX];
const struct xc_dom_params *params;
+
+ /* PV: Pointer to the in-guest P2M. */
+ void *p2m_guest;
};
/* get guest IO ABI protocol */
@@ -296,6 +299,8 @@ static xen_pfn_t move_l3_below_4G(struct xc_dom_image *dom,
xen_pfn_t l3pfn,
xen_pfn_t l3mfn)
{
+ struct xc_dom_image_x86 *domx86 = dom->arch_private;
+ uint32_t *p2m_guest = domx86->p2m_guest;
xen_pfn_t new_l3mfn;
struct xc_mmu *mmu;
void *l3tab;
@@ -313,9 +318,7 @@ static xen_pfn_t move_l3_below_4G(struct xc_dom_image *dom,
if ( !new_l3mfn )
goto out;
- dom->p2m_host[l3pfn] = new_l3mfn;
- if ( xc_dom_update_guest_p2m(dom) != 0 )
- goto out;
+ p2m_guest[l3pfn] = dom->p2m_host[l3pfn] = new_l3mfn;
if ( xc_add_mmu_update(dom->xch, mmu,
(((unsigned long long)new_l3mfn)
@@ -444,7 +447,17 @@ static int setup_pgtables_pv(struct xc_dom_image *dom)
static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
{
struct xc_dom_image_x86 *domx86 = dom->arch_private;
- xen_pfn_t l3mfn, l3pfn;
+ uint32_t *p2m_guest = domx86->p2m_guest;
+ xen_pfn_t l3mfn, l3pfn, i;
+
+ /* Copy dom->p2m_host[] into the guest. */
+ for ( i = 0; i < dom->p2m_size; ++i )
+ {
+ if ( dom->p2m_host[i] != INVALID_PFN )
+ p2m_guest[i] = dom->p2m_host[i];
+ else
+ p2m_guest[i] = -1;
+ }
l3pfn = domx86->maps[0].lvls[2].pfn;
l3mfn = xc_dom_p2m(dom, l3pfn);
@@ -488,6 +501,19 @@ static int alloc_pgtables_x86_64(struct xc_dom_image *dom)
static int setup_pgtables_x86_64(struct xc_dom_image *dom)
{
+ struct xc_dom_image_x86 *domx86 = dom->arch_private;
+ uint64_t *p2m_guest = domx86->p2m_guest;
+ xen_pfn_t i;
+
+ /* Copy dom->p2m_host[] into the guest. */
+ for ( i = 0; i < dom->p2m_size; ++i )
+ {
+ if ( dom->p2m_host[i] != INVALID_PFN )
+ p2m_guest[i] = dom->p2m_host[i];
+ else
+ p2m_guest[i] = -1;
+ }
+
return setup_pgtables_pv(dom);
}
@@ -495,11 +521,14 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
static int alloc_p2m_list(struct xc_dom_image *dom, size_t p2m_alloc_size)
{
+ struct xc_dom_image_x86 *domx86 = dom->arch_private;
+
if ( xc_dom_alloc_segment(dom, &dom->p2m_seg, "phys2mach",
0, p2m_alloc_size) )
return -1;
- dom->p2m_guest = xc_dom_seg_to_ptr(dom, &dom->p2m_seg);
- if ( dom->p2m_guest == NULL )
+
+ domx86->p2m_guest = xc_dom_seg_to_ptr(dom, &dom->p2m_seg);
+ if ( domx86->p2m_guest == NULL )
return -1;
return 0;
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
` (2 preceding siblings ...)
2019-12-17 20:15 ` [Xen-devel] [PATCH 3/4] tools/dombuilder: Remove p2m_guest from the common interface Andrew Cooper
@ 2019-12-17 20:15 ` Andrew Cooper
2019-12-23 18:23 ` Julien Grall
2020-01-03 14:25 ` Jan Beulich
2019-12-31 16:37 ` [Xen-devel] [PATCH 0/4] " Wei Liu
4 siblings, 2 replies; 14+ messages in thread
From: Andrew Cooper @ 2019-12-17 20:15 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Varad Gautam, Ian Jackson, Volodymyr Babchuk
xc_dom_p2m() and dom->p2m_host[] implement a linear transform for translated
domains, but waste a substantial chunk of RAM doing so.
ARM literally never reads dom->p2m_host[] (because of the xc_dom_translated()
short circuit in xc_dom_p2m()). Drop it all.
x86 HVM does use dom->p2m_host[] for xc_domain_populate_physmap_exact() calls
when populating 4k pages. Reuse the same tactic from 2M/1G ranges and use an
on-stack array instead. Drop the memory allocation.
x86 PV guests do use dom->p2m_host[] as a non-identity transform. Rename the
field to pv_p2m to make it clear it is PV-only.
No change in the constructed guests.
Reported-by: Varad Gautam <vrd@amazon.de>
Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Varad Gautam <vrd@amazon.de>
---
stubdom/grub/kexec.c | 28 ++++++++---------
tools/libxc/include/xc_dom.h | 19 ++++++------
tools/libxc/xc_dom_arm.c | 9 ------
tools/libxc/xc_dom_x86.c | 72 ++++++++++++++++++--------------------------
4 files changed, 52 insertions(+), 76 deletions(-)
diff --git a/stubdom/grub/kexec.c b/stubdom/grub/kexec.c
index 10891eabcc..0e68b969a2 100644
--- a/stubdom/grub/kexec.c
+++ b/stubdom/grub/kexec.c
@@ -87,17 +87,17 @@ static void do_exchange(struct xc_dom_image *dom, xen_pfn_t target_pfn, xen_pfn_
xen_pfn_t target_mfn;
for (source_pfn = 0; source_pfn < start_info.nr_pages; source_pfn++)
- if (dom->p2m_host[source_pfn] == source_mfn)
+ if (dom->pv_p2m[source_pfn] == source_mfn)
break;
ASSERT(source_pfn < start_info.nr_pages);
- target_mfn = dom->p2m_host[target_pfn];
+ target_mfn = dom->pv_p2m[target_pfn];
/* Put target MFN at source PFN */
- dom->p2m_host[source_pfn] = target_mfn;
+ dom->pv_p2m[source_pfn] = target_mfn;
/* Put source MFN at target PFN */
- dom->p2m_host[target_pfn] = source_mfn;
+ dom->pv_p2m[target_pfn] = source_mfn;
}
int kexec_allocate(struct xc_dom_image *dom)
@@ -110,7 +110,7 @@ int kexec_allocate(struct xc_dom_image *dom)
pages_moved2pfns = realloc(pages_moved2pfns, new_allocated * sizeof(*pages_moved2pfns));
for (i = allocated; i < new_allocated; i++) {
/* Exchange old page of PFN i with a newly allocated page. */
- xen_pfn_t old_mfn = dom->p2m_host[i];
+ xen_pfn_t old_mfn = dom->pv_p2m[i];
xen_pfn_t new_pfn;
xen_pfn_t new_mfn;
@@ -122,7 +122,7 @@ int kexec_allocate(struct xc_dom_image *dom)
/*
* If PFN of newly allocated page (new_pfn) is less then currently
* requested PFN (i) then look for relevant PFN/MFN pair. In this
- * situation dom->p2m_host[new_pfn] no longer contains proper MFN
+ * situation dom->pv_p2m[new_pfn] no longer contains proper MFN
* because original page with new_pfn was moved earlier
* to different location.
*/
@@ -132,10 +132,10 @@ int kexec_allocate(struct xc_dom_image *dom)
pages_moved2pfns[i] = new_pfn;
/* Put old page at new PFN */
- dom->p2m_host[new_pfn] = old_mfn;
+ dom->pv_p2m[new_pfn] = old_mfn;
/* Put new page at PFN i */
- dom->p2m_host[i] = new_mfn;
+ dom->pv_p2m[i] = new_mfn;
}
allocated = new_allocated;
@@ -282,11 +282,11 @@ void kexec(void *kernel, long kernel_size, void *module, long module_size, char
dom->p2m_size = dom->total_pages;
/* setup initial p2m */
- dom->p2m_host = malloc(sizeof(*dom->p2m_host) * dom->p2m_size);
+ dom->pv_p2m = malloc(sizeof(*dom->pv_p2m) * dom->p2m_size);
/* Start with our current P2M */
for (i = 0; i < dom->p2m_size; i++)
- dom->p2m_host[i] = pfn_to_mfn(i);
+ dom->pv_p2m[i] = pfn_to_mfn(i);
if ( (rc = xc_dom_build_image(dom)) != 0 ) {
printk("xc_dom_build_image returned %d\n", rc);
@@ -373,7 +373,7 @@ void kexec(void *kernel, long kernel_size, void *module, long module_size, char
_boot_oldpdmfn = virt_to_mfn(start_info.pt_base);
DEBUG("boot old pd mfn %lx\n", _boot_oldpdmfn);
DEBUG("boot pd virt %lx\n", dom->pgtables_seg.vstart);
- _boot_pdmfn = dom->p2m_host[PHYS_PFN(dom->pgtables_seg.vstart - dom->parms.virt_base)];
+ _boot_pdmfn = dom->pv_p2m[PHYS_PFN(dom->pgtables_seg.vstart - dom->parms.virt_base)];
DEBUG("boot pd mfn %lx\n", _boot_pdmfn);
_boot_stack = _boot_target + PAGE_SIZE;
DEBUG("boot stack %lx\n", _boot_stack);
@@ -384,13 +384,13 @@ void kexec(void *kernel, long kernel_size, void *module, long module_size, char
/* Keep only useful entries */
for (nr_m2p_updates = pfn = 0; pfn < start_info.nr_pages; pfn++)
- if (dom->p2m_host[pfn] != pfn_to_mfn(pfn))
+ if (dom->pv_p2m[pfn] != pfn_to_mfn(pfn))
nr_m2p_updates++;
m2p_updates = malloc(sizeof(*m2p_updates) * nr_m2p_updates);
for (i = pfn = 0; pfn < start_info.nr_pages; pfn++)
- if (dom->p2m_host[pfn] != pfn_to_mfn(pfn)) {
- m2p_updates[i].ptr = PFN_PHYS(dom->p2m_host[pfn]) | MMU_MACHPHYS_UPDATE;
+ if (dom->pv_p2m[pfn] != pfn_to_mfn(pfn)) {
+ m2p_updates[i].ptr = PFN_PHYS(dom->pv_p2m[pfn]) | MMU_MACHPHYS_UPDATE;
m2p_updates[i].val = pfn;
i++;
}
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index b7d0faf7e1..d2e316f35e 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -123,16 +123,12 @@ struct xc_dom_image {
/* other state info */
uint32_t f_active[XENFEAT_NR_SUBMAPS];
+
/*
- * p2m_host maps guest physical addresses an offset from
- * rambase_pfn (see below) into gfns.
- *
- * For a pure PV guest this means that it maps GPFNs into MFNs for
- * a hybrid guest this means that it maps GPFNs to GPFNS.
- *
- * Note that the input is offset by rambase.
+ * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs. It is
+ * eventually copied into guest context.
*/
- xen_pfn_t *p2m_host;
+ xen_pfn_t *pv_p2m;
/* physical memory
*
@@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image *dom, xen_pfn_t pfn)
{
if ( xc_dom_translated(dom) )
return pfn;
- if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages)
+
+ /* x86 PV only now. */
+ if ( pfn >= dom->total_pages )
return INVALID_MFN;
- return dom->p2m_host[pfn - dom->rambase_pfn];
+
+ return dom->pv_p2m[pfn];
}
#endif /* _XC_DOM_H */
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 7e0fb9169f..931404c222 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -348,9 +348,6 @@ static int populate_guest_memory(struct xc_dom_image *dom,
}
}
- for ( pfn = 0; pfn < nr_pfns; pfn++ )
- dom->p2m_host[pfn] = base_pfn + pfn;
-
out:
free(extents);
return rc < 0 ? rc : 0;
@@ -359,7 +356,6 @@ static int populate_guest_memory(struct xc_dom_image *dom,
static int meminit(struct xc_dom_image *dom)
{
int i, rc;
- xen_pfn_t pfn;
uint64_t modbase;
uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
@@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
assert(ramsize == 0); /* Too much RAM is rejected above */
dom->p2m_size = p2m_size;
- dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
- if ( dom->p2m_host == NULL )
- return -EINVAL;
- for ( pfn = 0; pfn < p2m_size; pfn++ )
- dom->p2m_host[pfn] = INVALID_PFN;
/* setup initial p2m and allocate guest memory */
for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ )
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index f21662c8b9..819afcb03f 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -318,7 +318,7 @@ static xen_pfn_t move_l3_below_4G(struct xc_dom_image *dom,
if ( !new_l3mfn )
goto out;
- p2m_guest[l3pfn] = dom->p2m_host[l3pfn] = new_l3mfn;
+ p2m_guest[l3pfn] = dom->pv_p2m[l3pfn] = new_l3mfn;
if ( xc_add_mmu_update(dom->xch, mmu,
(((unsigned long long)new_l3mfn)
@@ -450,11 +450,11 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
uint32_t *p2m_guest = domx86->p2m_guest;
xen_pfn_t l3mfn, l3pfn, i;
- /* Copy dom->p2m_host[] into the guest. */
+ /* Copy dom->pv_p2m[] into the guest. */
for ( i = 0; i < dom->p2m_size; ++i )
{
- if ( dom->p2m_host[i] != INVALID_PFN )
- p2m_guest[i] = dom->p2m_host[i];
+ if ( dom->pv_p2m[i] != INVALID_PFN )
+ p2m_guest[i] = dom->pv_p2m[i];
else
p2m_guest[i] = -1;
}
@@ -505,11 +505,11 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
uint64_t *p2m_guest = domx86->p2m_guest;
xen_pfn_t i;
- /* Copy dom->p2m_host[] into the guest. */
+ /* Copy dom->pv_p2m[] into the guest. */
for ( i = 0; i < dom->p2m_size; ++i )
{
- if ( dom->p2m_host[i] != INVALID_PFN )
- p2m_guest[i] = dom->p2m_host[i];
+ if ( dom->pv_p2m[i] != INVALID_PFN )
+ p2m_guest[i] = dom->pv_p2m[i];
else
p2m_guest[i] = -1;
}
@@ -1245,11 +1245,11 @@ static int meminit_pv(struct xc_dom_image *dom)
return -EINVAL;
}
- dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
- if ( dom->p2m_host == NULL )
+ dom->pv_p2m = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
+ if ( dom->pv_p2m == NULL )
return -EINVAL;
for ( pfn = 0; pfn < dom->p2m_size; pfn++ )
- dom->p2m_host[pfn] = INVALID_PFN;
+ dom->pv_p2m[pfn] = INVALID_PFN;
/* allocate guest memory */
for ( i = 0; i < nr_vmemranges; i++ )
@@ -1269,7 +1269,7 @@ static int meminit_pv(struct xc_dom_image *dom)
pfn_base = vmemranges[i].start >> PAGE_SHIFT;
for ( pfn = pfn_base; pfn < pfn_base+pages; pfn++ )
- dom->p2m_host[pfn] = pfn;
+ dom->pv_p2m[pfn] = pfn;
pfn_base_idx = pfn_base;
while ( super_pages ) {
@@ -1279,7 +1279,7 @@ static int meminit_pv(struct xc_dom_image *dom)
for ( pfn = pfn_base_idx, j = 0;
pfn < pfn_base_idx + (count << SUPERPAGE_2MB_SHIFT);
pfn += SUPERPAGE_2MB_NR_PFNS, j++ )
- extents[j] = dom->p2m_host[pfn];
+ extents[j] = dom->pv_p2m[pfn];
rc = xc_domain_populate_physmap(dom->xch, dom->guest_domid, count,
SUPERPAGE_2MB_SHIFT, memflags,
extents);
@@ -1292,7 +1292,7 @@ static int meminit_pv(struct xc_dom_image *dom)
{
mfn = extents[j];
for ( k = 0; k < SUPERPAGE_2MB_NR_PFNS; k++, pfn++ )
- dom->p2m_host[pfn] = mfn + k;
+ dom->pv_p2m[pfn] = mfn + k;
}
pfn_base_idx = pfn;
}
@@ -1301,7 +1301,7 @@ static int meminit_pv(struct xc_dom_image *dom)
{
allocsz = min_t(uint64_t, 1024 * 1024, pages - j);
rc = xc_domain_populate_physmap_exact(dom->xch, dom->guest_domid,
- allocsz, 0, memflags, &dom->p2m_host[pfn_base + j]);
+ allocsz, 0, memflags, &dom->pv_p2m[pfn_base + j]);
if ( rc )
{
@@ -1428,25 +1428,6 @@ static int meminit_hvm(struct xc_dom_image *dom)
}
dom->p2m_size = p2m_size;
- dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) *
- dom->p2m_size);
- if ( dom->p2m_host == NULL )
- {
- DOMPRINTF("Could not allocate p2m");
- goto error_out;
- }
-
- for ( i = 0; i < p2m_size; i++ )
- dom->p2m_host[i] = ((xen_pfn_t)-1);
- for ( vmemid = 0; vmemid < nr_vmemranges; vmemid++ )
- {
- uint64_t pfn;
-
- for ( pfn = vmemranges[vmemid].start >> PAGE_SHIFT;
- pfn < vmemranges[vmemid].end >> PAGE_SHIFT;
- pfn++ )
- dom->p2m_host[pfn] = pfn;
- }
/*
* Try to claim pages for early warning of insufficient memory available.
@@ -1488,14 +1469,16 @@ static int meminit_hvm(struct xc_dom_image *dom)
* We attempt to allocate 1GB pages if possible. It falls back on 2MB
* pages if 1GB allocation fails. 4KB pages will be used eventually if
* both fail.
- *
- * Under 2MB mode, we allocate pages in batches of no more than 8MB to
- * ensure that we can be preempted and hence dom0 remains responsive.
*/
if ( dom->device_model )
{
+ xen_pfn_t extents[0xa0];
+
+ for ( i = 0; i < ARRAY_SIZE(extents); ++i )
+ extents[i] = i;
+
rc = xc_domain_populate_physmap_exact(
- xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
+ xch, domid, 0xa0, 0, memflags, extents);
if ( rc != 0 )
{
DOMPRINTF("Could not populate low memory (< 0xA0).\n");
@@ -1538,7 +1521,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
if ( count > max_pages )
count = max_pages;
- cur_pfn = dom->p2m_host[cur_pages];
+ cur_pfn = cur_pages;
/* Take care the corner cases of super page tails */
if ( ((cur_pfn & (SUPERPAGE_1GB_NR_PFNS-1)) != 0) &&
@@ -1564,8 +1547,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
xen_pfn_t sp_extents[nr_extents];
for ( i = 0; i < nr_extents; i++ )
- sp_extents[i] =
- dom->p2m_host[cur_pages+(i<<SUPERPAGE_1GB_SHIFT)];
+ sp_extents[i] = cur_pages + (i << SUPERPAGE_1GB_SHIFT);
done = xc_domain_populate_physmap(xch, domid, nr_extents,
SUPERPAGE_1GB_SHIFT,
@@ -1604,8 +1586,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
xen_pfn_t sp_extents[nr_extents];
for ( i = 0; i < nr_extents; i++ )
- sp_extents[i] =
- dom->p2m_host[cur_pages+(i<<SUPERPAGE_2MB_SHIFT)];
+ sp_extents[i] = cur_pages + (i << SUPERPAGE_2MB_SHIFT);
done = xc_domain_populate_physmap(xch, domid, nr_extents,
SUPERPAGE_2MB_SHIFT,
@@ -1624,8 +1605,13 @@ static int meminit_hvm(struct xc_dom_image *dom)
/* Fall back to 4kB extents. */
if ( count != 0 )
{
+ xen_pfn_t extents[count];
+
+ for ( i = 0; i < count; ++i )
+ extents[i] = cur_pages + i;
+
rc = xc_domain_populate_physmap_exact(
- xch, domid, count, 0, new_memflags, &dom->p2m_host[cur_pages]);
+ xch, domid, count, 0, new_memflags, extents);
cur_pages += count;
stat_normal_pages += count;
}
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks
2019-12-17 20:15 ` [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks Andrew Cooper
@ 2019-12-23 18:12 ` Julien Grall
0 siblings, 0 replies; 14+ messages in thread
From: Julien Grall @ 2019-12-23 18:12 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Ian Jackson, Varad Gautam, Stefano Stabellini, Volodymyr Babchuk,
Wei Liu
Hi Andrew,
On 17/12/2019 21:15, Andrew Cooper wrote:
> Currently, the setup_pgtable() hook is optional, but alloc_pgtable() hook is
> not. Both are specific to x86 PV guests, and stubbed in various ways by the
> dombuilders for translated guests (x86 HVM, ARM).
>
> Make alloc_pgtables() optional, and drop all the stubs for translated guest
> types.
>
> No change in the constructed guests.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien@xen.org>
Cheers,
> ---
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Varad Gautam <vrd@amazon.de>
> ---
> tools/libxc/include/xc_dom.h | 3 ++-
> tools/libxc/xc_dom_arm.c | 21 ---------------------
> tools/libxc/xc_dom_boot.c | 6 +++---
> tools/libxc/xc_dom_core.c | 3 ++-
> tools/libxc/xc_dom_x86.c | 7 -------
> 5 files changed, 7 insertions(+), 33 deletions(-)
>
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 5900bbe8fa..9ff1cb8b07 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -253,8 +253,9 @@ void xc_dom_register_loader(struct xc_dom_loader *loader);
> /* --- arch specific hooks ----------------------------------------- */
>
> struct xc_dom_arch {
> - /* pagetable setup */
> int (*alloc_magic_pages) (struct xc_dom_image * dom);
> +
> + /* pagetable setup - x86 PV only */
> int (*alloc_pgtables) (struct xc_dom_image * dom);
> int (*alloc_p2m_list) (struct xc_dom_image * dom);
> int (*setup_pgtables) (struct xc_dom_image * dom);
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 5b9eca6087..7e0fb9169f 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -47,23 +47,6 @@ const char *xc_domain_get_native_protocol(xc_interface *xch,
> }
>
> /* ------------------------------------------------------------------------ */
> -/*
> - * arm guests are hybrid and start off with paging disabled, therefore no
> - * pagetables and nothing to do here.
> - */
> -static int alloc_pgtables_arm(struct xc_dom_image *dom)
> -{
> - DOMPRINTF_CALLED(dom->xch);
> - return 0;
> -}
> -
> -static int setup_pgtables_arm(struct xc_dom_image *dom)
> -{
> - DOMPRINTF_CALLED(dom->xch);
> - return 0;
> -}
> -
> -/* ------------------------------------------------------------------------ */
>
> static int alloc_magic_pages(struct xc_dom_image *dom)
> {
> @@ -539,8 +522,6 @@ static struct xc_dom_arch xc_dom_32 = {
> .page_shift = PAGE_SHIFT_ARM,
> .sizeof_pfn = 8,
> .alloc_magic_pages = alloc_magic_pages,
> - .alloc_pgtables = alloc_pgtables_arm,
> - .setup_pgtables = setup_pgtables_arm,
> .start_info = start_info_arm,
> .shared_info = shared_info_arm,
> .vcpu = vcpu_arm32,
> @@ -555,8 +536,6 @@ static struct xc_dom_arch xc_dom_64 = {
> .page_shift = PAGE_SHIFT_ARM,
> .sizeof_pfn = 8,
> .alloc_magic_pages = alloc_magic_pages,
> - .alloc_pgtables = alloc_pgtables_arm,
> - .setup_pgtables = setup_pgtables_arm,
> .start_info = start_info_arm,
> .shared_info = shared_info_arm,
> .vcpu = vcpu_arm64,
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index 918ee4d045..79dbbf6571 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -199,9 +199,9 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
> /* initial mm setup */
> if ( (rc = xc_dom_update_guest_p2m(dom)) != 0 )
> return rc;
> - if ( dom->arch_hooks->setup_pgtables )
> - if ( (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
> - return rc;
> + if ( dom->arch_hooks->setup_pgtables &&
> + (rc = dom->arch_hooks->setup_pgtables(dom)) != 0 )
> + return rc;
>
> /* start info page */
> if ( dom->arch_hooks->start_info )
> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> index 9bd04cb2d5..fc77804a7e 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -1247,7 +1247,8 @@ int xc_dom_build_image(struct xc_dom_image *dom)
> goto err;
> if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 )
> goto err;
> - if ( dom->arch_hooks->alloc_pgtables(dom) != 0 )
> + if ( dom->arch_hooks->alloc_pgtables &&
> + dom->arch_hooks->alloc_pgtables(dom) != 0 )
> goto err;
> if ( dom->alloc_bootstack )
> {
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 1ce3c798ef..d2acff1061 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1690,12 +1690,6 @@ static int bootlate_pv(struct xc_dom_image *dom)
> return 0;
> }
>
> -static int alloc_pgtables_hvm(struct xc_dom_image *dom)
> -{
> - DOMPRINTF("%s: doing nothing", __func__);
> - return 0;
> -}
> -
> /*
> * The memory layout of the start_info page and the modules, and where the
> * addresses are stored:
> @@ -1906,7 +1900,6 @@ static struct xc_dom_arch xc_hvm_32 = {
> .page_shift = PAGE_SHIFT_X86,
> .sizeof_pfn = 4,
> .alloc_magic_pages = alloc_magic_pages_hvm,
> - .alloc_pgtables = alloc_pgtables_hvm,
> .vcpu = vcpu_hvm,
> .meminit = meminit_hvm,
> .bootearly = bootearly,
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2019-12-17 20:15 ` [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
@ 2019-12-23 18:23 ` Julien Grall
2020-01-02 17:49 ` Andrew Cooper
2020-01-03 14:25 ` Jan Beulich
1 sibling, 1 reply; 14+ messages in thread
From: Julien Grall @ 2019-12-23 18:23 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Ian Jackson, Varad Gautam, Stefano Stabellini, Volodymyr Babchuk,
Wei Liu
Hi,
On 17/12/2019 21:15, Andrew Cooper wrote:
> xc_dom_p2m() and dom->p2m_host[] implement a linear transform for translated
> domains, but waste a substantial chunk of RAM doing so.
>
> ARM literally never reads dom->p2m_host[] (because of the xc_dom_translated()
> short circuit in xc_dom_p2m()). Drop it all.
>
> x86 HVM does use dom->p2m_host[] for xc_domain_populate_physmap_exact() calls
> when populating 4k pages. Reuse the same tactic from 2M/1G ranges and use an
> on-stack array instead. Drop the memory allocation.
>
> x86 PV guests do use dom->p2m_host[] as a non-identity transform. Rename the
> field to pv_p2m to make it clear it is PV-only.
Nice cleanup! This will probably make slightly faster guest boot :).
> @@ -359,7 +356,6 @@ static int populate_guest_memory(struct xc_dom_image *dom,
> static int meminit(struct xc_dom_image *dom)
> {
> int i, rc;
> - xen_pfn_t pfn;
> uint64_t modbase;
>
> uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
> @@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
> assert(ramsize == 0); /* Too much RAM is rejected above */
>
> dom->p2m_size = p2m_size;
Do we need to keep p2m_size?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
` (3 preceding siblings ...)
2019-12-17 20:15 ` [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
@ 2019-12-31 16:37 ` Wei Liu
4 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2019-12-31 16:37 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Varad Gautam,
Ian Jackson, Xen-devel, Volodymyr Babchuk
On Tue, Dec 17, 2019 at 08:15:46PM +0000, Andrew Cooper wrote:
> Vastly drop xl's memory usage for HVM (x86 and ARM) guest construction.
>
> See
> https://lore.kernel.org/xen-devel/1562159202-11316-1-git-send-email-vrd@amazon.de/T/#u
> for the origins of this work, but ultimately I think this is a far cleaner
> solution to the problem.
>
> Andrew Cooper (4):
> tools/dombuilder: xc_dom_x86 cleanup
> tools/dombuilder: Remove PV-only, mandatory hooks
> tools/dombuilder: Remove p2m_guest from the common interface
> tools/dombuilder: Don't allocate dom->p2m_host[] for translated
> domains
The code looks good to me.
Acked-by: Wei Liu <wl@xen.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2019-12-23 18:23 ` Julien Grall
@ 2020-01-02 17:49 ` Andrew Cooper
2020-01-03 10:44 ` Julien Grall
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2020-01-02 17:49 UTC (permalink / raw)
To: Julien Grall, Xen-devel
Cc: Ian Jackson, Varad Gautam, Stefano Stabellini, Volodymyr Babchuk,
Wei Liu
On 23/12/2019 18:23, Julien Grall wrote:
> Hi,
>
> On 17/12/2019 21:15, Andrew Cooper wrote:
>> xc_dom_p2m() and dom->p2m_host[] implement a linear transform for
>> translated
>> domains, but waste a substantial chunk of RAM doing so.
>>
>> ARM literally never reads dom->p2m_host[] (because of the
>> xc_dom_translated()
>> short circuit in xc_dom_p2m()). Drop it all.
>>
>> x86 HVM does use dom->p2m_host[] for
>> xc_domain_populate_physmap_exact() calls
>> when populating 4k pages. Reuse the same tactic from 2M/1G ranges
>> and use an
>> on-stack array instead. Drop the memory allocation.
>>
>> x86 PV guests do use dom->p2m_host[] as a non-identity transform.
>> Rename the
>> field to pv_p2m to make it clear it is PV-only.
>
> Nice cleanup! This will probably make slightly faster guest boot :).
>
>> @@ -359,7 +356,6 @@ static int populate_guest_memory(struct
>> xc_dom_image *dom,
>> static int meminit(struct xc_dom_image *dom)
>> {
>> int i, rc;
>> - xen_pfn_t pfn;
>> uint64_t modbase;
>> uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
>> @@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
>> assert(ramsize == 0); /* Too much RAM is rejected above */
>> dom->p2m_size = p2m_size;
>
> Do we need to keep p2m_size?
Yes, I think so.
There are some common checks which would fail if it becomes 0, and
importantly, it is used to locate safe gfns for temporary physmap mappings.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2020-01-02 17:49 ` Andrew Cooper
@ 2020-01-03 10:44 ` Julien Grall
2020-01-08 9:36 ` Julien Grall
0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2020-01-03 10:44 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Ian Jackson, Varad Gautam, Stefano Stabellini, Volodymyr Babchuk,
Wei Liu
Hi Andrew,
On 02/01/2020 17:49, Andrew Cooper wrote:
> On 23/12/2019 18:23, Julien Grall wrote:
>> Hi,
>>
>> On 17/12/2019 21:15, Andrew Cooper wrote:
>>> xc_dom_p2m() and dom->p2m_host[] implement a linear transform for
>>> translated
>>> domains, but waste a substantial chunk of RAM doing so.
>>>
>>> ARM literally never reads dom->p2m_host[] (because of the
>>> xc_dom_translated()
>>> short circuit in xc_dom_p2m()). Drop it all.
>>>
>>> x86 HVM does use dom->p2m_host[] for
>>> xc_domain_populate_physmap_exact() calls
>>> when populating 4k pages. Reuse the same tactic from 2M/1G ranges
>>> and use an
>>> on-stack array instead. Drop the memory allocation.
>>>
>>> x86 PV guests do use dom->p2m_host[] as a non-identity transform.
>>> Rename the
>>> field to pv_p2m to make it clear it is PV-only.
>>
>> Nice cleanup! This will probably make slightly faster guest boot :).
>>
>>> @@ -359,7 +356,6 @@ static int populate_guest_memory(struct
>>> xc_dom_image *dom,
>>> static int meminit(struct xc_dom_image *dom)
>>> {
>>> int i, rc;
>>> - xen_pfn_t pfn;
>>> uint64_t modbase;
>>> uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
>>> @@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
>>> assert(ramsize == 0); /* Too much RAM is rejected above */
>>> dom->p2m_size = p2m_size;
>>
>> Do we need to keep p2m_size?
>
> Yes, I think so.
>
> There are some common checks which would fail if it becomes 0, and
> importantly, it is used to locate safe gfns for temporary physmap mappings.
Do you mean the scratch page? If so, on Arm we use a fix address in the
memory map rather than p2m_size.
I have looked at the usage of the p2m_size in the common code and I
didn't spot any path that can be used by Arm and using p2m_size.
Also, I don't think p2m_size is calculated correctly on Arm. It only
englobe the RAM and doesn't take into account the rest of the mappings
(e.g MMIO...). So I am not sure how this could be used in common code.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2019-12-17 20:15 ` [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-23 18:23 ` Julien Grall
@ 2020-01-03 14:25 ` Jan Beulich
2020-01-03 15:02 ` Andrew Cooper
1 sibling, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2020-01-03 14:25 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Varad Gautam,
Ian Jackson, Xen-devel, Volodymyr Babchuk
On 17.12.2019 21:15, Andrew Cooper wrote:
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -123,16 +123,12 @@ struct xc_dom_image {
>
> /* other state info */
> uint32_t f_active[XENFEAT_NR_SUBMAPS];
> +
> /*
> - * p2m_host maps guest physical addresses an offset from
> - * rambase_pfn (see below) into gfns.
The removal of this part of the comment and ...
> - * For a pure PV guest this means that it maps GPFNs into MFNs for
> - * a hybrid guest this means that it maps GPFNs to GPFNS.
> - *
> - * Note that the input is offset by rambase.
> + * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs. It is
> + * eventually copied into guest context.
> */
> - xen_pfn_t *p2m_host;
> + xen_pfn_t *pv_p2m;
>
> /* physical memory
> *
> @@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image *dom, xen_pfn_t pfn)
> {
> if ( xc_dom_translated(dom) )
> return pfn;
> - if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages)
> +
> + /* x86 PV only now. */
> + if ( pfn >= dom->total_pages )
> return INVALID_MFN;
> - return dom->p2m_host[pfn - dom->rambase_pfn];
> +
> + return dom->pv_p2m[pfn];
> }
... the dropping of all uses of rambase_pfn here make it look
like you're doing away with that concept altogether. Is this
really correct? If so, I guess you want to
- say a word in this regard in the description, the more that
you don't fully get rid of this (the structure field and
some uses elsewhere remain),
- drop/adjust the respective comment fragment next to the
field a little further down in the structure.
> @@ -1245,11 +1245,11 @@ static int meminit_pv(struct xc_dom_image *dom)
> return -EINVAL;
> }
>
> - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
> - if ( dom->p2m_host == NULL )
> + dom->pv_p2m = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
Since you're touching the line anyway, perhaps better
sizeof(*dom->pv_p2m)?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2020-01-03 14:25 ` Jan Beulich
@ 2020-01-03 15:02 ` Andrew Cooper
2020-01-03 15:33 ` Jan Beulich
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2020-01-03 15:02 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Varad Gautam,
Ian Jackson, Xen-devel, Volodymyr Babchuk
On 03/01/2020 14:25, Jan Beulich wrote:
> On 17.12.2019 21:15, Andrew Cooper wrote:
>> --- a/tools/libxc/include/xc_dom.h
>> +++ b/tools/libxc/include/xc_dom.h
>> @@ -123,16 +123,12 @@ struct xc_dom_image {
>>
>> /* other state info */
>> uint32_t f_active[XENFEAT_NR_SUBMAPS];
>> +
>> /*
>> - * p2m_host maps guest physical addresses an offset from
>> - * rambase_pfn (see below) into gfns.
> The removal of this part of the comment and ...
>
>> - * For a pure PV guest this means that it maps GPFNs into MFNs for
>> - * a hybrid guest this means that it maps GPFNs to GPFNS.
>> - *
>> - * Note that the input is offset by rambase.
>> + * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs. It is
>> + * eventually copied into guest context.
>> */
>> - xen_pfn_t *p2m_host;
>> + xen_pfn_t *pv_p2m;
>>
>> /* physical memory
>> *
>> @@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image *dom, xen_pfn_t pfn)
>> {
>> if ( xc_dom_translated(dom) )
>> return pfn;
>> - if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages)
>> +
>> + /* x86 PV only now. */
>> + if ( pfn >= dom->total_pages )
>> return INVALID_MFN;
>> - return dom->p2m_host[pfn - dom->rambase_pfn];
>> +
>> + return dom->pv_p2m[pfn];
>> }
> ... the dropping of all uses of rambase_pfn here make it look
> like you're doing away with that concept altogether. Is this
> really correct?
Well - it is effectively dead code here, because of the
xc_dom_translated() in context above, and it being 0 outside of ARM.
The (nonzero) value is used by other bits of ARM code.
> If so, I guess you want to
> - say a word in this regard in the description, the more that
> you don't fully get rid of this (the structure field and
> some uses elsewhere remain),
> - drop/adjust the respective comment fragment next to the
> field a little further down in the structure.
The domain builder is made almost exclusively of bitrot, but I don't
have an ARM system to do any testing on. Given how fragile the code is,
I'm not comfortable doing speculative deletion of code I'm not certain
is unused.
>
>> @@ -1245,11 +1245,11 @@ static int meminit_pv(struct xc_dom_image *dom)
>> return -EINVAL;
>> }
>>
>> - dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
>> - if ( dom->p2m_host == NULL )
>> + dom->pv_p2m = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
> Since you're touching the line anyway, perhaps better
> sizeof(*dom->pv_p2m)?
Will fix.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2020-01-03 15:02 ` Andrew Cooper
@ 2020-01-03 15:33 ` Jan Beulich
0 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2020-01-03 15:33 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Varad Gautam,
Ian Jackson, Xen-devel, Volodymyr Babchuk
On 03.01.2020 16:02, Andrew Cooper wrote:
> On 03/01/2020 14:25, Jan Beulich wrote:
>> On 17.12.2019 21:15, Andrew Cooper wrote:
>>> --- a/tools/libxc/include/xc_dom.h
>>> +++ b/tools/libxc/include/xc_dom.h
>>> @@ -123,16 +123,12 @@ struct xc_dom_image {
>>>
>>> /* other state info */
>>> uint32_t f_active[XENFEAT_NR_SUBMAPS];
>>> +
>>> /*
>>> - * p2m_host maps guest physical addresses an offset from
>>> - * rambase_pfn (see below) into gfns.
>> The removal of this part of the comment and ...
>>
>>> - * For a pure PV guest this means that it maps GPFNs into MFNs for
>>> - * a hybrid guest this means that it maps GPFNs to GPFNS.
>>> - *
>>> - * Note that the input is offset by rambase.
>>> + * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs. It is
>>> + * eventually copied into guest context.
>>> */
>>> - xen_pfn_t *p2m_host;
>>> + xen_pfn_t *pv_p2m;
>>>
>>> /* physical memory
>>> *
>>> @@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image *dom, xen_pfn_t pfn)
>>> {
>>> if ( xc_dom_translated(dom) )
>>> return pfn;
>>> - if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages)
>>> +
>>> + /* x86 PV only now. */
>>> + if ( pfn >= dom->total_pages )
>>> return INVALID_MFN;
>>> - return dom->p2m_host[pfn - dom->rambase_pfn];
>>> +
>>> + return dom->pv_p2m[pfn];
>>> }
>> ... the dropping of all uses of rambase_pfn here make it look
>> like you're doing away with that concept altogether. Is this
>> really correct?
>
> Well - it is effectively dead code here, because of the
> xc_dom_translated() in context above, and it being 0 outside of ARM.
>
> The (nonzero) value is used by other bits of ARM code.
>
>> If so, I guess you want to
>> - say a word in this regard in the description, the more that
>> you don't fully get rid of this (the structure field and
>> some uses elsewhere remain),
>> - drop/adjust the respective comment fragment next to the
>> field a little further down in the structure.
>
> The domain builder is made almost exclusively of bitrot, but I don't
> have an ARM system to do any testing on. Given how fragile the code is,
> I'm not comfortable doing speculative deletion of code I'm not certain
> is unused.
My remark was about this (non-Arm) part of the comment:
* An x86 PV guest has one or more blocks of physical RAM,
* consisting of total_pages starting at rambase_pfn. The start
* address and size of each block is controlled by vNUMA
* structures.
I did in no way mean to ask for speculative deletion of code.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains
2020-01-03 10:44 ` Julien Grall
@ 2020-01-08 9:36 ` Julien Grall
0 siblings, 0 replies; 14+ messages in thread
From: Julien Grall @ 2020-01-08 9:36 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Ian Jackson, Varad Gautam, Stefano Stabellini, Volodymyr Babchuk,
Wei Liu
Hi,
On 03/01/2020 10:44, Julien Grall wrote:
> Hi Andrew,
>
> On 02/01/2020 17:49, Andrew Cooper wrote:
>> On 23/12/2019 18:23, Julien Grall wrote:
>>> Hi,
>>>
>>> On 17/12/2019 21:15, Andrew Cooper wrote:
>>>> xc_dom_p2m() and dom->p2m_host[] implement a linear transform for
>>>> translated
>>>> domains, but waste a substantial chunk of RAM doing so.
>>>>
>>>> ARM literally never reads dom->p2m_host[] (because of the
>>>> xc_dom_translated()
>>>> short circuit in xc_dom_p2m()). Drop it all.
>>>>
>>>> x86 HVM does use dom->p2m_host[] for
>>>> xc_domain_populate_physmap_exact() calls
>>>> when populating 4k pages. Reuse the same tactic from 2M/1G ranges
>>>> and use an
>>>> on-stack array instead. Drop the memory allocation.
>>>>
>>>> x86 PV guests do use dom->p2m_host[] as a non-identity transform.
>>>> Rename the
>>>> field to pv_p2m to make it clear it is PV-only.
>>>
>>> Nice cleanup! This will probably make slightly faster guest boot :).
>>>
>>>> @@ -359,7 +356,6 @@ static int populate_guest_memory(struct
>>>> xc_dom_image *dom,
>>>> static int meminit(struct xc_dom_image *dom)
>>>> {
>>>> int i, rc;
>>>> - xen_pfn_t pfn;
>>>> uint64_t modbase;
>>>> uint64_t ramsize = (uint64_t)dom->total_pages <<
>>>> XC_PAGE_SHIFT;
>>>> @@ -423,11 +419,6 @@ static int meminit(struct xc_dom_image *dom)
>>>> assert(ramsize == 0); /* Too much RAM is rejected above */
>>>> dom->p2m_size = p2m_size;
>>>
>>> Do we need to keep p2m_size?
>>
>> Yes, I think so.
>>
>> There are some common checks which would fail if it becomes 0, and
>> importantly, it is used to locate safe gfns for temporary physmap
>> mappings.
>
> Do you mean the scratch page? If so, on Arm we use a fix address in the
> memory map rather than p2m_size.
>
> I have looked at the usage of the p2m_size in the common code and I
> didn't spot any path that can be used by Arm and using p2m_size.
>
> Also, I don't think p2m_size is calculated correctly on Arm. It only
> englobe the RAM and doesn't take into account the rest of the mappings
> (e.g MMIO...). So I am not sure how this could be used in common code.
Thinking a bit more of this. As the p2m_size was IHMO already buggy, it
should not make any difference after this series. Therefore removing
p2m_size can be probably done on a follow-up patch.
So I am happy with the patch as-is:
Acked-by: Julien Grall <julien@xen.org>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-01-08 9:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 20:15 [Xen-devel] [PATCH 0/4] Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 1/4] tools/dombuilder: xc_dom_x86 cleanup Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 2/4] tools/dombuilder: Remove PV-only, mandatory hooks Andrew Cooper
2019-12-23 18:12 ` Julien Grall
2019-12-17 20:15 ` [Xen-devel] [PATCH 3/4] tools/dombuilder: Remove p2m_guest from the common interface Andrew Cooper
2019-12-17 20:15 ` [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains Andrew Cooper
2019-12-23 18:23 ` Julien Grall
2020-01-02 17:49 ` Andrew Cooper
2020-01-03 10:44 ` Julien Grall
2020-01-08 9:36 ` Julien Grall
2020-01-03 14:25 ` Jan Beulich
2020-01-03 15:02 ` Andrew Cooper
2020-01-03 15:33 ` Jan Beulich
2019-12-31 16:37 ` [Xen-devel] [PATCH 0/4] " Wei Liu
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.