Re: [Xen-devel] [PATCH] x86/svm: Fix svm_update_guest_efer() for domains using shadow paging
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] > Sent: Saturday, October 6, 2018 1:02 AM > > When using shadow paging, EFER.NX is a Xen controlled bit, and is required > by > the shadow pagefault handler to distinguish instruction fetches from data > accesses. > > This can be observed by a guest which has NX and SMEP clear but SMAP > active by > attempting to execute code on a user mapping. The first attempt to build > the > target shadow will #PF so is handled by the shadow code, but when walking > the > the guest pagetables, the lack of PFEC_insn_fetch being signalled causes the > shadow code to mistake the instruction fetch for a data fetch, and believe > that it is a real guest fault. As a result, the guest receives #PF[-d-srP] > for an action which should complete successfully. > > The suspicious-looking gymnastics with LME is actually a subtle corner case > with shadow paging. When dropping out of Long Mode, a guests choice of > LME > and Xen's choice of CR0.PG cause hardware to operate in Long Mode, but > the > shadow code to operate in 2-on-3 mode. > > In addition to describing this corner case in the SVM side, extend the > comment > for the same fix on the VT-x side. (I have a suspicion that I've just worked > out why VT-x doesn't tolerate LMA != LME when Unrestricted Guest is clear.) > > Signed-off-by: Andrew Cooper Reviewed-by: Kevin Tian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: move vendor independent CPU save/restore logic to shared code
> From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Friday, October 5, 2018 7:32 PM > > A few pieces of the handling here are (no longer?) vendor specific, and > hence there's no point in replicating the code. Make sure not otherwise > pre-filled fields of struct hvm_hw_cpu instances are zero filled before > calling the vendor "save" hook, eliminating the need for the hook > functions to zero individual fields. > > Signed-off-by: Jan Beulich Reviewed-by: Kevin Tian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: [PATCH v3 3/4] x86/HVM: implement memory read caching
> From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Tuesday, October 2, 2018 6:39 PM > > >>> On 25.09.18 at 16:25, wrote: > > Emulation requiring device model assistance uses a form of instruction > > re-execution, assuming that the second (and any further) pass takes > > exactly the same path. This is a valid assumption as far use of CPU > > registers goes (as those can't change without any other instruction > > executing in between), but is wrong for memory accesses. In particular > > it has been observed that Windows might page out buffers underneath > an > > instruction currently under emulation (hitting between two passes). If > > the first pass translated a linear address successfully, any subsequent > > pass needs to do so too, yielding the exact same translation. > > > > Introduce a cache (used by just guest page table accesses for now) to > > make sure above described assumption holds. This is a very simplistic > > implementation for now: Only exact matches are satisfied (no overlaps or > > partial reads or anything). > > > > As to the actual data page in this scenario, there are a couple of > > aspects to take into consideration: > > - We must be talking about an insn accessing two locations (two memory > > ones, one of which is MMIO, or a memory and an I/O one). > > - If the non I/O / MMIO side is being read, the re-read (if it occurs at > > all) is having its result discarded, by taking the shortcut through > > the first switch()'s STATE_IORESP_READY case in hvmemul_do_io(). Note > > how, among all the re-issue sanity checks there, we avoid comparing > > the actual data. > > - If the non I/O / MMIO side is being written, it is the OSes > > responsibility to avoid actually moving page contents to disk while > > there might still be a write access in flight - this is no different > > in behavior from bare hardware. > > - Read-modify-write accesses are, as always, complicated, and while we > > deal with them better nowadays than we did in the past, we're still > > not quite there to guarantee hardware like behavior in all cases > > anyway. Nothing is getting worse by the changes made here, afaict. > > > > Signed-off-by: Jan Beulich > > Acked-by: Tim Deegan > > Reviewed-by: Paul Durrant > > SVM and VMX maintainers? > Reviewed-by: Kevin Tian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/18] INSTALL: Mention kconfig
On Mon, Oct 08, 2018 at 03:24:41PM +0100, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH 10/18] INSTALL: Mention kconfig"): > > On 08.10.18 at 16:08, wrote: > > > Thanks, I'll take that as an ack. (Assuming you did indeed mean > > > `accepting' rather than `excepting'.) > > > > Well, no, it was not meant as an ack, merely as the absence of further > > objections. (And yes, I did mean "accepting".) > > Err, OK. > > Doug, do you want me to propose a different or expanded wording ? > > Ian. No. I think this good. It sheds light on something that was black magic and that's good. I think you've started the conversation on how we see XEN_CONFIG_EXPERT evolving (due to the shim config) and that's great too. +1 from me. -- Doug ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 128493: regressions - FAIL
flight 128493 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/128493/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 125898 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 125898 test-amd64-i386-examine 8 reboot fail REGR. vs. 125898 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl7 xen-boot fail REGR. vs. 125898 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 125898 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 125898 test-amd64-amd64-examine 8 reboot fail REGR. vs. 125898 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 125898 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-credit1 7 xen-bootfail baseline untested test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125898 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125898 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125898 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125898 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125898 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125898 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14
[Xen-devel] [PATCH 1/3] xen/xsm: remove unnecessary #define
this #define is unnecessary since XSM_INLINE is redefined in xsm/dummy.h, it's a risk of build breakage, so remove it. Signed-off-by: Xin Li --- CC: Daniel De Graaf CC: George Dunlap CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Sergey Dyasli CC: Andrew Cooper CC: Ming Lu v5: 1. move the removal of #define to this new patch. 2. fix wrong git author --- xen/xsm/dummy.c | 1 - 1 file changed, 1 deletion(-) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 3290d04527..06a674fad0 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -11,7 +11,6 @@ */ #define XSM_NO_WRAPPERS -#define XSM_INLINE /* */ #include struct xsm_operations dummy_xsm_ops; -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 4/8] libxl: support unmapping static shared memory areas during domain destruction
From: Zhongze Liu Author: Zhongze Liu Add libxl__sshm_del to unmap static shared memory areas mapped by libxl__sshm_add during domain creation. The unmapping process is: * For a master: decrease the refcount of the sshm region, if the refcount reaches 0, cleanup the whole sshm path. * For a slave: 1) unmap the shared pages, and cleanup related xs entries. If the system works normally, all the shared pages will be unmapped, so there won't be page leaks. In case of errors, the unmapping process will go on and unmap all the other pages that can be unmapped, so the other pages won't be leaked, either. 2) Decrease the refcount of the sshm region, if the refcount reaches 0, cleanup the whole sshm path. This is for the proposal "Allow setting up shared memory areas between VMs from xl config file" (see [1]). [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Signed-off-by: Stefano Stabellini Cc: Ian Jackson Cc: Wei Liu Cc: Stefano Stabellini Cc: Julien Grall Cc: xen-de...@lists.xen.org --- Changes in v5: - fix typos - add comments - cannot move unmap before xenstore transaction because it needs to retrieve begin/size values from xenstore --- tools/libxl/libxl_domain.c | 5 ++ tools/libxl/libxl_internal.h | 2 + tools/libxl/libxl_sshm.c | 107 +++ 3 files changed, 114 insertions(+) diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c index 3377bba..3f7ffa6 100644 --- a/tools/libxl/libxl_domain.c +++ b/tools/libxl/libxl_domain.c @@ -1060,6 +1060,11 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis) goto out; } +rc = libxl__sshm_del(gc, domid); +if (rc) { +LOGD(ERROR, domid, "Deleting static shm failed."); +} + if (libxl__device_pci_destroy_all(gc, domid) < 0) LOGD(ERROR, domid, "Pci shutdown failed"); rc = xc_domain_pause(ctx->xch, domid); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 6f31a3d..e86d356 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -4442,6 +4442,8 @@ static inline const char *libxl__qemu_qmp_path(libxl__gc *gc, int domid) _hidden int libxl__sshm_add(libxl__gc *gc, uint32_t domid, libxl_static_shm *sshm, int len); +_hidden int libxl__sshm_del(libxl__gc *gc, uint32_t domid); + _hidden int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid, libxl_static_shm *sshms, int len); _hidden int libxl__sshm_setdefault(libxl__gc *gc, uint32_t domid, diff --git a/tools/libxl/libxl_sshm.c b/tools/libxl/libxl_sshm.c index f61b80c..9672056 100644 --- a/tools/libxl/libxl_sshm.c +++ b/tools/libxl/libxl_sshm.c @@ -94,6 +94,113 @@ int libxl__sshm_check_overlap(libxl__gc *gc, uint32_t domid, return 0; } +/* + * Decrease the refcount of an sshm. When refcount reaches 0, + * clean up the whole sshm path. + * Xenstore operations are done within the same transaction. + */ +static void libxl__sshm_decref(libxl__gc *gc, xs_transaction_t xt, + const char *sshm_path) +{ +int count; +const char *count_path, *count_string; + +count_path = GCSPRINTF("%s/usercnt", sshm_path); +if (libxl__xs_read_checked(gc, xt, count_path, _string)) +return; +count = atoi(count_string); + +if (--count == 0) { +libxl__xs_path_cleanup(gc, xt, sshm_path); +return; +} + +count_string = GCSPRINTF("%d", count); +libxl__xs_write_checked(gc, xt, count_path, count_string); + +return; +} + +static void libxl__sshm_do_unmap(libxl__gc *gc, uint32_t domid, const char *id, + uint64_t begin, uint64_t size) +{ +uint64_t end; +begin >>= XC_PAGE_SHIFT; +size >>= XC_PAGE_SHIFT; +end = begin + size; +for (; begin < end; ++begin) { +if (xc_domain_remove_from_physmap(CTX->xch, domid, begin)) { +SSHM_ERROR(domid, id, + "unable to unmap shared page at 0x%"PRIx64".", + begin); +} +} +} + +static void libxl__sshm_del_slave(libxl__gc *gc, xs_transaction_t xt, + uint32_t domid, const char *id, bool isretry) +{ +const char *slave_path, *begin_str, *size_str; +uint64_t begin, size; + +slave_path = GCSPRINTF("%s/slaves/%"PRIu32, SSHM_PATH(id), domid); + +begin_str = libxl__xs_read(gc, xt, GCSPRINTF("%s/begin", slave_path)); +size_str = libxl__xs_read(gc, xt, GCSPRINTF("%s/size", slave_path)); +begin = strtoull(begin_str, NULL, 16); +size = strtoull(size_str, NULL, 16); + +libxl__sshm_do_unmap(gc, domid, id, begin, size); +libxl__xs_path_cleanup(gc, xt, slave_path); +} + +/* Delete static_shm entries in the xensotre. */ +int libxl__sshm_del(libxl__gc *gc, uint32_t domid) +{ +int rc, i; +
[Xen-devel] [PATCH v8 7/8] xen/arm: export shared memory regions as reserved-memory on device tree
Shared memory regions need to be advertised to the guest. Fortunately, a device tree binding for special memory regions already exist: reserved-memory. Add a reserved-memory node for each shared memory region, for both masters and slaves. Signed-off-by: Stefano Stabellini --- Changes in v8: - code style - id is added to device tree Changes in v7: - change node name to xen-shmem - add compatible property - add id property --- tools/libxl/libxl_arch.h | 2 +- tools/libxl/libxl_arm.c | 61 +--- tools/libxl/libxl_dom.c | 2 +- tools/libxl/libxl_x86.c | 2 +- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h index 63c26cc..417e710 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -36,7 +36,7 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, /* setup arch specific hardware description, i.e. DTB on ARM */ _hidden int libxl__arch_domain_init_hw_description(libxl__gc *gc, - libxl_domain_build_info *info, + libxl_domain_config *d_config, libxl__domain_build_state *state, struct xc_dom_image *dom); /* finalize arch specific hardware description. */ diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index 054ad58..c1624c0 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -436,6 +436,58 @@ static int make_memory_nodes(libxl__gc *gc, void *fdt, return 0; } +static int make_reserved_nodes(libxl__gc *gc, void *fdt, + libxl_domain_config *d_config) +{ +int res, i; +const char *name; + +if (d_config->num_sshms == 0) +return 0; + +res = fdt_begin_node(fdt, "reserved-memory"); +if (res) return res; + +res = fdt_property_cell(fdt, "#address-cells", GUEST_ROOT_ADDRESS_CELLS); +if (res) return res; + +res = fdt_property_cell(fdt, "#size-cells", GUEST_ROOT_SIZE_CELLS); +if (res) return res; + +res = fdt_property(fdt, "ranges", NULL, 0); +if (res) return res; + +for (i = 0; i < d_config->num_sshms; i++) { +uint64_t start = d_config->sshms[i].begin; + +if (d_config->sshms[i].role == LIBXL_SSHM_ROLE_SLAVE) +start += d_config->sshms[i].offset; +name = GCSPRINTF("xen-shmem@%"PRIx64, start); + +res = fdt_begin_node(fdt, name); +if (res) return res; + +res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS, +GUEST_ROOT_SIZE_CELLS, 1, start, +d_config->sshms[i].size); +if (res) return res; + +res = fdt_property_compat(gc, fdt, 1, "xen,shared-memory"); +if (res) return res; + +res = fdt_property_string(fdt, "id", d_config->sshms[i].id); +if (res) return res; + +res = fdt_end_node(fdt); +if (res) return res; +} + +res = fdt_end_node(fdt); +if (res) return res; + +return 0; +} + static int make_gicv2_node(libxl__gc *gc, void *fdt, uint64_t gicd_base, uint64_t gicd_size, uint64_t gicc_base, uint64_t gicc_size) @@ -811,10 +863,11 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt) #define FDT_MAX_SIZE (1<<20) -static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info, +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config, libxl__domain_build_state *state, struct xc_dom_image *dom) { +libxl_domain_build_info *info = _config->b_info; void *fdt = NULL; void *pfdt = NULL; int rc, res; @@ -897,6 +950,7 @@ next_resize: FDT( make_psci_node(gc, fdt) ); FDT( make_memory_nodes(gc, fdt, dom) ); +FDT( make_reserved_nodes(gc, fdt, d_config) ); switch (info->arch_arm.gic_version) { case LIBXL_GIC_VERSION_V2: @@ -946,12 +1000,13 @@ out: } int libxl__arch_domain_init_hw_description(libxl__gc *gc, - libxl_domain_build_info *info, + libxl_domain_config *d_config, libxl__domain_build_state *state, struct xc_dom_image *dom) { int rc; uint64_t val; +libxl_domain_build_info *info = _config->b_info; if (info->type != LIBXL_DOMAIN_TYPE_PVH) { LOG(ERROR, "Unsupported Arm guest type %s", @@ -971,7 +1026,7 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc, if (rc) return rc; -rc = libxl__prepare_dtb(gc, info, state, dom); +rc = libxl__prepare_dtb(gc, d_config, state, dom); if (rc) goto out; if
[Xen-devel] [PATCH v8 2/8] libxl: introduce a new structure to represent static shared memory regions
From: Zhongze Liu Author: Zhongze Liu Add a new structure to the IDL family to represent static shared memory regions as proposed in the proposal "Allow setting up shared memory areas between VMs from xl config file" (see [1]). And deleted some trailing white spaces. [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Reviewed-by: Stefano Stabellini Acked-by: Wei Liu Signed-off-by: Stefano Stabellini Cc: Wei Liu Cc: Ian Jackson Cc: Stefano Stabellini Cc: Julien Grall Cc: xen-de...@lists.xen.org --- Changes in v8: - move LIBXL_HAVE_SSHM up and add comment - fix typo - remove LIBXL_SSHM_ID_MAXLEN Changes in v5: - fix typos - add LIBXL_HAVE_SSHM - replace end with size --- tools/libxl/libxl.h | 8 tools/libxl/libxl_types.idl | 32 ++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 2cfc1b0..18b29d7 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -377,6 +377,11 @@ #define LIBXL_HAVE_EXTENDED_VKB 1 /* + * LIBXL_HAVE_SSHM indicates that libxl supports static shared memory regions. + */ +#define LIBXL_HAVE_SSHM 1 + +/* * libxl ABI compatibility * * The only guarantee which libxl makes regarding ABI compatibility @@ -2461,6 +2466,9 @@ int libxl_fd_set_nonblock(libxl_ctx *ctx, int fd, int nonblock); int libxl_qemu_monitor_command(libxl_ctx *ctx, uint32_t domid, const char *command_line, char **output); +/* Constant for libxl_static_shm */ +#define LIBXL_SSHM_RANGE_UNKNOWN UINT64_MAX + #include #endif /* LIBXL_H */ diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 3b8f967..c3498ad 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -565,10 +565,10 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("keymap", string), ("sdl", libxl_sdl_info), ("spice",libxl_spice_info), - + ("gfx_passthru", libxl_defbool), ("gfx_passthru_kind", libxl_gfx_passthru_kind), - + ("serial", string), ("boot", string), ("usb", libxl_defbool), @@ -903,6 +903,33 @@ libxl_device_vsnd = Struct("device_vsnd", [ ("pcms", Array(libxl_vsnd_pcm, "num_vsnd_pcms")) ]) +libxl_sshm_cachepolicy = Enumeration("sshm_cachepolicy", [ +(-1, "UNKNOWN"), +(0, "ARM_NORMAL"), # ARM policies should be < 32 +(32, "X86_NORMAL"), # X86 policies should be >= 32 +], init_val = "LIBXL_SSHM_CACHE_POLICY_UNKNOWN") + +libxl_sshm_prot = Enumeration("sshm_prot", [ +(-1, "UNKNOWN"), +(3, "RW"), +], init_val = "LIBXL_SSHM_PROT_UNKNOWN") + +libxl_sshm_role = Enumeration("sshm_role", [ +(-1, "UNKNOWN"), +(0, "MASTER"), +(1, "SLAVE"), +], init_val = "LIBXL_SSHM_ROLE_UNKNOWN") + +libxl_static_shm = Struct("static_shm", [ +("id", string), +("offset", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}), +("begin", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}), +("size", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}), +("prot", libxl_sshm_prot, {'init_val': 'LIBXL_SSHM_PROT_UNKNOWN'}), +("cache_policy", libxl_sshm_cachepolicy, {'init_val': 'LIBXL_SSHM_CACHEPOLICY_UNKNOWN'}), +("role", libxl_sshm_role, {'init_val': 'LIBXL_SSHM_ROLE_UNKNOWN'}), +]) + libxl_domain_config = Struct("domain_config", [ ("c_info", libxl_domain_create_info), ("b_info", libxl_domain_build_info), @@ -924,6 +951,7 @@ libxl_domain_config = Struct("domain_config", [ ("channels", Array(libxl_device_channel, "num_channels")), ("usbctrls", Array(libxl_device_usbctrl, "num_usbctrls")), ("usbdevs", Array(libxl_device_usbdev, "num_usbdevs")), +("sshms", Array(libxl_static_shm, "num_sshms")), ("on_poweroff", libxl_action_on_shutdown), ("on_reboot", libxl_action_on_shutdown), -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 1/8] xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing
From: Zhongze Liu Author: Zhongze Liu The existing XENMAPSPACE_gmfn_foreign subop of XENMEM_add_to_physmap forbids a Dom0 to map memory pages from one DomU to another, which restricts some useful yet not dangerous use cases -- such as sharing pages among DomU's so that they can do shm-based communication. This patch introduces XENMAPSPACE_gmfn_share to address this inconvenience, which is mostly the same as XENMAPSPACE_gmfn_foreign but has its own xsm check. Specifically, the patch: * Introduces a new av permission MMU__SHARE_MEM to denote if two domains can share memory by using the new subop; * Introduces xsm_map_gmfn_share() to check if (current) has proper permission over (t) AND MMU__SHARE_MEM is allowed between (d) and (t); * Modify the default xen.te to allow MMU__SHARE_MEM for normal domains that allow grant mapping/event channels. The new subop is marked unsupported for x86 because calling p2m_add_foregin on two DomU's is currently not supported on x86. This is for the proposal "Allow setting up shared memory areas between VMs from xl config file" (see [1]). [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Signed-off-by: Stefano Stabellini Cc: Daniel De Graaf Cc: Ian Jackson Cc: Wei Liu Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: xen-de...@lists.xen.org --- Changes in v8: - typo Changes in v7: - add additional checks - update comments to reflect that Changes in v5: - fix coding style - remove useless x86 hypervisor message for the unimplemented op - code style - improve/add comments --- tools/flask/policy/modules/xen.if | 2 ++ xen/arch/arm/mm.c | 7 ++- xen/include/public/memory.h | 8 xen/include/xsm/dummy.h | 14 ++ xen/include/xsm/xsm.h | 6 ++ xen/xsm/dummy.c | 1 + xen/xsm/flask/hooks.c | 9 + xen/xsm/flask/policy/access_vectors | 5 + 8 files changed, 51 insertions(+), 1 deletion(-) diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if index 4e06cfc..b0ab089 100644 --- a/tools/flask/policy/modules/xen.if +++ b/tools/flask/policy/modules/xen.if @@ -128,6 +128,8 @@ define(`domain_comms', ` domain_event_comms($1, $2) allow $1 $2:grant { map_read map_write copy unmap }; allow $2 $1:grant { map_read map_write copy unmap }; + allow $1 $2:mmu share_mem; + allow $2 $1:mmu share_mem; ') # domain_self_comms(domain) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7a06a33..7b7869f 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1245,6 +1245,7 @@ int xenmem_add_to_physmap_one( break; case XENMAPSPACE_gmfn_foreign: +case XENMAPSPACE_gmfn_share: { struct domain *od; p2m_type_t p2mt; @@ -1259,7 +1260,11 @@ int xenmem_add_to_physmap_one( return -EINVAL; } -rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od); +if ( space == XENMAPSPACE_gmfn_foreign ) +rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od); +else +rc = xsm_map_gmfn_share(XSM_TARGET, d, od); + if ( rc ) { rcu_unlock_domain(od); diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index 8fc27ce..631d10e 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -227,6 +227,14 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); Stage-2 using the Normal Memory Inner/Outer Write-Back Cacheable memory attribute. */ +#define XENMAPSPACE_gmfn_share 6 /* GMFN from another dom, + XENMEM_add_to_physmap_batch (and + currently ARM) only. Unlike + XENMAPSPACE_gmfn_foreign, it + requires current to have mapping + privileges instead of the + destination domain. */ + /* ` } */ /* diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index b0ac1f6..dd99593 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -535,6 +535,20 @@ static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, str return xsm_default_action(action, d, t); } +/* + * Be aware that this is not an exact default equivalence of its flask + * variant which also checks if @d and @t "are allowed to share memory + * pages", for now, we don't have a proper default equivalence of such a + * check. + */ +static XSM_INLINE int xsm_map_gmfn_share(XSM_DEFAULT_ARG struct domain *d, + struct domain *t) +{ +
[Xen-devel] [PATCH v8 6/8] docs: documentation about static shared memory regions
From: Zhongze Liu Author: Zhongze Liu Add docs to document the motivation, usage, use cases and other relevant information about the static shared memory feature. This is for the proposal "Allow setting up shared memory areas between VMs from xl config file". See: https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Signed-off-by: Stefano Stabellini Cc: Ian Jackson Cc: Wei Liu Cc: Stefano Stabellini Cc: Julien Grall Cc: xen-de...@lists.xen.org --- Changes in v6: - add clarifications on memory allocation Changes in v5: - fix typos --- docs/man/xl-static-shm-configuration.pod.5 | 264 + docs/man/xl.cfg.pod.5.in | 8 + docs/misc/xenstore-paths.markdown | 47 + 3 files changed, 319 insertions(+) create mode 100644 docs/man/xl-static-shm-configuration.pod.5 diff --git a/docs/man/xl-static-shm-configuration.pod.5 b/docs/man/xl-static-shm-configuration.pod.5 new file mode 100644 index 000..81ff3f1 --- /dev/null +++ b/docs/man/xl-static-shm-configuration.pod.5 @@ -0,0 +1,264 @@ +=head1 NAME + +xl-static-shm-configuration - XL Static Shared Memory Configuration Syntax + + +(B: This is currently only available to ARM guests.) + +=head1 DESCRIPTION + +The static_shm option allows users to statically setup shared memory regions +among a group of VMs, enabling guests without grant table support to do +shm-based communication. + +Every shared region is: + +=over 4 + +* Uniquely identified by a string that is no longer than 128 characters, which +is called an B in this document. + +* Backed by exactly one domain, which is called a B domain, and all +the other domains who are also sharing this region are called Bs. + +=back + +=head1 SYNTAX + +This document specifies syntax of the static shared memory configuration in +the xl config file. It has the following form: + +static_shm = [ "SSHM_SPEC", "SSHM_SPEC", ... ] + +where each C is in this form: + +[=,]* + +Valid examples of C are: + +id=ID1, begin=0x10, size=0x10, role=master, cache_policy=x86_normal +id=ID1, offset = 0, begin=0x50, size=0x10, role=slave, prot=rw +id=ID2, begin=0x30, size=0x10, role=master +id=ID2, offset = 0x1, begin=0x69, size=0x11, role=slave +id=ID2, offset = 0x1, begin=0x69, size=0x11, role=slave + +These might be specified in the domain config file like this: + +static_shm = ["id=ID2, offset = 0x1, begin=0x69, size=0x11, +role=slave"] + + +More formally, the string is a series of comma-separated keyword/value +pairs. Each parameter may be specified at most once. Default values apply if +the parameter is not specified. + +=head1 Parameters + +=over 4 + +=item B + +=over 4 + +=item Description + +The unique identifier of the shared memory region. + +Every identifier could appear only once in each xl config file. + +=item Supported values + +A string that contains alphanumerics and "_"s, and is no longer than 128 +characters. + +=item Default value + +None, this parameter is mandatory. + +=back + +=item B/B + +=over 4 + +=item Description + +The boundaries of the shared memory area. + +=item Supported values + +Same with B. + +=item Default Value + +None, this parameter is mandatory. + +=back + +=item B + +=over 4 + +=item Description + +Can only appear when B = slave. If set, the address mapping will not +start from the beginning the backing memory region, but from the middle +(B bytes away from the beginning) of it. See the graph below: + +With B = 0, the mapping will look like: + + backing memory region: # + | | + | | + | | + V V + slave's shared region: # + +With B > 0: + + backing memory region: # + |<-- offset -->|| | + | | + | | + V V + slave's memory region: # + +=item Supported values + +Decimals or hexadecimals with a prefix "0x", and should be the multiple of the +hypervisor page granularity (currently 4K on both ARM and x86). + +=item Default value + +0x0 + +=back + +=item B + +=over 4 + +=item Description + +The backing area would be taken from one domain, which we will mark as +the "master domain", and this domain should be created prior to any +other slave domains that depend on it. The master's shared memory range +is NOT allocated in addition to its regular memory. Hence, it is usually +a good idea to choose a subrange of the regular guest
[Xen-devel] [PATCH v8 5/8] libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files
From: Zhongze Liu Author: Zhongze Liu Add the parsing utils for the newly introduced libxl_static_sshm struct to the libxl/libxlu_* family. And add realated parsing code in xl to parse the struct from xl config files. This is for the proposal "Allow setting up shared memory areas between VMs from xl config file" (see [1]). [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Signed-off-by: Stefano Stabellini Cc: Wei Liu Cc: Ian Jackson Cc: Stefano Stabellini Cc: Julien Grall Cc: xen-de...@lists.xen.org --- Changes in v5: - remove alignment checks, they were moved to libxl --- tools/libxl/Makefile | 2 +- tools/libxl/libxlu_sshm.c | 206 ++ tools/libxl/libxlutil.h | 6 ++ tools/xl/xl_parse.c | 25 +- 4 files changed, 237 insertions(+), 2 deletions(-) create mode 100644 tools/libxl/libxlu_sshm.c diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 53af186..f3de189 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -177,7 +177,7 @@ AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \ AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \ - libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o + libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o libxlu_sshm.o $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h $(TEST_PROG_OBJS) _libxl.api-for-check: CFLAGS += $(CFLAGS_libxentoollog) $(CFLAGS_libxentoolcore) diff --git a/tools/libxl/libxlu_sshm.c b/tools/libxl/libxlu_sshm.c new file mode 100644 index 000..7c3e512 --- /dev/null +++ b/tools/libxl/libxlu_sshm.c @@ -0,0 +1,206 @@ +#include "libxl_osdeps.h" /* must come before any other headers */ +#include "libxlu_internal.h" +#include "xenctrl.h" + +#include + +#define PARAM_RE(EXPR) "^\\s*" EXPR "\\s*(,|$)" +#define WORD_RE "([_a-zA-Z0-9]+)" +#define EQU_RE PARAM_RE(WORD_RE "\\s*=\\s*" WORD_RE) + +#define RET_INVAL(msg, curr_str) do { \ +xlu__sshm_err(cfg, msg, curr_str); \ +rc = EINVAL;\ +goto out; \ +} while(0) + +/* set a member in libxl_static_shm and report an error if it's respecified, + * @curr_str indicates the head of the remaining string. */ +#define SET_VAL(var, name, type, value, curr_str) do { \ +if ((var) != LIBXL_SSHM_##type##_UNKNOWN && (var) != value) { \ +RET_INVAL("\"" name "\" respecified", curr_str);\ +} \ +(var) = value; \ +} while(0) + + +static void xlu__sshm_err(XLU_Config *cfg, const char *msg, + const char *curr_str) { +fprintf(cfg->report, +"%s: config parsing error in shared_memory: %s at '%s'\n", +cfg->config_source, msg, curr_str); +} + +static int parse_prot(XLU_Config *cfg, char *str, libxl_sshm_prot *prot) +{ +int rc; +libxl_sshm_prot new_prot; + +if (!strcmp(str, "rw")) { +new_prot = LIBXL_SSHM_PROT_RW; +} else { +RET_INVAL("invalid permission flags", str); +} + +SET_VAL(*prot, "permission flags", PROT, new_prot, str); + +rc = 0; + + out: +return rc; +} + +static int parse_cachepolicy(XLU_Config *cfg, char *str, + libxl_sshm_cachepolicy *policy) +{ +int rc; +libxl_sshm_cachepolicy new_policy; + +if (!strcmp(str, "ARM_normal")) { +new_policy = LIBXL_SSHM_CACHEPOLICY_ARM_NORMAL; +} else if (!strcmp(str, "x86_normal")) { +new_policy = LIBXL_SSHM_CACHEPOLICY_X86_NORMAL; +} else { +RET_INVAL("invalid cache policy", str); +} + +SET_VAL(*policy, "cache policy", CACHEPOLICY, new_policy, str); +rc = 0; + + out: +return rc; +} + +/* handle key = value pairs */ +static int handle_equ(XLU_Config *cfg, char *key, char *val, + libxl_static_shm *sshm) +{ +int rc; + +if (!strcmp(key, "id")) { +if (sshm->id && !strcmp(sshm->id, val)) { +RET_INVAL("id respecified", val); +} + +sshm->id = strdup(val); +if (!sshm->id) { +fprintf(stderr, "sshm parser out of memory\n"); +rc = ENOMEM; +goto out; +} +} else if (!strcmp(key, "role")) { +libxl_sshm_role new_role; + +if (!strcmp("master", val)) { +new_role = LIBXL_SSHM_ROLE_MASTER; +} else if (!strcmp("slave", val)) { +new_role = LIBXL_SSHM_ROLE_SLAVE; +} else { +RET_INVAL("invalid role", val); +} + +SET_VAL(sshm->role, "role", ROLE, new_role, val); +} else if (!strcmp(key, "begin") || +
[Xen-devel] [PATCH v8 3/8] libxl: support mapping static shared memory areas during domain creation
From: Zhongze Liu Author: Zhongze Liu Add libxl__sshm_add to map shared pages from one DomU to another, The mapping process involves the following steps: * Set defaults and check for further errors in the static_shm configs: overlapping areas, invalid ranges, duplicated master domain, not page aligned, no master domain etc. * Use xc_domain_add_to_physmap_batch to map the shared pages to slaves * When some of the pages can't be successfully mapped, roll back any successfully mapped pages so that the system stays in a consistent state. * Write information about static shared memory areas into the appropriate xenstore paths and set the refcount of the shared region accordingly. Temporarily mark this as unsupported on x86 because calling p2m_add_foreign on two domU's is currently not allowd on x86 (see the comments in x86/mm/p2m.c:p2m_add_foreign for more details). This is for the proposal "Allow setting up shared memory areas between VMs from xl config file" (see [1]). [1] https://lists.xen.org/archives/html/xen-devel/2017-08/msg03242.html Signed-off-by: Zhongze Liu Signed-off-by: Stefano Stabellini Cc: Wei Liu Cc: Ian Jackson Cc: Stefano Stabellini Cc: Julien Grall Cc: xen-de...@lists.xen.org --- Changes in v5: - fix typos - add comments - add value checks (including alignment checks) in sshm_setdefaults --- tools/libxl/Makefile | 3 +- tools/libxl/libxl_arch.h | 6 + tools/libxl/libxl_arm.c | 15 ++ tools/libxl/libxl_create.c | 27 +++ tools/libxl/libxl_internal.h | 14 ++ tools/libxl/libxl_sshm.c | 405 +++ tools/libxl/libxl_x86.c | 19 ++ 7 files changed, 488 insertions(+), 1 deletion(-) create mode 100644 tools/libxl/libxl_sshm.c diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 6da342e..53af186 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -140,7 +140,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \ libxl_vtpm.o libxl_nic.o libxl_disk.o libxl_console.o \ libxl_cpupool.o libxl_mem.o libxl_sched.o libxl_tmem.o \ libxl_9pfs.o libxl_domain.o libxl_vdispl.o \ - libxl_pvcalls.o libxl_vsnd.o libxl_vkb.o $(LIBXL_OBJS-y) + libxl_pvcalls.o libxl_vsnd.o libxl_vkb.o libxl_sshm.o \ + $(LIBXL_OBJS-y) LIBXL_OBJS += libxl_genid.o LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h index 930570e..63c26cc 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -73,6 +73,12 @@ int libxl__arch_extra_memory(libxl__gc *gc, const libxl_domain_build_info *info, uint64_t *out); +_hidden +bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info); + +_hidden +int libxl__arch_domain_sshm_cachepolicy_setdefault(libxl_static_shm *sshm); + #if defined(__i386__) || defined(__x86_64__) #define LAPIC_BASE_ADDRESS 0xfee0 diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index 25dc3de..054ad58 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -1138,6 +1138,21 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); } +bool libxl__arch_domain_support_sshm(const libxl_domain_build_info *b_info) +{ +return true; +} + +int libxl__arch_domain_sshm_cachepolicy_setdefault(libxl_static_shm *sshm) +{ +if (sshm->cache_policy == LIBXL_SSHM_CACHEPOLICY_UNKNOWN) +sshm->cache_policy = LIBXL_SSHM_CACHEPOLICY_ARM_NORMAL; +if (sshm->cache_policy >= LIBXL_SSHM_CACHEPOLICY_X86_NORMAL) +return ERROR_INVAL; + +return 0; +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 320dbed..45ae9e4 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -513,6 +513,14 @@ int libxl__domain_build(libxl__gc *gc, ret = ERROR_INVAL; goto out; } + +/* The p2m has been setup, we could map the static shared memory now. */ +ret = libxl__sshm_add(gc, domid, d_config->sshms, d_config->num_sshms); +if (ret != 0) { +LOG(ERROR, "failed to map static shared memory"); +goto out; +} + ret = libxl__build_post(gc, domid, info, state, vments, localents); out: return ret; @@ -949,6 +957,25 @@ static void initiate_domain_create(libxl__egc *egc, goto error_out; } +if (d_config->num_sshms != 0 && +!libxl__arch_domain_support_sshm(_config->b_info)) { +LOGD(ERROR, domid, "static_shm is not supported by this domain type."); +ret = ERROR_INVAL; +goto error_out; +} + +for (i = 0; i < d_config->num_sshms; ++i) { +ret =
[Xen-devel] [PATCH v8 8/8] xen/arm: map reserved-memory regions as normal memory in dom0
reserved-memory regions should be mapped as normal memory. At the moment, they get remapped as device memory because Xen doesn't know better. Add an explicit check for it. Signed-off-by: Stefano Stabellini --- xen/arch/arm/domain_build.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index f552154..c7df4cf 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1307,6 +1307,13 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n", path); +/* + * reserved-memory ranges should be mapped as normal memory in the + * p2m. + */ +if ( !strcmp(dt_node_name(node), "reserved-memory") ) +p2mt = p2m_ram_rw; + res = handle_device(d, node, p2mt); if ( res) return res; -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 0/8] Allow setting up shared memory areas between VMs from xl config files
Hi, This series implements a new xl config entry. Users can use the new config entry to statically setup shared memory areas among VMs that don't have grant table support so that they could communicate with each other through the static shared memory areas. It was originally developed by Zhongze, I am just updating the last few issued that were address during the last round of reviews in January. I tested the feature on ARM and works fine. Cheers, Stefano Main changes in v8: - remove "add xen,dmabuf nodes" patch - add "map reserved-memory regions as normal memory in dom0" patch - send new device tree binding request to devicetree.org The following changes since commit 85b00385827e4e061b2ff38b549c03d0f1e66b6a: xen/sched: Drop set_current_state() (2018-10-08 18:34:55 +0100) are available in the git repository at: http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git share_mem-v8 for you to fetch changes up to fc00e34ce9924b5915deacd881ae7cd6888510ba: xen/arm: map reserved-memory regions as normal memory in dom0 (2018-10-08 16:34:09 -0700) Stefano Stabellini (2): xen/arm: export shared memory regions as reserved-memory on device tree xen/arm: map reserved-memory regions as normal memory in dom0 Zhongze Liu (6): xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing libxl: introduce a new structure to represent static shared memory regions libxl: support mapping static shared memory areas during domain creation libxl: support unmapping static shared memory areas during domain destruction libxl:xl: add parsing code to parse "libxl_static_sshm" from xl config files docs: documentation about static shared memory regions docs/man/xl-static-shm-configuration.pod.5 | 264 +++ docs/man/xl.cfg.pod.5.in | 8 + docs/misc/xenstore-paths.markdown | 47 +++ tools/flask/policy/modules/xen.if | 2 + tools/libxl/Makefile | 5 +- tools/libxl/libxl.h| 8 + tools/libxl/libxl_arch.h | 8 +- tools/libxl/libxl_arm.c| 76 - tools/libxl/libxl_create.c | 27 ++ tools/libxl/libxl_dom.c| 2 +- tools/libxl/libxl_domain.c | 5 + tools/libxl/libxl_internal.h | 16 + tools/libxl/libxl_sshm.c | 512 + tools/libxl/libxl_types.idl| 32 +- tools/libxl/libxl_x86.c| 21 +- tools/libxl/libxlu_sshm.c | 206 tools/libxl/libxlutil.h| 6 + tools/xl/xl_parse.c| 25 +- xen/arch/arm/domain_build.c| 7 + xen/arch/arm/mm.c | 7 +- xen/include/public/memory.h| 8 + xen/include/xsm/dummy.h| 14 + xen/include/xsm/xsm.h | 6 + xen/xsm/dummy.c| 1 + xen/xsm/flask/hooks.c | 9 + xen/xsm/flask/policy/access_vectors| 5 + 26 files changed, 1315 insertions(+), 12 deletions(-) create mode 100644 docs/man/xl-static-shm-configuration.pod.5 create mode 100644 tools/libxl/libxl_sshm.c create mode 100644 tools/libxl/libxlu_sshm.c ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 128495: regressions - FAIL
flight 128495 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/128495/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-arndale 6 xen-install fail REGR. vs. 128449 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 128449 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 128449 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 128449 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 128449 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 128449 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuue2e3436add538be0e558cdc42f3e6b76e9deb0f9 baseline version: qemuu3c2d3042849686969add641bd38b08b9877b9e8f Last test of basis 128449 2018-10-06 11:37:10 Z2 days Testing same since 128495 2018-10-08 09:09:24 Z0 days1 attempts People who touched revisions under test: Eric Blake Gerd Hoffmann Peter Maydell Philippe Mathieu-Daudé remy.noel jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt
[Xen-devel] [PATCH] devicetree,xen: add xen,shared-memory binding
Introduce a device tree binding for Xen reserved-memory regions. They are used to share memory across VMs from the VM config files. (See static_shm config option.) Signed-off-by: Stefano Stabellini Cc: julien.gr...@arm.com diff --git a/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt b/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt new file mode 100644 index 000..a927a94 --- /dev/null +++ b/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt @@ -0,0 +1,20 @@ +* Xen hypervisor reserved-memory binding + +Expose one or more memory regions as reserved-memory to the guest +virtual machine. Typically, a region is configured at VM creation time +to be a shared memory area across multiple virtual machines for +communication among them. + +For each of these pre-shared memory regions, a range is exposed under +the /reserved-memory node as a child node. Each range sub-node is named +xen-shmem@ and has the following properties: + +- compatible: + compatible = xen,shared-memory" + +- reg: + the base guest physical address and size of the shared memory region + +- id: + a string that identifies the shared memory region as specified in + the VM config file ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus bisection] complete test-amd64-i386-xl-qemut-win10-i386
branch xen-unstable xenbranch xen-unstable job test-amd64-i386-xl-qemut-win10-i386 testid xen-boot Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Bug introduced: fb1c592cf4c9eeb84ec6bce13c13c11c7d05b6c7 Bug not present: 94710cac0ef4ee177a63b5227664b38c95bbf703 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/128516/ (Revision log too long, omitted.) For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-linus/test-amd64-i386-xl-qemut-win10-i386.xen-boot.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/linux-linus/test-amd64-i386-xl-qemut-win10-i386.xen-boot --summary-out=tmp/128516.bisection-summary --basis-template=125898 --blessings=real,real-bisect linux-linus test-amd64-i386-xl-qemut-win10-i386 xen-boot Searching for failure / basis pass: 128476 fail [host=chardonnay0] / 125898 ok. Failure / basis pass flights: 128476 / 125898 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest fb1c592cf4c9eeb84ec6bce13c13c11c7d05b6c7 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986 359970fd8b781fac2ddcbc84dd5b890075fa08ef Basis pass 94710cac0ef4ee177a63b5227664b38c95bbf703 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 1f7574763cbb2c85825b8cc4d81f386e767a476f Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git#94710cac0ef4ee177a63b5227664b38c95bbf703-fb1c592cf4c9eeb84ec6bce13c13c11c7d05b6c7 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 git://xenbits.xen.org/qemu-xen.git#43139135a8938de44f66333831d3a8655d07663a-de5b678ca4dcdfa83e322491d478d66df56c1986 git://xenbits.xen.org/xen.git#1f7574763cbb2c85825b8cc4d81f386e767a476f-359970fd8b781fac2ddcbc84dd5b890075fa08ef adhoc-revtuple-generator: tree discontiguous: linux-2.6 adhoc-revtuple-generator: tree discontiguous: qemu-xen Loaded 2006 nodes in revision graph Searching for test results: 125702 pass irrelevant 125898 pass 94710cac0ef4ee177a63b5227664b38c95bbf703 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 1f7574763cbb2c85825b8cc4d81f386e767a476f 125921 fail irrelevant 126069 fail irrelevant 126202 fail irrelevant 126310 fail irrelevant 126412 fail irrelevant 126550 fail irrelevant 126682 fail irrelevant 126888 fail irrelevant 126978 [] 127038 fail irrelevant 127108 fail irrelevant 127148 fail irrelevant 127193 fail irrelevant 127221 fail irrelevant 127256 fail irrelevant 127284 fail irrelevant 127315 fail irrelevant 127344 fail irrelevant 127364 fail irrelevant 127389 fail irrelevant 127403 fail irrelevant 127415 fail irrelevant 127443 fail irrelevant 127479 fail irrelevant 127458 fail irrelevant 127516 fail irrelevant 127497 fail irrelevant 127535 fail irrelevant 127551 fail irrelevant 127569 fail irrelevant 127617 fail irrelevant 127732 fail irrelevant 127793 fail irrelevant 127907 fail irrelevant 127976 fail irrelevant 127962 fail irrelevant 127991 fail irrelevant 128002 fail irrelevant 128022 fail irrelevant 128059 fail irrelevant 128114 fail irrelevant 128170 fail irrelevant 128264 fail irrelevant 128236 fail irrelevant 128278 fail irrelevant 128334 fail irrelevant 128312 fail irrelevant 128369 fail irrelevant 128407 fail irrelevant 128438 fail irrelevant 128488 pass 94710cac0ef4ee177a63b5227664b38c95bbf703 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 1f7574763cbb2c85825b8cc4d81f386e767a476f 128508 pass 94710cac0ef4ee177a63b5227664b38c95bbf703 c530a75c1e6a472b0eb9558310b518f0dfcd8860 9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 de5b678ca4dcdfa83e322491d478d66df56c1986
[Xen-devel] [xen-unstable-smoke test] 128513: tolerable all pass - PUSHED
flight 128513 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/128513/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 85b00385827e4e061b2ff38b549c03d0f1e66b6a baseline version: xen a696f4e63ca51693736a6cf7b911522287238653 Last test of basis 128509 2018-10-08 15:00:49 Z0 days Testing same since 128513 2018-10-08 18:01:11 Z0 days1 attempts People who touched revisions under test: Andrew Cooper George Dunlap Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git a696f4e63c..85b0038582 85b00385827e4e061b2ff38b549c03d0f1e66b6a -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 11/16] xen/arm: vsysreg: Add wrapper to handle sysreg access trapped by HCR_EL2.TVM
A follow-up patch will require to emulate some accesses to system registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes to the virtual memory control registers will be trapped to the hypervisor. This patch adds the infrastructure to passthrough the access to the host registers. Note that HCR_EL2.TVM will be set in a follow-up patch dynamically. Signed-off-by: Julien Grall --- xen/arch/arm/arm64/vsysreg.c | 57 1 file changed, 57 insertions(+) diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c index 6e60824572..1517879697 100644 --- a/xen/arch/arm/arm64/vsysreg.c +++ b/xen/arch/arm/arm64/vsysreg.c @@ -23,6 +23,46 @@ #include #include +/* + * Macro to help generating helpers for registers trapped when + * HCR_EL2.TVM is set. + * + * Note that it only traps NS write access from EL1. + */ +#define TVM_REG(reg)\ +static bool vreg_emulate_##reg(struct cpu_user_regs *regs, \ + uint64_t *r, bool read) \ +{ \ +GUEST_BUG_ON(read); \ +WRITE_SYSREG64(*r, reg);\ +\ +return true;\ +} + +/* Defining helpers for emulating sysreg registers. */ +TVM_REG(SCTLR_EL1) +TVM_REG(TTBR0_EL1) +TVM_REG(TTBR1_EL1) +TVM_REG(TCR_EL1) +TVM_REG(ESR_EL1) +TVM_REG(FAR_EL1) +TVM_REG(AFSR0_EL1) +TVM_REG(AFSR1_EL1) +TVM_REG(MAIR_EL1) +TVM_REG(AMAIR_EL1) +TVM_REG(CONTEXTIDR_EL1) + +/* Macro to generate easily case for co-processor emulation */ +#define GENERATE_CASE(reg) \ +case HSR_SYSREG_##reg: \ +{ \ +bool res; \ +\ +res = vreg_emulate_sysreg64(regs, hsr, vreg_emulate_##reg); \ +ASSERT(res);\ +break; \ +} + void do_sysreg(struct cpu_user_regs *regs, const union hsr hsr) { @@ -44,6 +84,23 @@ void do_sysreg(struct cpu_user_regs *regs, break; /* + * HCR_EL2.TVM + * + * ARMv8 (DDI 0487B.b): Table D1-37 + */ +GENERATE_CASE(SCTLR_EL1) +GENERATE_CASE(TTBR0_EL1) +GENERATE_CASE(TTBR1_EL1) +GENERATE_CASE(TCR_EL1) +GENERATE_CASE(ESR_EL1) +GENERATE_CASE(FAR_EL1) +GENERATE_CASE(AFSR0_EL1) +GENERATE_CASE(AFSR1_EL1) +GENERATE_CASE(MAIR_EL1) +GENERATE_CASE(AMAIR_EL1) +GENERATE_CASE(CONTEXTIDR_EL1) + +/* * MDCR_EL2.TDRA * * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 04/16] xen/arm: guest_walk_tables: Switch the return to bool
At the moment, guest_walk_tables can either return 0, -EFAULT, -EINVAL. The use of the last 2 are not clearly defined and used inconsistently in the code. The current only caller does not care about the return value and the value of it seems very limited (no way to differentiate between the 15ish error paths). So switch to bool to simplify the return and make the developer life a bit easier. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- This patch was originally sent separately and reviewed by Stefano. --- xen/arch/arm/guest_walk.c| 50 xen/arch/arm/mem_access.c| 2 +- xen/include/asm-arm/guest_walk.h | 8 +++ 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index 4a1b4cf2c8..7db7a7321b 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -28,9 +28,9 @@ * page table on a different vCPU, the following registers would need to be * loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1. */ -static int guest_walk_sd(const struct vcpu *v, - vaddr_t gva, paddr_t *ipa, - unsigned int *perms) +static bool guest_walk_sd(const struct vcpu *v, + vaddr_t gva, paddr_t *ipa, + unsigned int *perms) { int ret; bool disabled = true; @@ -79,7 +79,7 @@ static int guest_walk_sd(const struct vcpu *v, } if ( disabled ) -return -EFAULT; +return false; /* * The address of the L1 descriptor for the initial lookup has the @@ -97,12 +97,12 @@ static int guest_walk_sd(const struct vcpu *v, /* Access the guest's memory to read only one PTE. */ ret = access_guest_memory_by_ipa(d, paddr, , sizeof(short_desc_t), false); if ( ret ) -return -EINVAL; +return false; switch ( pte.walk.dt ) { case L1DESC_INVALID: -return -EFAULT; +return false; case L1DESC_PAGE_TABLE: /* @@ -122,10 +122,10 @@ static int guest_walk_sd(const struct vcpu *v, /* Access the guest's memory to read only one PTE. */ ret = access_guest_memory_by_ipa(d, paddr, , sizeof(short_desc_t), false); if ( ret ) -return -EINVAL; +return false; if ( pte.walk.dt == L2DESC_INVALID ) -return -EFAULT; +return false; if ( pte.pg.page ) /* Small page. */ { @@ -175,7 +175,7 @@ static int guest_walk_sd(const struct vcpu *v, *perms |= GV2M_EXEC; } -return 0; +return true; } /* @@ -355,9 +355,9 @@ static bool check_base_size(unsigned int output_size, uint64_t base) * page table on a different vCPU, the following registers would need to be * loaded: TCR_EL1, TTBR0_EL1, TTBR1_EL1, and SCTLR_EL1. */ -static int guest_walk_ld(const struct vcpu *v, - vaddr_t gva, paddr_t *ipa, - unsigned int *perms) +static bool guest_walk_ld(const struct vcpu *v, + vaddr_t gva, paddr_t *ipa, + unsigned int *perms) { int ret; bool disabled = true; @@ -442,7 +442,7 @@ static int guest_walk_ld(const struct vcpu *v, */ if ( (input_size > TCR_EL1_IPS_48_BIT_VAL) || (input_size < TCR_EL1_IPS_MIN_VAL) ) -return -EFAULT; +return false; } else { @@ -487,7 +487,7 @@ static int guest_walk_ld(const struct vcpu *v, } if ( disabled ) -return -EFAULT; +return false; /* * The starting level is the number of strides (grainsizes[gran] - 3) @@ -498,12 +498,12 @@ static int guest_walk_ld(const struct vcpu *v, /* Get the IPA output_size. */ ret = get_ipa_output_size(d, tcr, _size); if ( ret ) -return -EFAULT; +return false; /* Make sure the base address does not exceed its configured size. */ ret = check_base_size(output_size, ttbr); if ( !ret ) -return -EFAULT; +return false; /* * Compute the base address of the first level translation table that is @@ -523,12 +523,12 @@ static int guest_walk_ld(const struct vcpu *v, /* Access the guest's memory to read only one PTE. */ ret = access_guest_memory_by_ipa(d, paddr, , sizeof(lpae_t), false); if ( ret ) -return -EFAULT; +return false; /* Make sure the base address does not exceed its configured size. */ ret = check_base_size(output_size, pfn_to_paddr(pte.walk.base)); if ( !ret ) -return -EFAULT; +return false; /* * If page granularity is 64K, make sure the address is aligned @@ -537,7 +537,7 @@ static int guest_walk_ld(const struct vcpu *v, if ( (output_size < TCR_EL1_IPS_52_BIT_VAL) && (gran
[Xen-devel] [RFC 14/16] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit)
With the recent changes, a P2M entry may be populated but may as not valid. In some situation, it would be useful to know whether the entry has been marked available to guest in order to perform a specific action. So extend p2m_get_entry to return the value of bit[0] (valid bit). Signed-off-by: Julien Grall --- xen/arch/arm/mem_access.c | 6 +++--- xen/arch/arm/p2m.c| 20 xen/include/asm-arm/p2m.h | 3 ++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c index 9239bdf323..f434510b2a 100644 --- a/xen/arch/arm/mem_access.c +++ b/xen/arch/arm/mem_access.c @@ -70,7 +70,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, * No setting was found in the Radix tree. Check if the * entry exists in the page-tables. */ -mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL); +mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) return -ESRCH; @@ -199,7 +199,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag, * We had a mem_access permission limiting the access, but the page type * could also be limiting, so we need to check that as well. */ -mfn = p2m_get_entry(p2m, gfn, , NULL, NULL); +mfn = p2m_get_entry(p2m, gfn, , NULL, NULL, NULL); if ( mfn_eq(mfn, INVALID_MFN) ) goto err; @@ -405,7 +405,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr, gfn = gfn_next_boundary(gfn, order) ) { p2m_type_t t; -mfn_t mfn = p2m_get_entry(p2m, gfn, , NULL, ); +mfn_t mfn = p2m_get_entry(p2m, gfn, , NULL, , NULL); if ( !mfn_eq(mfn, INVALID_MFN) ) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 12b459924b..df6b48d73b 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -306,10 +306,14 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, * * If the entry is not present, INVALID_MFN will be returned and the * page_order will be set according to the order of the invalid range. + * + * valid will contain the value of bit[0] (e.g valid bit) of the + * entry. */ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, p2m_type_t *t, p2m_access_t *a, -unsigned int *page_order) +unsigned int *page_order, +bool *valid) { paddr_t addr = gfn_to_gaddr(gfn); unsigned int level = 0; @@ -317,6 +321,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, int rc; mfn_t mfn = INVALID_MFN; p2m_type_t _t; +bool _valid; /* Convenience aliases */ const unsigned int offsets[4] = { @@ -334,6 +339,10 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, *t = p2m_invalid; +/* Allow valid to be NULL */ +valid = valid?: &_valid; +*valid = false; + /* XXX: Check if the mapping is lower than the mapped gfn */ /* This gfn is higher than the highest the p2m map currently holds */ @@ -379,6 +388,9 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, * to the GFN. */ mfn = mfn_add(mfn, gfn_x(gfn) & ((1UL << level_orders[level]) - 1)); + +if ( valid ) +*valid = lpae_is_valid(entry); } out_unmap: @@ -397,7 +409,7 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t) struct p2m_domain *p2m = p2m_get_hostp2m(d); p2m_read_lock(p2m); -mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL); +mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL, NULL); p2m_read_unlock(p2m); return mfn; @@ -1464,7 +1476,7 @@ int relinquish_p2m_mapping(struct domain *d) for ( ; gfn_x(start) < gfn_x(end); start = gfn_next_boundary(start, order) ) { -mfn_t mfn = p2m_get_entry(p2m, start, , NULL, ); +mfn_t mfn = p2m_get_entry(p2m, start, , NULL, , NULL); count++; /* @@ -1527,7 +1539,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) for ( ; gfn_x(start) < gfn_x(end); start = next_gfn ) { -mfn_t mfn = p2m_get_entry(p2m, start, , NULL, ); +mfn_t mfn = p2m_get_entry(p2m, start, , NULL, , NULL); next_gfn = gfn_next_boundary(start, order); diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index d7afa2bbe8..92213dd1ab 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -211,7 +211,8 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t); */ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, p2m_type_t *t, p2m_access_t *a, -unsigned int *page_order); +unsigned int *page_order, +bool *valid); /* * Direct set a p2m entry: only for use by the P2M code. -- 2.11.0
[Xen-devel] [RFC 16/16] xen/arm: Track page accessed between batch of Set/Way operations
At the moment, the implementation of Set/Way operations will go through all the entries of the guest P2M and flush them. However, this is very expensive and may render unusable a guest OS using them. For instance, Linux 32-bit will use Set/Way operations during secondary CPU bring-up. As the implementation is really expensive, it may be possible to hit the CPU bring-up timeout. To limit the Set/Way impact, we track what pages has been of the guest has been accessed between batch of Set/Way operations. This is done using bit[0] (aka valid bit) of the P2M entry. This patch adds a new per-arch helper is introduced to perform actions just before the guest is first unpaused. This will be used to invalidate the P2M to track access from the start of the guest. Signed-off-by: Julien Grall --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Konrad Rzeszutek Wilk Cc: Tim Deegan Cc: Wei Liu --- xen/arch/arm/domain.c | 14 ++ xen/arch/arm/domain_build.c | 7 +++ xen/arch/arm/p2m.c | 32 +++- xen/arch/x86/domain.c | 4 xen/common/domain.c | 5 - xen/include/asm-arm/p2m.h | 2 ++ xen/include/xen/domain.h| 2 ++ 7 files changed, 64 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index feebbf5a92..f439f4657a 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -738,6 +738,20 @@ int arch_domain_soft_reset(struct domain *d) return -ENOSYS; } +void arch_domain_creation_finished(struct domain *d) +{ +/* + * To avoid flushing the whole guest RAM on the first Set/Way, we + * invalidate the P2M to track what has been accessed. + * + * This is only turned when IOMMU is not used or the page-table are + * not shared because bit[0] (e.g valid bit) unset will result + * IOMMU fault that could be not fixed-up. + */ +if ( !iommu_use_hap_pt(d) ) +p2m_invalidate_root(p2m_get_hostp2m(d)); +} + static int is_guest_pv32_psr(uint32_t psr) { switch (psr & PSR_MODE_MASK) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index f552154e93..de96516faa 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2249,6 +2249,13 @@ int __init construct_dom0(struct domain *d) v->is_initialised = 1; clear_bit(_VPF_down, >pause_flags); +/* + * XXX: We probably want a command line option to invalidate or not + * the P2M. This is because invalidating the P2M will not work with + * IOMMU, however if this is not done there will be an impact. + */ +p2m_invalidate_root(p2m_get_hostp2m(d)); + return 0; } diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index a3d4c563b1..8e0c31d7ac 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1079,6 +1079,22 @@ static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) p2m->need_flush = true; } +/* + * Invalidate all entries in the root page-tables. This is + * useful to get fault on entry and do an action. + */ +void p2m_invalidate_root(struct p2m_domain *p2m) +{ +unsigned int i; + +p2m_write_lock(p2m); + +for ( i = 0; i < P2M_ROOT_LEVEL; i++ ) +p2m_invalidate_table(p2m, page_to_mfn(p2m->root + i)); + +p2m_write_unlock(p2m); +} + bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn) { struct p2m_domain *p2m = p2m_get_hostp2m(d); @@ -1539,7 +1555,8 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) for ( ; gfn_x(start) < gfn_x(end); start = next_gfn ) { -mfn_t mfn = p2m_get_entry(p2m, start, , NULL, , NULL); +bool valid; +mfn_t mfn = p2m_get_entry(p2m, start, , NULL, , ); next_gfn = gfn_next_boundary(start, order); @@ -1547,6 +1564,13 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) ) continue; +/* + * Page with valid bit (bit [0]) unset does not need to be + * cleaned + */ +if ( !valid ) +continue; + /* XXX: Implement preemption */ while ( gfn_x(start) < gfn_x(next_gfn) ) { @@ -1571,6 +1595,12 @@ static void p2m_flush_vm(struct vcpu *v) /* XXX: Handle preemption */ p2m_cache_flush_range(v->domain, p2m->lowest_mapped_gfn, p2m->max_mapped_gfn); + +/* + * Invalidate the p2m to track which page was modified by the guest + * between call of p2m_flush_vm(). + */ +p2m_invalidate_root(p2m); } /* diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 9371efc8c7..2b6d1c01a1 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -723,6 +723,10 @@ int arch_domain_soft_reset(struct domain *d) return ret; } +void arch_domain_creation_finished(struct domain *d) +{
[Xen-devel] [RFC 01/16] xen/arm: Introduce helpers to clear/flags flags in HCR_EL2
A couple of places in the code will need to clear/set flags in HCR_EL2 for a given vCPU and then replicate into the hardware. Introduce helpers and replace open-coded version. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- The patch was previously sent separately and reviewed by Stefano. I haven't find a good place for those new helpers so stick to processor.h at the moment. This require to use macro rather than inline helpers given that processor.h is usually the root of all headers. --- xen/arch/arm/traps.c| 3 +-- xen/include/asm-arm/processor.h | 18 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 51d2e42c77..9251ae50b8 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -682,8 +682,7 @@ static void inject_vabt_exception(struct cpu_user_regs *regs) break; } -current->arch.hcr_el2 |= HCR_VA; -WRITE_SYSREG(current->arch.hcr_el2, HCR_EL2); +vcpu_hcr_set_flags(current, HCR_VA); } /* diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index 8016cf306f..975c8ff097 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -840,6 +840,24 @@ void abort_guest_exit_end(void); : : : "memory"); \ } while (0) +/* + * Clear/Set flags in HCR_EL2 for a given vCPU. It only supports the current + * vCPU for now. + */ +#define vcpu_hcr_clear_flags(v, flags) \ +do {\ +ASSERT((v) == current); \ +(v)->arch.hcr_el2 &= ~(flags); \ +WRITE_SYSREG((v)->arch.hcr_el2, HCR_EL2); \ +} while (0) + +#define vcpu_hcr_set_flags(v, flags)\ +do {\ +ASSERT((v) == current); \ +(v)->arch.hcr_el2 |= (flags); \ +WRITE_SYSREG((v)->arch.hcr_el2, HCR_EL2); \ +} while (0) + #endif /* __ASSEMBLY__ */ #endif /* __ASM_ARM_PROCESSOR_H */ /* -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 00/16] xen/arm: Implement Set/Way operations
Hi all, As discussed during Linaro Connect with Stefano, I am sending an early version of the Set/Way implementation to gather feedback on the approach. For more details on the issue, see the commit message of patch #15. A branch with the code is available can be clone from: https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git branch dev-cacheflush Cheers, Julien Grall (16): xen/arm: Introduce helpers to clear/flags flags in HCR_EL2 xen/arm: Introduce helpers to get/set an MFN from/to an LPAE entry xen/arm: Allow lpae_is_{table, mapping} helpers to work on invalid entry xen/arm: guest_walk_tables: Switch the return to bool xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h xen/arm: p2m: Introduce a helper to generate P2M table entry from a page xen/arm: p2m: Introduce p2m_is_valid and use it xen/arm: p2m: Handle translation fault in get_page_from_gva xen/arm: p2m: Introduce a function to resolve translation fault xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM xen/arm: vsysreg: Add wrapper to handle sysreg access trapped by HCR_EL2.TVM xen/arm: Rework p2m_cache_flush to take a range [begin, end) xen/arm: p2m: Allow to flush cache on any RAM region xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit) xen/arm: Implement Set/Way operations xen/arm: Track page accessed between batch of Set/Way operations xen/arch/arm/arm64/vsysreg.c | 82 xen/arch/arm/domain.c| 14 ++ xen/arch/arm/domain_build.c | 7 + xen/arch/arm/domctl.c| 2 +- xen/arch/arm/guest_walk.c| 52 ++--- xen/arch/arm/mem_access.c| 8 +- xen/arch/arm/mm.c| 12 +- xen/arch/arm/p2m.c | 405 ++- xen/arch/arm/traps.c | 36 +--- xen/arch/arm/vcpreg.c| 167 xen/arch/x86/domain.c| 4 + xen/common/domain.c | 5 +- xen/include/asm-arm/cpregs.h | 1 + xen/include/asm-arm/guest_walk.h | 8 +- xen/include/asm-arm/lpae.h | 14 +- xen/include/asm-arm/p2m.h| 28 ++- xen/include/asm-arm/processor.h | 18 ++ xen/include/asm-arm/traps.h | 24 +++ xen/include/xen/domain.h | 2 + 19 files changed, 764 insertions(+), 125 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 03/16] xen/arm: Allow lpae_is_{table, mapping} helpers to work on invalid entry
Currently, lpae_is_{table, mapping} helpers will always return false on entries with the valid bit unset. However, it would be useful to have them operating on any entry. For instance to store information in advance but still request a fault. With that change, the p2m is now providing an overlay for *_is_{table, mapping} that will check the valid bit of the entry. Signed-off-by: Julien Grall --- The patch was previously sent separately. --- xen/arch/arm/guest_walk.c | 2 +- xen/arch/arm/mm.c | 2 +- xen/arch/arm/p2m.c | 22 ++ xen/include/asm-arm/lpae.h | 11 +++ 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index e3e21bdad3..4a1b4cf2c8 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -566,7 +566,7 @@ static int guest_walk_ld(const struct vcpu *v, * PTE is invalid or holds a reserved entry (PTE<1:0> == x0)) or if the PTE * maps a memory block at level 3 (PTE<1:0> == 01). */ -if ( !lpae_is_mapping(pte, level) ) +if ( !lpae_is_valid(pte) || !lpae_is_mapping(pte, level) ) return -EFAULT; /* Make sure that the lower bits of the PTE's base address are zero. */ diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 0bc31b1d9b..987fcb9162 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -996,7 +996,7 @@ static int create_xen_entries(enum xenmap_operation op, for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1)) { entry = _second[second_linear_offset(addr)]; -if ( !lpae_is_table(*entry, 2) ) +if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) ) { rc = create_xen_table(entry); if ( rc < 0 ) { diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index f8a2f6459e..8fffb42889 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -219,6 +219,20 @@ static p2m_access_t p2m_mem_access_radix_get(struct p2m_domain *p2m, gfn_t gfn) return radix_tree_ptr_to_int(ptr); } +/* + * lpae_is_* helpers don't check whether the valid bit is set in the + * PTE. Provide our own overlay to check the valid bit. + */ +static inline bool p2m_is_mapping(lpae_t pte, unsigned int level) +{ +return lpae_is_valid(pte) && lpae_is_mapping(pte, level); +} + +static inline bool p2m_is_superpage(lpae_t pte, unsigned int level) +{ +return lpae_is_valid(pte) && lpae_is_superpage(pte, level); +} + #define GUEST_TABLE_MAP_FAILED 0 #define GUEST_TABLE_SUPER_PAGE 1 #define GUEST_TABLE_NORMAL_PAGE 2 @@ -262,7 +276,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, /* The function p2m_next_level is never called at the 3rd level */ ASSERT(level < 3); -if ( lpae_is_mapping(*entry, level) ) +if ( p2m_is_mapping(*entry, level) ) return GUEST_TABLE_SUPER_PAGE; mfn = lpae_get_mfn(*entry); @@ -642,7 +656,7 @@ static void p2m_free_entry(struct p2m_domain *p2m, return; /* Nothing to do but updating the stats if the entry is a super-page. */ -if ( lpae_is_superpage(entry, level) ) +if ( p2m_is_superpage(entry, level) ) { p2m->stats.mappings[level]--; return; @@ -697,7 +711,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry, * a superpage. */ ASSERT(level < target); -ASSERT(lpae_is_superpage(*entry, level)); +ASSERT(p2m_is_superpage(*entry, level)); page = alloc_domheap_page(NULL, 0); if ( !page ) @@ -836,7 +850,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m, /* We need to split the original page. */ lpae_t split_pte = *entry; -ASSERT(lpae_is_superpage(*entry, level)); +ASSERT(p2m_is_superpage(*entry, level)); if ( !p2m_split_superpage(p2m, _pte, level, target, offsets) ) { diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h index 17fdc6074f..545b0c8f24 100644 --- a/xen/include/asm-arm/lpae.h +++ b/xen/include/asm-arm/lpae.h @@ -133,16 +133,19 @@ static inline bool lpae_is_valid(lpae_t pte) return pte.walk.valid; } +/* + * lpae_is_* don't check the valid bit. This gives an opportunity for the + * callers to operate on the entry even if they are not valid. For + * instance to store information in advance. + */ static inline bool lpae_is_table(lpae_t pte, unsigned int level) { -return (level < 3) && lpae_is_valid(pte) && pte.walk.table; +return (level < 3) && pte.walk.table; } static inline bool lpae_is_mapping(lpae_t pte, unsigned int level) { -if ( !lpae_is_valid(pte) ) -return false; -else if ( level == 3 ) +if ( level == 3 ) return pte.walk.table; else return !pte.walk.table; -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org
[Xen-devel] [RFC 07/16] xen/arm: p2m: Introduce p2m_is_valid and use it
The LPAE format allows to store information in an entry even with the valid bit unset. In a follow-up patch, we will take advantage of this feature to re-purpose the valid bit for generating a translation fault even if an entry contains valid information. So we need a different way to know whether an entry contains valid information. It is possible to use the information hold in the p2m_type to know for that purpose. Indeed all entries containing valid information will have a valid p2m type (i.e p2m_type != p2m_invalid). This patch introduces a new helper p2m_is_valid, which implements that idea, and replace most of lpae_is_valid call with the new helper. The ones remaining are for TLBs handling and entries accounting. With the renaming there are 2 others changes required: - Generate table entry with a valid p2m type - Detect new mapping for proper stats accounting Signed-off-by: Julien Grall --- xen/arch/arm/p2m.c | 34 +++--- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 6c76298ebc..2a1e7e9be2 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -220,17 +220,26 @@ static p2m_access_t p2m_mem_access_radix_get(struct p2m_domain *p2m, gfn_t gfn) } /* + * In the case of the P2M, the valid bit is used for other purpose. Use + * the type to check whether an entry is valid. + */ +static inline bool p2m_is_valid(lpae_t pte) +{ +return pte.p2m.type != p2m_invalid; +} + +/* * lpae_is_* helpers don't check whether the valid bit is set in the * PTE. Provide our own overlay to check the valid bit. */ static inline bool p2m_is_mapping(lpae_t pte, unsigned int level) { -return lpae_is_valid(pte) && lpae_is_mapping(pte, level); +return p2m_is_valid(pte) && lpae_is_mapping(pte, level); } static inline bool p2m_is_superpage(lpae_t pte, unsigned int level) { -return lpae_is_valid(pte) && lpae_is_superpage(pte, level); +return p2m_is_valid(pte) && lpae_is_superpage(pte, level); } #define GUEST_TABLE_MAP_FAILED 0 @@ -264,7 +273,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, entry = *table + offset; -if ( !lpae_is_valid(*entry) ) +if ( !p2m_is_valid(*entry) ) { if ( read_only ) return GUEST_TABLE_MAP_FAILED; @@ -356,7 +365,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, entry = table[offsets[level]]; -if ( lpae_is_valid(entry) ) +if ( p2m_is_valid(entry) ) { *t = entry.p2m.type; @@ -544,8 +553,11 @@ static lpae_t page_to_p2m_table(struct page_info *page) /* * The access value does not matter because the hardware will ignore * the permission fields for table entry. + * + * We use p2m_ram_rw so the entry has a valid type. This is important + * for p2m_is_valid() to return valid on table entries. */ -return mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, p2m_access_rwx); +return mfn_to_p2m_entry(page_to_mfn(page), p2m_ram_rw, p2m_access_rwx); } static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool clean_pte) @@ -569,7 +581,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry) struct page_info *page; lpae_t *p; -ASSERT(!lpae_is_valid(*entry)); +ASSERT(!p2m_is_valid(*entry)); page = alloc_domheap_page(NULL, 0); if ( page == NULL ) @@ -626,7 +638,7 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m, gfn_t gfn, */ static void p2m_put_l3_page(const lpae_t pte) { -ASSERT(lpae_is_valid(pte)); +ASSERT(p2m_is_valid(pte)); /* * TODO: Handle other p2m types @@ -654,11 +666,11 @@ static void p2m_free_entry(struct p2m_domain *p2m, struct page_info *pg; /* Nothing to do if the entry is invalid. */ -if ( !lpae_is_valid(entry) ) +if ( !p2m_is_valid(entry) ) return; /* Nothing to do but updating the stats if the entry is a super-page. */ -if ( p2m_is_superpage(entry, level) ) +if ( level == 3 && entry.p2m.table ) { p2m->stats.mappings[level]--; return; @@ -951,7 +963,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m, else p2m->need_flush = true; } -else /* new mapping */ +else if ( !p2m_is_valid(orig_pte) ) /* new mapping */ p2m->stats.mappings[level]++; p2m_write_pte(entry, pte, p2m->clean_pte); @@ -965,7 +977,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m, * Free the entry only if the original pte was valid and the base * is different (to avoid freeing when permission is changed). */ -if ( lpae_is_valid(orig_pte) && +if ( p2m_is_valid(orig_pte) && !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) ) p2m_free_entry(p2m, orig_pte, level); -- 2.11.0 ___ Xen-devel mailing list
[Xen-devel] [RFC 08/16] xen/arm: p2m: Handle translation fault in get_page_from_gva
A follow-up patch will re-purpose the valid bit of LPAE entries to generate fault even on entry containing valid information. This means that when translation a guest VA to guest PA (e.g IPA) will fail if the Stage-2 entries used have the valid bit unset. Because of that, we need to fallback to walk the page-table in software to check whether the fault was expected. This patch adds the software page-table walk on all the translation fault. It would be possible in the future to avoid pointless walk when the fault in PAR_EL1 is not a translation fault. Signed-off-by: Julien Grall --- There are a couple of TODO in the code. They are clean-up and performance improvement (e.g when the fault cannot be handled) that could be delayed after the series has been merged. --- xen/arch/arm/p2m.c | 65 -- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 2a1e7e9be2..ec956bc151 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -1438,6 +1439,8 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, struct page_info *page = NULL; paddr_t maddr = 0; uint64_t par; +mfn_t mfn; +p2m_type_t t; /* * XXX: To support a different vCPU, we would need to load the @@ -1454,8 +1457,30 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, par = gvirt_to_maddr(va, , flags); p2m_read_unlock(p2m); +/* + * gvirt_to_maddr may fail if the entry does not have the valid bit + * set. Fallback + * to the second method: + * 1) Translate the VA to IPA using software lookup -> Stage-1 page-table + * may not be accessible because the stage-2 entries may have valid + * bit unset. + * 2) Software lookup of the MFN + * + * Note that when memaccess is enabled, we instead all directly + * p2m_mem_access_check_and_get_page(...). Because the function is a + * a variant of the methods described above, it will be able to + * handle entry with valid bit unset. + * + * TODO: Integrate more nicely memaccess with the rest of the + * function. + * TODO: Use the fault error in PAR_EL1 to avoid pointless + * translation. + */ if ( par ) { +paddr_t ipa; +unsigned int perms; + /* * When memaccess is enabled, the translation GVA to MADDR may * have failed because of a permission fault. @@ -1463,20 +1488,46 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va, if ( p2m->mem_access_enabled ) return p2m_mem_access_check_and_get_page(va, flags, v); -dprintk(XENLOG_G_DEBUG, -"%pv: gvirt_to_maddr failed va=%#"PRIvaddr" flags=0x%lx par=%#"PRIx64"\n", -v, va, flags, par); -return NULL; +/* + * The software stage-1 table walk can still fail, e.g, if the + * GVA is not mapped. + */ +if ( !guest_walk_tables(v, va, , ) ) +{ +dprintk(XENLOG_G_DEBUG, "%pv: Failed to walk page-table va %#"PRIvaddr"\n", v, va); +return NULL; +} + +/* + * Check permission that are assumed by the caller. For instance + * in case of guestcopy, the caller assumes that the translated + * page can be accessed with the requested permissions. If this + * is not the case, we should fail. + * + * Please note that we do not check for the GV2M_EXEC + * permission. This is fine because the hardware-based translation + * instruction does not test for execute permissions. + */ +if ( (flags & GV2M_WRITE) && !(perms & GV2M_WRITE) ) +return NULL; + +mfn = p2m_lookup(d, gaddr_to_gfn(ipa), ); +if ( mfn_eq(INVALID_MFN, mfn) ) +return NULL; + +/* We consider that RAM is always mapped read-write */ } +else +mfn = maddr_to_mfn(maddr); -if ( !mfn_valid(maddr_to_mfn(maddr)) ) +if ( !mfn_valid(mfn) ) { dprintk(XENLOG_G_DEBUG, "%pv: Invalid MFN %#"PRI_mfn"\n", -v, mfn_x(maddr_to_mfn(maddr))); +v, mfn_x(mfn)); return NULL; } -page = mfn_to_page(maddr_to_mfn(maddr)); +page = mfn_to_page(mfn); ASSERT(page); if ( unlikely(!get_page(page, d)) ) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 15/16] xen/arm: Implement Set/Way operations
Set/Way operations are used to perform maintenance on a given cache. At the moment, Set/Way operations are not trapped and therefore a guest OS will directly act on the local cache. However, a vCPU may migrate to another pCPU in the middle of the processor. This will result to have cache with stall data (Set/Way are not propagated) potentially causing crash. This may be the cause of heisenbug noticed in Osstest [1]. Furthermore, Set/Way operations are not available on system cache. This means that OS, such as Linux 32-bit, relying on those operations to fully clean the cache before disabling MMU may break because data may sits in system caches and not in RAM. For more details about Set/Way, see the talk "The Art of Virtualizing Cache Maintenance" given at Xen Summit 2018 [2]. In the context of Xen, we need to trap Set/Way operations and emulate them. From the Arm Arm (B1.14.4 in DDI 046C.c), Set/Way operations are difficult to virtualized. So we can assume that a guest OS using them will suffer the consequence (i.e slowness) until developer removes all the usage of Set/Way. As the software is not allowed to infer the Set/Way to Physical Address mapping, Xen will need to go through the guest P2M and clean & invalidate all the entries mapped. Because Set/Way happen in batch (a loop on all Set/Way of a cache), Xen would need to go through the P2M for every instructions. This is quite expensive and would severely impact the guest OS. The implementation is re-using the KVM policy to limit the number of flush: - If we trap a Set/Way operations, we enable VM trapping (i.e HVC_EL2.TVM) to detect cache being turned on/off, and do a full clean. - We clean the caches when turning on and off - Once the caches are enabled, we stop trapping VM instructions [1] https://lists.xenproject.org/archives/html/xen-devel/2017-09/msg03191.html [2] https://fr.slideshare.net/xen_com_mgr/virtualizing-cache Signed-off-by: Julien Grall --- xen/arch/arm/arm64/vsysreg.c | 27 +- xen/arch/arm/p2m.c | 68 xen/arch/arm/traps.c | 2 +- xen/arch/arm/vcpreg.c| 23 +++ xen/include/asm-arm/p2m.h| 16 +++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c index 1517879697..43c6c3e30d 100644 --- a/xen/arch/arm/arm64/vsysreg.c +++ b/xen/arch/arm/arm64/vsysreg.c @@ -40,7 +40,20 @@ static bool vreg_emulate_##reg(struct cpu_user_regs *regs, \ } /* Defining helpers for emulating sysreg registers. */ -TVM_REG(SCTLR_EL1) +static bool vreg_emulate_SCTLR_EL1(struct cpu_user_regs *regs, uint64_t *r, + bool read) +{ +struct vcpu *v = current; +bool cache_enabled = vcpu_has_cache_enabled(v); + +GUEST_BUG_ON(read); +WRITE_SYSREG(*r, SCTLR_EL1); + +p2m_toggle_cache(v, cache_enabled); + +return true; +} + TVM_REG(TTBR0_EL1) TVM_REG(TTBR1_EL1) TVM_REG(TCR_EL1) @@ -84,6 +97,18 @@ void do_sysreg(struct cpu_user_regs *regs, break; /* + * HCR_EL2.TSW + * + * ARMv8 (DDI 0487B.b): Table D1-42 + */ +case HSR_SYSREG_DCISW: +case HSR_SYSREG_DCCSW: +case HSR_SYSREG_DCCISW: +if ( hsr.sysreg.read ) +p2m_set_way_flush(current); +break; + +/* * HCR_EL2.TVM * * ARMv8 (DDI 0487B.b): Table D1-37 diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index df6b48d73b..a3d4c563b1 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1564,6 +1564,74 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) return 0; } +static void p2m_flush_vm(struct vcpu *v) +{ +struct p2m_domain *p2m = p2m_get_hostp2m(v->domain); + +/* XXX: Handle preemption */ +p2m_cache_flush_range(v->domain, p2m->lowest_mapped_gfn, + p2m->max_mapped_gfn); +} + +/* + * See note at ARMv7 ARM B1.14.4 (DDI 0406C.c) (TL;DR: S/W ops are not + * easily virtualized). + * + * Main problems: + * - S/W ops are local to a CPU (not broadcast) + * - We have line migration behind our back (speculation) + * - System caches don't support S/W at all (damn!) + * + * In the face of the above, the best we can do is to try and convert + * S/W ops to VA ops. Because the guest is not allowed to infer the S/W + * to PA mapping, it can only use S/W to nuke the whole cache, which is + * rather a good thing for us. + * + * Also, it is only used when turning caches on/off ("The expected + * usage of the cache maintenance instructions that operate by set/way + * is associated with the powerdown and powerup of caches, if this is + * required by the implementation."). + * + * We use the following policy: + * - If we trap a S/W operation, we enabled VM trapping to detect + * caches being turned on/off, and do a full clean. + * + * - We flush the caches on both caches being turned on and off. +
[Xen-devel] [RFC 10/16] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM
A follow-up patch will require to emulate some accesses to some co-processors registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes to the virtual memory control registers will be trapped to the hypervisor. This patch adds the infrastructure to passthrough the access to host registers. For convenience a bunch of helpers have been added to generate the different helpers. Note that HCR_EL2.TVM will be set in a follow-up patch dynamically. Signed-off-by: Julien Grall --- xen/arch/arm/vcpreg.c| 144 +++ xen/include/asm-arm/cpregs.h | 1 + 2 files changed, 145 insertions(+) diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index b04d996fd3..49529b97cd 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -24,6 +24,122 @@ #include #include +/* + * Macros to help generating helpers for registers trapped when + * HCR_EL2.TVM is set. + * + * Note that it only traps NS write access from EL1. + * + * - TVM_REG() should not be used outside of the macros. It is there to + *help defining TVM_REG32() and TVM_REG64() + * - TVM_REG32(regname, xreg) and TVM_REG64(regname, xreg) are used to + *resp. generate helper accessing 32-bit and 64-bit register. "regname" + *been the Arm32 name and "xreg" the Arm64 name. + * - UPDATE_REG32_COMBINED(lowreg, hireg, xreg) are used to generate a + * pair of registers share the same Arm32 registers. "lowreg" and + * "higreg" been resp. the Arm32 name and "xreg" the Arm64 name. "lowreg" + * will use xreg[31:0] and "hireg" will use xreg[63:32]. + */ + +/* The name is passed from the upper macro to workaround macro expansion. */ +#define TVM_REG(sz, func, reg...) \ +static bool func(struct cpu_user_regs *regs, uint##sz##_t *r, bool read)\ +{ \ +GUEST_BUG_ON(read); \ +WRITE_SYSREG##sz(*r, reg); \ +\ +return true;\ +} + +#define TVM_REG32(regname, xreg) TVM_REG(32, vreg_emulate_##regname, xreg) +#define TVM_REG64(regname, xreg) TVM_REG(64, vreg_emulate_##regname, xreg) + +#ifdef CONFIG_ARM_32 +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ +/* Use TVM_REG directly to workaround macro expansion. */ \ +TVM_REG(32, vreg_emulate_##lowreg, lowreg) \ +TVM_REG(32, vreg_emulate_##hireg, hireg) + +#else /* CONFIG_ARM_64 */ +#define TVM_REG32_COMBINED(lowreg, hireg, xreg) \ +static bool vreg_emulate_##xreg(struct cpu_user_regs *regs, uint32_t *r,\ +bool read, bool hi) \ +{ \ +register_t reg = READ_SYSREG(xreg); \ +\ +GUEST_BUG_ON(read); \ +if ( hi ) /* reg[63:32] is AArch32 register hireg */\ +{ \ +reg &= GENMASK(31, 0); \ +reg |= ((uint64_t)*r) << 32;\ +} \ +else /* reg[31:0] is AArch32 register lowreg. */\ +{ \ +reg &= GENMASK(31, 0); \ +reg |= *r; \ +} \ +WRITE_SYSREG(reg, xreg);\ +\ +return true;\ +} \ +\ +static bool vreg_emulate_##lowreg(struct cpu_user_regs *regs, uint32_t *r, \ + bool read)\ +{ \ +return vreg_emulate_##xreg(regs, r, read, false); \ +} \ +\ +static bool vreg_emulate_##hireg(struct cpu_user_regs *regs, uint32_t *r, \ +
[Xen-devel] [RFC 09/16] xen/arm: p2m: Introduce a function to resolve translation fault
Currently a Stage-2 translation fault could happen: 1) MMIO emulation 2) When the page-tables is been updated using Break-Before-Make 3) Page not mapped A follow-up patch will re-purpose the valid bit in an entry to generate translation fault. This would be used to do an action on each entries to track page used for a given period. A new function is introduced to try to resolve a translation fault. This will include 2) and the new way to generate fault explained above. To avoid invalidating all the page-tables entries in one go. It is possible to invalidate the top-level table and then on trap invalidate the table one-level down. This will be repeated until a block/page entry has been reached. At the moment, there are no action done when reaching a block/page entry but setting the valid bit to 1. Signed-off-by: Julien Grall --- xen/arch/arm/p2m.c | 127 +++ xen/arch/arm/traps.c | 7 +-- 2 files changed, 131 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index ec956bc151..af445d3313 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1043,6 +1043,133 @@ int p2m_set_entry(struct p2m_domain *p2m, return rc; } +/* Invalidate all entries in the table. The p2m should be write locked. */ +static void p2m_invalidate_table(struct p2m_domain *p2m, mfn_t mfn) +{ +lpae_t *table; +unsigned int i; + +ASSERT(p2m_is_write_locked(p2m)); + +table = map_domain_page(mfn); + +for ( i = 0; i < LPAE_ENTRIES; i++ ) +{ +lpae_t pte = table[i]; + +pte.p2m.valid = 0; + +p2m_write_pte([i], pte, p2m->clean_pte); +} + +unmap_domain_page(table); + +p2m->need_flush = true; +} + +bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn) +{ +struct p2m_domain *p2m = p2m_get_hostp2m(d); +unsigned int level = 0; +bool resolved = false; +lpae_t entry, *table; +paddr_t addr = gfn_to_gaddr(gfn); + +/* Convenience aliases */ +const unsigned int offsets[4] = { +zeroeth_table_offset(addr), +first_table_offset(addr), +second_table_offset(addr), +third_table_offset(addr) +}; + +p2m_write_lock(p2m); + +/* This gfn is higher than the highest the p2m map currently holds */ +if ( gfn_x(gfn) > gfn_x(p2m->max_mapped_gfn) ) +goto out; + +table = p2m_get_root_pointer(p2m, gfn); +/* + * The table should always be non-NULL because the gfn is below + * p2m->max_mapped_gfn and the root table pages are always present. + */ +BUG_ON(table == NULL); + +/* + * Go down the page-tables until an entry has the valid bit unset or + * a block/page entry has been hit. + */ +for ( level = P2M_ROOT_LEVEL; level <= 3; level++ ) +{ +int rc; + +entry = table[offsets[level]]; + +if ( level == 3 ) +break; + +/* Stop as soon as we hit an entry with the valid bit unset. */ +if ( !lpae_is_valid(entry) ) +break; + +rc = p2m_next_level(p2m, true, level, , offsets[level]); +if ( rc == GUEST_TABLE_MAP_FAILED ) +goto out_unmap; +else if ( rc != GUEST_TABLE_NORMAL_PAGE ) +break; +} + +/* + * If the valid bit of the entry is set, it means someone was playing with + * the Stage-2 page table. Nothing to do and mark the fault as resolved. + */ +if ( lpae_is_valid(entry) ) +{ +resolved = true; +goto out_unmap; +} + +/* + * The valid bit is unset. If the entry is still not valid then the fault + * cannot be resolved, exit and report it. + */ +if ( !p2m_is_valid(entry) ) +goto out_unmap; + +/* + * Now we have an entry with valid bit unset, but still valid from + * the P2M point of view. + * + * For entry pointing to a table, the table will be invalidated. + * For entry pointing to a block/page, no work to do for now. + */ +if ( lpae_is_table(entry, level) ) +p2m_invalidate_table(p2m, lpae_get_mfn(entry)); + +/* + * Now that the work on the entry is done, set the valid bit to prevent + * another fault on that entry. + */ +resolved = true; +entry.p2m.valid = 1; + +p2m_write_pte(table + offsets[level], entry, p2m->clean_pte); + +/* + * No need to flush the TLBs as the modified entry had the valid bit + * unset. + */ + +out_unmap: +unmap_domain_page(table); + +out: +p2m_write_unlock(p2m); + +return resolved; +} + static inline int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr, diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index b40798084d..169b57cb6b 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1882,6 +1882,8 @@ static bool try_map_mmio(gfn_t gfn) return
[Xen-devel] [RFC 02/16] xen/arm: Introduce helpers to get/set an MFN from/to an LPAE entry
The new helpers make it easier to read the code by abstracting the way to set/get an MFN from/to an LPAE entry. The helpers are using "walk" as the bits are common across different LPAE stages. At the same time, use the new helpers to replace the various open-coding place. Signed-off-by: Julien Grall --- This patch was originally sent separately. --- xen/arch/arm/mm.c | 10 +- xen/arch/arm/p2m.c | 19 ++- xen/include/asm-arm/lpae.h | 3 +++ 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7a06a33e21..0bc31b1d9b 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -238,7 +238,7 @@ void dump_pt_walk(paddr_t ttbr, paddr_t addr, /* For next iteration */ unmap_domain_page(mapping); -mapping = map_domain_page(_mfn(pte.walk.base)); +mapping = map_domain_page(lpae_get_mfn(pte)); } unmap_domain_page(mapping); @@ -323,7 +323,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned attr) ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK)); -e.pt.base = mfn_x(mfn); +lpae_set_mfn(e, mfn); return e; } @@ -490,7 +490,7 @@ mfn_t domain_page_map_to_mfn(const void *ptr) ASSERT(slot >= 0 && slot < DOMHEAP_ENTRIES); ASSERT(map[slot].pt.avail != 0); -return _mfn(map[slot].pt.base + offset); +return mfn_add(lpae_get_mfn(map[slot]), offset); } #endif @@ -851,7 +851,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn, /* mfn_to_virt is not valid on the 1st 1st mfn, since it * is not within the xenheap. */ first = slot == xenheap_first_first_slot ? -xenheap_first_first : __mfn_to_virt(p->pt.base); +xenheap_first_first : mfn_to_virt(lpae_get_mfn(*p)); } else if ( xenheap_first_first_slot == -1) { @@ -1007,7 +1007,7 @@ static int create_xen_entries(enum xenmap_operation op, BUG_ON(!lpae_is_valid(*entry)); -third = __mfn_to_virt(entry->pt.base); +third = mfn_to_virt(lpae_get_mfn(*entry)); entry = [third_table_offset(addr)]; switch ( op ) { diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 30cfb01498..f8a2f6459e 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -265,7 +265,7 @@ static int p2m_next_level(struct p2m_domain *p2m, bool read_only, if ( lpae_is_mapping(*entry, level) ) return GUEST_TABLE_SUPER_PAGE; -mfn = _mfn(entry->p2m.base); +mfn = lpae_get_mfn(*entry); unmap_domain_page(*table); *table = map_domain_page(mfn); @@ -349,7 +349,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn, if ( a ) *a = p2m_mem_access_radix_get(p2m, gfn); -mfn = _mfn(entry.p2m.base); +mfn = lpae_get_mfn(entry); /* * The entry may point to a superpage. Find the MFN associated * to the GFN. @@ -519,7 +519,7 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a) ASSERT(!(mfn_to_maddr(mfn) & ~PADDR_MASK)); -e.p2m.base = mfn_x(mfn); +lpae_set_mfn(e, mfn); return e; } @@ -621,7 +621,7 @@ static void p2m_put_l3_page(const lpae_t pte) */ if ( p2m_is_foreign(pte.p2m.type) ) { -mfn_t mfn = _mfn(pte.p2m.base); +mfn_t mfn = lpae_get_mfn(pte); ASSERT(mfn_valid(mfn)); put_page(mfn_to_page(mfn)); @@ -655,7 +655,7 @@ static void p2m_free_entry(struct p2m_domain *p2m, return; } -table = map_domain_page(_mfn(entry.p2m.base)); +table = map_domain_page(lpae_get_mfn(entry)); for ( i = 0; i < LPAE_ENTRIES; i++ ) p2m_free_entry(p2m, *(table + i), level + 1); @@ -669,7 +669,7 @@ static void p2m_free_entry(struct p2m_domain *p2m, */ p2m_tlb_flush_sync(p2m); -mfn = _mfn(entry.p2m.base); +mfn = lpae_get_mfn(entry); ASSERT(mfn_valid(mfn)); pg = mfn_to_page(mfn); @@ -688,7 +688,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry, bool rv = true; /* Convenience aliases */ -mfn_t mfn = _mfn(entry->p2m.base); +mfn_t mfn = lpae_get_mfn(*entry); unsigned int next_level = level + 1; unsigned int level_order = level_orders[next_level]; @@ -719,7 +719,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry, * the necessary fields. So the correct permission are kept. */ pte = *entry; -pte.p2m.base = mfn_x(mfn_add(mfn, i << level_order)); +lpae_set_mfn(pte, mfn_add(mfn, i << level_order)); /* * First and second level pages set p2m.table = 0, but third @@ -952,7 +952,8 @@ static int __p2m_set_entry(struct p2m_domain *p2m, * Free the entry only if the original pte was valid and the base * is different (to avoid freeing when permission is changed). */ -if (
[Xen-devel] [RFC 06/16] xen/arm: p2m: Introduce a helper to generate P2M table entry from a page
Generate P2M table entry requires to set some default values which are worth to explain in a comment. At the moment, there are 2 places where such entry are created but only one as proper comment. Some move the code to generate P2M table entry in a separate helper. This will be helpful in a follow-up patch to make modification on the defaults. At the same time, switch the default access from p2m->default_access to p2m_access_rwx. This should not matter as permission are ignored for table by the hardware. Signed-off-by: Julien Grall --- xen/arch/arm/p2m.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 8fffb42889..6c76298ebc 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -538,6 +538,16 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a) return e; } +/* Generate table entry with correct attributes. */ +static lpae_t page_to_p2m_table(struct page_info *page) +{ +/* + * The access value does not matter because the hardware will ignore + * the permission fields for table entry. + */ +return mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, p2m_access_rwx); +} + static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool clean_pte) { write_pte(p, pte); @@ -558,7 +568,6 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry) { struct page_info *page; lpae_t *p; -lpae_t pte; ASSERT(!lpae_is_valid(*entry)); @@ -576,14 +585,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry) unmap_domain_page(p); -/* - * The access value does not matter because the hardware will ignore - * the permission fields for table entry. - */ -pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, - p2m->default_access); - -p2m_write_pte(entry, pte, p2m->clean_pte); +p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte); return 0; } @@ -764,14 +766,11 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry, unmap_domain_page(table); -pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, - p2m->default_access); - /* * Even if we failed, we should install the newly allocated LPAE * entry. The caller will be in charge to free the sub-tree. */ -p2m_write_pte(entry, pte, p2m->clean_pte); +p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte); return rv; } -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 05/16] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
GUEST_BUG_ON may be used in other files doing guest emulation. Signed-off-by: Julien Grall --- The patch was previously sent separately. --- xen/arch/arm/traps.c| 24 xen/include/asm-arm/traps.h | 24 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 9251ae50b8..b40798084d 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -68,30 +68,6 @@ static inline void check_stack_alignment_constraints(void) { #endif } -/* - * GUEST_BUG_ON is intended for checking that the guest state has not been - * corrupted in hardware and/or that the hardware behaves as we - * believe it should (i.e. that certain traps can only occur when the - * guest is in a particular mode). - * - * The intention is to limit the damage such h/w bugs (or spec - * misunderstandings) can do by turning them into Denial of Service - * attacks instead of e.g. information leaks or privilege escalations. - * - * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state! - * - * Compared with regular BUG_ON it dumps the guest vcpu state instead - * of Xen's state. - */ -#define guest_bug_on_failed(p) \ -do {\ -show_execution_state(guest_cpu_user_regs());\ -panic("Guest Bug: %pv: '%s', line %d, file %s\n", \ - current, p, __LINE__, __FILE__); \ -} while (0) -#define GUEST_BUG_ON(p) \ -do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0) - #ifdef CONFIG_ARM_32 static int debug_stack_lines = 20; #define stack_words_per_line 8 diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h index 70b52d1d16..0acf7de67d 100644 --- a/xen/include/asm-arm/traps.h +++ b/xen/include/asm-arm/traps.h @@ -9,6 +9,30 @@ # include #endif +/* + * GUEST_BUG_ON is intended for checking that the guest state has not been + * corrupted in hardware and/or that the hardware behaves as we + * believe it should (i.e. that certain traps can only occur when the + * guest is in a particular mode). + * + * The intention is to limit the damage such h/w bugs (or spec + * misunderstandings) can do by turning them into Denial of Service + * attacks instead of e.g. information leaks or privilege escalations. + * + * GUEST_BUG_ON *MUST* *NOT* be used to check for guest controllable state! + * + * Compared with regular BUG_ON it dumps the guest vcpu state instead + * of Xen's state. + */ +#define guest_bug_on_failed(p) \ +do {\ +show_execution_state(guest_cpu_user_regs());\ +panic("Guest Bug: %pv: '%s', line %d, file %s\n", \ + current, p, __LINE__, __FILE__); \ +} while (0) +#define GUEST_BUG_ON(p) \ +do { if ( unlikely(p) ) guest_bug_on_failed(#p); } while (0) + int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr); void advance_pc(struct cpu_user_regs *regs, const union hsr hsr); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 12/16] xen/arm: Rework p2m_cache_flush to take a range [begin, end)
The function will be easier to re-use in a follow-up patch if you have only the begin and end. At the same time, rename the function to reflect the change in the prototype. Signed-off-by: Julien Grall --- xen/arch/arm/domctl.c | 2 +- xen/arch/arm/p2m.c| 3 +-- xen/include/asm-arm/p2m.h | 7 +-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 4587c75826..c10f568aad 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -61,7 +61,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, if ( e < s ) return -EINVAL; -return p2m_cache_flush(d, _gfn(s), domctl->u.cacheflush.nr_pfns); +return p2m_cache_flush_range(d, _gfn(s), _gfn(e)); } case XEN_DOMCTL_bind_pt_irq: { diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index af445d3313..8537b7bab1 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1507,10 +1507,9 @@ int relinquish_p2m_mapping(struct domain *d) return rc; } -int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr) +int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) { struct p2m_domain *p2m = p2m_get_hostp2m(d); -gfn_t end = gfn_add(start, nr); gfn_t next_gfn; p2m_type_t t; unsigned int order; diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index c03557544a..d7afa2bbe8 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -224,8 +224,11 @@ int p2m_set_entry(struct p2m_domain *p2m, p2m_type_t t, p2m_access_t a); -/* Clean & invalidate caches corresponding to a region of guest address space */ -int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr); +/* + * Clean & invalidate caches corresponding to a region [start,end) of guest + * address space. + */ +int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end); /* * Map a region in the guest p2m with a specific p2m type. -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC 13/16] xen/arm: p2m: Allow to flush cache on any RAM region
Currently, we only allow to flush cache on region mapped as p2m_ram_{rw,ro}. There are no real problem to flush cache on any RAM region such as grants and foreign mapping. Therefore, relax the cache to allow flushing the cache on any RAM region. Signed-off-by: Julien Grall --- xen/arch/arm/p2m.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 8537b7bab1..12b459924b 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1532,7 +1532,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t start, gfn_t end) next_gfn = gfn_next_boundary(start, order); /* Skip hole and non-RAM page */ -if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_ram(t) ) +if ( mfn_eq(mfn, INVALID_MFN) || !p2m_is_any_ram(t) ) continue; /* XXX: Implement preemption */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Xen boot failure on QEMU (WAS: Re: [PATCH v3] xen:arm: Populate arm64 image header)
(+ Peter Maydell and Stefano) Hi Steward, Thank you for the bug report. On 05/10/2018 23:17, Stewart Hildebrand wrote: On 11/09/2018 17:48, Amit Singh Tomar wrote: diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index d63734f..ef87b5c 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -120,8 +127,8 @@ efi_head: add x13, x18, #0x16 b real_start /* branch to kernel start */ .quad 0/* Image load offset from start of RAM */ -.quad 0/* reserved */ -.quad 0/* reserved */ +.quad _end - start /* Effective size of kernel image, little-endian */ +.quad __HEAD_FLAGS /* Informative flags, little-endian */ .quad 0/* reserved */ .quad 0/* reserved */ .quad 0/* reserved */ Since 17bd254a xen:arm: Populate arm64 image header, qemu-system-aarch64 has not been too happy about booting Xen. Trying to launch qemu-system-aarch64 gives the following error: rom: requested regions overlap (rom bootloader. free=0x400d0150, addr=0x4000) qemu-system-aarch64: rom check and register reset failed Reverting 17bd254a allowed it to boot again. Alternatively, setting the image offset to some value allowed it to boot again. diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index ef87b5c..8879c77 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -126,7 +126,7 @@ efi_head: */ add x13, x18, #0x16 b real_start /* branch to kernel start */ -.quad 0/* Image load offset from start of RAM */ +.quad 0x0008 /* Image load offset from start of RAM */ .quad _end - start /* Effective size of kernel image, little-endian */ .quad __HEAD_FLAGS /* Informative flags, little-endian */ .quad 0/* reserved */ I'm not sure if this is a fault of qemu, or if Xen should put some value in the image load offset field? Per the Linux arm64 booting protocol [1], the load offset can definitely be 0. The bootloader (here QEMU) should not assume a specific text offset, Linux actually provides an option to randomize the text offset in order to test that assumption (see ARM64_RANDOMIZE_TEXT_OFFSET). I have CCed Stefano and Peter who could give more details on how QEMU is handling the Image protocol. For reference, I'm using the following script to build and launch qemu+Xen https://gist.github.com/stewdk/110f43e0cc1d905fc6ed4c7e10d8d35e Cheers, [1] https://www.kernel.org/doc/Documentation/arm64/booting.txt -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 128509: tolerable all pass - PUSHED
flight 128509 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/128509/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen a696f4e63ca51693736a6cf7b911522287238653 baseline version: xen 7b20a865bc105fe566156201c8e6c37ef692e3dd Last test of basis 128500 2018-10-08 11:00:56 Z0 days Testing same since 128509 2018-10-08 15:00:49 Z0 days1 attempts People who touched revisions under test: Bastian Blank Ian Jackson Jan Beulich Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 7b20a865bc..a696f4e63c a696f4e63ca51693736a6cf7b911522287238653 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/5] xen/domain: Allocate d->vcpu[] earlier during domain_create()
On 08/10/18 14:51, Jan Beulich wrote: On 05.10.18 at 16:54, wrote: >> @@ -405,19 +419,6 @@ struct domain *domain_create(domid_t domid, >> >> if ( !is_idle_domain(d) ) >> { >> -/* Check d->max_vcpus and allocate d->vcpu[]. */ >> -err = -EINVAL; >> -if ( config->max_vcpus < 1 || >> - config->max_vcpus > domain_max_vcpus(d) ) >> -goto fail; > Ah, there it goes away. But I think it would be more logical for this to > happen in the previous patch. Anyway > Reviewed-by: Jan Beulich > for both, preferably (but not necessarily) with the removal moved > there. The x86 side is trivial, but the ARM side is not. I don't think patches 3 and 4 should be merged, but I'll let Julien/Stefano have the final say. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/5] RFC: test/depriv: Add a tool to check process-level depriv
Anthony PERARD writes ("Re: [PATCH 5/5] RFC: test/depriv: Add a tool to check process-level depriv"): > On Fri, Oct 05, 2018 at 05:57:01PM +0100, George Dunlap wrote: > > +# TEST: Process / group id > > +# > > +# Read /proc//status, checking Uid and Gid lines > > +# > > +# Uid should be xen-qemuuser-range-base+$domid > > +# Gid should be 65534 ("nobody") > > That is wrong. Gid doesn't have to be nobody. gid can be chosen when > creating the base user id. (And I'm pretty sure "nobody" should be > avoided.) The gid is not really relevant but nobody is sometimes chosen as a gid that no process has so is perhaps a poor choice. A single specific gid for all of these would be fine. Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/5] RFC: test/depriv: Add a tool to check process-level depriv
On Fri, Oct 05, 2018 at 05:57:01PM +0100, George Dunlap wrote: > +# TEST: Process / group id > +# > +# Read /proc//status, checking Uid and Gid lines > +# > +# Uid should be xen-qemuuser-range-base+$domid > +# Gid should be 65534 ("nobody") That is wrong. Gid doesn't have to be nobody. gid can be chosen when creating the base user id. (And I'm pretty sure "nobody" should be avoided.) > +# FIXME: deal with other UID configurations? > +echo -n "Process UID: " > +tgt_uid=$(id -u xen-qemuuser-range-base) > +tgt_uid=$(( $tgt_uid + $domid )) > + > +# Example input: > +# Uid: 1193119311931193 > +input=$(grep ^Uid: /proc/$dmpid/status) > +if [[ "$input" =~ > ^Uid:[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)[[:space:]]+([0-9]+)$ > ]] ; then > +result="PASSED" > +for i in {1..4}; do > + if [[ "${BASH_REMATCH[$i]}" != "$tgt_uid" ]] ; then > + result="FAILED" > + failed="true" > + break > + fi > +done > +else > +result="FAILED" > +failed="true" > +fi > +echo $result > + > +# Example input: > +# Gid: 10020 10020 10020 10020 > +echo -n "Process GID: " > +tgt_gid=$(id -g nobody) This should be `id -g xen-qemuuser-range-base`. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 02/17] x86/mm: make mm.c build with !CONFIG_PV
>>> On 04.10.18 at 17:43, wrote: > @@ -1277,6 +1274,8 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain > *l1e_owner) > } > > > +#ifdef CONFIG_PV > + > /* Could you please avoid inserting yet another blank line here, and instead make use of the existing so far one too many? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/sched: Drop set_current_state()
On 10/08/2018 03:48 PM, Andrew Cooper wrote: > This appears to have been a Linux-ism which found its way into the Xen > codebase with the IA64 port, and remained after IA64 was removed. > > As far as I can tell from code archeology, none of the other architectures > have ever had a current->state field. > > Signed-off-by: Andrew Cooper Not sure exactly what the rules are, but in case you need it: Acked-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/17] x86/shadow: put PV L1TF functions under CONFIG_PV
>>> On 04.10.18 at 17:43, wrote: > Signed-off-by: Wei Liu If applicable (not sure whether Tim would have to ack it instead): Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/sched: Drop set_current_state()
>>> On 08.10.18 at 16:48, wrote: > This appears to have been a Linux-ism which found its way into the Xen > codebase with the IA64 port, and remained after IA64 was removed. > > As far as I can tell from code archeology, none of the other architectures > have ever had a current->state field. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use HVM_PARAM_[BUF]IOREQ_PFN
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 08 October 2018 15:59 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: RE: [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use > HVM_PARAM_[BUF]IOREQ_PFN > > >>> On 08.10.18 at 16:38, wrote: > >> -Original Message- > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 08 October 2018 14:29 > >> To: Paul Durrant > >> Cc: Andrew Cooper ; Wei Liu > >> ; xen-devel > >> Subject: Re: [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use > >> HVM_PARAM_[BUF]IOREQ_PFN > >> > >> >>> On 05.10.18 at 15:43, wrote: > >> > Since commit 2c257bd6 "x86/hvm: remove default ioreq server (again)" > the > >> > GFNs allocated by the toolstack and set in HVM_PARAM_IOREQ_PFN and > >> > HVM_PARAM_BUFIOREQ_PFN have been unused. This patch allows them to be > >> used > >> > by (non-default) ioreq servers. > >> > > >> > NOTE: This fixes a compatibility issue. A guest created on a version > of > >> > Xen that pre-dates the initial ioreq server implementation and > >> then > >> > migrated in will currently fail to resume because its migration > >> > stream will lack values for HVM_PARAM_IOREQ_SERVER_PFN and > >> > HVM_PARAM_NR_IOREQ_SERVER_PAGES *unless* the system has an > >> > emulator domain that uses direct resource mapping (which > depends > >> > on the version of privcmd it happens to have) in which case it > >> > will not require use of GFNs for the ioreq server shared > >> > pages. > >> > >> Meaning this wants to be backported till where? > >> > > > > This fix is 4.12 specific because it is predicated on removal of default > > ioreq server support. > > Ah, good. > > >> > A similar compatibility issue with migrated-in VMs exists with Xen > 4.11 > >> > because the upstream QEMU fall-back to use legacy ioreq server was > >> broken > >> > when direct resource mapping was introduced. > >> > This is because, prior to the resource mapping patches, it was the > >> creation > >> > of the non-default ioreq server that failed if GFNs were not > available > >> > whereas, as of 4.11, it is retrieval of the info that fails which > does > >> not > >> > trigger the fall-back. > >> > >> Are you working on a fix or workaround for this, too, then? > >> > > > > Not yet. I'm not sure how to approach this. There are a few options: > > > > 1. Backport default IOREQ server removal and this fix > > 2. Do a bespoke 4.11 fix that forces IOREQ server creation to fail if > there > > are no GFNs available, thus triggering the default IOREQ server fallback > in > > QEMU. > > 3. Upstream a fix into QEMU to do a fallback at the point that it fails > to > > get GFNs i.e. have it close its IOREQ server and then fall back to > default. > > > > The problem with 1 is that it breaks qemu trad. 2 is probably simplest, > but > > if the emualator can do resource mapping it is unnecessary. 3 is > probably > > best, but it's not our fix to deliver. > > > > Thoughts? > > 2 indeed looks best to me then. Though I'm not sure I understand > what you say correctly: Would triggering the default IOREQ server > fallback be a step backwards, if the emulator is capable and able to > use resource mapping? Yes. With resource mapping the emulator doesn't rely in pages on special GFNs and so it would indeed be a step backwards... > If so, somehow avoiding this would of > course be nice, and I'd then assume this isn't reasonable to achieve > without a qemu side change, in which case the solution wouldn't be > any better than 3 anymore. > ...but avoiding would indeed mean a change in QEMU. Given that the chances of having a VM migrated all the way in from some ancient Xen without a reboot along the way at any point is probably quite small, and that no-one is likely to notice a fall-back to default IOREQ server unless they are really looking for it, I'd say let's go with 2. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use HVM_PARAM_[BUF]IOREQ_PFN
>>> On 08.10.18 at 16:38, wrote: >> -Original Message- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: 08 October 2018 14:29 >> To: Paul Durrant >> Cc: Andrew Cooper ; Wei Liu >> ; xen-devel >> Subject: Re: [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use >> HVM_PARAM_[BUF]IOREQ_PFN >> >> >>> On 05.10.18 at 15:43, wrote: >> > Since commit 2c257bd6 "x86/hvm: remove default ioreq server (again)" the >> > GFNs allocated by the toolstack and set in HVM_PARAM_IOREQ_PFN and >> > HVM_PARAM_BUFIOREQ_PFN have been unused. This patch allows them to be >> used >> > by (non-default) ioreq servers. >> > >> > NOTE: This fixes a compatibility issue. A guest created on a version of >> > Xen that pre-dates the initial ioreq server implementation and >> then >> > migrated in will currently fail to resume because its migration >> > stream will lack values for HVM_PARAM_IOREQ_SERVER_PFN and >> > HVM_PARAM_NR_IOREQ_SERVER_PAGES *unless* the system has an >> > emulator domain that uses direct resource mapping (which depends >> > on the version of privcmd it happens to have) in which case it >> > will not require use of GFNs for the ioreq server shared >> > pages. >> >> Meaning this wants to be backported till where? >> > > This fix is 4.12 specific because it is predicated on removal of default > ioreq server support. Ah, good. >> > A similar compatibility issue with migrated-in VMs exists with Xen 4.11 >> > because the upstream QEMU fall-back to use legacy ioreq server was >> broken >> > when direct resource mapping was introduced. >> > This is because, prior to the resource mapping patches, it was the >> creation >> > of the non-default ioreq server that failed if GFNs were not available >> > whereas, as of 4.11, it is retrieval of the info that fails which does >> not >> > trigger the fall-back. >> >> Are you working on a fix or workaround for this, too, then? >> > > Not yet. I'm not sure how to approach this. There are a few options: > > 1. Backport default IOREQ server removal and this fix > 2. Do a bespoke 4.11 fix that forces IOREQ server creation to fail if there > are no GFNs available, thus triggering the default IOREQ server fallback in > QEMU. > 3. Upstream a fix into QEMU to do a fallback at the point that it fails to > get GFNs i.e. have it close its IOREQ server and then fall back to default. > > The problem with 1 is that it breaks qemu trad. 2 is probably simplest, but > if the emualator can do resource mapping it is unnecessary. 3 is probably > best, but it's not our fix to deliver. > > Thoughts? 2 indeed looks best to me then. Though I'm not sure I understand what you say correctly: Would triggering the default IOREQ server fallback be a step backwards, if the emulator is capable and able to use resource mapping? If so, somehow avoiding this would of course be nice, and I'd then assume this isn't reasonable to achieve without a qemu side change, in which case the solution wouldn't be any better than 3 anymore. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 128497: all pass - PUSHED
flight 128497 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/128497/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd c0b412ce93b9d3ee750e5f262b50e64c52d300cc baseline version: freebsd 8f45b071b58d4a00be551ddcc52e78a06ed6fbc7 Last test of basis 128413 2018-10-05 09:19:12 Z3 days Testing same since 128497 2018-10-08 09:19:52 Z0 days1 attempts People who touched revisions under test: allanjude dbaio emaste jamie jhibbits kevans lidl mav mjg shurd thj trasz tuexen vangyzen jobs: build-amd64-freebsd-againpass build-amd64-freebsd pass build-amd64-xen-freebsd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/freebsd.git 8f45b071b58..c0b412ce93b c0b412ce93b9d3ee750e5f262b50e64c52d300cc -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/sched: Drop set_current_state()
This appears to have been a Linux-ism which found its way into the Xen codebase with the IA64 port, and remained after IA64 was removed. As far as I can tell from code archeology, none of the other architectures have ever had a current->state field. Signed-off-by: Andrew Cooper --- CC: George Dunlap CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Julien Grall --- xen/include/xen/sched.h | 1 - 1 file changed, 1 deletion(-) diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index c5540fa..0ddff03 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -620,7 +620,6 @@ void __domain_crash(struct domain *d); */ void noreturn asm_domain_crash_synchronous(unsigned long addr); -#define set_current_state(_s) do { current->state = (_s); } while (0) void scheduler_init(void); int sched_init_vcpu(struct vcpu *v, unsigned int processor); void sched_destroy_vcpu(struct vcpu *v); -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use HVM_PARAM_[BUF]IOREQ_PFN
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 08 October 2018 14:29 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: Re: [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use > HVM_PARAM_[BUF]IOREQ_PFN > > >>> On 05.10.18 at 15:43, wrote: > > Since commit 2c257bd6 "x86/hvm: remove default ioreq server (again)" the > > GFNs allocated by the toolstack and set in HVM_PARAM_IOREQ_PFN and > > HVM_PARAM_BUFIOREQ_PFN have been unused. This patch allows them to be > used > > by (non-default) ioreq servers. > > > > NOTE: This fixes a compatibility issue. A guest created on a version of > > Xen that pre-dates the initial ioreq server implementation and > then > > migrated in will currently fail to resume because its migration > > stream will lack values for HVM_PARAM_IOREQ_SERVER_PFN and > > HVM_PARAM_NR_IOREQ_SERVER_PAGES *unless* the system has an > > emulator domain that uses direct resource mapping (which depends > > on the version of privcmd it happens to have) in which case it > > will not require use of GFNs for the ioreq server shared > > pages. > > Meaning this wants to be backported till where? > This fix is 4.12 specific because it is predicated on removal of default ioreq server support. > > A similar compatibility issue with migrated-in VMs exists with Xen 4.11 > > because the upstream QEMU fall-back to use legacy ioreq server was > broken > > when direct resource mapping was introduced. > > This is because, prior to the resource mapping patches, it was the > creation > > of the non-default ioreq server that failed if GFNs were not available > > whereas, as of 4.11, it is retrieval of the info that fails which does > not > > trigger the fall-back. > > Are you working on a fix or workaround for this, too, then? > Not yet. I'm not sure how to approach this. There are a few options: 1. Backport default IOREQ server removal and this fix 2. Do a bespoke 4.11 fix that forces IOREQ server creation to fail if there are no GFNs available, thus triggering the default IOREQ server fallback in QEMU. 3. Upstream a fix into QEMU to do a fallback at the point that it fails to get GFNs i.e. have it close its IOREQ server and then fall back to default. The problem with 1 is that it breaks qemu trad. 2 is probably simplest, but if the emualator can do resource mapping it is unnecessary. 3 is probably best, but it's not our fix to deliver. Thoughts? Paul > > --- a/xen/arch/x86/hvm/ioreq.c > > +++ b/xen/arch/x86/hvm/ioreq.c > > @@ -237,6 +237,26 @@ bool handle_hvm_io_completion(struct vcpu *v) > > return true; > > } > > > > +static gfn_t hvm_alloc_legacy_ioreq_gfn(struct hvm_ioreq_server *s) > > +{ > > +struct domain *d = s->target; > > +unsigned int i; > > + > > +BUILD_BUG_ON(HVM_PARAM_IOREQ_PFN > > > + sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); > > +BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN > > > + sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); > > +BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN != HVM_PARAM_IOREQ_PFN + 1); > > + > > +for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ ) > > +{ > > +if ( !test_and_set_bit(i, >arch.hvm.ioreq_gfn.legacy_mask) ) > > +return _gfn(d->arch.hvm.params[i]); > > Aren't you risking to use GFN 0 here, if the param was never set? > Since in theory GFN 0 could be valid here, perhaps whether these > MFNs may be used should be tracked by starting out legacy_mask > such that allocations are impossible, while the setting of the > params would then make the slots available for allocating from? > > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V3 2/2] Xen/PCIback: Implement PCI flr/slot/bus reset with 'reset' SysFS attribute
On 10/3/18 11:51 AM, Pasi Kärkkäinen wrote: > On Wed, Sep 19, 2018 at 11:05:26AM +0200, Roger Pau Monné wrote: >> On Tue, Sep 18, 2018 at 02:09:53PM -0400, Boris Ostrovsky wrote: >>> On 9/18/18 5:32 AM, George Dunlap wrote: > On Sep 18, 2018, at 8:15 AM, Pasi Kärkkäinen wrote: > > Hi, > > On Mon, Sep 17, 2018 at 02:06:02PM -0400, Boris Ostrovsky wrote: >> What about the toolstack changes? Have they been accepted? I vaguely >> recall there was a discussion about those changes but don't remember how >> it ended. >> > I don't think toolstack/libxl patch has been applied yet either. > > > "[PATCH V1 0/1] Xen/Tools: PCI reset using 'reset' SysFS attribute": > https://lists.xen.org/archives/html/xen-devel/2017-12/msg00664.html > > "[PATCH V1 1/1] Xen/libxl: Perform PCI reset using 'reset' SysFS > attribute": > https://lists.xen.org/archives/html/xen-devel/2017-12/msg00663.html >>> >>> Will this patch work for *BSD? Roger? >> At least FreeBSD don't support pci-passthrough, so none of this works >> ATM. There's no sysfs on BSD, so much of what's in libxl_pci.c will >> have to be moved to libxl_linux.c when BSD support is added. >> > Ok. That sounds like it's OK for the initial pci 'reset' implementation in > xl/libxl to be linux-only.. > Are these two patches still needed? ISTR they were written originally to deal with guest trying to use device that was previously assigned to another guest. But pcistub_put_pci_dev() calls __pci_reset_function_locked() which first tries FLR, and it looks like it was added relatively recently. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/18] INSTALL: Mention kconfig
>>> On 08.10.18 at 16:08, wrote: > Jan Beulich writes ("Re: [PATCH 10/18] INSTALL: Mention kconfig"): >> On 05.10.18 at 19:29, wrote: >> > +silently overriden. The only way to find which configuration options >> >> Isn't it "overridden", or are both spellings okay? > > It is; I was wrong. > >> > +this varible there is nothing stopping you setting dangerously >> >> variable > > Fixed. > >> Beyond these nits I'm okay with the text now, accepting the anomaly >> pointed out by Doug. > > Thanks, I'll take that as an ack. (Assuming you did indeed mean > `accepting' rather than `excepting'.) Well, no, it was not meant as an ack, merely as the absence of further objections. (And yes, I did mean "accepting".) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] stubdom/grub.patches: Drop docs changes, for licensing reasons
Ian Jackson writes ("[PATCH] stubdom/grub.patches: Drop docs changes, for licensing reasons"): > The patch file 00cvs is an import of a new upstream version of > grub1 from upstream CVS. FTR, I intend to backport this change to the earliest (security-supported) tree that it cleanly applies to. If someone thinks this is a bad idea please let me know. Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 18/18] tools/debugger/kdd: Install as `xen-kdd', not just `kdd'
On 08/10/2018 16:01, Ian Jackson wrote: > Tim Deegan writes ("Re: [PATCH 18/18] tools/debugger/kdd: Install as > `xen-kdd', not just `kdd'"): >> At 18:29 +0100 on 05 Oct (1538764157), Ian Jackson wrote: >>> `kdd' is an unfortunate namespace landgrab. >> >> Bah, humbug, etc. :) Can we have a note in the changelog for the next >> release to warn the few kdd users that we've done this? > > Mmm, sorry. Err, where are such release notes collected ? I believe this is a manual process triggered by the Community Manager. I'm fine with collecting RN snipplets for the upcoming release, of course. Ian, another question: could we add a link to the release note of the related version on: https://xenbits.xen.org/docs/unstable/support-matrix.html like it was done on: https://wiki.xenproject.org/wiki/Xen_Project_Release_Features Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 00/18] Miscellaneous build and docs, fixes from Debian
Ian Jackson writes ("[PATCH 00/18] Miscellaneous build and docs, fixes from Debian"): > Bastian Blank (1): > Ian Jackson (17): Thanks to the reviewers. I have pushed to staging these patches, which were acked/reviewed and seemed to me to be uncontroversial: > docs/man: Fix two typos detected by the Debian lintian tool > tools/xentrace/xenalyze: Fix typos detected by lintian > Various: Fix typos `unkown', `retreive' (detected by lintian) > Various: Fix typo `occured' > Various: Fix typo `reseting' > tools/python/xen/lowlevel: Fix typo `sucess' > Various: Fix typo `infomation' > Various: Fix typo `mappping' > docs/man: Provide properly-formatted NAME sections > docs/man/xen-pv-channel.pod.7: Remove a spurious blank line > tools/xenstat: Fix shared library version > libfsimage: Honour general LDFLAGS Subject to further comments, I will repost the remainder as a v2 of the series. Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/vtd: fix iommu_share_p2m_table
>>> On 08.10.18 at 15:52, wrote: > Commit 2916951c1 "mm / iommu: include need_iommu() test in > iommu_use_hap_pt()" changed the check in iommu_share_p2m_table to use > need_iommu(d) (as part of iommu_use_hap_pt) instead of iommu_enabled, > which broke the check because at the point in domain construction > where iommu_share_p2m_table is called need_iommu(d) will always return > false. > > Fix this by reverting to the previous logic. > > While there turn the hap_enabled check into an ASSERT, since the only > caller of iommu_share_p2m_table already performs the hap_enabled check > before calling the function. > > Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -505,7 +505,13 @@ int iommu_do_domctl( > > void iommu_share_p2m_table(struct domain* d) > { > -if ( iommu_use_hap_pt(d) ) > +ASSERT(hap_enabled(d)); > +/* > + * iommu_use_hap_pt cannot be used here because at the point in the > domain > + * construction where iommu_share_p2m_table get called need_iommu(d) will > + * always return false. > + */ I'm fighting with myself whether to shorten this comment while committing - it's certainly not "brief". Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/18] INSTALL: Mention kconfig
Jan Beulich writes ("Re: [PATCH 10/18] INSTALL: Mention kconfig"): > On 08.10.18 at 16:08, wrote: > > Thanks, I'll take that as an ack. (Assuming you did indeed mean > > `accepting' rather than `excepting'.) > > Well, no, it was not meant as an ack, merely as the absence of further > objections. (And yes, I did mean "accepting".) Err, OK. Doug, do you want me to propose a different or expanded wording ? Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] mm/page_alloc: add bootscrub=idle cmdline option
>>> On 03.10.18 at 12:28, wrote: > Scrubbing RAM during boot may take a long time on machines with lots > of RAM. Add 'idle' option which marks all pages dirty initially so they > would eventually be scrubbed in idle-loop on every online CPU. > > Performance of idle-loop scrubbing is worse than bootscrub but it's > guaranteed that the allocator will return scrubbed pages by doing > eager scrubbing during allocation (unless MEMF_no_scrub was provided). I've commented on this before: Without clarifying what "performance" means in this context, the statement's truth cannot be verified. To certain (many?) people, performance in the context of booting a system may mean the time it takes until the system is fully up. I think idle scrubbing helps this rather than making it worse. > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -227,7 +227,7 @@ that byte `0x12345678` is bad, you would place > `badpage=0x12345` on > Xen's command line. > > ### bootscrub > -> `= ` > +> `= | idle` > > > Default: `true` Any reason not to make "idle" the default? Iirc that was the plan anyway. > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -161,8 +161,32 @@ string_param("badpage", opt_badpage); > /* > * no-bootscrub -> Free pages are not zeroed during boot. > */ > -static bool_t opt_bootscrub __initdata = 1; > -boolean_param("bootscrub", opt_bootscrub); > +enum { > +BOOTSCRUB_OFF = 0, > +BOOTSCRUB_ON, > +BOOTSCRUB_IDLE, > +}; > +static int __read_mostly opt_bootscrub = BOOTSCRUB_ON; Why "int" when you've just made yourself an enum? > +static int __init parse_bootscrub_param(const char *s) > +{ > +if ( *s == '\0' ) > +return 0; > + > +if ( !strcmp(s, "idle") ) > +opt_bootscrub = BOOTSCRUB_IDLE; > +else > +opt_bootscrub = parse_bool(s, NULL); > + > +if ( opt_bootscrub < 0 ) Can you please follow the model used elsewhere, where parse_bool() gets called first? Also can "bootscrub" alone please have the same meaning as "bootscrub=yes"? > +{ > +opt_bootscrub = BOOTSCRUB_ON; > +return -EINVAL; > +} > + > +return 0; > +} > + > +custom_param("bootscrub", parse_bootscrub_param); No blank line between function and its own custom_param() please. > @@ -1763,7 +1787,8 @@ static void init_heap_pages( > nr_pages -= n; > } > > -free_heap_pages(pg + i, 0, scrub_debug); > +free_heap_pages(pg + i, 0, scrub_debug || > + opt_bootscrub == BOOTSCRUB_IDLE); So this is the hunk explaining why the variable can't be __initdata. The question though is whether the resulting post-init behavior is actually correct; it's certainly odd for a boot related option to have an effect post-boot as well. > @@ -2039,8 +2064,11 @@ void __init heap_init_late(void) > */ > setup_low_mem_virq(); > > -if ( opt_bootscrub ) > +if ( opt_bootscrub == BOOTSCRUB_ON ) > scrub_heap_pages(); > +else if ( opt_bootscrub == BOOTSCRUB_IDLE ) > +printk("Scrubbing Free RAM on %d nodes in background\n", > + num_online_nodes()); switch()? And is the reference to the node count really meaningful in the logged message? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 07/18] Various: Fix typo `infomation'
Jan Beulich writes ("Re: [PATCH 07/18] Various: Fix typo `infomation'"): > On 05.10.18 at 19:29, wrote: > > Signed-off-by: Ian Jackson > > Acked-by: Jan Beulich > > Funny how often the same spelling mistakes repeat... Mmm. In one of my own projects I had a persistent problem with `pseudo' (which appeared in some textual protocol elements, so the spelling was important). I eventually wrote a test case which fails iff the source code contains `ps[u]edo' (written that way to avoid tripping itself). Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/18] INSTALL: Mention kconfig
Jan Beulich writes ("Re: [PATCH 10/18] INSTALL: Mention kconfig"): > On 05.10.18 at 19:29, wrote: > > +silently overriden. The only way to find which configuration options > > Isn't it "overridden", or are both spellings okay? It is; I was wrong. > > +this varible there is nothing stopping you setting dangerously > > variable Fixed. > Beyond these nits I'm okay with the text now, accepting the anomaly > pointed out by Doug. Thanks, I'll take that as an ack. (Assuming you did indeed mean `accepting' rather than `excepting'.) Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/18] INSTALL: Mention kconfig
Doug Goldstein writes ("Re: [PATCH 10/18] INSTALL: Mention kconfig"): > On Fri, Oct 05, 2018 at 06:29:09PM +0100, Ian Jackson wrote: > > Firstly, add a reference to the documentation for the kconfig system. > > > > Secondly, warn the user about the XEN_CONFIG_EXPERT problem. > > Reviewed-by: Doug Goldstein Thanks. > > +Xen Hypervisor > > +== > > + > > +Xen itself is configured via a `kconfig' system borrowed from Linux. > > +See docs/misc/kconfig.txt. > > + > > +Note that unlike with Linux, and contrary to that document, you cannot > > +look at Kconfig files, or the default or generated config files etc., > > +to find available configuration options. This is because it is only > > +supported (and security supported) by the Xen Project, to change a > > +small subset of the options. Attempts to change other options will be > > +silently overriden. The only way to find which configuration options > > +are available is to run `make menuconfig' or the like. > > + > > +You can counter-override this behaviour by setting XEN_CONFIG_EXPERT=y > > +in your environment. However, doing this is not supported and the > > +resulting configurations do not receive security support. If you set > > +this varible there is nothing stopping you setting dangerously > > +experimental combinations of features - not even any warnings. > > Not really true because the shim is supported and relies on > XEN_CONFIG_EXPERT It sounds like you are saying that what I write above is not accurate. I didn't intend it to cover any setting of XEN_CONFIG_EXPERT by the Xen build system itself. I'm not sure how that interacts wit your R-B either. Normally a R-B would imply approval of the contents. > but certainly better to make users aware than blindly > hiding everything from them. I'd still argue that eventually we'll get > rid of this because most distros build with XEN_CONFIG_EXPERT on and > most Xen devs I know just have it set in their environment. Well, yes. I decided that documenting the situation was the best way to throw light on it and possibly even get it changed. Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 18/18] tools/debugger/kdd: Install as `xen-kdd', not just `kdd'
Tim Deegan writes ("Re: [PATCH 18/18] tools/debugger/kdd: Install as `xen-kdd', not just `kdd'"): > At 18:29 +0100 on 05 Oct (1538764157), Ian Jackson wrote: > > `kdd' is an unfortunate namespace landgrab. > > Bah, humbug, etc. :) Can we have a note in the changelog for the next > release to warn the few kdd users that we've done this? Mmm, sorry. Err, where are such release notes collected ? CC'ing Juergen as RM. > > Signed-off-by: Ian Jackson > > Acked-by: Tim Deegan Thanks, Ian. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/vtd: fix iommu_share_p2m_table
> -Original Message- > From: Roger Pau Monne [mailto:roger@citrix.com] > Sent: 08 October 2018 14:53 > To: xen-devel@lists.xenproject.org > Cc: Roger Pau Monne ; Jan Beulich > ; Paul Durrant > Subject: [PATCH v2] x86/vtd: fix iommu_share_p2m_table > > Commit 2916951c1 "mm / iommu: include need_iommu() test in > iommu_use_hap_pt()" changed the check in iommu_share_p2m_table to use > need_iommu(d) (as part of iommu_use_hap_pt) instead of iommu_enabled, > which broke the check because at the point in domain construction > where iommu_share_p2m_table is called need_iommu(d) will always return > false. > > Fix this by reverting to the previous logic. > > While there turn the hap_enabled check into an ASSERT, since the only > caller of iommu_share_p2m_table already performs the hap_enabled check > before calling the function. > > Signed-off-by: Roger Pau Monné Reviewed-by: Paul Durrant > --- > Cc: Jan Beulich > Cc: Paul Durrant > --- > Changes since v1: > - Add a comment about why iommu_use_hap_pt cannot be used in >iommu_share_p2m_table. > - Expand commit message. > - Convert the hap_enabled check into an ASSERT. > --- > xen/drivers/passthrough/iommu.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/passthrough/iommu.c > b/xen/drivers/passthrough/iommu.c > index debb5e6fe1..75bc410c2c 100644 > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -505,7 +505,13 @@ int iommu_do_domctl( > > void iommu_share_p2m_table(struct domain* d) > { > -if ( iommu_use_hap_pt(d) ) > +ASSERT(hap_enabled(d)); > +/* > + * iommu_use_hap_pt cannot be used here because at the point in the > domain > + * construction where iommu_share_p2m_table get called need_iommu(d) > will > + * always return false. > + */ > +if ( iommu_enabled && iommu_hap_pt_share ) > iommu_get_ops()->share_p2m(d); > } > > -- > 2.19.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/vtd: fix iommu_share_p2m_table
Commit 2916951c1 "mm / iommu: include need_iommu() test in iommu_use_hap_pt()" changed the check in iommu_share_p2m_table to use need_iommu(d) (as part of iommu_use_hap_pt) instead of iommu_enabled, which broke the check because at the point in domain construction where iommu_share_p2m_table is called need_iommu(d) will always return false. Fix this by reverting to the previous logic. While there turn the hap_enabled check into an ASSERT, since the only caller of iommu_share_p2m_table already performs the hap_enabled check before calling the function. Signed-off-by: Roger Pau Monné --- Cc: Jan Beulich Cc: Paul Durrant --- Changes since v1: - Add a comment about why iommu_use_hap_pt cannot be used in iommu_share_p2m_table. - Expand commit message. - Convert the hap_enabled check into an ASSERT. --- xen/drivers/passthrough/iommu.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index debb5e6fe1..75bc410c2c 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -505,7 +505,13 @@ int iommu_do_domctl( void iommu_share_p2m_table(struct domain* d) { -if ( iommu_use_hap_pt(d) ) +ASSERT(hap_enabled(d)); +/* + * iommu_use_hap_pt cannot be used here because at the point in the domain + * construction where iommu_share_p2m_table get called need_iommu(d) will + * always return false. + */ +if ( iommu_enabled && iommu_hap_pt_share ) iommu_get_ops()->share_p2m(d); } -- 2.19.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/5] xen/domain: Allocate d->vcpu[] earlier during domain_create()
>>> On 05.10.18 at 16:54, wrote: > @@ -405,19 +419,6 @@ struct domain *domain_create(domid_t domid, > > if ( !is_idle_domain(d) ) > { > -/* Check d->max_vcpus and allocate d->vcpu[]. */ > -err = -EINVAL; > -if ( config->max_vcpus < 1 || > - config->max_vcpus > domain_max_vcpus(d) ) > -goto fail; Ah, there it goes away. But I think it would be more logical for this to happen in the previous patch. Anyway Reviewed-by: Jan Beulich for both, preferably (but not necessarily) with the removal moved there. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/5] xen/domain: Audit config->max_vcpus during {, arch_}check_domain_config()
>>> On 05.10.18 at 16:54, wrote: > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -601,6 +601,8 @@ void vcpu_switch_to_aarch64_mode(struct vcpu *v) > > int arch_check_domain_config(struct xen_domctl_createdomain *config) > { > +unsigned int max_vcpus = 0; Is the initializer really needed here considering ... > @@ -619,6 +621,22 @@ int arch_check_domain_config(struct > xen_domctl_createdomain *config) > } > } > > +/* Calculate the maximum number of vcpus from the selected GIC > version... */ > +switch ( config->arch.gic_version ) > +{ > +case GIC_V2: max_vcpus = 8; break; > +case GIC_V3: max_vcpus = 255; break; > + > +default: > +return -EOPNOTSUPP; ... this? > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -297,6 +297,9 @@ static int check_domain_config(struct > xen_domctl_createdomain *config) > XEN_DOMCTL_CDF_xs_domain) ) > return -EINVAL; > > +if ( config->max_vcpus < 1 ) > +return -EINVAL; > + > return arch_check_domain_config(config); > } Any reason you don't remove the now redundant check from domain_create(), which would allow ditching altogether x86's domain_max_vcpus()? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] vtpmmgr compiled from from git strange output
Hi! Just cloned master from git and compiled stubdom/vtpmmgr and am getting weird output from it. Like this: INFO[TPM]: TPM2_PCR_Read TPM Manager - disk format 0 root seal: zu; sector of 84: zu root: zu v=zu itree: 36; sector of 112: zu group: zu v=zu id=zu md=zu group seal: zu; 72 in parent: zu; sector of 5: zu vtpm: zu+zu; sector of 20: zu INFO[VTPM]: disk_read_sector 0 INFO[VTPM]: disk_read_sector 1 load_root_pre: n/n INFO[VTPM]: Assuming first time initialization. INFO[TPM]: TPM2_CreatePrimary INFO[VTPM]: SRK handle: 0x8000 INFO[TPM]: TPM2_Create Looking at the history in git it seems like the %zu was patched quite dome time ago. Why was this done even if the supporting c-lib doesn't seem to support it? Am I missing something here? And still vtpmmrg will not even touch the disk image to persistantly save the Hash Key of the vtpm:s. Am I beating a dead horse here? Best Dag ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/5] xen/domain: Introduce a new check_domain_config() helper
>>> On 05.10.18 at 16:54, wrote: > Call it from the head of domain_create() (before doing any memory > allocations), which will apply the checks to dom0 as well as domU's. > > For now, just subsume the XEN_DOMCTL_CDF_* check from XEN_DOMCTL_createdomain. > This means that the corner case of the toolstack providing bad configuration > will burn a domid, but production setups shouldn't ever get into this > situation. "Burn" as in "skip in the current round", not as in "leak" afaiu? > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -288,6 +288,18 @@ static void _domain_destroy(struct domain *d) > free_domain_struct(d); > } > > +static int check_domain_config(struct xen_domctl_createdomain *config) I was tempted to ask for the parameter to be constified, but since on its own the code movement here makes no sense (and the description also doesn't supply any hint), I've peeked into patch 2, where I found that Arm's arch_check_domain_config() actually modifies the config. With that I don't consider "check" the right term for the function name; "sanitize" or "massage" perhaps? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/5] xen/domain: Introduce a new arch_check_domain_config() helper
>>> On 05.10.18 at 16:54, wrote: > On the ARM side, lift the code to select the appropriate GIC version when > NATIVE is requested. > > Signed-off-by: Andrew Cooper The minimal x86 pieces Acked-by: Jan Beulich (possibly subject to renaming as per patch 1's comment) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/hvm/ioreq: allow ioreq servers to use HVM_PARAM_[BUF]IOREQ_PFN
>>> On 05.10.18 at 15:43, wrote: > Since commit 2c257bd6 "x86/hvm: remove default ioreq server (again)" the > GFNs allocated by the toolstack and set in HVM_PARAM_IOREQ_PFN and > HVM_PARAM_BUFIOREQ_PFN have been unused. This patch allows them to be used > by (non-default) ioreq servers. > > NOTE: This fixes a compatibility issue. A guest created on a version of > Xen that pre-dates the initial ioreq server implementation and then > migrated in will currently fail to resume because its migration > stream will lack values for HVM_PARAM_IOREQ_SERVER_PFN and > HVM_PARAM_NR_IOREQ_SERVER_PAGES *unless* the system has an > emulator domain that uses direct resource mapping (which depends > on the version of privcmd it happens to have) in which case it > will not require use of GFNs for the ioreq server shared > pages. Meaning this wants to be backported till where? > A similar compatibility issue with migrated-in VMs exists with Xen 4.11 > because the upstream QEMU fall-back to use legacy ioreq server was broken > when direct resource mapping was introduced. > This is because, prior to the resource mapping patches, it was the creation > of the non-default ioreq server that failed if GFNs were not available > whereas, as of 4.11, it is retrieval of the info that fails which does not > trigger the fall-back. Are you working on a fix or workaround for this, too, then? > --- a/xen/arch/x86/hvm/ioreq.c > +++ b/xen/arch/x86/hvm/ioreq.c > @@ -237,6 +237,26 @@ bool handle_hvm_io_completion(struct vcpu *v) > return true; > } > > +static gfn_t hvm_alloc_legacy_ioreq_gfn(struct hvm_ioreq_server *s) > +{ > +struct domain *d = s->target; > +unsigned int i; > + > +BUILD_BUG_ON(HVM_PARAM_IOREQ_PFN > > + sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); > +BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN > > + sizeof(d->arch.hvm.ioreq_gfn.legacy_mask) * 8); > +BUILD_BUG_ON(HVM_PARAM_BUFIOREQ_PFN != HVM_PARAM_IOREQ_PFN + 1); > + > +for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ ) > +{ > +if ( !test_and_set_bit(i, >arch.hvm.ioreq_gfn.legacy_mask) ) > +return _gfn(d->arch.hvm.params[i]); Aren't you risking to use GFN 0 here, if the param was never set? Since in theory GFN 0 could be valid here, perhaps whether these MFNs may be used should be tracked by starting out legacy_mask such that allocations are impossible, while the setting of the params would then make the slots available for allocating from? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/hvm: make sure HVM_PARAM_[BUF]IOREQ_PFN can only be set once
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 08 October 2018 14:20 > To: Paul Durrant > Cc: Andrew Cooper ; Wei Liu > ; xen-devel > Subject: Re: [PATCH 1/2] x86/hvm: make sure HVM_PARAM_[BUF]IOREQ_PFN can > only be set once > > >>> On 05.10.18 at 15:43, wrote: > > These parameters should have always been in the 'set once' category > > but this has, so far, not been enforced. > > Hmm, now that I'm looking at patch 2 I see where this is coming from, > but a hint towards this here would have helped, if this is to be a > separate patch (folding into the second patch with a respective > remark would perhaps have been better anyway). > Ok. I'll squash them together if I need to do a v2. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/hvm: make sure HVM_PARAM_[BUF]IOREQ_PFN can only be set once
>>> On 05.10.18 at 15:43, wrote: > These parameters should have always been in the 'set once' category > but this has, so far, not been enforced. Hmm, now that I'm looking at patch 2 I see where this is coming from, but a hint towards this here would have helped, if this is to be a separate patch (folding into the second patch with a respective remark would perhaps have been better anyway). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/xsm: remove unnecessary #define
On Mon, Oct 08, 2018 at 06:59:01PM +0800, Xin Li wrote: > From: root This needs to be deleted while committing. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/hvm: make sure HVM_PARAM_[BUF]IOREQ_PFN can only be set once
>>> On 05.10.18 at 15:43, wrote: > These parameters should have always been in the 'set once' category > but this has, so far, not been enforced. But now that we're not even handling these anymore, why is there a need to start doing so? If anything wouldn't it be better to add them to the deprecated group in that same function (and maybe also its "get" counterpart)? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 128500: tolerable all pass - PUSHED
flight 128500 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/128500/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 7b20a865bc105fe566156201c8e6c37ef692e3dd baseline version: xen 91d4eca7add6a7a114bc05cc6d38223a0c0b5575 Last test of basis 128426 2018-10-05 16:00:58 Z2 days Testing same since 128500 2018-10-08 11:00:56 Z0 days1 attempts People who touched revisions under test: Christian Lindig Yang Qian jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 91d4eca7ad..7b20a865bc 7b20a865bc105fe566156201c8e6c37ef692e3dd -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] preparations for 4.11.1 and 4.8.5
All, both releases are due in about a month's time. Please point out backports you find missing from their respective staging branches, but which you consider relevant. On top of what I've just pushed there I have 2fb57e4bee x86: silence false log messages for plain "xpti" / "pv-l1tf" 51e0cb4593 x86: split opt_xpti 0b89643ef6 x86: split opt_pv_l1tf 8743d2dea5 x86: fix "xpti=" and "pv-l1tf=" yet again e30c47cd8b vtd: add missing check for shared EPT... queued already - no need to point these out separately. I've noticed only now that even 4.8.4 has gone out already after its full support time period was over, due to the significant delay between its preparations announcement and it actually having become ready. Since its tree was maintained as if another stable release would be done, I think we will want to cut such a release. In that event, 4.8.5 is going to be the last XenProject coordinated release from that branch. If we decide otherwise, we'd then announce after the fact that 4.8.4 actually was. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/svm: Fix svm_update_guest_efer() for domains using shadow paging
>>> On 08.10.18 at 13:03, wrote: > On 08/10/18 11:12, Jan Beulich wrote: > On 05.10.18 at 19:02, wrote: >>> --- a/xen/arch/x86/hvm/svm/svm.c >>> +++ b/xen/arch/x86/hvm/svm/svm.c >>> @@ -649,13 +649,32 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int > cr, unsigned int flags) >>> static void svm_update_guest_efer(struct vcpu *v) >>> { >>> struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; >>> -bool lma = v->arch.hvm.guest_efer & EFER_LMA; >>> -uint64_t new_efer; >>> +unsigned long guest_efer = v->arch.hvm.guest_efer, >>> +xen_efer = read_efer(); >>> >>> -new_efer = (v->arch.hvm.guest_efer | EFER_SVME) & ~EFER_LME; >>> -if ( lma ) >>> -new_efer |= EFER_LME; >>> -vmcb_set_efer(vmcb, new_efer); >>> +if ( paging_mode_shadow(v->domain) ) >>> +{ >>> +/* EFER.NX is a Xen-owned bit and is not under guest control. */ >>> +guest_efer &= ~EFER_NX; >>> +guest_efer |= xen_efer & EFER_NX; >>> + >>> +/* >>> + * CR0.PG is a Xen-owned bit, and remains set even when the guest >>> has >>> + * logically disabled paging. >>> + * >>> + * LMA was calculated using the guest CR0.PG setting, but LME needs >>> + * clearing to avoid interacting with Xen's CR0.PG setting. As >>> writes >>> + * to CR0 are intercepted, it is safe to leave LME clear at this >>> + * point, and fix up both LME and LMA when CR0.PG is set. >>> + */ >>> +if ( !(guest_efer & EFER_LMA) ) >>> +guest_efer &= ~EFER_LME; >>> +} >> I think this wants an "else", either ASSERT()ing that what the removed >> code did is actually the case (arranged for by the callers), or >> retaining the original logic in some form. > > No - the original logic does not want keeping. It is a latent bug in > the HAP case, because if the guest could write to CR0, setting CR0.PG > would fail to activate long mode. Hmm, perhaps we're talking of different parts of the original code: I think you talk about the clearing of LME, whereas I am mostly concerned about the dropped implication of LME from LMA. Also note that I suggested keeping some form of the old code only as one option; the other was to add an ASSERT() verifying that the callers have taken care of that implication. In particular for these ... >> This looks particularly relevant >> when hvm_efer_valid() was called with -1 as its cr0_pg argument, as >> in that case there was not necessarily any correlation enforced >> between EFER.LMA and CR0.PG. > > On the subject of passing -1, all of that logic is horrible, but it is > only ever used when LME/LMA is being recalculated around other state > changes. ... cases I couldn't really convince myself that all callers really guarantee this. Anyway - don't read this as an objection to the change. I agree SVM and VMX should be in line with one another wherever possible. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
>>> On 08.10.18 at 12:50, wrote: > On Mon, Oct 08, 2018 at 12:37:39PM +0200, Roger Pau Monné wrote: >> On Mon, Oct 08, 2018 at 04:33:04AM -0600, Jan Beulich wrote: >> > >>> On 08.10.18 at 12:14, wrote: >> > > --- a/xen/drivers/passthrough/iommu.c >> > > +++ b/xen/drivers/passthrough/iommu.c >> > > @@ -505,7 +505,7 @@ int iommu_do_domctl( >> > > >> > > void iommu_share_p2m_tableiommu_share_p2m_table(struct domain* d) >> > > { >> > > -if ( iommu_use_hap_pt(d) ) >> > > +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) >> > >> > Considering the only caller, the hap_enabled() part is (and was) not >> > needed here. >> >> Would you like me to remove it in this patch? > > I've converted it to an ASSERT instead of just removing it. That's fine of course (and I was about to suggest this in reply to your earlier mail, but luckily have looked here first). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Backports to stable
>>> On 08.10.18 at 12:40, wrote: > On Mon, Oct 08, 2018 at 03:29:06AM -0600, Jan Beulich wrote: >> >>> On 07.10.18 at 03:04, wrote: >> > I'd like to propose backporting GCC7/8 fixes to all stable branches. Below >> > is a list up to stable-4.6, but some of the patches are already on >> > select branches (developed during that release cycle, or already >> > backported). >> >> I continue to be opposed to backporting anything that is not needed >> for dealing security issues to branches which are in security-support- >> only mode, i.e. anything older than 4.8 at this point in time. > > Ok, noted. It's hard to compile those still security-supported versions > on recent systems. But if one need such combination (old Xen + new other > things), then can also apply those patches locally. I just wanted to > reduce work duplication. > >> Furthermore I notice that 4.6 has just moved out of security support, >> at the end of last week. > > Hmm, 18+18 months from October 13, 2015 is at the end of this week. Quite possible - I'm not judging by announcement date, but by the time stamp of xen/Makefile (which is always the last thing to get updated, for the version adjustment). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/xsm: remove unnecessary #define
>>> On 08.10.18 at 12:59, wrote: > From: root > > this #define is unnecessary since XSM_INLINE is redefined in > xsm/dummy.h, so remove it. And it is actually a latent risk of build breakage, if the other one got updated without this one following suit. > Signed-off-by: Xin Li Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for multi-page case
>>> On 25.09.18 at 17:30, wrote: > On 25/09/18 13:41, Jan Beulich wrote: > On 20.09.18 at 14:41, wrote: >>> On 13/09/18 11:12, Jan Beulich wrote: The function does two translations in one go for a single guest access. Any failure of the first translation step (guest linear -> guest physical), resulting in #PF, ought to take precedence over any failure of the second step (guest physical -> host physical). >>> Why? What is the basis of this presumption? >>> >>> As far as what real hardware does... >>> >>> This test sets up a ballooned page and a read-only page. I.e. a second >>> stage fault on the first part of a misaligned access, and a first stage >>> fault on the second part of the access. >>> >>> (d1) --- Xen Test Framework --- >>> (d1) Environment: HVM 64bit (Long mode 4 levels) >>> (d1) Test splitfault >>> (d1) About to read >>> (XEN) *** EPT qual 0181, gpa 0011cffc >>> (d1) Reading PTR: got >>> (d1) About to write >>> (XEN) *** EPT qual 0182, gpa 0011cffc >>> (d1) ** >>> (d1) PANIC: Unhandled exception at 0008:001047e0 >>> (d1) Vec 14 #PF[-d-sWP] %cr2 0011d000 >>> (d1) ** >>> >>> The second stage fault is recognised first, which is contrary to your >>> presumption, i.e. the code in its current form appears to be correct. >> Coming back to this example of yours: As a first step, are we in >> agreement that with the exception of very complex instructions >> (FSAVE, FXSAVE, XSAVE etc) instructions are supposed to work in an >> all-or-nothing manner when it comes to updating of architectural >> state (be it registers or memory)? > > No. Read Chapter Intel Vol3 8.1 and 8.2, which makes it quite clear > that misaligned accesses may be split access, and observably so to other > processors in the system. > > I've even found a new bit in it which says that >quadword SSE accesses > may even result in a partial write being completed before #PF is raised. As said before, I'm all with you with the "may" part for FPU, SSE, and alike more complex accesses. Short of being able to think of a way to compare (in a more direct way than via EPT behavior, as your test did) native and emulated behavior (I had coded something up until I realized that it doesn't test what I want to test), I've written a small test for real hardware. Would this debugging patch --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -87,7 +87,7 @@ GLOBAL(boot_cpu_compat_gdt_table) * of physical memory. In any case the VGA hole should be mapped with type UC. * Uses 1x 4k page. */ -l1_identmap: +GLOBAL(l1_identmap) pfn = 0 .rept L1_PAGETABLE_ENTRIES /* VGA hole (0xa-0xc) should be mapped UC-. */ --- a/xen/arch/x86/ioport_emulate.c +++ b/xen/arch/x86/ioport_emulate.c @@ -112,8 +112,48 @@ static struct dmi_system_id __initdata i { } }; +extern unsigned long l1_identmap[L1_PAGETABLE_ENTRIES];//temp static int __init ioport_quirks_init(void) { + {//temp + unsigned char*hole = __va(0xb); + unsigned long l1e = l1_identmap[0xb0]; + int i; + + show_page_walk((long)hole); + + l1_identmap[0xb0] &= ~_PAGE_RW; + l1_identmap[0xb0] |= _PAGE_PWT; + for(;;) { + flush_area_all(hole, FLUSH_TLB|FLUSH_TLB_GLOBAL); + show_page_walk((long)hole); + + for(i = 3; i > 0; --i) { +unsigned long cr2 = 0; + +memset(hole - 4, 0, 4); +asm("1: addl $-1, %0; 2:\n\t" +".pushsection .fixup,\"ax\"\n" +"3: mov %%cr2, %1; jmp 2b\n\t" +".popsection\n\t" +_ASM_EXTABLE(1b, 3b) +: "+m" (hole[-i]), "+r" (cr2) :: "memory"); + +printk("CR2=%lx data: %02x %02x %02x %02x\n", cr2, + hole[-4], hole[-3], hole[-2], hole[-1]); + } + + if(l1_identmap[0xb0] & (1UL << (paddr_bits - 1))) +break; + + l1_identmap[0xb0] |= ((1UL << paddr_bits) - 1) & PAGE_MASK; + } + + l1_identmap[0xb0] = l1e; + flush_area_all(hole, FLUSH_TLB|FLUSH_TLB_GLOBAL); + show_page_walk((long)hole); + } + dmi_check_system(ioport_quirks_tbl); return 0; } producing this output (XEN) Pagetable walk from 830b: (XEN) L4[0x106] = 8000cf0ba063 (XEN) L3[0x000] = cf0b4063 (XEN) L2[0x000] = cf0b3063 (XEN) L1[0x0b0] = 000b0373 (XEN) Pagetable walk from 830b: (XEN) L4[0x106] = 8000cf0ba063 (XEN) L3[0x000] = cf0b4063 (XEN) L2[0x000] = cf0b3063 (XEN) L1[0x0b0] = 000b0379 (XEN) CR2=830b data: 00 00 00 00 (XEN) CR2=830b data: 00 00 00 00 (XEN) CR2=830b data: 00 00 00 00 (XEN) Pagetable walk from 830b: (XEN) L4[0x106] = 8000cf0ba063 (XEN) L3[0x000] = cf0b4063 (XEN) L2[0x000] = cf0b3063
Re: [Xen-devel] [PATCH] x86/svm: Fix svm_update_guest_efer() for domains using shadow paging
On 08/10/18 11:12, Jan Beulich wrote: On 05.10.18 at 19:02, wrote: >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -649,13 +649,32 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int >> cr, unsigned int flags) >> static void svm_update_guest_efer(struct vcpu *v) >> { >> struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; >> -bool lma = v->arch.hvm.guest_efer & EFER_LMA; >> -uint64_t new_efer; >> +unsigned long guest_efer = v->arch.hvm.guest_efer, >> +xen_efer = read_efer(); >> >> -new_efer = (v->arch.hvm.guest_efer | EFER_SVME) & ~EFER_LME; >> -if ( lma ) >> -new_efer |= EFER_LME; >> -vmcb_set_efer(vmcb, new_efer); >> +if ( paging_mode_shadow(v->domain) ) >> +{ >> +/* EFER.NX is a Xen-owned bit and is not under guest control. */ >> +guest_efer &= ~EFER_NX; >> +guest_efer |= xen_efer & EFER_NX; >> + >> +/* >> + * CR0.PG is a Xen-owned bit, and remains set even when the guest >> has >> + * logically disabled paging. >> + * >> + * LMA was calculated using the guest CR0.PG setting, but LME needs >> + * clearing to avoid interacting with Xen's CR0.PG setting. As >> writes >> + * to CR0 are intercepted, it is safe to leave LME clear at this >> + * point, and fix up both LME and LMA when CR0.PG is set. >> + */ >> +if ( !(guest_efer & EFER_LMA) ) >> +guest_efer &= ~EFER_LME; >> +} > I think this wants an "else", either ASSERT()ing that what the removed > code did is actually the case (arranged for by the callers), or > retaining the original logic in some form. No - the original logic does not want keeping. It is a latent bug in the HAP case, because if the guest could write to CR0, setting CR0.PG would fail to activate long mode. Nothing goes wrong because SVM doesn't have a guest/host mask which allows selective updating of > This looks particularly relevant > when hvm_efer_valid() was called with -1 as its cr0_pg argument, as > in that case there was not necessarily any correlation enforced > between EFER.LMA and CR0.PG. On the subject of passing -1, all of that logic is horrible, but it is only ever used when LME/LMA is being recalculated around other state changes. This patch brings the SVM logic in line with VT-x logic (c/s fd32dcfe4) - SVM and VT-x are a whole lot less different than our code suggests. If there were problems with the common code, we've seen them already. Either both functions want changing, or neither, but I don't think it is a useful check to make. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/3] xen/xsm: Add new SILO mode for XSM
When SILO is enabled, there would be no page-sharing or event notifications between unprivileged VMs (no grant tables or event channels). Signed-off-by: Xin Li --- CC: Daniel De Graaf CC: George Dunlap CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Sergey Dyasli CC: Andrew Cooper CC: Ming Lu v5: 1. use __maybe_unused instead of __attribute__ ((unused)), and remove unnecessary #ifdef CONFIG_XSM_SILO. 2. rename cur_dom to currd. 3. move the removal of #define in dummy.c to a seperate patch. 4. remove a blank line in silo.c. --- docs/misc/xen-command-line.markdown | 5 +- xen/common/Kconfig | 15 xen/include/xsm/dummy.h | 3 +- xen/include/xsm/xsm.h | 6 ++ xen/xsm/Makefile| 1 + xen/xsm/silo.c | 108 xen/xsm/xsm_core.c | 11 +++ 7 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 xen/xsm/silo.c diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 67e062ecd7..2c7046eb86 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -900,7 +900,7 @@ Note that specifying zero as domU value means zero, while for dom0 it means to use the default. ### xsm -> `= dummy | flask` +> `= dummy | flask | silo` > Default: `dummy` @@ -911,6 +911,9 @@ the hypervisor was compiled with XSM support. (the dummy module) will be applied. It's also used when XSM is compiled out. * `flask`: this is the policy based access control. To choose this, the separated option in kconfig must also be enabled. +* `silo`: this will deny any unmediated communication channels between + unprivileged VMs. To choose this, the separated option in kconfig must also + be enabled. ### flask > `= permissive | enforcing | late | disabled` diff --git a/xen/common/Kconfig b/xen/common/Kconfig index f802efb625..ce965fbf17 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -154,15 +154,30 @@ config XSM_FLASK_POLICY If unsure, say Y. +config XSM_SILO + def_bool y + prompt "SILO support" + depends on XSM + ---help--- + Enables SILO as the access control mechanism used by the XSM framework. + This is not the default module, add boot parameter xsm=silo to choose + it. This will deny any unmediated communication channels (grant tables + and event channels) between unprivileged VMs. + + If unsure, say Y. + choice prompt "Default XSM implementation" depends on XSM default XSM_FLASK_DEFAULT if XSM_FLASK + default XSM_SILO_DEFAULT if XSM_SILO default XSM_DUMMY_DEFAULT config XSM_DUMMY_DEFAULT bool "Match non-XSM behavior" config XSM_FLASK_DEFAULT bool "FLux Advanced Security Kernel" if XSM_FLASK + config XSM_SILO_DEFAULT + bool "SILO" if XSM_SILO endchoice config LATE_HWDOM diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index b0ac1f66b3..ae971822d5 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -48,7 +48,8 @@ void __xsm_action_mismatch_detected(void); * There is no xsm_default_t argument available, so the value from the assertion * is used to initialize the variable. */ -#define XSM_INLINE /* */ +#define XSM_INLINE __maybe_unused + #define XSM_DEFAULT_ARG /* */ #define XSM_DEFAULT_VOID void #define XSM_ASSERT_ACTION(def) xsm_default_t action = def; (void)action diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 3d67962493..3b192b5c31 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -733,6 +733,12 @@ extern const unsigned char xsm_flask_init_policy[]; extern const unsigned int xsm_flask_init_policy_size; #endif +#ifdef CONFIG_XSM_SILO +extern void silo_init(void); +#else +static inline void silo_init(void) {} +#endif + #else /* CONFIG_XSM */ #include diff --git a/xen/xsm/Makefile b/xen/xsm/Makefile index 8bb4a24f09..e4d581e065 100644 --- a/xen/xsm/Makefile +++ b/xen/xsm/Makefile @@ -1,5 +1,6 @@ obj-y += xsm_core.o obj-$(CONFIG_XSM) += xsm_policy.o obj-$(CONFIG_XSM) += dummy.o +obj-$(CONFIG_XSM_SILO) += silo.o subdir-$(CONFIG_XSM_FLASK) += flask diff --git a/xen/xsm/silo.c b/xen/xsm/silo.c new file mode 100644 index 00..4850756a3d --- /dev/null +++ b/xen/xsm/silo.c @@ -0,0 +1,108 @@ +/** + * xsm/silo.c + * + * SILO module for XSM (Xen Security Modules) + * + * Copyright (c) 2018 Citrix Systems Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but
[Xen-devel] [PATCH 1/3] xen/xsm: remove unnecessary #define
From: root this #define is unnecessary since XSM_INLINE is redefined in xsm/dummy.h, so remove it. Signed-off-by: Xin Li --- CC: Daniel De Graaf CC: George Dunlap CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Sergey Dyasli CC: Andrew Cooper CC: Ming Lu v5: move the removal of #define to this new patch. --- xen/xsm/dummy.c | 1 - 1 file changed, 1 deletion(-) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 3290d04527..06a674fad0 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -11,7 +11,6 @@ */ #define XSM_NO_WRAPPERS -#define XSM_INLINE /* */ #include struct xsm_operations dummy_xsm_ops; -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/3] xen/xsm: Introduce new boot parameter xsm
Introduce new boot parameter xsm to choose which xsm module is enabled, and set default to dummy. And add new option in Kconfig to choose the default XSM implementation. Signed-off-by: Xin Li --- CC: Daniel De Graaf CC: George Dunlap CC: Jan Beulich CC: Konrad Rzeszutek Wilk CC: Stefano Stabellini CC: Tim Deegan CC: Wei Liu CC: Sergey Dyasli CC: Andrew Cooper CC: Ming Lu v5: 1. wording and style refine. 2. use ASSERT_UNREACHABLE() instead of unreachable printk. --- docs/misc/xen-command-line.markdown | 13 xen/common/Kconfig | 13 +++- xen/xsm/xsm_core.c | 47 - 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 1ffd586224..67e062ecd7 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -899,6 +899,19 @@ hardware domain is architecture dependent. Note that specifying zero as domU value means zero, while for dom0 it means to use the default. +### xsm +> `= dummy | flask` + +> Default: `dummy` + +Specify which XSM module should be enabled. This option is only available if +the hypervisor was compiled with XSM support. + +* `dummy`: this is the default choice. Basic restriction for common deployment + (the dummy module) will be applied. It's also used when XSM is compiled out. +* `flask`: this is the policy based access control. To choose this, the + separated option in kconfig must also be enabled. + ### flask > `= permissive | enforcing | late | disabled` diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 1a6d6281c1..f802efb625 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -116,7 +116,7 @@ config XSM config XSM_FLASK def_bool y - prompt "FLux Advanced Security Kernel support" if EXPERT = "y" + prompt "FLux Advanced Security Kernel support" depends on XSM ---help--- Enables FLASK (FLux Advanced Security Kernel) as the access control @@ -154,6 +154,17 @@ config XSM_FLASK_POLICY If unsure, say Y. +choice + prompt "Default XSM implementation" + depends on XSM + default XSM_FLASK_DEFAULT if XSM_FLASK + default XSM_DUMMY_DEFAULT + config XSM_DUMMY_DEFAULT + bool "Match non-XSM behavior" + config XSM_FLASK_DEFAULT + bool "FLux Advanced Security Kernel" if XSM_FLASK +endchoice + config LATE_HWDOM bool "Dedicated hardware domain" default n diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c index 9645e244c3..9e5c1b07a2 100644 --- a/xen/xsm/xsm_core.c +++ b/xen/xsm/xsm_core.c @@ -31,6 +31,38 @@ struct xsm_operations *xsm_ops; +enum xsm_bootparam { +XSM_BOOTPARAM_DUMMY, +XSM_BOOTPARAM_FLASK, +}; + +static enum xsm_bootparam __initdata xsm_bootparam = +#ifdef CONFIG_XSM_FLASK_DEFAULT +XSM_BOOTPARAM_FLASK; +#else +XSM_BOOTPARAM_DUMMY; +#endif + +static int __init parse_xsm_param(const char *s) +{ +int rc = 0; + +if ( !strcmp(s, "dummy") ) +xsm_bootparam = XSM_BOOTPARAM_DUMMY; +#ifdef CONFIG_XSM_FLASK +else if ( !strcmp(s, "flask") ) +xsm_bootparam = XSM_BOOTPARAM_FLASK; +#endif +else +{ +printk("XSM: Unknown boot parameter xsm=%s\n", s); +rc = -EINVAL; +} + +return rc; +} +custom_param("xsm", parse_xsm_param); + static inline int verify(struct xsm_operations *ops) { /* verify the security_operations structure exists */ @@ -57,7 +89,20 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size) } xsm_ops = _xsm_ops; -flask_init(policy_buffer, policy_size); + +switch ( xsm_bootparam ) +{ +case XSM_BOOTPARAM_DUMMY: +break; + +case XSM_BOOTPARAM_FLASK: +flask_init(policy_buffer, policy_size); +break; + +default: +ASSERT_UNREACHABLE(); +break; +} return 0; } -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Question, How to share interrupt between Doms
On 08/10/2018 03:37, Peng Fan wrote: Hi Julien Hi Peng, -Original Message- From: Julien Grall [mailto:julien.gr...@arm.com] Sent: 2018年10月5日 1:27 To: Peng Fan ; Stefano Stabellini Cc: xen-devel@lists.xenproject.org; Andre Przywara Subject: Re: Question, How to share interrupt between Doms Hi Peng, On 04/10/2018 02:12, Peng Fan wrote: -Original Message- From: Julien Grall [mailto:julien.gr...@arm.com] Sent: 2018年10月3日 0:03 To: Peng Fan ; Stefano Stabellini Cc: xen-devel@lists.xenproject.org; Andre Przywara Subject: Re: Question, How to share interrupt between Doms On 02/10/2018 09:32, Peng Fan wrote: Hi Julien, Stefano, Hi Peng, Do you have any suggestions on how to share one interrupt between Doms? Sharing interrupts are usually a pain. You would need to forward the interrupts to all the domains using that interrupt and wait for them to EOI. This has security implications because you don't want DomA to prevent DomB receiving another interrupt because the previous one has not been EOIed correctly. The issue is that a gpio controller has 32 in/out port, however it only has one binded interrupt. The interrupt handler needs to check the status bits to check which port has interrupt coming. In my case, there are different devices using gpio interrupt that needs to be assigned to different doms. From what you wrote, it looks like you expect the GPIO controller to be shared with multiple domains. I don't think it is safe to do that. You need one domain (or Xen) to fully manage the controller. All the other domain will have to access either a virtual GPIO controller or PV one. In the former, interrupt would be virtual, while the latter the interrupt would be through even channel. So sharing interrupt should not be necessary. Did I miss anything? When interrupts comes, the dom0 will handle that. Then forward the interrupt to domu. But I did not find a good method to forward the interrupt and hook the interrupt in domu dts and domu driver. In Domu, driver needs use request irq and the dts needs interrupt=. But when dom0 notify remote, there is no hook in frontend driver and the other driver interrupt handler. You say that Dom0 will receive the interrupt. So Dom0 is access directly the GPIO controller. Right? Yes. What about the guests? Do they access directly the GPIO controller? Or did you introduce a PV protocol for this? Guest use PV to access GPIO in Dom0. When interrupt comes to dom0, the pv use event channel to forward the interrupt to Domu, I did not find a good way to do interrupt forwarding and let domu handle the interrupt as without Virtualization. Do you mean using a SPIs rather than an event channel to deliver the interrupt to the guest? I use generic_handle_irq() when frontent received the forwarded interrupt from dom0 in a eventchannel interrupt, but no work. Do you see any error? Would it be possible to paste logs? Another issue is in backend, request_threaded_irq for the gpio, some gpio will trigger interrupt before domu could handle the interrupt, this is because gpio default status or some on board devices pull up/down the gpio. I am not sure to understand the issue with that. It should not be different from the hardware case. Would it be possible to expand it? -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
On Mon, Oct 08, 2018 at 12:37:39PM +0200, Roger Pau Monné wrote: > On Mon, Oct 08, 2018 at 04:33:04AM -0600, Jan Beulich wrote: > > >>> On 08.10.18 at 12:14, wrote: > > > --- a/xen/drivers/passthrough/iommu.c > > > +++ b/xen/drivers/passthrough/iommu.c > > > @@ -505,7 +505,7 @@ int iommu_do_domctl( > > > > > > void iommu_share_p2m_tableiommu_share_p2m_table(struct domain* d) > > > { > > > -if ( iommu_use_hap_pt(d) ) > > > +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) > > > > Considering the only caller, the hap_enabled() part is (and was) not > > needed here. > > Would you like me to remove it in this patch? I've converted it to an ASSERT instead of just removing it. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Backports to stable
On Mon, Oct 08, 2018 at 03:29:06AM -0600, Jan Beulich wrote: > >>> On 07.10.18 at 03:04, wrote: > > I'd like to propose backporting GCC7/8 fixes to all stable branches. Below > > is a list up to stable-4.6, but some of the patches are already on > > select branches (developed during that release cycle, or already > > backported). > > I continue to be opposed to backporting anything that is not needed > for dealing security issues to branches which are in security-support- > only mode, i.e. anything older than 4.8 at this point in time. Ok, noted. It's hard to compile those still security-supported versions on recent systems. But if one need such combination (old Xen + new other things), then can also apply those patches locally. I just wanted to reduce work duplication. > Furthermore I notice that 4.6 has just moved out of security support, > at the end of last week. Hmm, 18+18 months from October 13, 2015 is at the end of this week. > > e0a97098e2 x86: fix section type mismatch in mm.c > > This describes itself as a Clang fix - has it become relevant for > gcc now too? Yes, for gcc 8.1 at least. > Anyway - this has been in even the original 4.7.0, > so as per above not a candidate for any actively maintained > branch. > > > # This one doesn't apply cleanly, because acpi stuff moved > > # tools/firmware/hvmloader/acpi -> tools/acpi > > 858dbaaeda libacpi: fixes for iasl >= 20180427 > > Iirc I've applied this back to 4.8 already, and quite some time ago. Yes, this one is missing only in 4.7 and 4.6. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
On Mon, Oct 08, 2018 at 04:33:04AM -0600, Jan Beulich wrote: > >>> On 08.10.18 at 12:14, wrote: > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -505,7 +505,7 @@ int iommu_do_domctl( > > > > void iommu_share_p2m_tableiommu_share_p2m_table(struct domain* d) > > { > > -if ( iommu_use_hap_pt(d) ) > > +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) > > Considering the only caller, the hap_enabled() part is (and was) not > needed here. Would you like me to remove it in this patch? I've merely changed iommu_share_p2m_table back to use the same logic as before 2916951c1. > I also would prefer to see a comment attached here, > to (briefly) explain why this can't be iommu_use_hap_pt(). > > Is this what Wei also has found to break his PVH Dom0? Yes, I think so. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
>>> On 08.10.18 at 12:27, wrote: >> -Original Message- >> From: Roger Pau Monne [mailto:roger@citrix.com] >> Sent: 08 October 2018 11:15 >> To: xen-devel@lists.xenproject.org >> Cc: Roger Pau Monne ; Jan Beulich >> ; Paul Durrant >> Subject: [PATCH] x86/vtd: fix iommu_share_p2m_table >> >> Commit 2916951c1 changed the check in iommu_share_p2m_table to use >> need_iommu(d) instead of iommu_enabled, which broke the check because >> at the point in domain construction where iommu_share_p2m_table is >> called need_iommu(d) will always return false. >> >> Fix this by reverting to the previous logic. > > Thanks for finding this. > > Above, I think you need to include the title of commit 2916951c1 and also > the text should say that the need_iommu() check became implicit in > iommu_hap_pt_share() (otherwise folks looking at the patch may be bemused > because need_iommu is not in the code). Indeed, it did take me more than just a moment to make the connection between description and code change (in this regard). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
>>> On 08.10.18 at 12:14, wrote: > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -505,7 +505,7 @@ int iommu_do_domctl( > > void iommu_share_p2m_table(struct domain* d) > { > -if ( iommu_use_hap_pt(d) ) > +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) Considering the only caller, the hap_enabled() part is (and was) not needed here. I also would prefer to see a comment attached here, to (briefly) explain why this can't be iommu_use_hap_pt(). Is this what Wei also has found to break his PVH Dom0? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen PV: Sample new PV driver for buffer sharing between domains
On 08/10/2018 10:12, Omkar Bolla wrote: Hi, Hi, This is also okay, but problem here is I am using 4.8 stable xen because it is working on Hkey960(ArmV8) This is because you can't bring up secondary CPUs on the Hikey with Xen 4.11 [1], right? It would be nice to find where the bug was introduced because Xen 4.8 is out of support and does not contain the latest fixes (such as Meltdown/Spectre). Is there any other way to share buffer dynamically? You would have to write your own PV drivers or port the series to Xen 4.8. Cheers, [1] https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg21576.html -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
> -Original Message- > From: Roger Pau Monne [mailto:roger@citrix.com] > Sent: 08 October 2018 11:15 > To: xen-devel@lists.xenproject.org > Cc: Roger Pau Monne ; Jan Beulich > ; Paul Durrant > Subject: [PATCH] x86/vtd: fix iommu_share_p2m_table > > Commit 2916951c1 changed the check in iommu_share_p2m_table to use > need_iommu(d) instead of iommu_enabled, which broke the check because > at the point in domain construction where iommu_share_p2m_table is > called need_iommu(d) will always return false. > > Fix this by reverting to the previous logic. Thanks for finding this. Above, I think you need to include the title of commit 2916951c1 and also the text should say that the need_iommu() check became implicit in iommu_hap_pt_share() (otherwise folks looking at the patch may be bemused because need_iommu is not in the code). The code looks fine though so... Reviewed-by: Paul Durrant > > Signed-off-by: Roger Pau Monné > --- > Cc: Jan Beulich > Cc: Paul Durrant > --- > xen/drivers/passthrough/iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/drivers/passthrough/iommu.c > b/xen/drivers/passthrough/iommu.c > index debb5e6fe1..a9408f1de9 100644 > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -505,7 +505,7 @@ int iommu_do_domctl( > > void iommu_share_p2m_table(struct domain* d) > { > -if ( iommu_use_hap_pt(d) ) > +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) > iommu_get_ops()->share_p2m(d); > } > > -- > 2.19.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/vtd: fix iommu_share_p2m_table
Commit 2916951c1 changed the check in iommu_share_p2m_table to use need_iommu(d) instead of iommu_enabled, which broke the check because at the point in domain construction where iommu_share_p2m_table is called need_iommu(d) will always return false. Fix this by reverting to the previous logic. Signed-off-by: Roger Pau Monné --- Cc: Jan Beulich Cc: Paul Durrant --- xen/drivers/passthrough/iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index debb5e6fe1..a9408f1de9 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -505,7 +505,7 @@ int iommu_do_domctl( void iommu_share_p2m_table(struct domain* d) { -if ( iommu_use_hap_pt(d) ) +if ( iommu_enabled && hap_enabled(d) && iommu_hap_pt_share ) iommu_get_ops()->share_p2m(d); } -- 2.19.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/svm: Fix svm_update_guest_efer() for domains using shadow paging
>>> On 05.10.18 at 19:02, wrote: > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -649,13 +649,32 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int > cr, unsigned int flags) > static void svm_update_guest_efer(struct vcpu *v) > { > struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb; > -bool lma = v->arch.hvm.guest_efer & EFER_LMA; > -uint64_t new_efer; > +unsigned long guest_efer = v->arch.hvm.guest_efer, > +xen_efer = read_efer(); > > -new_efer = (v->arch.hvm.guest_efer | EFER_SVME) & ~EFER_LME; > -if ( lma ) > -new_efer |= EFER_LME; > -vmcb_set_efer(vmcb, new_efer); > +if ( paging_mode_shadow(v->domain) ) > +{ > +/* EFER.NX is a Xen-owned bit and is not under guest control. */ > +guest_efer &= ~EFER_NX; > +guest_efer |= xen_efer & EFER_NX; > + > +/* > + * CR0.PG is a Xen-owned bit, and remains set even when the guest has > + * logically disabled paging. > + * > + * LMA was calculated using the guest CR0.PG setting, but LME needs > + * clearing to avoid interacting with Xen's CR0.PG setting. As > writes > + * to CR0 are intercepted, it is safe to leave LME clear at this > + * point, and fix up both LME and LMA when CR0.PG is set. > + */ > +if ( !(guest_efer & EFER_LMA) ) > +guest_efer &= ~EFER_LME; > +} I think this wants an "else", either ASSERT()ing that what the removed code did is actually the case (arranged for by the callers), or retaining the original logic in some form. This looks particularly relevant when hvm_efer_valid() was called with -1 as its cr0_pg argument, as in that case there was not necessarily any correlation enforced between EFER.LMA and CR0.PG. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-sid test] 75372: trouble: blocked/broken
flight 75372 distros-debian-sid real [real] http://osstest.xensource.com/osstest/logs/75372/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-pvopsbroken build-i386 broken build-amd64-pvopsbroken build-armhf broken build-amd64 broken build-i386-pvops broken Tests which did not succeed, but are not blocking: test-amd64-amd64-i386-sid-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-i386-i386-sid-netboot-pvgrub 1 build-check(1) blocked n/a test-armhf-armhf-armhf-sid-netboot-pygrub 1 build-check(1)blocked n/a test-amd64-i386-amd64-sid-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-sid-netboot-pvgrub 1 build-check(1)blocked n/a build-armhf 4 host-install(4) broken like 75332 build-armhf-pvops 4 host-install(4) broken like 75332 build-i386-pvops 4 host-install(4) broken like 75332 build-amd64 4 host-install(4) broken like 75332 build-amd64-pvops 4 host-install(4) broken like 75332 build-i3864 host-install(4) broken like 75332 baseline version: flight 75332 jobs: build-amd64 broken build-armhf broken build-i386 broken build-amd64-pvopsbroken build-armhf-pvopsbroken build-i386-pvops broken test-amd64-amd64-amd64-sid-netboot-pvgrubblocked test-amd64-i386-i386-sid-netboot-pvgrub blocked test-amd64-i386-amd64-sid-netboot-pygrub blocked test-armhf-armhf-armhf-sid-netboot-pygrubblocked test-amd64-amd64-i386-sid-netboot-pygrub blocked sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/ocaml: Release the global lock before invoking block syscalls
On 08/10/18 08:58, Christian Lindig wrote: > >> On 8 Oct 2018, at 04:10, Yang Qian wrote: >> >> Functions related with event channel are parallelizable, so release global >> lock before invoking C function which will finally call block syscalls. >> >> Signed-off-by: Yang Qian >> --- >> tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 30 >> +-- >> 1 file changed, 28 insertions(+), 2 deletions(-) > Acked-by: Christian Lindig > > From an OCaml point of view this is looking good. But I would like to hear > from developers understanding event channels that this is indeed safe to do. They all end up as ioctl()'s into the kernel, and from there some become hypercalls into Xen. Either way, this patch is fine. Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 17/18] xenmon: Install as xenmon, not xenmon.py
On Fri, Oct 05, 2018 at 06:29:16PM +0100, Ian Jackson wrote: > Adding the implementation language as a suffix to a program name is > poor practice. > > Signed-off-by: Ian Jackson Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 20/23] xen: support console_switching between Dom0 and DomUs on ARM
>>> On 05.10.18 at 20:47, wrote: > @@ -391,31 +394,73 @@ static void dump_console_ring_key(unsigned char key) > free_xenheap_pages(buf, order); > } > > -/* CTRL- switches input direction between Xen and DOM0. */ > +/* > + * CTRL- switches input direction between Xen, Dom0 and > + * DomUs. > + */ Just like the title, this comment makes it sound as if any DomU could participate in this switching. > #define switch_code (opt_conswitch[0]-'a'+1) > -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. */ > +/* > + * console_rx=0 => input to xen > + * console_rx=1 => input to dom0 > + * console_rx=N => input dom(N-1) > + */ > +static unsigned int __read_mostly console_rx = 0; > > static void switch_serial_input(void) > { > -static char *input_str[2] = { "DOM0", "Xen" }; > -xen_rx = !xen_rx; > -printk("*** Serial input -> %s", input_str[xen_rx]); > +if ( console_rx++ == max_init_domid + 1 ) > +console_rx = 0; > + > +if ( !console_rx ) Please be consistent ... > +printk("*** Serial input to Xen"); > +else > +printk("*** Serial input to DOM%d", console_rx - 1); > + > if ( switch_code ) > -printk(" (type 'CTRL-%c' three times to switch input to %s)", > - opt_conswitch[0], input_str[!xen_rx]); > +printk(" (type 'CTRL-%c' three times to switch input)", > + opt_conswitch[0]); > printk("\n"); > } > > static void __serial_rx(char c, struct cpu_user_regs *regs) > { > -if ( xen_rx ) > +if ( console_rx == 0 ) ... in style. But perhaps ... > return handle_keypress(c, regs); > > -/* Deliver input to guest buffer, unless it is already full. */ > -if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE ) > -serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c; > -/* Always notify the guest: prevents receive path from getting stuck. */ > -send_global_virq(VIRQ_CONSOLE); > +if ( console_rx == 1 ) ... switch() would be better to use here anyway. > +{ > +/* Deliver input to hardware domain, unless it is already full. */ Looks like you've mis-edited the original comment. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 13/18] tools/xenstat: Fix shared library version
On Fri, Oct 05, 2018 at 06:29:12PM +0100, Ian Jackson wrote: > From: Bastian Blank > > libxenstat does not have a stable ABI. Set its version to the current > Xen release version. > > Signed-off-by: Ian Jackson Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel