Re: [Xen-devel] [PATCH] x86/svm: Fix svm_update_guest_efer() for domains using shadow paging

2018-10-08 Thread Tian, Kevin
> 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

2018-10-08 Thread Tian, Kevin
> 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

2018-10-08 Thread Tian, Kevin
> 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

2018-10-08 Thread Doug Goldstein
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

2018-10-08 Thread osstest service owner
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

2018-10-08 Thread Xin Li
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread osstest service owner
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

2018-10-08 Thread Stefano Stabellini
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

2018-10-08 Thread osstest service owner
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

2018-10-08 Thread osstest service owner
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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)

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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)

2018-10-08 Thread Julien Grall
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

2018-10-08 Thread Julien Grall
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)

2018-10-08 Thread Julien Grall

(+ 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

2018-10-08 Thread osstest service owner
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()

2018-10-08 Thread Andrew Cooper
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

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Anthony PERARD
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

2018-10-08 Thread Jan Beulich
>>> 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()

2018-10-08 Thread George Dunlap
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

2018-10-08 Thread Jan Beulich
>>> 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()

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Paul Durrant
> -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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread osstest service owner
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()

2018-10-08 Thread Andrew Cooper
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

2018-10-08 Thread Paul Durrant
> -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

2018-10-08 Thread Boris Ostrovsky
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Ian Jackson
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'

2018-10-08 Thread Juergen Gross
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

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Jan Beulich
>>> 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'

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Ian Jackson
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'

2018-10-08 Thread Ian Jackson
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

2018-10-08 Thread Paul Durrant
> -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

2018-10-08 Thread Roger Pau Monne
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()

2018-10-08 Thread Jan Beulich
>>> 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()

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Dag Nygren
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Paul Durrant
> -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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Wei Liu
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread osstest service owner
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

2018-10-08 Thread Jan Beulich
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Andrew Cooper
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

2018-10-08 Thread Xin Li
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

2018-10-08 Thread Xin Li
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

2018-10-08 Thread Xin Li
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

2018-10-08 Thread Julien Grall

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

2018-10-08 Thread Roger Pau Monné
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

2018-10-08 Thread Marek Marczykowski
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

2018-10-08 Thread Roger Pau Monné
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Julien Grall



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

2018-10-08 Thread Paul Durrant
> -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

2018-10-08 Thread Roger Pau Monne
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Platform Team regression test user
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

2018-10-08 Thread Andrew Cooper
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

2018-10-08 Thread Wei Liu
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

2018-10-08 Thread Jan Beulich
>>> 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

2018-10-08 Thread Wei Liu
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

  1   2   >