[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH RFC 04/18] libxl: add ability to set rambase_pfn via cfg file
Hello,
On 18/05/16 17:32, Andrii Anisov wrote:
From: Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>
In case of missing required property in cfg file
the default value (0x40000) should be used.
The memory layout for the guest is static and may change between
version. By allowing the user to specify the base of the RAM, you expose
the Xen internal. The user cannot use this option without knowing the
version of Xen 4.4 and looking at xen/include/public/arch-arm.h.
Also, you need to explain why a user would want to change the
rambase_pfn. I suspect it is for supporting direct mapped domain. If so,
I think the way forward is to expose the same memory layout as the
hardware rather than trying to make the layout half-static/dynamic.
Exposing the same layout as the hardware would allow you to map the
device MMIOs 1:1 and avoid unnecessary IPA -> PA translation via an
hypercall (see patch #10).
Lastly, the commit message should explain the restriction. For instance,
I would have expect to be able to specify any value. However based on
the check in xl, this is not possible.
Signed-off-by: Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>
Signed-off-by: Iurii Konovalenko <iurii.konovalenko@xxxxxxxxxxxxxxx>
Signed-off-by: Iurii Mykhalskyi <iurii.mykhalskyi@xxxxxxxxxxxxxxx>
---
tools/libxc/xc_dom_arm.c | 13 +++++++------
tools/libxl/libxl_arm.c | 10 ++++++++--
tools/libxl/libxl_create.c | 5 +++++
tools/libxl/libxl_dom.c | 6 +++---
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 13 +++++++++++++
7 files changed, 38 insertions(+), 11 deletions(-)libxc
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 5ee8eef..96df669 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -415,9 +415,12 @@ int arch_setup_meminit(struct xc_dom_image *dom)
uint64_t modbase;
uint64_t ramsize = (uint64_t)dom->total_pages << XC_PAGE_SHIFT;
+ uint64_t guest_rambase = (uint64_t)dom->rambase_pfn << XC_PAGE_SHIFT;
+ uint64_t guest_ramsize = (GUEST_RAM0_BASE + GUEST_RAM0_SIZE) -
+ guest_rambase;
- const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
- const uint64_t bankmax[] = GUEST_RAM_BANK_SIZES;
+ const uint64_t bankbase[] = {guest_rambase, GUEST_RAM1_BASE};
+ const uint64_t bankmax[] = {guest_ramsize, GUEST_RAM1_SIZE};
/* Convenient */
const uint64_t kernbase = dom->kernel_seg.vstart;
@@ -433,8 +436,6 @@ int arch_setup_meminit(struct xc_dom_image *dom)
xen_pfn_t p2m_size;
uint64_t bank0end;
- assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
-
if ( modsize + kernsize > bankmax[0] )
{
DOMPRINTF("%s: Not enough memory for the kernel+dtb+initrd",
@@ -448,11 +449,11 @@ int arch_setup_meminit(struct xc_dom_image *dom)
return -1;
}
- if ( ramsize > GUEST_RAM_MAX )
+ if ( ramsize > (bankmax[0] + bankmax[1]) )
{
DOMPRINTF("%s: ram size is too large for guest address space: "
"%"PRIx64" > %llx",
- __FUNCTION__, ramsize, GUEST_RAM_MAX);
+ __FUNCTION__, ramsize, bankmax[0] + bankmax[1]);
return -1;
}
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 0af8010..7f9547b 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -372,7 +372,10 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt,
{
int res, i;
const char *name;
- const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
+
+ uint64_t guest_rambase = (uint64_t)dom->rambase_pfn << XC_PAGE_SHIFT;
+
+ const uint64_t bankbase[] = {guest_rambase, GUEST_RAM1_BASE};
for (i = 0; i < GUEST_RAM_BANKS; i++) {
name = GCSPRINTF("memory@%"PRIx64, bankbase[i]);
@@ -914,7 +917,10 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc
*gc,
{
void *fdt = dom->devicetree_blob;
int i;
- const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
+
+ uint64_t guest_rambase = (uint64_t)dom->rambase_pfn << XC_PAGE_SHIFT;
+
+ const uint64_t bankbase[] = {guest_rambase, GUEST_RAM1_BASE};
const struct xc_dom_seg *ramdisk = dom->ramdisk_blob ?
&dom->ramdisk_seg : NULL;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 350e274..1c0579c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -182,6 +182,11 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
b_info->target_memkb = b_info->max_memkb;
+#ifdef GUEST_RAM_BASE
+ if (b_info->rambase == LIBXL_INVALID_RAM_BASE)
+ b_info->rambase = GUEST_RAM0_BASE;
xl is not the only front-end for libxl. So you need to validate the
value of rambase here.
+#endif
+
libxl_defbool_setdefault(&b_info->claim_mode, false);
libxl_defbool_setdefault(&b_info->localtime, false);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 2da3ac4..be6598f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -703,12 +703,12 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
LOGE(ERROR, "xc_dom_boot_xen_init failed");
goto out;
}
-#ifdef GUEST_RAM_BASE
- if ( (ret = xc_dom_rambase_init(dom, GUEST_RAM_BASE)) != 0 ) {
+
Spurious change.
+ if ( (ret = xc_dom_rambase_init(dom, info->rambase)) != 0 ) {
LOGE(ERROR, "xc_dom_rambase failed");
goto out;
}
-#endif
+
if ( (ret = xc_dom_parse_image(dom)) != 0 ) {
LOGE(ERROR, "xc_dom_parse_image failed");
goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1699f32..5b0b50a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -100,6 +100,7 @@
#define LIBXL_HVM_EXTRA_MEMORY 2048
#define LIBXL_MIN_DOM0_MEM (128*1024)
#define LIBXL_INVALID_GFN (~(uint64_t)0)
+#define LIBXL_INVALID_RAM_BASE (~(uint64_t)0)
/* use 0 as the domid of the toolstack domain for now */
#define LIBXL_TOOLSTACK_DOMID 0
#define QEMU_SIGNATURE "DeviceModelRecord0002"
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 502a148..b6cc8d2 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -416,6 +416,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("target_memkb", MemKB),
("video_memkb", MemKB),
("shadow_memkb", MemKB),
+ ("rambase", uint64, {'init_val': 'LIBXL_INVALID_RAM_BASE'}),
Any change in libxl_types.idl should also come with a new define in
libxl.h to allow toolstack built on top of libxl works with various
version of Xen.
("rtc_timeoffset", uint32),
("exec_ssidref", uint32),
("exec_ssid_label", string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9a2870e..28d57c3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -31,6 +31,7 @@
#include <sys/select.h>
#include <sys/utsname.h> /* for utsname in xl info */
#include <xentoollog.h>
+#include <xenctrl.h>
#include <ctype.h>
#include <inttypes.h>
#include <limits.h>
@@ -1352,6 +1353,18 @@ static void parse_config_data(const char *config_source,
if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
b_info->max_memkb = l * 1024;
+#ifdef GUEST_RAM_BASE
+ if (!xlu_cfg_get_long (config, "rambase_pfn", &l, 0)) {
New options need to be documented in docs/man/xl.cfg.pod.5.
+ /* TODO add more detailed check for valid value */
+ uint64_t rambase = (uint64_t)l << XC_PAGE_SHIFT;
+ if (rambase > (GUEST_RAM0_BASE + GUEST_RAM0_SIZE)) {
+ fprintf(stderr, "ERROR: invalid value 0x%lx for
\"rambase_pfn\"\n", l);
+ exit (1);
+ }
+ b_info->rambase = rambase;
+ }
+#endif
+
if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
vcpus = l;
if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, l)) {
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
