Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
Hello Stefano, On 05.02.19 11:17, Andrii Anisov wrote: Thank you. I already referred this thread in the email to the vendor as the first step. But have no answer yet :( We've got an answer from the vendor. They agree with arguments and have provided a patch to eliminate Set/Way usage. We've checked and it does what is needed. So we do not need this WA any more. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 01/17] argo: Introduce the Kconfig option to govern inclusion of Argo
Defines CONFIG_ARGO when enabled. Default: disabled. When the Kconfig option is enabled, the Argo hypercall implementation will be included, allowing use of the hypervisor-mediated interdomain communication mechanism. Argo is implemented for x86 and ARM hardware platforms. Availability of the option depends on EXPERT and Argo is currently an experimental feature. Signed-off-by: Christopher Clark Acked-by: Jan Beulich --- v3 added Jan's Ack v2 #01 feedback, Jan: replace def_bool/prompt with bool v1 #02 feedback, Jan: default Kconfig off, use EXPERT, fix whitespace xen/common/Kconfig | 19 +++ 1 file changed, 19 insertions(+) diff --git a/xen/common/Kconfig b/xen/common/Kconfig index a79cd40..0438462 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -202,6 +202,25 @@ config LATE_HWDOM If unsure, say N. +config ARGO + bool "Argo: hypervisor-mediated interdomain communication" if EXPERT = "y" + ---help--- + Enables a hypercall for domains to ask the hypervisor to perform + data transfer of messages between domains. + + This allows communication channels to be established that do not + require any shared memory between domains; the hypervisor is the + entity that each domain interacts with. The hypervisor is able to + enforce Mandatory Access Control policy over the communication. + + If XSM_FLASK is enabled, XSM policy can govern which domains may + communicate via the Argo system. + + This feature does nothing if the "argo" boot parameter is not present. + Argo is disabled at runtime by default. + + If unsure, say N. + menu "Schedulers" visible if EXPERT = "y" -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 04/17] argo: init, destroy and soft-reset, with enable command line opt
Initialises basic data structures and performs teardown of argo state
for domain shutdown.
Inclusion of the Argo implementation is dependent on CONFIG_ARGO.
Introduces a new Xen command line parameter 'argo': bool to enable/disable
the argo hypercall. Defaults to disabled.
New headers:
public/argo.h: with definions of addresses and ring structure, including
indexes for atomic update for communication between domain and hypervisor.
xen/argo.h: to expose the hooks for integration into domain lifecycle:
argo_init: per-domain init of argo data structures for domain_create.
argo_destroy: teardown for domain_destroy and the error exit
path of domain_create.
argo_soft_reset: reset of domain state for domain_soft_reset.
Adds a new field to struct domain: struct argo_domain *argo;
In accordance with recent work on _domain_destroy, argo_destroy is
idempotent. It will tear down: all rings registered by this domain, all
rings where this domain is the single sender (ie. specified partner,
non-wildcard rings), and all pending notifications where this domain is
awaiting signal about available space in the rings of other domains.
A count will be maintained of the number of rings that a domain has
registered in order to limit it below the fixed maximum limit defined here.
Macros are defined to verify the internal locking state within the argo
implementation. The macros are ASSERTed on entry to functions to validate
and document the required lock state prior to calling.
The hash function for the hashtables that hold ring state is derived from
the string hashing function djb2 (http://www.cse.yorku.ca/~oz/hash.html)
by Daniel J. Bernstein. Basic testing with a limited number of domains and
ports has shown reasonable distribution for the table size.
The software license on the public header is the BSD license, standard
procedure for the public Xen headers. The public header was originally
posted under a GPL license at: [1]:
https://lists.xenproject.org/archives/html/xen-devel/2013-05/msg02710.html
The following ACK by Lars Kurth is to confirm that only people being
employees of Citrix contributed to the header files in the series posted at
[1] and that thus the copyright of the files in question is fully owned by
Citrix. The ACK also confirms that Citrix is happy for the header files to
be published under a BSD license in this series (which is based on [1]).
Signed-off-by: Christopher Clark
Acked-by: Lars Kurth
Reviewed-by: Ross Philipson
Tested-by: Chris Patterson
Reviewed-by: Roger Pau Monné
---
v7 #02 Julien/Jan/Stefano: apply range check to numeric hypercall args
v7 #10 Jan: avoid code duplication: forward compat ops to native
v7 #04 Roger: add Reviewed-by
v7 #04 Roger: list_first_entry_or_null in partner_rings_remove
v7 #04 Roger: list_first_entry_or_null in domain_rings_remove_all
v7 #04 Roger: list_first_entry_or_null in wildcard_rings_pending_remove
v7 #04 Roger: list_first_entry_or_null in pending_remove_all
v7 #04 Roger: add space to compat switch ( cmd )
v7 #04 Roger: add space to switch ( cmd )
v7 #07 Roger: remove stray newline in compat_argo_op
v6 #09 Jan: introduce compat ABI
v6 #04 Jan: xlat.lst: move argo struct entries to alphabetical position
v6 #04 Roger: use list_for_each_entry{_safe} for looping
v5 #04 Roger: tweak command line doc: remove statement about top level bool
v5: add compat validation macros to primary source file: common/argo.c
v5: dropped external file for compat macros: common/compat/argo.c
v4: removed FIXME for removing argo_destroy from domain_kill
v4 Jan: amend the command line doc text referring to build configuration
v4 : use standard data structures as per common code
v4 Jan: replace hash_index with djb2-derived hash algorithm
v4 Andrew: switch argo command line option to list argo=
v4 #04 Roger: drop unneeded init of ring_count in argo_domain_init
v4 #04 Roger: replace if (ring_info->mfns) with ASSERTs in ring_unmap
v4 #04 Roger: rewrite the locking verification macros
v4 #04 Roger: make L1 lock description comment clearer about R(L1) and W(L1)
v4 Andrew: fix split of dprintk in ring_map_info across v4 commits
v3 #04 Andrew: use xzalloc for struct argo_domain in argo_init
v3 #04 Andrew: reference CONFIG_ARGO in the command line documentation
v3 #07 Jan: rename ring_find_info to find_ring_info
v3 #04 Andrew: don't truncate args do_argo_op printk
v3 #07 Jan: fix numeric entries in printk format strings
v3 #10 Roger: move find functions to top of file and drop prototypes
v3 #04 Jan: meld compat check for hypercall arg types
v3 #04 Roger/Jan: make lock names clearer and assert their state
v3 #04 Jan: port -> aport with type; distinguish argo port from evtchn
v3 #04 Jan: reorder call to argo_init_domain in argo_init
v3 #04 Jan: ring_remove_mfns: zero count before freeing arrays
v3 #04 Jason/Roger: soft_reset: can assume reinit is ok if d->argo set
v3 #04 Roger: remove unused and confusing d->argo_lock
v3 #04 Roger: add simple inlines in
[Xen-devel] [PATCH v8 for-4.12 07/17] argo: implement the register op
The register op is used by a domain to register a region of memory for receiving messages from either a specified other domain, or, if specifying a wildcard, any domain. This operation creates a mapping within Xen's private address space that will remain resident for the lifetime of the ring. In subsequent commits, the hypervisor will use this mapping to copy data from a sending domain into this registered ring, making it accessible to the domain that registered the ring to receive data. Wildcard any-sender rings are default disabled and registration will be refused with EPERM unless they have been specifically enabled with the new mac-permissive flag that is added to the argo boot option here. The reason why the default for wildcard rings is 'deny' is that there is currently no means to protect the ring from DoS by a noisy domain spamming the ring, affecting other domains ability to send to it. This will be addressed with XSM policy controls in subsequent work. Since denying access to any-sender rings is a significant functional constraint, the new option "mac-permissive" for the argo bootparam enables overriding this. eg: "argo=1,mac-permissive=1" The p2m type of the memory supplied by the guest for the ring must be p2m_ram_rw and the memory will be pinned as PGT_writable_page while the ring is registered. This hypercall op and its interface currently only supports 4K-sized pages. Signed-off-by: Christopher Clark Tested-by: Chris Patterson Reviewed-by: Roger Pau Monné --- v7 #09 Jan: avoid code duplication, forward compat ops to native v7 #09 Jan: comments in do/compat_argo_op to highlight arg difference v7 #09 self: use the 64-bit fixed width gfn type v7 #07 Roger: add Reviewed-by v7 #07 Roger: add missing space character to if condition v7 #07 Roger: move #endif and drop second CONFIG_COMPAT ifdef v6 #09 Jan: add compat ABI v6 #07 Jan: add argo_message_header to xlat.lst and invoke the CHECK v6 #07 Jan: xlat.lst: move argo struct entries to alphabetical position v5 #07 Roger: add BUILD_BUG_ON for MAX_RING_SIZE, PAGE_SIZE v5 #07 Roger: gprintk(XENLOG_ERR,.. for denied existing ring v5: add compat validation macros to primary source file: common/argo.c v5 : convert hypercall arg structs to struct form for compat checking v5: dropped external file for compat macros: common/compat/argo.c v4 v3#07 Jan: shrink critical sections in register_ring v4 v3#07 Jan: revise register flag MASK in header, note 32-bitness of args v4 feedback: use standard data structures per common code, not loop macros v4 Andrew: use the single argo command line option list v4 #07 Jan: rewrite find_ring_mfn to use check_get_page_from_gfn v4 #07 Roger: add FIXME to ring_map_page for vmap contiguous ring mapping v3 #07 Jan: comment: minimum ring size is based on minimum-sized message v3 #04 Andrew: reference CONFIG_ARGO in the command line documentation v3 #07 Jan: register_ring: fold else, if into else-if to drop indent v3 #07 Jan: remove no longer used guest_handle_is_aligned macros v3 #07 Jan: remove dead code from find_ring_mfns v3 #07 Jan: fix format string indention in printks v3 #07 Jan: remove redundant bounds check on npage in find_ring_mfns v3 #08 self/Roger: improve dprintk output in find_ring_info like find_send_info v3 #07 Jan: rename ring_find_info to find_ring_info v3 #07 Jan: use array_index_nospec in ring_map_page v3 #07 Jan: fix numeric entries in printk format strings v3 #7 Jan: drop unneeded parentheses from ROUNDUP_MESSAGE defn v3 #10 Roger: move find functions to top of file and drop prototypes v3 #03 meld compat check for hypercall arg register struct v3 #04 Roger/Jan: make lock names clearer and assert their state v3 #04 Jan: port -> aport with type; distinguish argo port from evtchn v3 feedback #07 Eric: fix header max ring size comment units v3 feedback #04 Roger: mfn_mapping: void* instead of uint8_t* v3 use %u for printing unsigned ints in find_ring_mfns v3 feedback #04 Jan: uint32_t -> unsigned int for npage in register_ring v3 feedback #04 Roger: drop npages struct member, calculate from len v3 : register_ring: uint32_t -> unsigned int for private_tx_ptr v3 feedback Roger/Jan: ASSERT currd is current->domain or use 'd' variable name v3 feedback #07 Roger: use opt_argo_mac_permissive : a boolean opt v3 feedback #04 Roger: reorder #includes to alphabetical order v3 feedback #07 Roger: drop comment re: Intel EPT/AMD NPT for write-only mapping v3 feedback #07 Roger: drop ptr arithmetic in update_tx_ptr, use ring struct cast v3 feedback #07 Roger: drop newline in ring_map_page v3 feedback #07 Roger: drop unneeded null check before xfree v3 feedback #07 Roger: use return and drop out label in register_ring v3 Stefano: add 4K page constraint to header file comment & commit msg v3 Julien/Stefano: 4K granularity ok: use 64-bit gfns in register interface v2 self: disallow ring resize via reregister v2 feedback Jan: drop cookie, implement teardown v2 feedback Jan: drop message from argo_message_op v2 self: mo
[Xen-devel] [PATCH v8 for-4.12 03/17] argo: define argo_dprintk for subsystem debugging
A convenience for working on development of the argo subsystem:
setting a #define variable enables additional debug messages.
Signed-off-by: Christopher Clark
Acked-by: Jan Beulich
Reviewed-by: Roger Pau Monné
---
v3 added Roger's Reviewed-by
v3 added Jan's Ack
v2 #03 feedback, Jan: fix ifdef/define confusion error
v1 #04 feedback, Jan: fix dprintk implementation
xen/common/argo.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/xen/common/argo.c b/xen/common/argo.c
index dd2390d..ada1aaf 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -19,6 +19,15 @@
#include
#include
+/* Change this to #define ARGO_DEBUG here to enable more debug messages */
+#undef ARGO_DEBUG
+
+#ifdef ARGO_DEBUG
+#define argo_dprintk(format, args...) printk("argo: " format, ## args )
+#else
+#define argo_dprintk(format, ... ) ((void)0)
+#endif
+
long
do_argo_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
XEN_GUEST_HANDLE_PARAM(void) arg2, unsigned long raw_arg3,
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 05/17] errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI
EMSGSIZE: Argo's sendv operation will return EMSGSIZE when an excess amount of data, across all iovs, has been supplied, exceeding either the statically configured maximum size of a transmittable message, or the (variable) size of the ring registered by the destination domain. ECONNREFUSED: Argo's register operation will return ECONNREFUSED if a ring is being registered to communicate with a specific remote domain that does exist but is not argo-enabled. These codes are described by POSIX here: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html EMSGSIZE : "Message too large" ECONNREFUSED : "Connection refused". The numeric values assigned to each are taken from Linux, as is the case for the existing error codes. EMSGSIZE : 90 ECONNREFUSED : 111 Signed-off-by: Christopher Clark Acked-by: Jan Beulich --- xen/include/public/errno.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h index 305c112..e1d02fc 100644 --- a/xen/include/public/errno.h +++ b/xen/include/public/errno.h @@ -102,6 +102,7 @@ XEN_ERRNO(EILSEQ, 84) /* Illegal byte sequence */ XEN_ERRNO(ERESTART,85) /* Interrupted system call should be restarted */ #endif XEN_ERRNO(ENOTSOCK,88) /* Socket operation on non-socket */ +XEN_ERRNO(EMSGSIZE,90) /* Message too large. */ XEN_ERRNO(EOPNOTSUPP, 95) /* Operation not supported on transport endpoint */ XEN_ERRNO(EADDRINUSE, 98) /* Address already in use */ XEN_ERRNO(EADDRNOTAVAIL, 99) /* Cannot assign requested address */ @@ -109,6 +110,7 @@ XEN_ERRNO(ENOBUFS, 105)/* No buffer space available */ XEN_ERRNO(EISCONN, 106)/* Transport endpoint is already connected */ XEN_ERRNO(ENOTCONN,107)/* Transport endpoint is not connected */ XEN_ERRNO(ETIMEDOUT, 110)/* Connection timed out */ +XEN_ERRNO(ECONNREFUSED,111)/* Connection refused */ #undef XEN_ERRNO #endif /* XEN_ERRNO */ -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 02/17] argo: introduce the argo_op hypercall boilerplate
Presence is gated upon CONFIG_ARGO.
Registers the hypercall previously reserved for this.
Takes 5 arguments, does nothing and returns -ENOSYS.
Implementation will provide a compat ABI so COMPAT_CALL is the selected
macro for the hypercall tables.
Signed-off-by: Christopher Clark
Acked-by: Jan Beulich
---
v7 adjust arg3, arg4 names in do_argo_op for range validation in later patch
v7 added Jan Acked-by
v6 dropped Jan Acked-by due to change of implementation and commit msg
v6 switched to COMPAT_CALL and provides compat_argo_op
v6 feedback #3 Julien: add argo_op to the ARM hypercall table
v2 Copyright line: add 2019
v2 feedback #3 Jan: drop "message" from argo_message_op
v2 feedback #3 Jan: add Acked-by
v1 feedback #15 Jan: handle upper-halves of hypercall args
v1 feedback #15 Jan: use unsigned where negative values impossible
xen/arch/arm/traps.c| 3 +++
xen/arch/x86/guest/hypercall_page.S | 2 +-
xen/arch/x86/hvm/hypercall.c| 3 +++
xen/arch/x86/hypercall.c| 3 +++
xen/arch/x86/pv/hypercall.c | 3 +++
xen/common/Makefile | 1 +
xen/common/argo.c | 38 +
xen/include/public/xen.h| 2 +-
xen/include/xen/hypercall.h | 18 ++
9 files changed, 71 insertions(+), 2 deletions(-)
create mode 100644 xen/common/argo.c
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 221c762..e1e8ac9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1397,6 +1397,9 @@ static arm_hypercall_t arm_hypercall_table[] = {
HYPERCALL(platform_op, 1),
HYPERCALL_ARM(vcpu_op, 3),
HYPERCALL(vm_assist, 2),
+#ifdef CONFIG_ARGO
+HYPERCALL(argo_op, 5),
+#endif
};
#ifndef NDEBUG
diff --git a/xen/arch/x86/guest/hypercall_page.S
b/xen/arch/x86/guest/hypercall_page.S
index fdd2e72..26afabf 100644
--- a/xen/arch/x86/guest/hypercall_page.S
+++ b/xen/arch/x86/guest/hypercall_page.S
@@ -59,7 +59,7 @@ DECLARE_HYPERCALL(sysctl)
DECLARE_HYPERCALL(domctl)
DECLARE_HYPERCALL(kexec_op)
DECLARE_HYPERCALL(tmem_op)
-DECLARE_HYPERCALL(xc_reserved_op)
+DECLARE_HYPERCALL(argo_op)
DECLARE_HYPERCALL(xenpmu_op)
DECLARE_HYPERCALL(arch_0)
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 19d1263..5bb1750 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -134,6 +134,9 @@ static const hypercall_table_t hvm_hypercall_table[] = {
#ifdef CONFIG_TMEM
HYPERCALL(tmem_op),
#endif
+#ifdef CONFIG_ARGO
+COMPAT_CALL(argo_op),
+#endif
COMPAT_CALL(platform_op),
#ifdef CONFIG_PV
COMPAT_CALL(mmuext_op),
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 032de8f..93e7860 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -64,6 +64,9 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
ARGS(domctl, 1),
ARGS(kexec_op, 2),
ARGS(tmem_op, 1),
+#ifdef CONFIG_ARGO
+ARGS(argo_op, 5),
+#endif
ARGS(xenpmu_op, 2),
#ifdef CONFIG_HVM
ARGS(hvm_op, 2),
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c
index 5d11911..f452dd5 100644
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -77,6 +77,9 @@ const hypercall_table_t pv_hypercall_table[] = {
#ifdef CONFIG_TMEM
HYPERCALL(tmem_op),
#endif
+#ifdef CONFIG_ARGO
+COMPAT_CALL(argo_op),
+#endif
HYPERCALL(xenpmu_op),
#ifdef CONFIG_HVM
HYPERCALL(hvm_op),
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 56fc201..59ac7de 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_ARGO) += argo.o
obj-y += bitmap.o
obj-y += bsearch.o
obj-$(CONFIG_CORE_PARKING) += core_parking.o
diff --git a/xen/common/argo.c b/xen/common/argo.c
new file mode 100644
index 000..dd2390d
--- /dev/null
+++ b/xen/common/argo.c
@@ -0,0 +1,38 @@
+/**
+ * Argo : Hypervisor-Mediated data eXchange
+ *
+ * Derived from v4v, the version 2 of v2v.
+ *
+ * Copyright (c) 2010, Citrix Systems
+ * Copyright (c) 2018-2019 BAE Systems
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include
+#include
+
+long
+do_argo_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
+ XEN_GUEST_HANDLE_PARAM(void) arg2, unsigned long raw_arg3,
+ unsigned long raw_arg4)
+{
+return -ENOSYS;
+}
+
+#ifdef CONFIG_COMPAT
+long
+compat_argo_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
+ XEN_
[Xen-devel] [PATCH v8 for-4.12 00/17] Argo: hypervisor-mediated interdomain communication
Version eight of this series: Note: This version may not address the currently open discussion on the ARM hypercall argument convention and type selection for hypercall parameters. * Range check applied to numeric args in native hypercall entry (ref: the above open discussion) * Revises the compat ABI and implementation - avoids duplication of hypercall op implementations via forwarding to native for ops other than sendv - register op uses an always-64-bit fixed width pfn type for consistent ABI as well as compat reuse of the native op - tested communication between VMs on x86-64 host with: 32-bit PV, 32-bit HVM and 64-bit PV guests * Applies list_first_entry_or_null macro in multiple loops to replace previous use of a list foreach to address review feedback * Removed stale comments from the public header New to this series: * Adds an initial version of a design document for Argo - based on work previously sent to the mailing list, covers the implementation's granular locking * Adds a SUPPORT.md section for the feature and Experimental statement Christopher Clark (17): argo: Introduce the Kconfig option to govern inclusion of Argo argo: introduce the argo_op hypercall boilerplate argo: define argo_dprintk for subsystem debugging argo: init, destroy and soft-reset, with enable command line opt errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI xen/arm: introduce guest_handle_for_field() argo: implement the register op argo: implement the unregister op argo: implement the sendv op; evtchn: expose send_guest_global_virq argo: implement the notify op xsm, argo: XSM control for argo register xsm, argo: XSM control for argo message send operation xsm, argo: XSM control for any access to argo by a domain xsm, argo: notify: don't describe rings that cannot be sent to MAINTAINERS: add new section for Argo and self as maintainer SUPPORT.md : add new entry for the Argo feature docs, argo: add design document for Argo MAINTAINERS |7 + SUPPORT.md |4 + docs/designs/argo.pandoc | 448 + docs/misc/xen-command-line.pandoc| 20 + tools/flask/policy/modules/guest_features.te |7 + xen/arch/arm/traps.c |3 + xen/arch/x86/guest/hypercall_page.S |2 +- xen/arch/x86/hvm/hypercall.c |3 + xen/arch/x86/hypercall.c |3 + xen/arch/x86/pv/hypercall.c |3 + xen/common/Kconfig | 19 + xen/common/Makefile |1 + xen/common/argo.c| 2364 ++ xen/common/domain.c |9 + xen/common/event_channel.c |2 +- xen/include/Makefile |1 + xen/include/asm-arm/guest_access.h |3 + xen/include/public/argo.h| 267 +++ xen/include/public/errno.h |2 + xen/include/public/xen.h |4 +- xen/include/xen/argo.h | 44 + xen/include/xen/event.h |7 + xen/include/xen/hypercall.h | 18 + xen/include/xen/sched.h |5 + xen/include/xlat.lst |9 + xen/include/xsm/dummy.h | 25 + xen/include/xsm/xsm.h| 31 + xen/xsm/dummy.c |6 + xen/xsm/flask/hooks.c| 41 +- xen/xsm/flask/include/avc.h |4 +- xen/xsm/flask/policy/access_vectors | 16 + xen/xsm/flask/policy/security_classes|1 + 32 files changed, 3370 insertions(+), 9 deletions(-) create mode 100644 docs/designs/argo.pandoc create mode 100644 xen/common/argo.c create mode 100644 xen/include/public/argo.h create mode 100644 xen/include/xen/argo.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 08/17] argo: implement the unregister op
Takes a single argument: a handle to the ring unregistration struct,
which specifies the port and partner domain id or wildcard.
The ring's entry is removed from the hashtable of registered rings;
any entries for pending notifications are removed; and the ring is
unmapped from Xen's address space.
If the ring had been registered to communicate with a single specified
domain (ie. a non-wildcard ring) then the partner domain state is removed
from the partner domain's argo send_info hash table.
Signed-off-by: Christopher Clark
Reviewed-by: Roger Pau Monné
Tested-by: Chris Patterson
---
v7 #09 Jan: avoid code duplication, forward compat ops to native
v6 #09 Jan: add compat ABI
v6 Chris: add Tested-by
v6 Roger: add Reviewed-by
v6 #04 Jan: xlat.lst: move argo struct entries to alphabetical position
v6 #04 Roger: use list_for_each_entry for looping
v5: add compat validation macros to primary source file: common/argo.c
v5: dropped external file for compat macros: common/compat/argo.c
v4 # Jan: shrink the critical sections in unregister
v4 : use standard data structures as per common code
v4 #08 Roger: skip send_info lookup for wildcard rings
v4: add ASSERT_UNREACHABLE for missing sender domain or send_info
v4: reduce indentation by using goto
v4: add unlikely to currd->argo check
v4 #08 Jan: move put_domain outside L2 critical section
v4: include ring data in debug output when ring not found
v3 #08 Jan: pull xfree out of exclusive critical sections in unregister_ring
v3 #08 Jan: rename send_find_info to find_send_info
v3 #07 Jan: rename ring_find_info to find_ring_info
v3 #08 Roger: use return and remove the out label in unregister_ring
v3 #08 Roger: better debug output in send_find_info
v3 #10 Roger: move find functions to top of file and drop prototypes
v3 #04 Jan: meld compat check for unregister_ring struct
v3 #04 Roger/Jan: make lock names clearer and assert their state
v3 #04 Jan: port -> aport with type; distinguish argo port from evtchn
v3 feedback Roger/Jan: ASSERT currd is current->domain or use 'd' variable name
v3 feedback #07 Roger: const the argo_ring_id structs in send_find_info
v2 feedback Jan: drop cookie, implement teardown
v2 feedback Jan: drop message from argo_message_op
v2 self: OVERHAUL
v2 self: reorder logic to shorten critical section
v1 #13 feedback Jan: revise use of guest_handle_okay vs __copy ops
v1 feedback Roger, Jan: drop argo prefix on static functions
v1,2 feedback Jan/Roger/Paul: drop errno returning guest access functions
v1 #5 (#14) feedback Paul: use currd in do_argo_message_op
v1 #5 (#14) feedback Paul: full use currd in argo_unregister_ring
v1 #13 (#14) feedback Paul: replace do/while with goto; reindent
v1 self: add blank lines in unregister case in do_argo_message_op
v1: #13 feedback Jan: public namespace: prefix with xen
v1: #13 feedback Jan: blank line after op case in do_argo_message_op
v1: #14 feedback Jan: replace domain id override with validation
v1: #18 feedback Jan: meld the ring count limit into the series
v1: feedback #15 Jan: verify zero in unused hypercall args
xen/common/argo.c | 126 ++
xen/include/public/argo.h | 19 +++
xen/include/xlat.lst | 1 +
3 files changed, 146 insertions(+)
diff --git a/xen/common/argo.c b/xen/common/argo.c
index 814dd0c..f3e468d 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -37,6 +37,7 @@ CHECK_argo_addr;
CHECK_argo_register_ring;
CHECK_argo_ring;
CHECK_argo_ring_message_header;
+CHECK_argo_unregister_ring;
#endif
#define MAX_RINGS_PER_DOMAIN128U
@@ -53,6 +54,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_argo_addr_t);
DEFINE_XEN_GUEST_HANDLE(xen_argo_gfn_t);
DEFINE_XEN_GUEST_HANDLE(xen_argo_register_ring_t);
DEFINE_XEN_GUEST_HANDLE(xen_argo_ring_t);
+DEFINE_XEN_GUEST_HANDLE(xen_argo_unregister_ring_t);
static bool __read_mostly opt_argo;
static bool __read_mostly opt_argo_mac_permissive;
@@ -360,6 +362,36 @@ find_ring_info(const struct domain *d, const struct
argo_ring_id *id)
return NULL;
}
+static struct argo_send_info *
+find_send_info(const struct domain *d, const struct argo_ring_id *id)
+{
+struct argo_send_info *send_info;
+const struct list_head *bucket;
+
+ASSERT(LOCKING_send_L2(d));
+
+/* List is not modified here. Search and return the match if found. */
+bucket = &d->argo->send_hash[hash_index(id)];
+
+list_for_each_entry(send_info, bucket, node)
+{
+const struct argo_ring_id *cmpid = &send_info->id;
+
+if ( cmpid->aport == id->aport &&
+ cmpid->domain_id == id->domain_id &&
+ cmpid->partner_id == id->partner_id )
+{
+argo_dprintk("found send_info for ring(%u:%x %u)\n",
+ id->domain_id, id->aport, id->partner_id);
+return send_info;
+}
+}
+argo_dprintk("no send_info for ring(%u:%x %u)\n",
+ id->domain_id, id->aport, id->partner_id);
+
+return NULL;
+}
+
st
[Xen-devel] [PATCH v8 for-4.12 09/17] argo: implement the sendv op; evtchn: expose send_guest_global_virq
sendv operation is invoked to perform a synchronous send of buffers contained in iovs to a remote domain's registered ring. It takes: * A destination address (domid, port) for the ring to send to. It performs a most-specific match lookup, to allow for wildcard. * A source address, used to inform the destination of where to reply. * The address of an array of iovs containing the data to send * .. and the length of that array of iovs * and a 32-bit message type, available to communicate message context data (eg. kernel-to-kernel, separate from the application data). If insufficient space exists in the destination ring, it will return -EAGAIN and Xen will notify the caller when sufficient space becomes available. Accesses to the ring indices are appropriately atomic. The rings are mapped into Xen's private address space to write as needed and the mappings are retained for later use. Notifications are sent to guests via VIRQ and send_guest_global_virq is exposed in the change to enable argo to call it. VIRQ_ARGO is claimed from the VIRQ previously reserved for this purpose (#11). The VIRQ notification method is used rather than sending events using evtchn functions directly because: * no current event channel type is an exact fit for the intended behaviour. ECS_IPI is closest, but it disallows migration to other VCPUs which is not necessarily a requirement for Argo. * at the point of argo_init, allocation of an event channel is complicated by none of the guest VCPUs being initialized yet and the event channel logic expects that a valid event channel has a present VCPU. * at the point of signalling a notification, the VIRQ logic is already defensive: if d->vcpu[0] is NULL, the notification is just silently dropped, whereas the evtchn_send logic is not so defensive: vcpu[0] must not be NULL, otherwise a null pointer dereference occurs. Using a VIRQ removes the need for the guest to query to determine which event channel notifications will be delivered on. This is also likely to simplify establishing future L0/L1 nested hypervisor argo communication. Signed-off-by: Christopher Clark Tested-by: Chris Patterson Reviewed-by: Roger Pau Monné --- v7 #09 Jan: avoid code duplication, forward compat ops to native v7 self: tidy XEN_ARGO_MAX_RING_SIZE comment in the public header v7 #09 Jan: drop stale XEN_ARGO_MAXIOV comment in the public header v7 #09 Roger: add Reviewed-by v7 #09 Roger/Jan: replace message_type validation check v7 #09 Roger/Jan: rewrite message size bounds check v6 #09 Jan: introduce compat ABI v6 : hoist read of iovs from guest from ringbuf_insert to do_argo_op v6 : simplify init of the NULL_hnd in ringbuf_insert v6 #09 Jan: xlat.lst remove argo_iov as generated macro not in use v6 #04 Jan: xlat.lst: move argo struct entries to alphabetical position v6 #09 Roger: fix reference to VIRQ_ARGO in commit message v6 #09 Roger: use list_for_each_entry for looping v5 #09 Roger: add comment explaining post-iovs tx_ptr round up + wrap v5 #09 Roger: remove redundant len bounds check vs MAX_ARGO_MESSAGE_SIZE v5 #09 Roger: ringbuf_insert: WARN not ERR on empty iovs v5 #09 Roger: bugfix: set rc = -EFAULT if !guest_handle_okay v5 use EBUSY when cannot add to the pending ent queue: many domains active v5: add compat validation macros to primary source file: common/argo.c v5: dropped external file for compat macros: common/compat/argo.c v5 : convert hypercall arg structs to struct form for compat checking v5 : switch argo_iov to needs translation in xlat.lst v4 Jan: remove use of fixed-width types from iov_count, ringbuf_insert v4 #07 Jan: shrink critical sections in sendv v3 #07 Jan: header: note 32-bitness of hypercall message tuype arg v4 : use standard data structures as per common code v4 self: bugfix memcpy_to_guest_ring: head_len must check (offset + len) v4 #09 Roger: drop MESSAGE from VIRQ_ARGO_MESSAGE v3 #07 Jan: rename ring_find_info* to find_ring_info* v3 #07 Jan: fix numeric entries in printk format strings v3 #10 Roger: move find functions to top of file and drop prototypes v3 #04 Jan: meld compat struct checking for hypercall args v3 #04 Roger/Jan: make lock names clearer and assert their state v3 #04 Jan: port -> aport with type; distinguish argo port from evtchn v3 feedback #09 Eric: fix len & offset sanity check in memcpy_to_guest_ring v3 feedback #04 Roger: newline fix in wildcard_pending_list_insert v3 feedback #04 Roger: drop npages struct member, calculate from len v3 #09 Roger: simplify EFAULT return in memcpy_to_guest_ring v3 #09 Roger: add newline before return in get_sanitized_ring v3 #09 Roger: replace while with for loop in iov_count v3 #09 Roger: drop 0 in struct init in ringbuf_insert v3 #09 Roger: comment for XEN_ARGO_MAXIOV: warn of stack overflow risk v3 #09 Roger: simplify while loop: for instead in ringbuf_insert v3 #09 Roger: drop out label for returns in ringbuf_insert v3 #09 Roger: drop newline in pending_queue v3 #09 Roger: replace second go
[Xen-devel] [PATCH v8 for-4.12 11/17] xsm, argo: XSM control for argo register
XSM controls for argo ring registration with two distinct cases, where
the ring being registered is:
1) Single source: registering a ring for communication to receive messages
from a specified single other domain.
Default policy: allow.
2) Any source: registering a ring for communication to receive messages
from any, or all, other domains (ie. wildcard).
Default policy: deny, with runtime policy configuration via bootparam.
This commit modifies the signature of core XSM hook functions in order to
apply 'const' to arguments, needed in order for 'const' to be accepted in
signature of functions that invoke them.
Signed-off-by: Christopher Clark
Acked-by: Daniel De Graaf
Tested-by: Chris Patterson
---
v6 Chris: apply const to avc_audit_data sdom and tdom struct members
v6 Chris: apply const to args in dummy.h function signatures
v6 Chris: fix missing return type in xsm.h inline functions
v3 Daniel/Jan: add to the default xsm policy for the register op
v3 hoist opt_argo_mac_permissive check to allow default policy to match non-XSM
v3 was: Acked-by: Daniel De Graaf
v3 Add Daniel's Acked-by ; note minor changes required for v4
v3 feedback #07 Roger: use opt_argo_mac_permissive : a boolean opt
v2 feedback #9 Jan: refactor to use argo-mac bootparam at point of introduction
v1 feedback Paul: replace use of strncmp with strcmp
v1 feedback #16 Jan: apply const to function signatures
v1 feedback #14 Jan: add blank line before return in parse_argo_mac_param
tools/flask/policy/modules/guest_features.te | 6 ++
xen/common/argo.c| 11 +--
xen/include/xsm/dummy.h | 14 ++
xen/include/xsm/xsm.h| 19 +++
xen/xsm/dummy.c | 4
xen/xsm/flask/hooks.c| 27 ---
xen/xsm/flask/include/avc.h | 4 ++--
xen/xsm/flask/policy/access_vectors | 11 +++
xen/xsm/flask/policy/security_classes| 1 +
9 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/tools/flask/policy/modules/guest_features.te
b/tools/flask/policy/modules/guest_features.te
index 9ac9780..d00769e 100644
--- a/tools/flask/policy/modules/guest_features.te
+++ b/tools/flask/policy/modules/guest_features.te
@@ -5,6 +5,12 @@ allow domain_type xen_t:xen tmem_op;
# pmu_ctrl is for)
allow domain_type xen_t:xen2 pmu_use;
+# Allow all domains:
+# to register single-sender (unicast) rings to partner with any domain; and
+# to register any-sender (wildcard) rings that can be sent to by any domain.
+allow domain_type xen_t:argo { register_any_source };
+allow domain_type domain_type:argo { register_single_source };
+
# Allow guest console output to the serial console. This is used by PV Linux
# and stub domains for early boot output, so don't audit even when we deny it.
# Without XSM, this is enabled only if the Xen was compiled in debug mode.
diff --git a/xen/common/argo.c b/xen/common/argo.c
index dce90ee..1a9a0e8 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
@@ -1652,8 +1653,10 @@ register_ring(struct domain *currd,
if ( reg.partner_id == XEN_ARGO_DOMID_ANY )
{
-if ( !opt_argo_mac_permissive )
-return -EPERM;
+ret = opt_argo_mac_permissive ? xsm_argo_register_any_source(currd) :
+-EPERM;
+if ( ret )
+return ret;
}
else
{
@@ -1664,6 +1667,10 @@ register_ring(struct domain *currd,
return -ESRCH;
}
+ret = xsm_argo_register_single_source(currd, dst_d);
+if ( ret )
+goto out;
+
send_info = xzalloc(struct argo_send_info);
if ( !send_info )
{
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index a29d1ef..9abfd69 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -720,6 +720,20 @@ static XSM_INLINE int xsm_dm_op(XSM_DEFAULT_ARG struct
domain *d)
#endif /* CONFIG_X86 */
+#ifdef CONFIG_ARGO
+static XSM_INLINE int xsm_argo_register_single_source(const struct domain *d,
+ const struct domain *t)
+{
+return 0;
+}
+
+static XSM_INLINE int xsm_argo_register_any_source(const struct domain *d)
+{
+return 0;
+}
+
+#endif /* CONFIG_ARGO */
+
#include
static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
{
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3b192b5..0b40714 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -181,6 +181,11 @@ struct xsm_operations {
#endif
int (*xen_version) (uint32_t cmd);
int (*domain_resource_map) (struct domain *d);
+#ifdef CONFIG_ARGO
+int (*argo_register_single_source) (const struct domain *d,
+
[Xen-devel] [PATCH v8 for-4.12 12/17] xsm, argo: XSM control for argo message send operation
Default policy: allow.
Signed-off-by: Christopher Clark
Reviewed-by: Paul Durrant
Acked-by: Daniel De Graaf
Tested-by: Chris Patterson
---
v3 Daniel/Jan: add to the default xsm policy for the send op
v3 Add Daniel's Acked-by
v2: reordered commit sequence to after sendv implementation
v1 feedback Jan #16: apply const to function signatures
v1 version was: Reviewed-by: Paul Durrant
tools/flask/policy/modules/guest_features.te | 7 ---
xen/common/argo.c| 11 +++
xen/include/xsm/dummy.h | 6 ++
xen/include/xsm/xsm.h| 6 ++
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c| 7 +++
xen/xsm/flask/policy/access_vectors | 2 ++
7 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/tools/flask/policy/modules/guest_features.te
b/tools/flask/policy/modules/guest_features.te
index d00769e..ca52257 100644
--- a/tools/flask/policy/modules/guest_features.te
+++ b/tools/flask/policy/modules/guest_features.te
@@ -6,10 +6,11 @@ allow domain_type xen_t:xen tmem_op;
allow domain_type xen_t:xen2 pmu_use;
# Allow all domains:
-# to register single-sender (unicast) rings to partner with any domain; and
-# to register any-sender (wildcard) rings that can be sent to by any domain.
+# to register single-sender (unicast) rings to partner with any domain;
+# to register any-sender (wildcard) rings that can be sent to by any domain;
+# and send messages to rings.
allow domain_type xen_t:argo { register_any_source };
-allow domain_type domain_type:argo { register_single_source };
+allow domain_type domain_type:argo { send register_single_source };
# Allow guest console output to the serial console. This is used by PV Linux
# and stub domains for early boot output, so don't audit even when we deny it.
diff --git a/xen/common/argo.c b/xen/common/argo.c
index 1a9a0e8..ce42e69 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -1990,6 +1990,17 @@ sendv(struct domain *src_d, xen_argo_addr_t *src_addr,
if ( !dst_d )
return -ESRCH;
+ret = xsm_argo_send(src_d, dst_d);
+if ( ret )
+{
+gprintk(XENLOG_ERR, "argo: XSM REJECTED %i -> %i\n",
+src_d->domain_id, dst_d->domain_id);
+
+put_domain(dst_d);
+
+return ret;
+}
+
read_lock(&L1_global_argo_rwlock);
if ( !src_d->argo )
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 9abfd69..9ae69cc 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -732,6 +732,12 @@ static XSM_INLINE int xsm_argo_register_any_source(const
struct domain *d)
return 0;
}
+static XSM_INLINE int xsm_argo_send(const struct domain *d,
+const struct domain *t)
+{
+return 0;
+}
+
#endif /* CONFIG_ARGO */
#include
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 0b40714..4211892 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -185,6 +185,7 @@ struct xsm_operations {
int (*argo_register_single_source) (const struct domain *d,
const struct domain *t);
int (*argo_register_any_source) (const struct domain *d);
+int (*argo_send) (const struct domain *d, const struct domain *t);
#endif
};
@@ -715,6 +716,11 @@ static inline int xsm_argo_register_any_source(const
struct domain *d)
return xsm_ops->argo_register_any_source(d);
}
+static inline int xsm_argo_send(const struct domain *d, const struct domain *t)
+{
+return xsm_ops->argo_send(d, t);
+}
+
#endif /* CONFIG_ARGO */
#endif /* XSM_NO_WRAPPERS */
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index ed236b0..ffac774 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -155,5 +155,6 @@ void __init xsm_fixup_ops (struct xsm_operations *ops)
#ifdef CONFIG_ARGO
set_to_dummy_if_null(ops, argo_register_single_source);
set_to_dummy_if_null(ops, argo_register_any_source);
+set_to_dummy_if_null(ops, argo_send);
#endif
}
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index fcb7487..76c012c 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1732,6 +1732,12 @@ static int flask_argo_register_any_source(const struct
domain *d)
return avc_has_perm(domain_sid(d), SECINITSID_XEN, SECCLASS_ARGO,
ARGO__REGISTER_ANY_SOURCE, NULL);
}
+
+static int flask_argo_send(const struct domain *d, const struct domain *t)
+{
+return domain_has_perm(d, t, SECCLASS_ARGO, ARGO__SEND);
+}
+
#endif
long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
@@ -1871,6 +1877,7 @@ static struct xsm_operations flask_ops = {
#ifdef CONFIG_ARGO
.argo_register_single_source = flask_argo_register_single_source,
.argo_register_any_source = flask_argo_register_any_source,
+.argo_send = flask_argo_send,
#endif
};
diff --git a/xen/xsm/flask/policy/access_vecto
[Xen-devel] [PATCH v8 for-4.12 16/17] SUPPORT.md : add new entry for the Argo feature
Status: Experimental Signed-off-by: Christopher Clark --- SUPPORT.md | 4 1 file changed, 4 insertions(+) diff --git a/SUPPORT.md b/SUPPORT.md index 7c8493c..19fc8d7 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -617,6 +617,10 @@ Virtual Performance Management Unit for HVM guests Disabled by default (enable with hypervisor command line option). This feature is not security supported: see http://xenbits.xen.org/xsa/advisory-163.html +### Argo: Inter-domain message delivery by hypercall + +Status: Experimental + ### x86/PCI Device Passthrough Status, x86 PV: Supported, with caveats -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 14/17] xsm, argo: notify: don't describe rings that cannot be sent to
Signed-off-by: Christopher Clark
Acked-by: Daniel De Graaf
Tested-by: Chris Patterson
---
v3 #10 Roger: drop out label, use return -EFAULT in fill_ring_data
v3: Add Daniel's Acked-by
xen/common/argo.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/xen/common/argo.c b/xen/common/argo.c
index 7523f32..13052b9 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -1342,6 +1342,17 @@ fill_ring_data(const struct domain *currd,
if ( !dst_d || !dst_d->argo )
goto out;
+/*
+ * Don't supply information about rings that a guest is not
+ * allowed to send to.
+ */
+ret = xsm_argo_send(currd, dst_d);
+if ( ret )
+{
+put_domain(dst_d);
+return ret;
+}
+
read_lock(&dst_d->argo->rings_L2_rwlock);
ring_info = find_ring_info_by_match(dst_d, ent.ring.aport,
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 10/17] argo: implement the notify op
Queries for data about space availability in registered rings and
causes notification to be sent when space has become available.
The hypercall op populates a supplied data structure with information about
ring state and if insufficient space is currently available in a given ring,
the hypervisor will record the domain's expressed interest and notify it
when it observes that space has become available.
Checks for free space occur when this notify op is invoked, so it may be
intentionally invoked with no data structure to populate
(ie. a NULL argument) to trigger such a check and consequent notifications.
Limit the maximum number of notify requests in a single operation to a
simple fixed limit of 256.
Signed-off-by: Christopher Clark
Tested-by: Chris Patterson
Reviewed-by: Roger Pau Monné
---
v7 #09 Jan: avoid code duplication, forward compat ops to native
v7 #10 Roger: list_first_entry_or_null in pending_notify
v6 #09 Jan: add compat ABI
v6 #04 Jan: xlat.lst: move argo struct entries to alphabetical position
v6: rewrap comment to fix line length
v6 #10 Roger: use list_for_each_entry{_safe} for looping
v5: add EBUSY ent flag when too many domains are already on pending list
v5: reorder notify flags: error flags last, fixed state first
v5: add compat validation macros to primary source file: common/argo.c
v5 : convert hypercall arg structs to struct form for compat checking
v5: dropped external file for compat macros: common/compat/argo.c
v4 #10 Roger: consolidate notify flags; infer pending notify if needed
v4 bugfix: take L3 before accessing ring_info in fill_ring_data
v4 #10 Roger: shorten notify flag names: drop _DATA_F
v4 #10 self/Roger: fill_ring_data: check pending_requeue error code
v4 : use standard data structures as per common code
v4 #10 Roger: lower indentation in fill_ring_data by using goto
v4 #10 Roger: reword the XEN_ARGO_RING_DATA_F_SUFFICIENT comment
v4 fix location of a FIXME that was incorrectly moved by this later commit
v3 #07 Jan: fix format string indention in printks
v3 (general) Jan: drop fixed width types for ringbuf_payload_space
v3 #07 Jan: rename ring_find_info_by_match to find_ring_info_by_match
v3 #07 Jan: fix numeric entries in printk format strings
v3: ringbuf_payload_space: simpler return 0 if get_sanitized_ring fails
v3 #10 Roger: simplify ringbuf_payload_space for empty rings
v3 #10 Roger: ringbuf_payload_space: add comment to explain how ret < INT32_MAX
v3 #10 Roger: drop out label, use return -EFAULT in fill_ring_data
v3 #10 Roger: add newline in signal_domid
v3 #10 Roger: move find functions to top of file and drop prototypes
v3 #04 Jan: meld the compat hypercall arg checking
v3 #04 Roger/Jan: make lock names clearer and assert their state
v3 #04 Jan: port -> aport with type; distinguish argo port from evtchn
v3 self: drop braces in foreach of notify_check_pending
v3 feedback Roger/Jan: ASSERT currd is current->domain or use 'd' variable name
v2 feedback Jan: drop cookie, implement teardown
v2 notify: add flag to indicate ring is shared
v2 argument name for fill_ring_data arg is now currd
v2 self: check ring size vs request and flag error rather than queue signal
v2 feedback Jan: drop 'message' from 'argo_message_op'
v2 self: simplify signal_domid, drop unnecessary label + goto
v2 self: skip the cookie check in pending_cancel
v2 self: implement npending limit on number of pending entries
v1 feedback #16 Jan: sanitize_ring in ringbuf_payload_space
v2 self: inline fill_ring_data_array
v2 self: avoid retesting dst_d for put_domain
v2 self/Jan: remove use of magic verification field and tidy up
v1 feedback #16 Jan: remove testing of magic in guest-supplied structure
v2 self: s/argo_pending_ent/pending_ent/g
v2 feedback v1#13 Roger: use OS-supplied roundup; drop from public header
v1,2 feedback Jan/Roger/Paul: drop errno returning guest access functions
v1 feedback Roger, Jan: drop argo prefix on static functions
v2 self: reduce indentation via goto out if arg NULL
v1 feedback #13 Jan: resolve checking of array handle and use of __copy
v1 #5 (#16) feedback Paul: notify op: use currd in do_argo_message_op
v1 #5 (#16) feedback Paul: notify op: use currd in argo_notify
v1 #5 (#16) feedback Paul: notify op: use currd in argo_notify_check_pending
v1 #5 (#16) feedback Paul: notify op: use currd in argo_fill_ring_data_array
v1 #13 (#16) feedback Paul: notify op: do/while: reindent only
v1 #13 (#16) feedback Paul: notify op: do/while: goto
v1 : add compat xlat.lst entries
v1: add definition for copy_field_from_guest_errno
v1 #13 feedback Jan: make 'ring data' comment comply with single-line style
v1 feedback #13 Jan: use __copy; so define and use __copy_field_to_guest_errno
v1: #13 feedback Jan: public namespace: prefix with xen
v1: #13 feedback Jan: add blank line after case in do_argo_message_op
v1: self: rename ent id to domain_id
v1: self: ent id-> domain_id
v1: self: drop signal if domain_cookie mismatches
v1. feedback #15 Jan: make loop i unsigned
v1. self: drop unnec
[Xen-devel] [PATCH v8 for-4.12 06/17] xen/arm: introduce guest_handle_for_field()
ARM port of c/s bb544585: "introduce guest_handle_for_field()"
This helper turns a field of a GUEST_HANDLE into a GUEST_HANDLE.
Signed-off-by: Christopher Clark
Reviewed-by: Paul Durrant
Reviewed-by: Stefano Stabellini
---
v3: Added Stefano's Reviewed-by
v2: Added Paul's Reviewed-by
xen/include/asm-arm/guest_access.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 224d2a0..8997a1c 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -63,6 +63,9 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa,
void *buf,
_y; \
})
+#define guest_handle_for_field(hnd, type, fld) \
+((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
+
#define guest_handle_from_ptr(ptr, type)\
((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
#define const_guest_handle_from_ptr(ptr, type) \
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 13/17] xsm, argo: XSM control for any access to argo by a domain
Will inhibit initialization of the domain's argo data structure to
prevent receiving any messages or notifications and access to any of
the argo hypercall operations.
Signed-off-by: Christopher Clark
Acked-by: Daniel De Graaf
Tested-by: Chris Patterson
---
v7 self: fix return of check error code
v7 #09 Jan: avoid code duplication, forward compat ops to native
v6 #09 Jan: add compat ABI
v6 Chris: apply const to args in dummy.h function signatures
v6 Chris: fix missing return type in xsm.h inline functions
v3 Daniel/Jan: add to the default xsm policy for enable
v3 Add Daniel's Acked-by
v3 #04 Jason/Roger: soft_reset: can assume reinit is ok if d->argo set
v2 self: fix xsm use in soft-reset prior to introduction
v1 #5 (#17) feedback Paul: XSM control for any access: use currd
v1 #16 feedback Jan: apply const to function signatures
tools/flask/policy/modules/guest_features.te | 4 ++--
xen/common/argo.c| 16
xen/include/xsm/dummy.h | 5 +
xen/include/xsm/xsm.h| 6 ++
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c| 7 +++
xen/xsm/flask/policy/access_vectors | 3 +++
7 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/tools/flask/policy/modules/guest_features.te
b/tools/flask/policy/modules/guest_features.te
index ca52257..fe4835d 100644
--- a/tools/flask/policy/modules/guest_features.te
+++ b/tools/flask/policy/modules/guest_features.te
@@ -5,11 +5,11 @@ allow domain_type xen_t:xen tmem_op;
# pmu_ctrl is for)
allow domain_type xen_t:xen2 pmu_use;
-# Allow all domains:
+# Allow all domains to enable the Argo interdomain communication hypercall;
# to register single-sender (unicast) rings to partner with any domain;
# to register any-sender (wildcard) rings that can be sent to by any domain;
# and send messages to rings.
-allow domain_type xen_t:argo { register_any_source };
+allow domain_type xen_t:argo { enable register_any_source };
allow domain_type domain_type:argo { send register_single_source };
# Allow guest console output to the serial console. This is used by PV Linux
diff --git a/xen/common/argo.c b/xen/common/argo.c
index ce42e69..7523f32 100644
--- a/xen/common/argo.c
+++ b/xen/common/argo.c
@@ -2078,6 +2078,10 @@ do_argo_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg1,
if ( unlikely(!opt_argo) )
return -EOPNOTSUPP;
+rc = xsm_argo_enable(currd);
+if ( rc )
+return rc;
+
switch ( cmd )
{
case XEN_ARGO_OP_register_ring:
@@ -2216,6 +2220,10 @@ compat_argo_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg1,
if ( unlikely(!opt_argo) )
return -EOPNOTSUPP;
+rc = xsm_argo_enable(currd);
+if ( rc )
+return rc;
+
argo_dprintk("->compat_argo_op(%u,%p,%p,%lu,0x%lx)\n", cmd,
(void *)arg1.p, (void *)arg2.p, arg3, arg4);
@@ -2277,7 +2285,7 @@ argo_init(struct domain *d)
{
struct argo_domain *argo;
-if ( !opt_argo )
+if ( !opt_argo || xsm_argo_enable(d) )
{
argo_dprintk("argo disabled, domid: %u\n", d->domain_id);
return 0;
@@ -2334,9 +2342,9 @@ argo_soft_reset(struct domain *d)
wildcard_rings_pending_remove(d);
/*
- * Since opt_argo cannot change at runtime, if d->argo is true then
- * opt_argo must be true, and we can assume that init is allowed to
- * proceed again here.
+ * Since neither opt_argo or xsm_argo_enable(d) can change at runtime,
+ * if d->argo is true then both opt_argo and xsm_argo_enable(d) must be
+ * true, and we can assume that init is allowed to proceed again here.
*/
argo_domain_init(d->argo);
}
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 9ae69cc..e628b1c 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -721,6 +721,11 @@ static XSM_INLINE int xsm_dm_op(XSM_DEFAULT_ARG struct
domain *d)
#endif /* CONFIG_X86 */
#ifdef CONFIG_ARGO
+static XSM_INLINE int xsm_argo_enable(const struct domain *d)
+{
+return 0;
+}
+
static XSM_INLINE int xsm_argo_register_single_source(const struct domain *d,
const struct domain *t)
{
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4211892..8a78d8a 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -182,6 +182,7 @@ struct xsm_operations {
int (*xen_version) (uint32_t cmd);
int (*domain_resource_map) (struct domain *d);
#ifdef CONFIG_ARGO
+int (*argo_enable) (const struct domain *d);
int (*argo_register_single_source) (const struct domain *d,
const struct domain *t);
int (*argo_register_any_source) (const struct domain *d);
@@ -705,6 +706,11 @@ static inline int xsm_domain_resource_map(xsm_default_t
def, struct domain *d)
[Xen-devel] [PATCH v8 for-4.12 15/17] MAINTAINERS: add new section for Argo and self as maintainer
Signed-off-by: Christopher Clark Reviewed-by: Roger Pau Monné --- v7 #015 Roger: Add Reviewed-by v5 whitespace: tabs MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e99d39e..a0cda4f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -158,6 +158,13 @@ S: Supported F: xen/arch/x86/hvm/svm/ F: xen/arch/x86/cpu/vpmu_amd.c +ARGO +M: Christopher Clark +S: Maintained +F: xen/include/public/argo.h +F: xen/include/xen/argo.h +F: xen/common/argo.c + ARINC653 SCHEDULER M: Josh Whitehead M: Robert VanVossen -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v8 for-4.12 17/17] docs, argo: add design document for Argo
Document provides a brief introduction to the Argo interdomain communication mechanism and a detailed description of the granular locking used within the Argo implementation. Signed-off-by: Christopher Clark --- docs/designs/argo.pandoc | 448 +++ 1 file changed, 448 insertions(+) create mode 100644 docs/designs/argo.pandoc diff --git a/docs/designs/argo.pandoc b/docs/designs/argo.pandoc new file mode 100644 index 000..2ce253b --- /dev/null +++ b/docs/designs/argo.pandoc @@ -0,0 +1,448 @@ +# Argo + +## Introduction + +Argo is an interdomain communication mechanism. It provides Xen hypervisor +primitives to transmit data between VMs, by performing data copies into +receive memory rings registered by domains. It does not require memory +sharing between VMs and does not use the grant tables or Xenstore. + +Argo has requirements for performance isolation between domains, to prevent +negative performance impact from malicious or disruptive activity of other +domains, or even other VCPUs of the same domain operating other rings. + +## Hypervisor-Mediated data eXchange (HMX) + +This term references inter-VM communication protocols that have this +key architectural point: The hypervisor is responsible for performing the +write of data into the guest-accessible memory buffer, in the manner +according to the agreed transfer protocol. This structure ensures that +there is strength to the transport mechanism, because the transmitting side +of the communication is the hypervisor, which can be trusted by the receiver, +and the buffer is isolated from access by any other potential sources +outside the receiver. + +The receiver can trust that the hypervisor will: + +- Provide a protocol implementation adhering to hardware synchronization +requirements for concurrent access to system memory by communicating +components +- Deliver data only from an approved source, enforcing policy for Mandatory +Access Control. +- Indicate the correct sender of the data. +- Transmit only the intended data, adhering to the access protocol of the data +structure in the buffer. If the memory region is being used as a ring, then: +- Data writes will only occur within the ring region that is indicated as +available for incoming data by the ring indexes. +- The indicated length of data written will exactly match the length of +data actually written. +- The write for each piece of data will occur only once. +- Data will be written sequentially in the order that it is sent. +- Issue notification of data delivered correctly. + +This structure allows for augmentation by the hypervisor to identify the +sending entity within the source VM, and then provide the receiver with +assured context information about the data source. This enables the receiver +to make decisions based on fine-grained knowledge of the source of the data. + +This structure is also of strong interest for nested virtualization: +transport via the hypervisor can enable construction of efficient +communications between VMs at different levels of nesting. + +# Locking + +Since Argo operates a data path between domains, sections of this code are +*hot* when the communication paths are in use. To encourage high performance, a +goal is to limit mutual exclusion to only where required and enable significant +concurrency. + +Avoidance of deadlock is essential and since state must frequently be updated +that pertains to more than one domain, a locking protocol defines which locks +are needed and the order of their acquistion. + +## Structure + +The granular locking structure of Argo enables: + +1. Performance isolation of guests +2. Avoidance of DoS of rings by domains that are not authorized to send to them +3. Deadlock-free teardown of state across multiple domains on domain destroy +4. Performance of guests using Argo with concurrent operation of rings. + +Argo uses three per-domain locks to protect three separate data structures. +Access to the ring_hash data structure is confined to domains that a +ring-registering domain has authorized to send data via the ring. The complete +set of Argo locks is: + +* Global : `L1_global_argo_rwlock` +* Per-domain: `rings_L2_rwlock` +* Per-domain: `send_L2_lock` +* Per-domain: `wildcard_L2_lock` +* Per-ring: `L3_lock` + +## Protected State + +The data structures being protected by the locks are all per-domain. The only +global Argo state is the `L1_global_argo_rwlock` used to coordinate access to +data structures of other domains. + +### State: Rings registered and owned by a domain + +This includes the state to run that ring, such as memory frame numbers and +established mappings. Per-ring state is protected by its own lock, so that +multiple VCPUs of the same domain operating different rings do not inhibit the +performance of each other. + +The per-domain ring state also includes the list of pending notifications for +other domains that are waiting for ring space availabi
Re: [Xen-devel] [PATCH for-4.12 4/8] x86/shadow: alloc enough pages so initialization doesn't fail
On Tue, Feb 05, 2019 at 10:32:08AM -0700, Jan Beulich wrote: > >>> On 05.02.19 at 16:53, wrote: > > On Tue, Feb 05, 2019 at 08:15:27AM -0700, Jan Beulich wrote: > >> >>> On 05.02.19 at 14:52, wrote: > >> > I don't think the amount of guest memory matters here, the following > >> > example with 8G of RAM and 8 vCPUs fails in the same way: > >> > > >> > # cat test.c > >> > test.c test.c.gcov test.cfg test.core > >> > root@:~ # cat test.cfg > >> > name = "test" > >> > type = "hvm" > >> > > >> > memory = 8192 > >> > vcpus = 8 > >> > hap = 0 > >> > # xl create test.cfg > >> > Parsing config from test.cfg > >> > libxl: error: libxl_create.c:578:libxl__domain_make: domain creation > >> > fail: > >> > Cannot allocate memory > >> > libxl: error: libxl_create.c:975:initiate_domain_create: cannot make > >> > domain: > >> > -3 > >> > > >> > And I think that's a perfectly suitable guest config. > >> > >> Indeed. And it doesn't seem to work for me anymore either. Must be > >> a regression, as I'm pretty sure it did still work not all that long ago. > >> Not even "shadow_memory=256" helps. > > > > No, because shadow_init is called from domain_create, and it's > > impossible to increase the shadow memory pool before the domain is > > actually created. > > Okay, I misunderstood the problem initially. Aiui this is a > regression from the early setting of ->max_vcpus, as that now Ahh, I wasn't able to figure out what caused this regression, it's indeed caused by max_vcpus being set at domain_create. > causes shadow_min_acceptable_pages() to block far more pages > than it did before from use for p2m allocs during domain creation. > I think I see an alternative way of fixing this for the moment > (without adding re-size logic yet when d->tot_pages grows), but > this will have to wait until tomorrow. Ack, I'm happy to implement it if you tell me the plan :). Maybe as an alternative the checks in shadow_alloc_p2m_page can be relaxed during domain creation, or the p2m allocated when the first memory page gets added to the domain? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 for-4.12 16/17] SUPPORT.md : add new entry for the Argo feature
On Wed, Feb 06, 2019 at 12:55:07AM -0800, Christopher Clark wrote: > Status: Experimental > > Signed-off-by: Christopher Clark Reviewed-by: Roger Pau Monné > --- > SUPPORT.md | 4 > 1 file changed, 4 insertions(+) > > diff --git a/SUPPORT.md b/SUPPORT.md > index 7c8493c..19fc8d7 100644 > --- a/SUPPORT.md > +++ b/SUPPORT.md > @@ -617,6 +617,10 @@ Virtual Performance Management Unit for HVM guests > Disabled by default (enable with hypervisor command line option). > This feature is not security supported: see > http://xenbits.xen.org/xsa/advisory-163.html > > +### Argo: Inter-domain message delivery by hypercall > + > +Status: Experimental > + Some features disabled by default at build time have: "Compile time disabled by default." Here. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: correctly deal with benign exceptions when combining two
On 06/02/2019 07:40, Jan Beulich wrote: > Benign exceptions, no matter whether they're first or second, will never > cause #DF (a benign exception being second can basically only be #AC, as > in the XSA-156 scenario). #DB can happen as well, but I'm not sure if this example is relevant here. Both of those loops where repeated exceptions caused by trying to deliver the first exception, rather than an issue of priority amongst simultaneous exceptions during the execution of an instruction. For VT-x, we're supposed to fill in the PENDING_DBG control rather than raise #DB directly, explicitly to allow the priority of interrupts to be expressed correctly. There is a very large quantity of untangling working to do, and very clear I'm not going to have time to fix even the STI/Singlestep issue in the 4.12 timeframe. > Sadly neither AMD nor Intel really define what happens with two benign > exceptions - the term "sequentially" used by both is poisoned by how the > combining of benign and non-benign exceptions is described. Since NMI, > #MC, and hardware interrupts are all benign and (perhaps with the > exception of #MC) can't occur second, favor the first in order to not > lose it. #MC has the highest priority so should only be recognised immediately after an instruction boundary. The interesting subset is then a #DB from task switch, then NMI, then #DB from other pending traps from the previous instruction, so I think it is quite possible for us to end up with a #DB stacked on top of the head of the NMI/#MC handler, if we follow a sequential model. (Lucky for XSA-260 then, if this case actually exists.) I don't however see a way of stacking #AC, because you can't know that one has occured until later in the instruction cycle than all other sources. What would happen is that you'd raise #AC from previous instruction, and then recognise #MC while starting to execute the #AC entry point. (I think) ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-coverity test] 132958: regressions - ALL FAIL
flight 132958 xen-unstable-coverity real [real] http://logs.test-lab.xenproject.org/osstest/logs/132958/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: coverity-amd647 coverity-upload fail REGR. vs. 132424 version targeted for testing: xen 8aa276235b93eeb4f81095c638970900e19b31e5 baseline version: xen 08b908ba63dee8bc313983c5e412852cbcbcda85 Last test of basis 132424 2019-01-23 09:19:14 Z 14 days Failing since132506 2019-01-27 09:18:42 Z 10 days4 attempts Testing same since 132958 2019-02-06 09:19:02 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Andrii Anisov Anthony PERARD Brian Woods Doug Goldstein George Dunlap Hans van Kranenburg Ian Jackson Jan Beulich Julien Grall Norbert Manthey Roger Pau Monne Roger Pau Monné Tamas K Lengyel Wei Liu jobs: coverity-amd64 fail 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 Not pushing. (No revision log; it would be 778 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC v2 2/2] x86/emulate: Send vm_event from emulate
>>
>> /* FPU sub-types which may be requested via ->get_fpu(). */
>> enum x86_emulate_fpu_type {
>> diff --git a/xen/include/asm-x86/hvm/emulate.h
>> b/xen/include/asm-x86/hvm/emulate.h
>> index 26a01e83a4..721e175b04 100644
>> --- a/xen/include/asm-x86/hvm/emulate.h
>> +++ b/xen/include/asm-x86/hvm/emulate.h
>> @@ -47,6 +47,7 @@ struct hvm_emulate_ctxt {
>> uint32_t intr_shadow;
>>
>> bool_t set_context;
>> +bool send_event;
>
> I'm not sure I see why a send_event field needs to be added to the
> ctxt struct, isn't is possible to pass this parameter to the relevant
> functions that care about it?
>
This was done to have as little code change as possible and it has a
precedent with the set_context bool. If a send_event parameter would be
used then it will have to be passed down the line and then a lot of
function signatures have to change. I think the current way is the best
way to have this done.
Thanks,
Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] VMX: don't ignore P2M setup error
set_mmio_p2m_entry() may fail, in particular with -ENOMEM. Don't ignore such an error, but instead cause domain creation to fail in such a case. Signed-off-by: Jan Beulich --- 4.12: Another candidate, but again open for discussion. --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2999,10 +2999,10 @@ static int vmx_alloc_vlapic_mapping(stru clear_domain_page(mfn); share_xen_page_with_guest(pg, d, SHARE_rw); d->arch.hvm.vmx.apic_access_mfn = mfn_x(mfn); -set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), mfn, - PAGE_ORDER_4K, p2m_get_hostp2m(d)->default_access); -return 0; +return set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), mfn, + PAGE_ORDER_4K, + p2m_get_hostp2m(d)->default_access); } static void vmx_free_vlapic_mapping(struct domain *d) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] VMX: don't ignore P2M setup error
On 06/02/2019 11:24, Jan Beulich wrote: > set_mmio_p2m_entry() may fail, in particular with -ENOMEM. Don't ignore > such an error, but instead cause domain creation to fail in such a case. > > Signed-off-by: Jan Beulich > --- > 4.12: Another candidate, but again open for discussion. I'm fine with that in 4.12: Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 for-4.12 17/17] docs, argo: add design document for Argo
On Wed, Feb 06, 2019 at 12:55:08AM -0800, Christopher Clark wrote: > Document provides a brief introduction to the Argo interdomain > communication mechanism and a detailed description of the granular > locking used within the Argo implementation. > > Signed-off-by: Christopher Clark Reviewed-by: Roger Pau Monné I think the document is fine, and can be expanded as more people interact with argos. > --- > docs/designs/argo.pandoc | 448 > +++ > 1 file changed, 448 insertions(+) > create mode 100644 docs/designs/argo.pandoc > > diff --git a/docs/designs/argo.pandoc b/docs/designs/argo.pandoc > new file mode 100644 > index 000..2ce253b > --- /dev/null > +++ b/docs/designs/argo.pandoc > @@ -0,0 +1,448 @@ > +# Argo > + > +## Introduction > + > +Argo is an interdomain communication mechanism. It provides Xen hypervisor > +primitives to transmit data between VMs, by performing data copies into > +receive memory rings registered by domains. It does not require memory > +sharing between VMs and does not use the grant tables or Xenstore. > + > +Argo has requirements for performance isolation between domains, to prevent > +negative performance impact from malicious or disruptive activity of other > +domains, or even other VCPUs of the same domain operating other rings. > + > +## Hypervisor-Mediated data eXchange (HMX) > + > +This term references inter-VM communication protocols that have this > +key architectural point: The hypervisor is responsible for performing the > +write of data into the guest-accessible memory buffer, in the manner > +according to the agreed transfer protocol. This structure ensures that > +there is strength to the transport mechanism, because the transmitting side > +of the communication is the hypervisor, which can be trusted by the receiver, > +and the buffer is isolated from access by any other potential sources > +outside the receiver. > + > +The receiver can trust that the hypervisor will: > + > +- Provide a protocol implementation adhering to hardware synchronization > +requirements for concurrent access to system memory by communicating > +components > +- Deliver data only from an approved source, enforcing policy for Mandatory > +Access Control. > +- Indicate the correct sender of the data. > +- Transmit only the intended data, adhering to the access protocol of the > data > +structure in the buffer. If the memory region is being used as a ring, then: > +- Data writes will only occur within the ring region that is indicated as > +available for incoming data by the ring indexes. > +- The indicated length of data written will exactly match the length of > +data actually written. > +- The write for each piece of data will occur only once. > +- Data will be written sequentially in the order that it is sent. > +- Issue notification of data delivered correctly. > + > +This structure allows for augmentation by the hypervisor to identify the > +sending entity within the source VM, and then provide the receiver with > +assured context information about the data source. This enables the receiver > +to make decisions based on fine-grained knowledge of the source of the data. > + > +This structure is also of strong interest for nested virtualization: > +transport via the hypervisor can enable construction of efficient > +communications between VMs at different levels of nesting. This AFAICT also requires some kind of cooperation between hypervisors? It might be good to expand why hypervisor mediated is better than shared memory in the nested virtualization case, since you mention this explicitly but don't give any details. > +## FAQ / Other Considerations > + > +### Why not have a single per-domain lock? > + > +Due to performance isolation / DoS avoidance: if there is a single per-domain > +lock, acquiring this lock will stall operations on other active rings owned > by > +the domain. A malicious domain can loop registering and unregistering rings, > +without any consent by the targetted domain, which would experience decreased > +throughput due to the contention on the single per-domain lock. I'm not sure I see how this is prevented, there's still a rings_L2_rwlock that AFAICT needs to be write-locked when adding a ring, and that would prevent any other accesses to the list of rings, thus stalling operations? I guess they key point here (and what alleviates the issue listed above) is using multiple locks allows each one to be locked for smaller periods of time, thus reducing the DoS. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
Gentle ping on the questions below. On Mon, Feb 04, 2019 at 11:07:12AM +0100, Roger Pau Monné wrote: > On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: > > On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné > > wrote: > > > > > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > > > wrote: > > > > > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > As discussed offline: > > > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > > - OpenXT has use cases for measured launch and nested virtualization, > > > > which influence downstream performance and security requirements for > > > > Argo and Xen > > FWIW, I don't see the connection between nested virtualization or > measured launch and the list macros. I think a little bit more context > would be helpful here in order to understand the issue. > > > > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > Patching the macros for OpenXT is perfectly fine, but it would be > better to understand and fix the problem upstream if possible. > > How are you patching the macros? > > What are you trying to achieve by patching them? > > Thanks, Roger. > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
Wrong 'To:' field in the previous email, sorry. Gentle ping on the questions below. On Mon, Feb 04, 2019 at 11:07:12AM +0100, Roger Pau Monné wrote: > On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: > > On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné > > wrote: > > > > > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > > > wrote: > > > > > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > As discussed offline: > > > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > > - OpenXT has use cases for measured launch and nested virtualization, > > > > which influence downstream performance and security requirements for > > > > Argo and Xen > > FWIW, I don't see the connection between nested virtualization or > measured launch and the list macros. I think a little bit more context > would be helpful here in order to understand the issue. > > > > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > Patching the macros for OpenXT is perfectly fine, but it would be > better to understand and fix the problem upstream if possible. > > How are you patching the macros? > > What are you trying to achieve by patching them? > > Thanks, Roger. > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: correctly deal with benign exceptions when combining two
>>> On 06.02.19 at 10:57, wrote: > On 06/02/2019 07:40, Jan Beulich wrote: >> Benign exceptions, no matter whether they're first or second, will never >> cause #DF (a benign exception being second can basically only be #AC, as >> in the XSA-156 scenario). > > #DB can happen as well, but I'm not sure if this example is relevant > here. Both of those loops where repeated exceptions caused by trying to > deliver the first exception, rather than an issue of priority amongst > simultaneous exceptions during the execution of an instruction. No, I don't think #DB falls into this category (I had it here before and then removed it): If a data breakpoint hits while delivering an exception or interrupt, that - being a trap - will be delivered _after_ the original event, i.e. on the first instruction of the other handler. That's also the way the XSA-156 advisory describes it. > For VT-x, we're supposed to fill in the PENDING_DBG control rather than > raise #DB directly, explicitly to allow the priority of interrupts to be > expressed correctly. There is a very large quantity of untangling > working to do, and very clear I'm not going to have time to fix even the > STI/Singlestep issue in the 4.12 timeframe. Are you saying there need to be any vendor specific adjustments to this function then? I would very much hope that the code here could remain vendor independent, with vendor specific adjustments done in vendor specific code, and independently of the proposed change here. >> Sadly neither AMD nor Intel really define what happens with two benign >> exceptions - the term "sequentially" used by both is poisoned by how the >> combining of benign and non-benign exceptions is described. Since NMI, >> #MC, and hardware interrupts are all benign and (perhaps with the >> exception of #MC) can't occur second, favor the first in order to not >> lose it. > > #MC has the highest priority so should only be recognised immediately > after an instruction boundary. Are you sure? What about an issue with one of the memory accesses involved in delivering a previously raised exception? > The interesting subset is then a #DB > from task switch, then NMI, then #DB from other pending traps from the > previous instruction, so I think it is quite possible for us to end up > with a #DB stacked on top of the head of the NMI/#MC handler, if we > follow a sequential model. (Lucky for XSA-260 then, if this case > actually exists.) But for that we'd first of all need callers of this function to record the fact that their exception was squashed. Plus, as per above, such a #DB is to be delivered after the one that caused it to be raised in the first place, so is not subject to the behavior of hvm_combine_hw_exceptions() itself, and hence beyond the scope of this patch. > I don't however see a way of stacking #AC, because you can't know that > one has occured until later in the instruction cycle than all other > sources. What would happen is that you'd raise #AC from previous > instruction, and then recognise #MC while starting to execute the #AC > entry point. (I think) Well - see XSA-156 for what hardware does in that situation. Besides eliminating that security issue, I don't think this is a very important case to deal with correctly, unless you're aware of OSes which allow handling #AC in ring 3. Irrespective of all of the above - what am I to take from your response? I.e. what adjustments (within the scope of this patch) do you see necessary for the change to become acceptable? It was my thinking that this change alone would have masked the original #DF issue you've run into, so would likely be worthwhile without any of the other related work you hint at. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
A previously bad situation has become worse with the early setting of
->max_vcpus: The value returned by shadow_min_acceptable_pages() has
further grown, and hence now holds back even more memory from use for
the p2m.
Make sh_min_allocation() account for all p2m memory needed for
shadow_enable() to succeed during domain creation (at which point the
domain has no memory at all allocated to it yet, and hence use of
d->tot_pages is meaningless).
Also make shadow_min_acceptable_pages() no longer needlessly add 1 to
the vCPU count.
Finally make the debugging printk() in shadow_alloc_p2m_page() a little
more useful by logging some of the relevant domain settings.
Reported-by: Roger Pau Monné
Signed-off-by: Jan Beulich
---
TBD: The question of course is whether such an "exact" calculation isn't
a little risky going forward, the more that the regression here
wasn't found by osstest, because domains with sufficiently few
vCPU-s weren't affected, due to the 4Mb minimum allocation enforced
by shadow_enable()'s call to shadow_set_allocation(). I would,
however, question this enforcement of a static minimum as well -
shadow_one_bit_enable() doesn't do so, for example.
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -966,7 +966,8 @@ const u8 sh_type_to_size[] = {
1 /* SH_type_oos_snapshot */
};
-/* Figure out the least acceptable quantity of shadow memory.
+/*
+ * Figure out the least acceptable quantity of shadow memory.
* The minimum memory requirement for always being able to free up a
* chunk of memory is very small -- only three max-order chunks per
* vcpu to hold the top level shadows and pages with Xen mappings in them.
@@ -975,11 +976,11 @@ const u8 sh_type_to_size[] = {
* instruction, we must be able to map a large number (about thirty) VAs
* at the same time, which means that to guarantee progress, we must
* allow for more than ninety allocated pages per vcpu. We round that
- * up to 128 pages, or half a megabyte per vcpu, and add 1 more vcpu's
- * worth to make sure we never return zero. */
+ * up to 128 pages, or half a megabyte per vcpu.
+ */
static unsigned int shadow_min_acceptable_pages(const struct domain *d)
{
-return (d->max_vcpus + 1) * 128;
+return d->max_vcpus * 128;
}
/* Dispatcher function: call the per-mode function that will unhook the
@@ -1322,8 +1323,11 @@ shadow_alloc_p2m_page(struct domain *d)
if ( !d->arch.paging.p2m_alloc_failed )
{
d->arch.paging.p2m_alloc_failed = 1;
-dprintk(XENLOG_ERR, "d%i failed to allocate from shadow pool\n",
-d->domain_id);
+dprintk(XENLOG_ERR,
+"d%d failed to allocate from shadow pool (tot=%u p2m=%u
min=%u)\n",
+d->domain_id, d->arch.paging.shadow.total_pages,
+d->arch.paging.shadow.p2m_pages,
+shadow_min_acceptable_pages(d));
}
paging_unlock(d);
return NULL;
@@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
{
/*
* Don't allocate less than the minimum acceptable, plus one page per
- * megabyte of RAM (for the p2m table).
+ * megabyte of RAM (for the p2m table, minimally enough for HVM's setting
+ * up of slot zero and VMX's setting up of the LAPIC page), plus one for
+ * HVM's 1-to-1 pagetable.
*/
-return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
+return shadow_min_acceptable_pages(d) +
+ max(d->tot_pages / 256,
+ is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
+: 0U) +
+ is_hvm_domain(d);
}
int shadow_set_allocation(struct domain *d, unsigned int pages, bool
*preempted)
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12 4/8] x86/shadow: alloc enough pages so initialization doesn't fail
>>> On 06.02.19 at 10:10, wrote: > On Tue, Feb 05, 2019 at 10:32:08AM -0700, Jan Beulich wrote: >> >>> On 05.02.19 at 16:53, wrote: >> > On Tue, Feb 05, 2019 at 08:15:27AM -0700, Jan Beulich wrote: >> >> >>> On 05.02.19 at 14:52, wrote: >> >> > I don't think the amount of guest memory matters here, the following >> >> > example with 8G of RAM and 8 vCPUs fails in the same way: >> >> > >> >> > # cat test.c >> >> > test.c test.c.gcov test.cfg test.core >> >> > root@:~ # cat test.cfg >> >> > name = "test" >> >> > type = "hvm" >> >> > >> >> > memory = 8192 >> >> > vcpus = 8 >> >> > hap = 0 >> >> > # xl create test.cfg >> >> > Parsing config from test.cfg >> >> > libxl: error: libxl_create.c:578:libxl__domain_make: domain creation >> >> > fail: > >> >> > Cannot allocate memory >> >> > libxl: error: libxl_create.c:975:initiate_domain_create: cannot make > domain: >> >> > -3 >> >> > >> >> > And I think that's a perfectly suitable guest config. >> >> >> >> Indeed. And it doesn't seem to work for me anymore either. Must be >> >> a regression, as I'm pretty sure it did still work not all that long ago. >> >> Not even "shadow_memory=256" helps. >> > >> > No, because shadow_init is called from domain_create, and it's >> > impossible to increase the shadow memory pool before the domain is >> > actually created. >> >> Okay, I misunderstood the problem initially. Aiui this is a >> regression from the early setting of ->max_vcpus, as that now > > Ahh, I wasn't able to figure out what caused this regression, it's > indeed caused by max_vcpus being set at domain_create. > >> causes shadow_min_acceptable_pages() to block far more pages >> than it did before from use for p2m allocs during domain creation. >> I think I see an alternative way of fixing this for the moment >> (without adding re-size logic yet when d->tot_pages grows), but >> this will have to wait until tomorrow. > > Ack, I'm happy to implement it if you tell me the plan :). Well, the plan was hard to spell out without actually trying out what is needed. You'll likely have seen already the resulting patch I've sent out for discussion. > Maybe as an alternative the checks in shadow_alloc_p2m_page can be > relaxed during domain creation, I've been considering this, but decided against it, because it would undermine the functionality in case the tool stack would not issue a set-allocation domctl. Mid-term (perhaps after 4.12) I think we need to settle on a more uniform model here, e.g. either always requiring a set-allocation request by the tool stack, or making the code not depend on it at all. > or the p2m allocated when the first memory page gets added to the domain? This might be possible as well, but perhaps requires a more intrusive change, in particular if you take into consideration the VMX change which was a byproduct of the playing done for this one. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] SVM: make nested page-fault tracing and logging consistent
Don't call __get_gfn_type_access() more than once, to make sure data
recorded for xentrace matches up with what gets logged in case of the
domain getting crashed.
As a side effect this also eliminates a type mismatch reported by
Norbert Manthey, as the first call now also needs to update the local
variable "p2mt".
Do a few cosmetics at the same time: Move a comment up a little, drop
the pointless "case 0" (seeing in particular the comment's wording),
and correct formatting.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1798,17 +1798,18 @@ static void svm_do_nested_pgfault(struct
} _d;
p2m = p2m_get_p2m(v);
+mfn = __get_gfn_type_access(p2m, gfn, &p2mt, &p2ma, 0, NULL, 0);
+
_d.gpa = gpa;
_d.qualification = 0;
-mfn = __get_gfn_type_access(p2m, gfn, &_d.p2mt, &p2ma, 0, NULL, 0);
_d.mfn = mfn_x(mfn);
+_d.p2mt = p2mt;
__trace_var(TRC_HVM_NPF, 0, sizeof(_d), &_d);
}
-switch (ret) {
-case 0:
-break;
+switch ( ret )
+{
case 1:
return;
case -1:
@@ -1818,10 +1819,12 @@ static void svm_do_nested_pgfault(struct
return;
}
+/* Everything else is an error. */
if ( p2m == NULL )
+{
p2m = p2m_get_p2m(v);
-/* Everything else is an error. */
-mfn = __get_gfn_type_access(p2m, gfn, &p2mt, &p2ma, 0, NULL, 0);
+mfn = __get_gfn_type_access(p2m, gfn, &p2mt, &p2ma, 0, NULL, 0);
+}
gdprintk(XENLOG_ERR,
"SVM violation gpa %#"PRIpaddr", mfn %#lx, type %i\n",
gpa, mfn_x(mfn), p2mt);
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] VMX: don't ignore P2M setup error
On 06/02/2019 10:24, Jan Beulich wrote: > set_mmio_p2m_entry() may fail, in particular with -ENOMEM. Don't ignore > such an error, but instead cause domain creation to fail in such a case. > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
On Wed, Feb 06, 2019 at 03:56:49AM -0700, Jan Beulich wrote:
> A previously bad situation has become worse with the early setting of
> ->max_vcpus: The value returned by shadow_min_acceptable_pages() has
> further grown, and hence now holds back even more memory from use for
> the p2m.
>
> Make sh_min_allocation() account for all p2m memory needed for
> shadow_enable() to succeed during domain creation (at which point the
> domain has no memory at all allocated to it yet, and hence use of
> d->tot_pages is meaningless).
>
> Also make shadow_min_acceptable_pages() no longer needlessly add 1 to
> the vCPU count.
>
> Finally make the debugging printk() in shadow_alloc_p2m_page() a little
> more useful by logging some of the relevant domain settings.
>
> Reported-by: Roger Pau Monné
> Signed-off-by: Jan Beulich
> ---
> TBD: The question of course is whether such an "exact" calculation isn't
> a little risky going forward, the more that the regression here
> wasn't found by osstest, because domains with sufficiently few
> vCPU-s weren't affected, due to the 4Mb minimum allocation enforced
> by shadow_enable()'s call to shadow_set_allocation(). I would,
> however, question this enforcement of a static minimum as well -
> shadow_one_bit_enable() doesn't do so, for example.
I wondered the same while doing my fix, whether it was best to account
for the exact amount of pages needed for shadow_enable to succeed, or
whether to add a fixed amount of slack that was greater than the
actual requirements of shadow_enable.
Since you have done the accounting, and I expect shadow_enable to not
change it's memory requirements very often I guess we can go with the
exact amount for now.
>
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -966,7 +966,8 @@ const u8 sh_type_to_size[] = {
> 1 /* SH_type_oos_snapshot */
> };
>
> -/* Figure out the least acceptable quantity of shadow memory.
> +/*
> + * Figure out the least acceptable quantity of shadow memory.
> * The minimum memory requirement for always being able to free up a
> * chunk of memory is very small -- only three max-order chunks per
> * vcpu to hold the top level shadows and pages with Xen mappings in them.
> @@ -975,11 +976,11 @@ const u8 sh_type_to_size[] = {
> * instruction, we must be able to map a large number (about thirty) VAs
> * at the same time, which means that to guarantee progress, we must
> * allow for more than ninety allocated pages per vcpu. We round that
> - * up to 128 pages, or half a megabyte per vcpu, and add 1 more vcpu's
> - * worth to make sure we never return zero. */
> + * up to 128 pages, or half a megabyte per vcpu.
> + */
> static unsigned int shadow_min_acceptable_pages(const struct domain *d)
> {
> -return (d->max_vcpus + 1) * 128;
> +return d->max_vcpus * 128;
> }
>
> /* Dispatcher function: call the per-mode function that will unhook the
> @@ -1322,8 +1323,11 @@ shadow_alloc_p2m_page(struct domain *d)
> if ( !d->arch.paging.p2m_alloc_failed )
> {
> d->arch.paging.p2m_alloc_failed = 1;
> -dprintk(XENLOG_ERR, "d%i failed to allocate from shadow pool\n",
> -d->domain_id);
> +dprintk(XENLOG_ERR,
> +"d%d failed to allocate from shadow pool (tot=%u p2m=%u
> min=%u)\n",
> +d->domain_id, d->arch.paging.shadow.total_pages,
> +d->arch.paging.shadow.p2m_pages,
> +shadow_min_acceptable_pages(d));
> }
> paging_unlock(d);
> return NULL;
> @@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
> {
> /*
> * Don't allocate less than the minimum acceptable, plus one page per
> - * megabyte of RAM (for the p2m table).
> + * megabyte of RAM (for the p2m table, minimally enough for HVM's setting
> + * up of slot zero and VMX's setting up of the LAPIC page), plus one for
> + * HVM's 1-to-1 pagetable.
> */
> -return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
> +return shadow_min_acceptable_pages(d) +
> + max(d->tot_pages / 256,
> + is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
> +: 0U) +
> + is_hvm_domain(d);
Should the call to shadow_set_allocation be changed so it attempts to
allocate sh_min_allocation(d) + d->arch.paging.shadow.p2m_pages?
It seems a little misleading to check whether there's a certain amount
of pages in the pool (sh_min_allocation(d) +
d->arch.paging.shadow.p2m_pages) and then set the allocation to 4M
unconditionally.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
>>> On 05.02.19 at 15:56, wrote: > On Mon, Feb 4, 2019 at 9:37 AM Jan Beulich wrote: >> >>> On 01.02.19 at 19:52, wrote: >> What I'm not sure I see is what you mean to >> express with all you wrote in terms of finding a way out of the >> current situation (besides requesting a vote) > > If you're just tired of this discussion and want it to be done, then > of course we can just take a vote. > > But ideally I think votes are best when everyone sees the landscape of > the decision clearly, and agrees on exactly what it is they disagree > about. Furthermore, it seems to me from reading this discussion that > it's more than just a few specific examples that I disagree with you > about, but about a number of principles; as such, investing time > trying to come to a common understanding should pay dividends in the > form of reduced friction in the future. Which I appreciate and agree with. I've been mentioning the option of a vote solely to show a way to make forward progress without us reaching agreement. > Before I expressed an opinion, I wanted to make sure that I hadn't > misunderstood you or missed a big aspect of the discussion. > >> > Improvements this series seeks to make, as I understand it, include >> > (in order of urgency): >> > >> > 1. Fixing one concrete instance of "UB hazard" >> >> Right, and we want to work around compiler bugs here. >> >> > 2. Minimize risk of further "UB hazard" in this bit of functionality >> > 3. Retain the effort Stefano has put in identifying all the places >> > where such UB hazards need to be addressed. >> > 4. Move towards MISRA-C compliance. >> > >> > As far as I can tell, primary objections you've leveled at the options >> > which try to address 2-4 are: >> > >> > a. "UB hazard" still not zero >> > b. MISRA compliancy no 100% >> > c. Ugly >> > d. Inefficient >> > >> > (Obviously some proposals have had more technical discussion.) >> > >> > Anything I missed? >> >> I don't think so, especially since various aspects can fall under "ugly" >> and/or "inefficient". > > Well for instance, other objections that you might have made that I > don't include in those include: > > * Incorrect (i.e., known ways in which the patch will break functionality) > * Misleading / confusing (i.e., someone modifying it is likely to > introduce regressions) > * Fragile (i.e., likely to break due to small or unrelated changes). There are none of the first category that I'm aware of, but to me the latter two categories at least overlap with "inefficient", and to me especially the var.S approach falls in the fragile and maybe also in the misleading/confusing category. > [snip] >>: Improving on a. and >> b. is not a good excuse to extend c., at least not unequivocally. >> Whether d. actually matters is a separate aspect, partly because it >> may mean different things (it could e.g. be taken as another >> wording for a. and b.). > > I take it you mean 2 and 4 (reduced UB hazard and increased chance of > MISRA-C compliance) are not a good excuse for c (ugly). > > The "ugliness" here involves, variously: > * Passing a variable through an asm "barrier" > * Casing pointers to other types, sometimes multiple times at once > > Most of them are a handful of lines hidden behind a macro in a header file. > > To me, on a scale of 1 to 10, I'd give them an ugliness factor 2 or 3. > > On the other hand, I find 2-4 compelling: > > * I consider your suggested approach, of using simple > pointer-to-pointer casting, or casting to a pointer after asm and > comparing the resulting pointers, to have a reasonably high chance of > tripping over UB behavior at some point in the future. Regardless of > the outcome of that -- whether we change our work-around again or > whether the compiler authors change the compilers -- both we and our > users and customers will have had a lot of hassle to deal with. > Avoiding that hassle is worth the slight ugliness introduced by the > other solutions. > > * Stefano has done a fair amount of work identifying the places that > need to be changed. We know that we're likely to need to make *some > sort* of change like this for MISRA compliance at some point. > Throwing away work that then will need to be duplicated is both a > waste of time, and of developer motivation. Even if we didn't think > it would impove UB behavior *or* get us closer to MISRA C compliance, > retaining the work he's done would be worth accepting a patch creating > such a macro. > > * The patch takes the code base one step closer to being MISRA C > compliant, by setting up infrastructure likely needed by whatever it > needs. Even before we had the recommendation from MISRA C, I would > consider preparing for that eventuality to be worth the minor ugliness > introduced. > > And so, to me, the unitptr_t casting proposal seems like an obvious > "accept". > > Do you disagree with any of my assessments above? Did I miss anything > that should be factored in? Well, I think the picture you'v
Re: [Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
On 06/02/2019 10:56, Jan Beulich wrote:
> @@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
> {
> /*
> * Don't allocate less than the minimum acceptable, plus one page per
> - * megabyte of RAM (for the p2m table).
> + * megabyte of RAM (for the p2m table, minimally enough for HVM's setting
> + * up of slot zero and VMX's setting up of the LAPIC page), plus one for
> + * HVM's 1-to-1 pagetable.
What is in slot 0? Nothing comes to mind.
> */
> -return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
> +return shadow_min_acceptable_pages(d) +
> + max(d->tot_pages / 256,
> + is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
> +: 0U) +
I'm not sure cpu_has_vmx is the right check here. For one, there is a
series posted adding similar support for AMD, so this is going to be
stale shortly.
I'd err on the side of making it unconditional, but if you do want to be
as tight as possible, then cpu_has_vmx_apic_reg_virt is the correct
check for now.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
>>> On 06.02.19 at 13:00, wrote:
> On 06/02/2019 10:56, Jan Beulich wrote:
>> @@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
>> {
>> /*
>> * Don't allocate less than the minimum acceptable, plus one page per
>> - * megabyte of RAM (for the p2m table).
>> + * megabyte of RAM (for the p2m table, minimally enough for HVM's
>> setting
>> + * up of slot zero and VMX's setting up of the LAPIC page), plus one for
>> + * HVM's 1-to-1 pagetable.
>
> What is in slot 0? Nothing comes to mind.
I can only refer you to p2m_alloc_table(), which has
/* Initialise physmap tables for slot zero. Other code assumes this. */
p2m->defer_nested_flush = 1;
rc = p2m_set_entry(p2m, _gfn(0), INVALID_MFN, PAGE_ORDER_4K,
p2m_invalid, p2m->default_access);
p2m->defer_nested_flush = 0;
>> */
>> -return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
>> +return shadow_min_acceptable_pages(d) +
>> + max(d->tot_pages / 256,
>> + is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
>> +: 0U) +
>
> I'm not sure cpu_has_vmx is the right check here. For one, there is a
> series posted adding similar support for AMD, so this is going to be
> stale shortly.
>
> I'd err on the side of making it unconditional, but if you do want to be
> as tight as possible, then cpu_has_vmx_apic_reg_virt is the correct
> check for now.
Well, if anything then the qualifier vmx_alloc_vlapic_mapping() uses,
which is cpu_has_vmx_virtualize_apic_accesses (unless you tell me
that needs changing too). But if SVM is going to have a similar
requirement going forward, then making it unconditional is fine with
me.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.10-testing test] 132922: tolerable FAIL - PUSHED
flight 132922 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/132922/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 132630 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail 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-i386-libvirt 13 migrate-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-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-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-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail 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-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-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-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail 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-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 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 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 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 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 80c2955777ad0756dae5f0d31af9e531eb27c4d2 baseline version: xen 316e4426a185efefa078dd087c89a694b2149be8 Last test of basis 132630 2019-01-30 16:42:59 Z6 days Failing since132700 2019-02-01 11:06:46 Z5 days4 attempts Testing same since 132922 2019-02-05 05:40:31 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Juergen Gross Julien Grall Kevin Tian Razvan Cojocaru Sergey Dy
Re: [Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
>>> On 06.02.19 at 12:52, wrote:
> On Wed, Feb 06, 2019 at 03:56:49AM -0700, Jan Beulich wrote:
>> @@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
>> {
>> /*
>> * Don't allocate less than the minimum acceptable, plus one page per
>> - * megabyte of RAM (for the p2m table).
>> + * megabyte of RAM (for the p2m table, minimally enough for HVM's
>> setting
>> + * up of slot zero and VMX's setting up of the LAPIC page), plus one for
>> + * HVM's 1-to-1 pagetable.
>> */
>> -return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
>> +return shadow_min_acceptable_pages(d) +
>> + max(d->tot_pages / 256,
>> + is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
>> +: 0U) +
>> + is_hvm_domain(d);
>
> Should the call to shadow_set_allocation be changed so it attempts to
> allocate sh_min_allocation(d) + d->arch.paging.shadow.p2m_pages?
>
> It seems a little misleading to check whether there's a certain amount
> of pages in the pool (sh_min_allocation(d) +
> d->arch.paging.shadow.p2m_pages) and then set the allocation to 4M
> unconditionally.
Well, as said in the post-commit-message remark, I think we want to
get rid of this, but only with it properly replaced (which would likely
require hooking into paths increasing d->tot_pages). Right now all
we're after is dealing with the regression.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH RFC v3 2/2] x86/emulate: Send vm_event from emulate
This patch aims to have mem access vm events sent from the emulator.
This is useful in the case of page-walks that have to emulate
instructions in access denied pages.
We use hvmemul_map_linear_addr() ro intercept r/w access and
hvmemul_insn_fetch() to intercept exec access.
First we try to send a vm event and if the event is sent then emulation
returns X86EMUL_ACCESS_EXCEPTION. If the event is not sent then the
emulation goes on as expected.
Signed-off-by: Alexandru Isaila
---
Changes since V2:
- Join "if" statements where possible
- Correct indentation where needed
- Remove rc initialization.
---
xen/arch/x86/hvm/emulate.c | 106 -
xen/arch/x86/hvm/vm_event.c| 2 +-
xen/arch/x86/mm/mem_access.c | 3 +-
xen/arch/x86/x86_emulate/x86_emulate.h | 1 +
xen/include/asm-x86/hvm/emulate.h | 4 +-
5 files changed, 109 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index a766eecc8e..e4acbdf271 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -26,6 +27,7 @@
#include
#include
#include
+#include
static void hvmtrace_io_assist(const ioreq_t *p)
{
@@ -530,6 +532,55 @@ static int hvmemul_do_mmio_addr(paddr_t mmio_gpa,
return hvmemul_do_io_addr(1, mmio_gpa, reps, size, dir, df, ram_gpa);
}
+static bool hvmemul_send_vm_event(paddr_t gpa, unsigned long gla, gfn_t gfn,
+ uint32_t pfec, struct hvm_emulate_ctxt *ctxt)
+{
+xenmem_access_t access;
+vm_event_request_t req = {};
+
+if ( !ctxt->send_event || !pfec ||
+p2m_get_mem_access(current->domain, gfn, &access,
+ altp2m_vcpu_idx(current)) != 0 )
+return false;
+
+switch ( access )
+{
+case XENMEM_access_x:
+case XENMEM_access_rx:
+if ( pfec & PFEC_write_access )
+req.u.mem_access.flags = MEM_ACCESS_R | MEM_ACCESS_W;
+break;
+
+case XENMEM_access_w:
+case XENMEM_access_rw:
+if ( pfec & PFEC_insn_fetch )
+req.u.mem_access.flags = MEM_ACCESS_X;
+break;
+
+case XENMEM_access_r:
+case XENMEM_access_n:
+if ( pfec & PFEC_write_access )
+req.u.mem_access.flags |= MEM_ACCESS_R | MEM_ACCESS_W;
+if ( pfec & PFEC_insn_fetch )
+req.u.mem_access.flags |= MEM_ACCESS_X;
+break;
+
+default:
+return false;
+}
+
+if ( !req.u.mem_access.flags )
+return false; /* No violation. */
+
+req.reason = VM_EVENT_REASON_MEM_ACCESS;
+req.u.mem_access.gfn = gfn_x(gfn);
+req.u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA | MEM_ACCESS_GLA_VALID;
+req.u.mem_access.gla = gla;
+req.u.mem_access.offset = gpa & ((1 << PAGE_SHIFT) - 1);
+
+return monitor_traps(current, true, &req) >= 0;
+}
+
/*
* Convert addr from linear to physical form, valid over the range
* [addr, addr + *reps * bytes_per_rep]. *reps is adjusted according to
@@ -636,6 +687,7 @@ static void *hvmemul_map_linear_addr(
unsigned int nr_frames = ((linear + bytes - !!bytes) >> PAGE_SHIFT) -
(linear >> PAGE_SHIFT) + 1;
unsigned int i;
+gfn_t gfn;
/*
* mfn points to the next free slot. All used slots have a page reference
@@ -674,7 +726,7 @@ static void *hvmemul_map_linear_addr(
ASSERT(mfn_x(*mfn) == 0);
res = hvm_translate_get_page(curr, addr, true, pfec,
- &pfinfo, &page, NULL, &p2mt);
+ &pfinfo, &page, &gfn, &p2mt);
switch ( res )
{
@@ -704,6 +756,23 @@ static void *hvmemul_map_linear_addr(
if ( pfec & PFEC_write_access )
{
+unsigned long reps = 1;
+struct hvm_emulate_ctxt old;
+int rc = 0;
+paddr_t gpa;
+
+old = *hvmemul_ctxt;
+rc = hvmemul_linear_to_phys(addr, &gpa, bytes, &reps,
+pfec, hvmemul_ctxt);
+if ( rc == X86EMUL_EXCEPTION )
+*hvmemul_ctxt = old;
+
+if ( hvmemul_send_vm_event(gpa, addr, gfn, pfec, hvmemul_ctxt) )
+{
+err = ERR_PTR(~X86EMUL_ACCESS_EXCEPTION);
+goto out;
+}
+
if ( p2m_is_discard_write(p2mt) )
{
err = ERR_PTR(~X86EMUL_OKAY);
@@ -1224,7 +1293,35 @@ int hvmemul_insn_fetch(
container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
/* Careful, as offset can wrap or truncate WRT insn_buf_eip. */
uint8_t insn_off = offset - hvmemul_ctxt->insn_buf_eip;
+paddr_t gpa;
+uint32_t pfec = PFEC_page_present | PFEC_insn_fetch;
+unsigned long addr, reps = 1;
+int rc;
+struct hvm_emulate_ctxt old;
+
+rc = hvmemul_virtual_to_linear
[Xen-devel] [PATCH RFC v3 1/2] x86/emulate: Move hvmemul_linear_to_phys
This is done so hvmemul_linear_to_phys() can be called from
hvmemul_map_linear_addr().
There is no functional change.
Signed-off-by: Alexandru Isaila
---
xen/arch/x86/hvm/emulate.c | 181 ++---
1 file changed, 90 insertions(+), 91 deletions(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 2d02ef1521..a766eecc8e 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -530,6 +530,95 @@ static int hvmemul_do_mmio_addr(paddr_t mmio_gpa,
return hvmemul_do_io_addr(1, mmio_gpa, reps, size, dir, df, ram_gpa);
}
+/*
+ * Convert addr from linear to physical form, valid over the range
+ * [addr, addr + *reps * bytes_per_rep]. *reps is adjusted according to
+ * the valid computed range. It is always >0 when X86EMUL_OKAY is returned.
+ * @pfec indicates the access checks to be performed during page-table walks.
+ */
+static int hvmemul_linear_to_phys(
+unsigned long addr,
+paddr_t *paddr,
+unsigned int bytes_per_rep,
+unsigned long *reps,
+uint32_t pfec,
+struct hvm_emulate_ctxt *hvmemul_ctxt)
+{
+struct vcpu *curr = current;
+unsigned long pfn, npfn, done, todo, i, offset = addr & ~PAGE_MASK;
+int reverse;
+
+/*
+ * Clip repetitions to a sensible maximum. This avoids extensive looping in
+ * this function while still amortising the cost of I/O trap-and-emulate.
+ */
+*reps = min_t(unsigned long, *reps, 4096);
+
+/* With no paging it's easy: linear == physical. */
+if ( !(curr->arch.hvm.guest_cr[0] & X86_CR0_PG) )
+{
+*paddr = addr;
+return X86EMUL_OKAY;
+}
+
+/* Reverse mode if this is a backwards multi-iteration string operation. */
+reverse = (hvmemul_ctxt->ctxt.regs->eflags & X86_EFLAGS_DF) && (*reps > 1);
+
+if ( reverse && ((PAGE_SIZE - offset) < bytes_per_rep) )
+{
+/* Do page-straddling first iteration forwards via recursion. */
+paddr_t _paddr;
+unsigned long one_rep = 1;
+int rc = hvmemul_linear_to_phys(
+addr, &_paddr, bytes_per_rep, &one_rep, pfec, hvmemul_ctxt);
+if ( rc != X86EMUL_OKAY )
+return rc;
+pfn = _paddr >> PAGE_SHIFT;
+}
+else if ( (pfn = paging_gva_to_gfn(curr, addr, &pfec)) ==
gfn_x(INVALID_GFN) )
+{
+if ( pfec & (PFEC_page_paged | PFEC_page_shared) )
+return X86EMUL_RETRY;
+*reps = 0;
+x86_emul_pagefault(pfec, addr, &hvmemul_ctxt->ctxt);
+return X86EMUL_EXCEPTION;
+}
+
+done = reverse ? bytes_per_rep + offset : PAGE_SIZE - offset;
+todo = *reps * bytes_per_rep;
+for ( i = 1; done < todo; i++ )
+{
+/* Get the next PFN in the range. */
+addr += reverse ? -PAGE_SIZE : PAGE_SIZE;
+npfn = paging_gva_to_gfn(curr, addr, &pfec);
+
+/* Is it contiguous with the preceding PFNs? If not then we're done. */
+if ( (npfn == gfn_x(INVALID_GFN)) ||
+ (npfn != (pfn + (reverse ? -i : i))) )
+{
+if ( pfec & (PFEC_page_paged | PFEC_page_shared) )
+return X86EMUL_RETRY;
+done /= bytes_per_rep;
+if ( done == 0 )
+{
+ASSERT(!reverse);
+if ( npfn != gfn_x(INVALID_GFN) )
+return X86EMUL_UNHANDLEABLE;
+*reps = 0;
+x86_emul_pagefault(pfec, addr & PAGE_MASK,
&hvmemul_ctxt->ctxt);
+return X86EMUL_EXCEPTION;
+}
+*reps = done;
+break;
+}
+
+done += PAGE_SIZE;
+}
+
+*paddr = ((paddr_t)pfn << PAGE_SHIFT) | offset;
+return X86EMUL_OKAY;
+}
+
/*
* Map the frame(s) covering an individual linear access, for writeable
* access. May return NULL for MMIO, or ERR_PTR(~X86EMUL_*) for other errors
@@ -692,97 +781,7 @@ static void hvmemul_unmap_linear_addr(
*mfn++ = _mfn(0);
}
#endif
-}
-
-/*
- * Convert addr from linear to physical form, valid over the range
- * [addr, addr + *reps * bytes_per_rep]. *reps is adjusted according to
- * the valid computed range. It is always >0 when X86EMUL_OKAY is returned.
- * @pfec indicates the access checks to be performed during page-table walks.
- */
-static int hvmemul_linear_to_phys(
-unsigned long addr,
-paddr_t *paddr,
-unsigned int bytes_per_rep,
-unsigned long *reps,
-uint32_t pfec,
-struct hvm_emulate_ctxt *hvmemul_ctxt)
-{
-struct vcpu *curr = current;
-unsigned long pfn, npfn, done, todo, i, offset = addr & ~PAGE_MASK;
-int reverse;
-
-/*
- * Clip repetitions to a sensible maximum. This avoids extensive looping in
- * this function while still amortising the cost of I/O trap-and-emulate.
- */
-*reps = min_t(unsigned long, *reps, 4096);
-
-/* With no paging it's easy: linear == physical. */
-if ( !(curr->arch.hvm.guest_cr[0] & X86_CR0_PG) )
-{
-*paddr = addr;
-ret
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 4/9] spec: add l1tf-barrier
On 2/5/19 15:43, Jan Beulich wrote: On 05.02.19 at 15:23, wrote: >> On 1/31/19 17:35, Jan Beulich wrote: >> On 29.01.19 at 15:43, wrote: @@ -1942,6 +1942,12 @@ Irrespective of Xen's setting, the feature is >> virtualised for HVM guests to use. By default, Xen will enable this mitigation on hardware believed to be vulnerable to L1TF. +On hardware vulnerable to L1TF, the `l1tf-barrier=` option can be used to force +or prevent Xen from protecting evaluations inside the hypervisor with a barrier +instruction to not load potentially secret information into L1 cache. By +default, Xen will enable this mitigation on hardware believed to be vulnerable +to L1TF. >>> ... and having SMT enabled, since aiui this is a non-issue without. >> In case flushing the L1 cache is not enabled, that is still an issue, >> because the transition guest -> hypervisor -> guest would allow to >> retrieve hypervisor data from the cache still. Do you want me to extend >> the logic to consider L1 cache flushing as well? > Well, I wouldn't be overly concerned of people disabling it from the > command line, but being kind to people without updated microcode > is perhaps a good idea. I will extend the commit message to state that this the CPU flag is set automatically independently of SMT and cache flushing. > @@ -100,6 +102,7 @@ static int __init parse_spec_ctrl(const char *s) opt_ibpb = false; opt_ssbd = false; opt_l1d_flush = 0; +opt_l1tf_barrier = 0; } else if ( val > 0 ) rc = -EINVAL; >>> Is this really something we want "spec-ctrl=no-xen" to disable? >>> It would seem to me that this should be restricted to "spec-ctrl=no". >> I have no strong opinion here. If you ask me to move it somewhere else, >> I will do that. I just want to make sure it's disable in case >> speculation mitigations should be disabled. > Unless anyone else voices a different opinion, I'd like to see it > moved as suggested. I will move the change above the disable_common label. @@ -843,6 +849,14 @@ void __init init_speculation_mitigations(void) opt_l1d_flush = cpu_has_bug_l1tf && !(caps & ARCH_CAPS_SKIP_L1DFL); /* + * By default, enable L1TF_VULN on L1TF-vulnerable hardware + */ >>> This ought to be a single line comment. >> Will fix. +if ( opt_l1tf_barrier == -1 ) +opt_l1tf_barrier = cpu_has_bug_l1tf; >>> At the very least opt_smt should be taken into account here. But >>> I guess this setting of the default may need to be deferred >>> further, until the topology of the system is known (there may >>> not be any hyperthreads after all). >> Again, cache flushing also has to be considered. So, I would like to >> keep it like this for now. > With the "for now" aspect properly explained in the description, > I guess that would be fine as a first step. I will extend the commit message accordingly. > +if ( cpu_has_bug_l1tf && opt_l1tf_barrier > 0) +setup_force_cpu_cap(X86_FEATURE_SC_L1TF_VULN); >>> Why the left side of the &&? >> IMHO, the CPU flag L1TF should only be set when the CPU is reported to >> be vulnerable, even if the command line wants to enforce mitigations. > What's the command line option good for if it doesn't trigger > patching in of the LFENCEs? Command line options exist, among > other purposes, to aid mitigating flaws in our determination of > what is a vulnerable platform. I will remove the extra conditional and enable patching based on the command line only. > +/* * We do not disable HT by default on affected hardware. * * Firstly, if the user intends to use exclusively PV, or HVM shadow >>> Furthermore, as per the comment and logic here and below a >>> !HVM configuration ought to be safe too, unless "pv-l1tf=" was >>> used (in which case we defer to the admin anyway), so it's >>> questionable whether the whole logic should be there in the >>> first place in this case. This would then in particular keep all >>> of this out for the PV shim. >> For the PV shim, I could add pv-shim to my check before enabling the CPU >> flag. > But the PV shim is just a special case. I'd like this code to be > compiled out for all !HVM configurations. The that introduces the evaluate_nospec macro does that already. Based on defined(CONFIG_HVM) lfence patching is disabled there. Do you want me to wrap this command line option into CONFIG_HVM checks as well? Best, Norbert Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/x
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 4/9] spec: add l1tf-barrier
>>> On 06.02.19 at 14:02, wrote: > On 2/5/19 15:43, Jan Beulich wrote: > On 05.02.19 at 15:23, wrote: >>> On 1/31/19 17:35, Jan Beulich wrote: >>> On 29.01.19 at 15:43, wrote: > +/* > * We do not disable HT by default on affected hardware. > * > * Firstly, if the user intends to use exclusively PV, or HVM shadow Furthermore, as per the comment and logic here and below a !HVM configuration ought to be safe too, unless "pv-l1tf=" was used (in which case we defer to the admin anyway), so it's questionable whether the whole logic should be there in the first place in this case. This would then in particular keep all of this out for the PV shim. >>> For the PV shim, I could add pv-shim to my check before enabling the CPU >>> flag. >> But the PV shim is just a special case. I'd like this code to be >> compiled out for all !HVM configurations. > > The that introduces the evaluate_nospec macro does that already. Based > on defined(CONFIG_HVM) lfence patching is disabled there. Oh, right. > Do you want me to wrap this command line option into CONFIG_HVM checks > as well? That would be nice; I have a patch for post-4.12 where I do something similar to opt_xpti_*. Therefore if you didn't do it here, I'd probably submit a fixup patch down the road. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 for-4.12 00/17] Argo: hypervisor-mediated interdomain communication
>>> On 06.02.19 at 09:54, wrote: > Version eight of this series: > > Note: This version may not address the currently open discussion on the > ARM hypercall argument convention and type selection for hypercall > parameters. > > * Range check applied to numeric args in native hypercall entry > (ref: the above open discussion) > > * Revises the compat ABI and implementation > - avoids duplication of hypercall op implementations via > forwarding to native for ops other than sendv > - register op uses an always-64-bit fixed width pfn type > for consistent ABI as well as compat reuse of the native op > - tested communication between VMs on x86-64 host with: > 32-bit PV, 32-bit HVM and 64-bit PV guests > > * Applies list_first_entry_or_null macro in multiple loops to > replace previous use of a list foreach to address review feedback > > * Removed stale comments from the public header > > New to this series: > > * Adds an initial version of a design document for Argo > - based on work previously sent to the mailing list, covers > the implementation's granular locking > > * Adds a SUPPORT.md section for the feature and Experimental statement > > Christopher Clark (17): > argo: Introduce the Kconfig option to govern inclusion of Argo > argo: introduce the argo_op hypercall boilerplate > argo: define argo_dprintk for subsystem debugging > argo: init, destroy and soft-reset, with enable command line opt > errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI > xen/arm: introduce guest_handle_for_field() > argo: implement the register op > argo: implement the unregister op > argo: implement the sendv op; evtchn: expose send_guest_global_virq > argo: implement the notify op > xsm, argo: XSM control for argo register > xsm, argo: XSM control for argo message send operation > xsm, argo: XSM control for any access to argo by a domain > xsm, argo: notify: don't describe rings that cannot be sent to > MAINTAINERS: add new section for Argo and self as maintainer > SUPPORT.md : add new entry for the Argo feature > docs, argo: add design document for Argo Where necessary and not already present Acked-by: Jan Beulich Jürgen, for this to be committed, your Rab would be needed, assuming you're still comfortable with this going in this late. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 for-4.12 00/17] Argo: hypervisor-mediated interdomain communication
On 06/02/2019 14:45, Jan Beulich wrote: On 06.02.19 at 09:54, wrote: >> Version eight of this series: >> >> Note: This version may not address the currently open discussion on the >> ARM hypercall argument convention and type selection for hypercall >> parameters. >> >> * Range check applied to numeric args in native hypercall entry >> (ref: the above open discussion) >> >> * Revises the compat ABI and implementation >> - avoids duplication of hypercall op implementations via >> forwarding to native for ops other than sendv >> - register op uses an always-64-bit fixed width pfn type >> for consistent ABI as well as compat reuse of the native op >> - tested communication between VMs on x86-64 host with: >> 32-bit PV, 32-bit HVM and 64-bit PV guests >> >> * Applies list_first_entry_or_null macro in multiple loops to >> replace previous use of a list foreach to address review feedback >> >> * Removed stale comments from the public header >> >> New to this series: >> >> * Adds an initial version of a design document for Argo >> - based on work previously sent to the mailing list, covers >> the implementation's granular locking >> >> * Adds a SUPPORT.md section for the feature and Experimental statement >> >> Christopher Clark (17): >> argo: Introduce the Kconfig option to govern inclusion of Argo >> argo: introduce the argo_op hypercall boilerplate >> argo: define argo_dprintk for subsystem debugging >> argo: init, destroy and soft-reset, with enable command line opt >> errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI >> xen/arm: introduce guest_handle_for_field() >> argo: implement the register op >> argo: implement the unregister op >> argo: implement the sendv op; evtchn: expose send_guest_global_virq >> argo: implement the notify op >> xsm, argo: XSM control for argo register >> xsm, argo: XSM control for argo message send operation >> xsm, argo: XSM control for any access to argo by a domain >> xsm, argo: notify: don't describe rings that cannot be sent to >> MAINTAINERS: add new section for Argo and self as maintainer >> SUPPORT.md : add new entry for the Argo feature >> docs, argo: add design document for Argo > > Where necessary and not already present > Acked-by: Jan Beulich > > Jürgen, for this to be committed, your Rab would be needed, assuming > you're still comfortable with this going in this late. What about the ARM hypercall parameters? Is this settled? If yes or if this question is solved this week: Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12 5/8] pvh/dom0: warn when dom0_mem is not set to a fixed value
>>> On 30.01.19 at 11:36, wrote: > There have been several reports of the dom0 builder running out of > memory when buildign a PVH dom0 without havingf specified a dom0_mem "building" and "having" > value. Print a warning message if dom0_mem is not set to a fixed value > when booting in PVH mode. Why does it need to be a fixed value? I.e. why can't you simply put this warning next to where the default gets established, when nr_pages is zero? > --- a/xen/arch/x86/dom0_build.c > +++ b/xen/arch/x86/dom0_build.c > @@ -344,6 +344,10 @@ unsigned long __init dom0_compute_nr_pages( > if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] ) > parse_dom0_mem(CONFIG_DOM0_MEM); > > +if ( is_hvm_domain(d) && !dom0_size.nr_pages ) > +printk( > +"WARNING: consider setting dom0_mem to a fixed value when using PVH mode\n"); Pretty unusual indentation. Is there any reason for you doing so? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 for-4.12 00/17] Argo: hypervisor-mediated interdomain communication
>>> On 06.02.19 at 14:53, wrote: > On 06/02/2019 14:45, Jan Beulich wrote: > On 06.02.19 at 09:54, wrote: >>> Version eight of this series: >>> >>> Note: This version may not address the currently open discussion on the >>> ARM hypercall argument convention and type selection for hypercall >>> parameters. >>> >>> * Range check applied to numeric args in native hypercall entry >>> (ref: the above open discussion) >>> >>> * Revises the compat ABI and implementation >>> - avoids duplication of hypercall op implementations via >>> forwarding to native for ops other than sendv >>> - register op uses an always-64-bit fixed width pfn type >>> for consistent ABI as well as compat reuse of the native op >>> - tested communication between VMs on x86-64 host with: >>> 32-bit PV, 32-bit HVM and 64-bit PV guests >>> >>> * Applies list_first_entry_or_null macro in multiple loops to >>> replace previous use of a list foreach to address review feedback >>> >>> * Removed stale comments from the public header >>> >>> New to this series: >>> >>> * Adds an initial version of a design document for Argo >>> - based on work previously sent to the mailing list, covers >>> the implementation's granular locking >>> >>> * Adds a SUPPORT.md section for the feature and Experimental statement >>> >>> Christopher Clark (17): >>> argo: Introduce the Kconfig option to govern inclusion of Argo >>> argo: introduce the argo_op hypercall boilerplate >>> argo: define argo_dprintk for subsystem debugging >>> argo: init, destroy and soft-reset, with enable command line opt >>> errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI >>> xen/arm: introduce guest_handle_for_field() >>> argo: implement the register op >>> argo: implement the unregister op >>> argo: implement the sendv op; evtchn: expose send_guest_global_virq >>> argo: implement the notify op >>> xsm, argo: XSM control for argo register >>> xsm, argo: XSM control for argo message send operation >>> xsm, argo: XSM control for any access to argo by a domain >>> xsm, argo: notify: don't describe rings that cannot be sent to >>> MAINTAINERS: add new section for Argo and self as maintainer >>> SUPPORT.md : add new entry for the Argo feature >>> docs, argo: add design document for Argo >> >> Where necessary and not already present >> Acked-by: Jan Beulich >> >> Jürgen, for this to be committed, your Rab would be needed, assuming >> you're still comfortable with this going in this late. > > What about the ARM hypercall parameters? Is this settled? My interpretation of Stefano's latest response was "yes, it is". > If yes or if this question is solved this week: > > Release-acked-by: Juergen Gross Thanks, I'll ask for last minute objections on irc and commit later this afternoon if I hear none. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH Makefile v2] asm: handle comments when creating header file
From: Norbert Manthey
In the early steps of compilation, the asm header files are created, such
as include/asm-$(TARGET_ARCH)/asm-offsets.h. These files depend on the
assembly file arch/$(TARGET_ARCH)/asm-offsets.s, which is generated
before. Depending on the used toolchain, there might be comments in the
assembly files. Especially the goto-gcc compiler of the bounded model
checker CBMC adds comments that start with a '#' symbol at the beginning
of the line.
This commit adds handling comments in assembler during the creation of the
asm header files, especially ignoring lines that start with '#', which
indicate comments for both ARM and x86 assembler. The used tool goto-as
produces exactly comments of this kind.
Signed-off-by: Norbert Manthey
Signed-off-by: Michael Tautschnig
---
xen/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/Makefile b/xen/Makefile
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -191,7 +191,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h:
arch/$(TARGET_ARCH)/asm-offsets.s
echo "#ifndef __ASM_OFFSETS_H__"; \
echo "#define __ASM_OFFSETS_H__"; \
echo ""; \
- sed -rne "/==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \
+ sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \
echo ""; \
echo "#endif") <$< >$@
--
2.7.4
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 test] 132919: regressions - trouble: blocked/broken/fail/pass
flight 132919 linux-3.18 real [real] http://logs.test-lab.xenproject.org/osstest/logs/132919/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-libvirt-raw broken test-amd64-i386-examine 8 reboot fail REGR. vs. 128858 test-amd64-i386-libvirt-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-libvirt 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-debianhvm-amd64 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-credit2 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 128858 test-amd64-amd64-xl 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 128858 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl7 xen-boot fail REGR. vs. 128858 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host fail REGR. vs. 128858 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host fail REGR. vs. 128858 test-amd64-amd64-qemuu-nested-intel 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-i386-pvgrub 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-rumprun-amd64 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 128858 test-amd64-amd64-xl-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 128858 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 128858 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 128858 Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt-raw 4 host-install(4) broken pass in 132798 test-armhf-armhf-libvirt 16 guest-start/debian.repeat fail in 132579 pass in 132798 test-armhf-armhf-libvirt-raw 11 guest-start fail in 132798 pass in 132652 test-amd64-i386-libvirt-pair 10 xen-boot/src_host fail pass in 132579 test-amd64-i386-libvirt-pair 11 xen-boot/dst_host fail pass in 132579 test-armhf-armhf-libvirt 7 xen-boot fail pass in 132798 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail pass in 132798 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 132798 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail pass in 132798 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 128858 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 13 saverestore-support-check fail in 132579 like 128858 test-armhf-armhf-libvirt 14 saverestore-support-check fail in 132579 like 128858 test-armhf-armhf-libvirt-raw 12 migrate-support-check fail in 132579 never pass test-armhf-armhf-libvirt13 migrate-support-check fail in 132579 never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail in 132579 never pass test-armhf-armhf-xl-vhd 12 migrate-support-check fail in 132579 never pass test-armhf-armhf-xl-vhd 13 saverestore-support-check fail in 132579 never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail in 132579 never pass test-amd64-amd64-examine 4 memdisk-try-append fail in 132652 li
Re: [Xen-devel] [PATCH RFC 0/6] Slotted channels for sync vm_events
On Wed, 2018-12-19 at 20:52 +0200, Petre Pircalabu wrote: > This patchset is a rework of the "multi-page ring buffer" for > vm_events > patch based on Andrew Cooper's comments. > For synchronous vm_events the ring waitqueue logic was unnecessary as > the > vcpu sending the request was blocked until a response was received. > To simplify the request/response mechanism, an array of slotted > channels > was created, one per vcpu. Each vcpu puts the request in the > corresponding slot and blocks until the response is received. > > I'm sending this patch as a RFC because, while I'm still working on > way to > measure the overall performance improvement, your feedback would be a > great > assistance. > Is anyone still using asynchronous vm_event requests? (the vcpu is not blocked and no response is expected). If not, I suggest that the feature should be removed as it (significantly) increases the complexity of the current implementation. Many thanks, Petre ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] XEN on R-CAR H3
HI, Trying to boot XEN on R-CAR H3 starter Kit board. Linux image based on 5.0.0-rc5 and XEN image is 4.12 tftp 0x4800 xen;tftp 0x7a00 Image; tftp 4a00 r8a7795-h3ulcb.dtb setenv xen_addr_r 0x4800 setenv fdt_addr_r 4a00 setenv kernel_addr_r 0x7a00 fdt addr $fdt_addr_r fdt resize fdt set /chosen xen,xen-bootargs "console=dtuart dom0_mem=384M" fdt set /chosen \#address-cells <1> fdt set /chosen \#size-cells <1> fdt mknod /chosen module@0 fdt resize fdt set /chosen/module@0 compatible "xen,linux-zimage" "xen,multiboot-module" fdt set /chosen/module@0 reg <$kernel_addr_r 0x180> setenv bootargs "console=hvc0 ro root=/dev/mmcblk0p2 clk_ignore_unused rootwait earlycon=xenboot" But when It boot it, I see following crash: booti 0x4800 - 0x4a00 ## Flattened Device Tree blob at 4a00 Booting using the fdt blob at 0x4a00 reserving fdt memory region: addr=4a00 size=1 Using Device Tree in place at 4a00, end 4a012fff Starting kernel ... UART enabled - - CPU booting - - Current EL 0008 - - Xen starting at EL2 - - Zero BSS - - Setting up control registers - - Turning on paging - - Ready - (XEN) Checking for initrd in /chosen (XEN) RAM: 4800 - 7fff (XEN) RAM: 0005 - 00053fff (XEN) RAM: 0006 - 00063fff (XEN) RAM: 0007 - 00073fff (XEN) RAM: 0005 - 00053fff (XEN) RAM: 0006 - 00063fff (XEN) RAM: 0007 - 00073fff (XEN) (XEN) MODULE[0]: 4a00 - 4a01 Device Tree (XEN) MODULE[1]: 7a00 - 7b80 Kernel (XEN) RESVD[0]: 4a00 - 4a01 (XEN) (XEN) (XEN) Command line: console=dtuart dtuart=serial0 dom0_mem=384M (XEN) PFN compression on bits 19...19 (XEN) Xen BUG at page_alloc.c:274 (XEN) [ Xen-4.12.0-rc arm64 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 0028b30c page_alloc.c#bootmem_region_add+0x184/0x194 (XEN) LR: 0028b36c (XEN) SP: 002d7d00 (XEN) CPSR: 83c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0050 X1: 0003 X2: 0006 (XEN) X3: 0054 X4: 88121000 X5: 0028 (XEN) X6: 0060 X7: 0004 X8: 0001 (XEN) X9: 000a X10: 002d7afa X11: 0031 (XEN) X12: 0002 X13: 0026e5e8 X14: 0020 (XEN) X15: 0040 X16: X17: (XEN) X18: 7d726e08 X19: 0050 X20: 0054 (XEN) X21: 00054000 X22: 0005 X23: 00054000 (XEN) X24: 0028b31c X25: 002b8400 X26: 4800 (XEN) X27: 00074000 X28: 0004 FP: 002d7d00 (XEN) (XEN) VTCR_EL2: 8000 (XEN) VTTBR_EL2: (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN)HCR_EL2: 0038 (XEN) TTBR0_EL2: 48114000 (XEN) (XEN)ESR_EL2: f201 (XEN) HPFAR_EL2: (XEN)FAR_EL2: (XEN) (XEN) Xen stack trace from sp=002d7d00: (XEN)002d7d40 0028b36c 0001 0005 (XEN)00054000 0005 00054000 002d7d90 (XEN)002d7d90 0029c8f8 0001 0005 (XEN)00054000 0005 00054000 0001 (XEN)002e 0005 002d7de0 0029dac0 (XEN)00054000 0005 00054000 002b83c0 (XEN) 002b83d8 4a00 4a01 (XEN)7d726a90 00200608 4800 47e0 (XEN)4a00 0040 (XEN)0001 (XEN) 0001 4a00 00013800 (XEN)002b83c0 0028b31c (XEN) 0003 0440 (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN)
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 8/9] common/grant_table: block speculative out-of-bound accesses
>>> On 29.01.19 at 15:43, wrote: > @@ -963,6 +965,9 @@ map_grant_ref( > PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", > op->ref, rgt->domain->domain_id); > > +/* Make sure the above check is not bypassed speculatively */ > +op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt)); > + > act = active_entry_acquire(rgt, op->ref); > shah = shared_entry_header(rgt, op->ref); > status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, > op->ref); Just FTR - this is a case where the change, according to prior discussion, is pretty unlikely to help at all. The compiler will have a hard time realizing that it could keep the result in a register past the active_entry_acquire() invocation, as that - due to the spin lock acquired there - acts as a compiler barrier. And looking at generated code (gcc 8.2) confirms that there's a reload from the stack. > @@ -2026,6 +2031,9 @@ gnttab_prepare_for_transfer( > goto fail; > } > > +/* Make sure the above check is not bypassed speculatively */ > +ref = array_index_nospec(ref, nr_grant_entries(rgt)); > + > sha = shared_entry_header(rgt, ref); > > scombo.word = *(u32 *)&sha->flags; > @@ -2223,7 +2231,8 @@ gnttab_transfer( > okay = gnttab_prepare_for_transfer(e, d, gop.ref); > spin_lock(&e->page_alloc_lock); > > -if ( unlikely(!okay) || unlikely(e->is_dying) ) > +/* Make sure this check is not bypassed speculatively */ > +if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) ) I'm still not really happy about this. The comment isn't helpful in connecting the use of evaluate_nospec() to the problem site (in the earlier hunk, which I've left in context), and I still don't understand why the e->is_dying is getting wrapped as well. Plus it occurs to me now that you're liable to render unlikely() ineffective here. So how about if ( unlikely(evaluate_nospec(!okay)) || unlikely(e->is_dying) ) ? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 6/9] is_control_domain: block speculation
>>> On 29.01.19 at 15:43, wrote:
> @@ -908,10 +909,10 @@ void watchdog_domain_destroy(struct domain *d);
> *(that is, this would not be suitable for a driver domain)
> * - There is never a reason to deny the hardware domain access to this
> */
> -#define is_hardware_domain(_d) ((_d) == hardware_domain)
> +#define is_hardware_domain(_d) evaluate_nospec((_d) == hardware_domain)
>
> /* This check is for functionality specific to a control domain */
> -#define is_control_domain(_d) ((_d)->is_privileged)
> +#define is_control_domain(_d) evaluate_nospec((_d)->is_privileged)
I'm afraid there's another fly in the ointment here: While looking at
the still questionable grant table change I've started wondering
about constructs like
case XENMEM_machphys_mapping:
{
struct xen_machphys_mapping mapping = {
.v_start = MACH2PHYS_VIRT_START,
.v_end = MACH2PHYS_VIRT_END,
.max_mfn = MACH2PHYS_NR_ENTRIES - 1
};
if ( !mem_hotplug && is_hardware_domain(current->domain) )
mapping.max_mfn = max_page - 1;
if ( copy_to_guest(arg, &mapping, 1) )
return -EFAULT;
return 0;
}
Granted the example here could be easily re-arranged, but there
are others where this is less easy or not possible at all. What I'm
trying to get at are constructs where the such-protected
predicates sit on the right side of && or || - afaict (also from
looking at some much simplified code examples) the intended
protection is gone in these cases.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] XEN on R-CAR H3
On 06.02.19 16:41, Amit Tomer wrote:
HI,
Hi
Trying to boot XEN on R-CAR H3 starter Kit board.
Linux image based on 5.0.0-rc5 and XEN image is 4.12
tftp 0x4800 xen;tftp 0x7a00 Image; tftp 4a00 r8a7795-h3ulcb.dtb
setenv xen_addr_r 0x4800
setenv fdt_addr_r 4a00
setenv kernel_addr_r 0x7a00
fdt addr $fdt_addr_r
fdt resize
fdt set /chosen xen,xen-bootargs "console=dtuart dom0_mem=384M"
fdt set /chosen \#address-cells <1>
fdt set /chosen \#size-cells <1>
fdt mknod /chosen module@0
fdt resize
fdt set /chosen/module@0 compatible "xen,linux-zimage" "xen,multiboot-module"
fdt set /chosen/module@0 reg <$kernel_addr_r 0x180>
setenv bootargs "console=hvc0 ro root=/dev/mmcblk0p2 clk_ignore_unused
rootwait earlycon=xenboot"
But when It boot it, I see following crash:
booti 0x4800 - 0x4a00
## Flattened Device Tree blob at 4a00
Booting using the fdt blob at 0x4a00
reserving fdt memory region: addr=4a00 size=1
Using Device Tree in place at 4a00, end
4a012fff
Starting kernel ...
UART enabled -
- CPU booting -
- Current EL 0008 -
- Xen starting at EL2 -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Checking for initrd in /chosen
(XEN) RAM: 4800 - 7fff
(XEN) RAM: 0005 - 00053fff
(XEN) RAM: 0006 - 00063fff
(XEN) RAM: 0007 - 00073fff
(XEN) RAM: 0005 - 00053fff
(XEN) RAM: 0006 - 00063fff
(XEN) RAM: 0007 - 00073fff
Memory nodes got duplicated somehow. Likely U-Boot did something incorrect.
Try to use single memory node in your device-tree instead of separated
by each bank nodes:
memory@4800 {
device_type = "memory";
/* first 128MB is reserved for secure area. */
reg = <0x0 0x4800 0x0 0x3800>,
<0x5 0x 0x0 0x4000>,
<0x6 0x 0x0 0x4000>,
<0x7 0x 0x0 0x4000>;
};
--
Regards,
Oleksandr Tyshchenko
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 8/9] common/grant_table: block speculative out-of-bound accesses
On 2/6/19 15:52, Jan Beulich wrote: On 29.01.19 at 15:43, wrote: >> @@ -963,6 +965,9 @@ map_grant_ref( >> PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", >> op->ref, rgt->domain->domain_id); >> >> +/* Make sure the above check is not bypassed speculatively */ >> +op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt)); >> + >> act = active_entry_acquire(rgt, op->ref); >> shah = shared_entry_header(rgt, op->ref); >> status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, >> op->ref); > Just FTR - this is a case where the change, according to prior > discussion, is pretty unlikely to help at all. The compiler will have > a hard time realizing that it could keep the result in a register past > the active_entry_acquire() invocation, as that - due to the spin > lock acquired there - acts as a compiler barrier. And looking at > generated code (gcc 8.2) confirms that there's a reload from the > stack. I could change this back to a prior version that protects each read operation. >> @@ -2026,6 +2031,9 @@ gnttab_prepare_for_transfer( >> goto fail; >> } >> >> +/* Make sure the above check is not bypassed speculatively */ >> +ref = array_index_nospec(ref, nr_grant_entries(rgt)); >> + >> sha = shared_entry_header(rgt, ref); >> >> scombo.word = *(u32 *)&sha->flags; >> @@ -2223,7 +2231,8 @@ gnttab_transfer( >> okay = gnttab_prepare_for_transfer(e, d, gop.ref); >> spin_lock(&e->page_alloc_lock); >> >> -if ( unlikely(!okay) || unlikely(e->is_dying) ) >> +/* Make sure this check is not bypassed speculatively */ >> +if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) ) > I'm still not really happy about this. The comment isn't helpful in > connecting the use of evaluate_nospec() to the problem site > (in the earlier hunk, which I've left in context), and I still don't > understand why the e->is_dying is getting wrapped as well. > Plus it occurs to me now that you're liable to render unlikely() > ineffective here. So how about > > if ( unlikely(evaluate_nospec(!okay)) || unlikely(e->is_dying) ) > > ? I will move the evaluate_nospec closer to the evaluation of okay, and will improve the comment mentioning that the okay variable represents whether the current reference is actually valid. Best, Norbert Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH Makefile v2] asm: handle comments when creating header file
>>> On 06.02.19 at 15:09, wrote: > From: Norbert Manthey > > In the early steps of compilation, the asm header files are created, such > as include/asm-$(TARGET_ARCH)/asm-offsets.h. These files depend on the > assembly file arch/$(TARGET_ARCH)/asm-offsets.s, which is generated > before. Depending on the used toolchain, there might be comments in the > assembly files. Especially the goto-gcc compiler of the bounded model > checker CBMC adds comments that start with a '#' symbol at the beginning > of the line. > > This commit adds handling comments in assembler during the creation of the > asm header files, especially ignoring lines that start with '#', which > indicate comments for both ARM and x86 assembler. The used tool goto-as > produces exactly comments of this kind. > > Signed-off-by: Norbert Manthey > Signed-off-by: Michael Tautschnig Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] XEN on R-CAR H3
Hi,Thanks for prompt reply.
> Memory nodes got duplicated somehow. Likely U-Boot did something incorrect.
>
> Try to use single memory node in your device-tree instead of separated
> by each bank nodes:
>
> memory@4800 {
> device_type = "memory";
> /* first 128MB is reserved for secure area. */
> reg = <0x0 0x4800 0x0 0x3800>,
><0x5 0x 0x0 0x4000>,
><0x6 0x 0x0 0x4000>,
><0x7 0x 0x0 0x4000>;
> };
>
> --
Sorry, I should have read the other thread completely where Andrii
Anisov has suggested the same.
Would try changes mentioned by you.
Thanks
-Amit
> Regards,
>
> Oleksandr Tyshchenko
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 132967: tolerable all pass - PUSHED
flight 132967 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/132967/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: 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 13 migrate-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 6b6ff07fbd9ec17586d1f52fac61e87e2e58 baseline version: xen 8aa276235b93eeb4f81095c638970900e19b31e5 Last test of basis 132947 2019-02-05 20:00:42 Z0 days Testing same since 132967 2019-02-06 13:00:36 Z0 days1 attempts People who touched revisions under test: Ian Jackson 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 8aa276235b..6b6ff07fbd 6b6ff07fbd9ec17586d1f52fac61e87e2e58 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 9/9] common/memory: block speculative out-of-bound accesses
>>> On 29.01.19 at 15:43, wrote:
> @@ -33,10 +34,10 @@ unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
>
> bool __mfn_valid(unsigned long mfn)
> {
> -return likely(mfn < max_page) &&
> - likely(!(mfn & pfn_hole_mask)) &&
> - likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
> - pdx_group_valid));
> +return evaluate_nospec(likely(mfn < max_page) &&
> + likely(!(mfn & pfn_hole_mask)) &&
> + likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
> + pdx_group_valid)));
Other than in the questionable grant table case, here I agree that
you want to wrap the entire construct. This has an unwanted effect
though: The test_bit() may still be speculated into with an out-of-
bounds mfn. (As mentioned elsewhere, operations on bit arrays are
an open issue altogether.) I therefore think you want to split this into
two:
bool __mfn_valid(unsigned long mfn)
{
return likely(evaluate_nospec(mfn < max_page)) &&
evaluate_nospec(likely(!(mfn & pfn_hole_mask)) &&
likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
pdx_group_valid)));
}
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ping: libfsimage path/file name changes
Ian, >>> On 28.01.19 at 08:55, wrote: > back in October you've added quite a number of "xen" prefixes to > various pieces there. Now that I've finally had time to connect this > change of yours with PV domain creation failures that I've since > been observing (not a bug in any way, merely resulting from the > fact that I'm running everything straight from the build tree) I > started wondering whether the environment variable inspected > by common/fsimage_plugin.c:load_plugins() shouldn't then also > gain a "XEN" prefix. I think this wants a resolution one way or the other for 4.12, i.e. either we stick to what is there also after 4.12, or we do the additional change for the same release as where all your other changes have been made. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 6/9] is_control_domain: block speculation
On 2/6/19 16:03, Jan Beulich wrote:
On 29.01.19 at 15:43, wrote:
>> @@ -908,10 +909,10 @@ void watchdog_domain_destroy(struct domain *d);
>> *(that is, this would not be suitable for a driver domain)
>> * - There is never a reason to deny the hardware domain access to this
>> */
>> -#define is_hardware_domain(_d) ((_d) == hardware_domain)
>> +#define is_hardware_domain(_d) evaluate_nospec((_d) == hardware_domain)
>>
>> /* This check is for functionality specific to a control domain */
>> -#define is_control_domain(_d) ((_d)->is_privileged)
>> +#define is_control_domain(_d) evaluate_nospec((_d)->is_privileged)
> I'm afraid there's another fly in the ointment here: While looking at
> the still questionable grant table change I've started wondering
> about constructs like
>
> case XENMEM_machphys_mapping:
> {
> struct xen_machphys_mapping mapping = {
> .v_start = MACH2PHYS_VIRT_START,
> .v_end = MACH2PHYS_VIRT_END,
> .max_mfn = MACH2PHYS_NR_ENTRIES - 1
> };
>
> if ( !mem_hotplug && is_hardware_domain(current->domain) )
> mapping.max_mfn = max_page - 1;
> if ( copy_to_guest(arg, &mapping, 1) )
> return -EFAULT;
>
> return 0;
> }
>
> Granted the example here could be easily re-arranged, but there
> are others where this is less easy or not possible at all. What I'm
> trying to get at are constructs where the such-protected
> predicates sit on the right side of && or || - afaict (also from
> looking at some much simplified code examples) the intended
> protection is gone in these cases.
I do not follow this. Independently of other conditionals in the if
statement, there should be an lfence instruction between the
"is_domain_control(...)" evaluation and accessing the max_page variable
- in case the code actually protects accessing that variable via that
function.
I validated this property for the above code snippet in the generated
assembly. However, I just noticed another problem: while my initial
version just placed the lfence instruction right into the code, not the
arch_barrier_nospec_true method is called via callq. I would like to get
the instructions to be embedded into the code directly, without the call
detour. In case I cannot force the compiler to do that, I would go back
to using a fixed lfence statement on all x86 platforms.
Best,
Norbert
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 9/9] common/memory: block speculative out-of-bound accesses
On 2/6/19 16:25, Jan Beulich wrote:
On 29.01.19 at 15:43, wrote:
>> @@ -33,10 +34,10 @@ unsigned long __read_mostly
>> pdx_group_valid[BITS_TO_LONGS(
>>
>> bool __mfn_valid(unsigned long mfn)
>> {
>> -return likely(mfn < max_page) &&
>> - likely(!(mfn & pfn_hole_mask)) &&
>> - likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
>> - pdx_group_valid));
>> +return evaluate_nospec(likely(mfn < max_page) &&
>> + likely(!(mfn & pfn_hole_mask)) &&
>> + likely(test_bit(pfn_to_pdx(mfn) /
>> PDX_GROUP_COUNT,
>> + pdx_group_valid)));
> Other than in the questionable grant table case, here I agree that
> you want to wrap the entire construct. This has an unwanted effect
> though: The test_bit() may still be speculated into with an out-of-
> bounds mfn. (As mentioned elsewhere, operations on bit arrays are
> an open issue altogether.) I therefore think you want to split this into
> two:
>
> bool __mfn_valid(unsigned long mfn)
> {
> return likely(evaluate_nospec(mfn < max_page)) &&
>evaluate_nospec(likely(!(mfn & pfn_hole_mask)) &&
>likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
>pdx_group_valid)));
> }
I can split the code. However, I wonder whether the test_bit accesses
should be protected separately, or actually as part of the test_bit
method themselves. Do you have any plans to do that already, because in
that case I would not have to modify the code.
Best,
Norbert
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> As per my earlier reply, I've yet to see proof of a "code-breaking
> optimization" that actually matches our case(s).
I have personally experienced a program being miscompiled because of
the mistaken belief by the compiler that _start and _end were
different objects. I have read haven't read back the whole history of
this but this is definitely a real bug.
I agree with George that this is a compiler bug. However, this bug is
not going to be fixed because the compiler community's behaviour is as
unreasonable as George paints :-(.
Our only option is some kind of bodge.
I don't believe in the asm fragment bodge because we don't have a
promise anywhere from the compiler authors that the asm hides pointer
provenance. I am not aware of any C technique which can be reliably
used to obscure pointer provenance and prevent this kind of
misoptimisation. Linux is skating on thin ice here.
That leaves:
(i) define indirection variables eg end_ in an assembly language file.
(ii) convert to uintptr_t before comparing
(i) is IMO wholly safe but it is a bit ugly and slightly less
performant.
I think (ii) is fairly safe. I doubt that we will find (ii) broken.
In particular, because there is little motivation for compiler authors
to try to `optimise it'. The difficulty with it providing automatic
way of detecting when we accidentallyf fail to cast. I suggest the
following:
extern const struct wombat _wombats_start[];
extern const struct abstract_symbol _wombats_end[];
and providing a macro which compares any pointer with a struct
abstract_symbol* by converting the latter to a uintptr_t. Direct
comparisons of _wombats_start with _wombats_end will result in a
compilation error due to type mismatch.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 8/9] common/grant_table: block speculative out-of-bound accesses
>>> On 06.02.19 at 16:06, wrote: > On 2/6/19 15:52, Jan Beulich wrote: > On 29.01.19 at 15:43, wrote: >>> @@ -963,6 +965,9 @@ map_grant_ref( >>> PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", >>> op->ref, rgt->domain->domain_id); >>> >>> +/* Make sure the above check is not bypassed speculatively */ >>> +op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt)); >>> + >>> act = active_entry_acquire(rgt, op->ref); >>> shah = shared_entry_header(rgt, op->ref); >>> status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt, >>> op->ref); >> Just FTR - this is a case where the change, according to prior >> discussion, is pretty unlikely to help at all. The compiler will have >> a hard time realizing that it could keep the result in a register past >> the active_entry_acquire() invocation, as that - due to the spin >> lock acquired there - acts as a compiler barrier. And looking at >> generated code (gcc 8.2) confirms that there's a reload from the >> stack. > I could change this back to a prior version that protects each read > operation. That or use block_speculation() with a comment explaining why. Also - why are there no changes at all to the unmap_grant_ref() / unmap_and_replace() call paths? Note in particular the security related comment next to the bounds check of op->ref there. I've gone through earlier review rounds, but I couldn't find an indication that this might have been the result of review feedback. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 6/9] is_control_domain: block speculation
>>> On 06.02.19 at 16:36, wrote:
> On 2/6/19 16:03, Jan Beulich wrote:
> On 29.01.19 at 15:43, wrote:
>>> @@ -908,10 +909,10 @@ void watchdog_domain_destroy(struct domain *d);
>>> *(that is, this would not be suitable for a driver domain)
>>> * - There is never a reason to deny the hardware domain access to this
>>> */
>>> -#define is_hardware_domain(_d) ((_d) == hardware_domain)
>>> +#define is_hardware_domain(_d) evaluate_nospec((_d) == hardware_domain)
>>>
>>> /* This check is for functionality specific to a control domain */
>>> -#define is_control_domain(_d) ((_d)->is_privileged)
>>> +#define is_control_domain(_d) evaluate_nospec((_d)->is_privileged)
>> I'm afraid there's another fly in the ointment here: While looking at
>> the still questionable grant table change I've started wondering
>> about constructs like
>>
>> case XENMEM_machphys_mapping:
>> {
>> struct xen_machphys_mapping mapping = {
>> .v_start = MACH2PHYS_VIRT_START,
>> .v_end = MACH2PHYS_VIRT_END,
>> .max_mfn = MACH2PHYS_NR_ENTRIES - 1
>> };
>>
>> if ( !mem_hotplug && is_hardware_domain(current->domain) )
>> mapping.max_mfn = max_page - 1;
>> if ( copy_to_guest(arg, &mapping, 1) )
>> return -EFAULT;
>>
>> return 0;
>> }
>>
>> Granted the example here could be easily re-arranged, but there
>> are others where this is less easy or not possible at all. What I'm
>> trying to get at are constructs where the such-protected
>> predicates sit on the right side of && or || - afaict (also from
>> looking at some much simplified code examples) the intended
>> protection is gone in these cases.
>
> I do not follow this. Independently of other conditionals in the if
> statement, there should be an lfence instruction between the
> "is_domain_control(...)" evaluation and accessing the max_page variable
> - in case the code actually protects accessing that variable via that
> function.
Hmm, yes, I may have been confused by looking at the && and ||
variants of the generated code, and mixing up the cases.
> I validated this property for the above code snippet in the generated
> assembly. However, I just noticed another problem: while my initial
> version just placed the lfence instruction right into the code, not the
> arch_barrier_nospec_true method is called via callq. I would like to get
> the instructions to be embedded into the code directly, without the call
> detour. In case I cannot force the compiler to do that, I would go back
> to using a fixed lfence statement on all x86 platforms.
I think we had made pretty clear that incurring the overhead even
onto unaffected platforms is not an option. Did you try whether
adding always_inline helps? (I take it that this is another case of
the size-of-asm issue that's being worked on in Linux as well iirc.)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v5 9/9] common/memory: block speculative out-of-bound accesses
>>> On 06.02.19 at 16:39, wrote:
> On 2/6/19 16:25, Jan Beulich wrote:
> On 29.01.19 at 15:43, wrote:
>>> @@ -33,10 +34,10 @@ unsigned long __read_mostly
>>> pdx_group_valid[BITS_TO_LONGS(
>>>
>>> bool __mfn_valid(unsigned long mfn)
>>> {
>>> -return likely(mfn < max_page) &&
>>> - likely(!(mfn & pfn_hole_mask)) &&
>>> - likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
>>> - pdx_group_valid));
>>> +return evaluate_nospec(likely(mfn < max_page) &&
>>> + likely(!(mfn & pfn_hole_mask)) &&
>>> + likely(test_bit(pfn_to_pdx(mfn) /
>>> PDX_GROUP_COUNT,
>>> + pdx_group_valid)));
>> Other than in the questionable grant table case, here I agree that
>> you want to wrap the entire construct. This has an unwanted effect
>> though: The test_bit() may still be speculated into with an out-of-
>> bounds mfn. (As mentioned elsewhere, operations on bit arrays are
>> an open issue altogether.) I therefore think you want to split this into
>> two:
>>
>> bool __mfn_valid(unsigned long mfn)
>> {
>> return likely(evaluate_nospec(mfn < max_page)) &&
>>evaluate_nospec(likely(!(mfn & pfn_hole_mask)) &&
>>likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT,
>>pdx_group_valid)));
>> }
>
> I can split the code. However, I wonder whether the test_bit accesses
> should be protected separately, or actually as part of the test_bit
> method themselves. Do you have any plans to do that already, because in
> that case I would not have to modify the code.
I don't think we want to do that in test_bit() and friends
themselves, as that would likely produce more unnecessary
changes than necessary ones. Even the change here
already looks to have much bigger impact than would be
wanted, as in the common case MFNs aren't guest controlled.
ISTR that originally you had modified just a single call site,
but I can't seem to find that in my inbox anymore. If that
was the case, what exactly were the criteria upon which
you had chosen this sole caller?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 132932: regressions - FAIL
flight 132932 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/132932/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. 132820 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail REGR. vs. 132820 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 132820 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 132820 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 132820 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 132820 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132820 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 132820 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 132820 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 132820 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 132820 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-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-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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-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-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 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-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-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 844293c73685f8198bc2f0c7c5a101b3fcfd538c baseline version: xen 755eb6403ec722db37f1b8f8b51e0b0ab661c003 Last test of basis 132820 2019-02-04 06:25:39 Z2 days Testing same since 132932 2019-02-05 08:57:56 Z1 days1 attempts People who touched revisions under test: Hans van Kranenburg Wei Liu jobs: build-a
Re: [Xen-devel] Ping: libfsimage path/file name changes
Jan Beulich writes ("Ping: libfsimage path/file name changes"):
> On 28.01.19 at 08:55, wrote:
> > back in October you've added quite a number of "xen" prefixes to
> > various pieces there. Now that I've finally had time to connect this
> > change of yours with PV domain creation failures that I've since
> > been observing (not a bug in any way, merely resulting from the
> > fact that I'm running everything straight from the build tree) I
> > started wondering whether the environment variable inspected
> > by common/fsimage_plugin.c:load_plugins() shouldn't then also
> > gain a "XEN" prefix.
>
> I think this wants a resolution one way or the other for 4.12, i.e.
> either we stick to what is there also after 4.12, or we do the
> additional change for the same release as where all your other
> changes have been made.
Yes, IMO it should. I will send a patch.
Also I have had reports of trouble with pygrub in the Debian Xen
packages, which I need to investigate and this may generate a further
patch or two.
Juergen, would that be OK with you in principle ? (I will ask again
for a formal release ack on concrete patch(es).)
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
>>> On 06.02.19 at 16:41, wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
>> As per my earlier reply, I've yet to see proof of a "code-breaking
>> optimization" that actually matches our case(s).
>
> I have personally experienced a program being miscompiled because of
> the mistaken belief by the compiler that _start and _end were
> different objects. I have read haven't read back the whole history of
> this but this is definitely a real bug.
>
> I agree with George that this is a compiler bug. However, this bug is
> not going to be fixed because the compiler community's behaviour is as
> unreasonable as George paints :-(.
>
> Our only option is some kind of bodge.
>
> I don't believe in the asm fragment bodge because we don't have a
> promise anywhere from the compiler authors that the asm hides pointer
> provenance. I am not aware of any C technique which can be reliably
> used to obscure pointer provenance and prevent this kind of
> misoptimisation. Linux is skating on thin ice here.
>
> That leaves:
>
> (i) define indirection variables eg end_ in an assembly language file.
> (ii) convert to uintptr_t before comparing
>
> (i) is IMO wholly safe but it is a bit ugly and slightly less
> performant.
>
> I think (ii) is fairly safe. I doubt that we will find (ii) broken.
> In particular, because there is little motivation for compiler authors
> to try to `optimise it'.
If you want to be "prepared" for them taking apart asm()-s, how
would you expect them to never "look through" casts?
> The difficulty with it providing automatic
> way of detecting when we accidentallyf fail to cast. I suggest the
> following:
>
> extern const struct wombat _wombats_start[];
> extern const struct abstract_symbol _wombats_end[];
>
> and providing a macro which compares any pointer with a struct
> abstract_symbol* by converting the latter to a uintptr_t. Direct
> comparisons of _wombats_start with _wombats_end will result in a
> compilation error due to type mismatch.
Hmm, that's certainly an interesting approach, and requires
care only when introducing a new pair of symbols of this kind.
But of course the macro you suggest to carry out the
comparison will have the same weakness as any open coded
cast to a suitable integer type. But there are benefits:
- it marks problem sites clearly (one of Stefano's goals),
- it isolates future changes to how exactly the comparisons
are to be done to the comparison macro(s)
- it doesn't undermine type safety of the main (start-of-
whatever) symbols (one of my goals),
- it allows the end-of-whatever symbols to also be handed to
functions in a type-safe manner (we could even have
per-instance structure flavors, such that unrelated "end"
symbols can't be mixed up; for example the type could
simply be a structure wrapping a field of the original base
type).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: libfsimage path/file name changes
On 06/02/2019 17:13, Ian Jackson wrote:
> Jan Beulich writes ("Ping: libfsimage path/file name changes"):
>> On 28.01.19 at 08:55, wrote:
>>> back in October you've added quite a number of "xen" prefixes to
>>> various pieces there. Now that I've finally had time to connect this
>>> change of yours with PV domain creation failures that I've since
>>> been observing (not a bug in any way, merely resulting from the
>>> fact that I'm running everything straight from the build tree) I
>>> started wondering whether the environment variable inspected
>>> by common/fsimage_plugin.c:load_plugins() shouldn't then also
>>> gain a "XEN" prefix.
>>
>> I think this wants a resolution one way or the other for 4.12, i.e.
>> either we stick to what is there also after 4.12, or we do the
>> additional change for the same release as where all your other
>> changes have been made.
>
> Yes, IMO it should. I will send a patch.
>
> Also I have had reports of trouble with pygrub in the Debian Xen
> packages, which I need to investigate and this may generate a further
> patch or two.
>
> Juergen, would that be OK with you in principle ? (I will ask again
> for a formal release ack on concrete patch(es).)
In principle bug fixes are fine, of course. Still depends on the time
they are sent and how risky they are.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [xen-unstable test] 132932: regressions - FAIL
>>> On 06.02.19 at 17:10, wrote: > flight 132932 xen-unstable real [real] > http://logs.test-lab.xenproject.org/osstest/logs/132932/ > > Regressions :-( > > Tests which did not succeed and are blocking, > including tests which could not be run: > test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs. > 132820 Hmm, the log is full of (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = ... This can hardly be a result of the three commits in range here. Assuming this is recurring / reproducible, is there a way to set up a bisect starting from an older flight? Otoh looking at a successful test in an older flight I find in http://logs.test-lab.xenproject.org/osstest/logs/132575/test-amd64-amd64-xl-credit2/serial-joubertin0.log much the same, just that there the box came up. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> On 06.02.19 at 16:41, wrote:
> > (i) define indirection variables eg end_ in an assembly language file.
> > (ii) convert to uintptr_t before comparing
> >
> > (i) is IMO wholly safe but it is a bit ugly and slightly less
> > performant.
> >
> > I think (ii) is fairly safe. I doubt that we will find (ii) broken.
> > In particular, because there is little motivation for compiler authors
> > to try to `optimise it'.
>
> If you want to be "prepared" for them taking apart asm()-s, how
> would you expect them to never "look through" casts?
I'm not sure what you mean by `look through casts'. I expect the
compiler to `look through casts' for both value and provenance
purposes.
But comparing uintptr_t's is never UB, no matter their value or
provenance. So there is simply unarguably no UB if the comparison is
done with uintptr_t's. The conversions themselves are ID, not UB.
Whether comparing pointer values is UB depends on their value and
provenance, and in general the compiler is able to look through most
transformations (including casts) to determine the ultimate provenance
- ie the provenance rules are not defeated by casts.
So that's de jure.
De facto, there is little incentive for a compiler to misoptimise
uintptr_t comparisons, and much incentive for it to get `better' at
disentangling pointer provenance for the purpose of (mis)optimising
pointer comparisons.
> > extern const struct wombat _wombats_start[];
> > extern const struct abstract_symbol _wombats_end[];
...
> Hmm, that's certainly an interesting approach, and requires
> care only when introducing a new pair of symbols of this kind.
> But of course the macro you suggest to carry out the
> comparison will have the same weakness as any open coded
> cast to a suitable integer type. But there are benefits:
libxl already relies on casting to uintptr_t as a way of avoiding the
rules restricting pointer comparisons. See these comments:
libxl_event.c l.476 libxl__watch_slot_contents
libxl_internal.h l.322 typedef struct libxl__ev_watch_slot
> - it marks problem sites clearly (one of Stefano's goals),
> - it isolates future changes to how exactly the comparisons
> are to be done to the comparison macro(s)
> - it doesn't undermine type safety of the main (start-of-
> whatever) symbols (one of my goals),
> - it allows the end-of-whatever symbols to also be handed to
> functions in a type-safe manner
Yes. However...
> (we could even have per-instance structure flavors, such that
> unrelated "end" symbols can't be mixed up; for example the type
> could simply be a structure wrapping a field of the original base
> type).
I think this would be difficult to achieve without writing a forbidden
pointer comparison in the macro. It could perhaps be achieved with
typeof() if that is available in hypervisor code.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [xen-unstable test] 132932: regressions - FAIL
Jan Beulich writes ("Re: [Xen-devel] [xen-unstable test] 132932: regressions -
FAIL"):
> >>> On 06.02.19 at 17:10, wrote:
> > flight 132932 xen-unstable real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/132932/
> >
> > Regressions :-(
> >
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> > test-amd64-amd64-amd64-pvgrub 7 xen-bootfail REGR. vs.
> > 132820
>
> Hmm, the log is full of
>
> (XEN) AMD-Vi: IO_PAGE_FAULT: domain = 0, device id = ...
>
> This can hardly be a result of the three commits in range here.
> Assuming this is recurring / reproducible, is there a way to set
> up a bisect starting from an older flight? Otoh looking at a
> successful test in an older flight I find in
> http://logs.test-lab.xenproject.org/osstest/logs/132575/test-amd64-amd64-xl-credit2/serial-joubertin0.log
> much the same, just that there the box came up.
This failure occured on joubertin1. The joubertins were put into
service again yesterday after a long absence. (The absence was due to
hardware trouble with the PDU etc. - not anything that would cause
this crash.)
So I think this is probably a host-specific crash.
The bisector has found a basis pass in 127489 and is working on it.
It has only just started. In particular it has not yet repro'd either
the basis pass or the new failure, and right now it is doing some
rebuilds of old (expired) build artifacts to repro the basis pass.
Results will appear here:
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable/test-amd64-amd64-amd64-pvgrub.xen-boot.html
and interim reports, or a report of a failure to bisect, will go to
the osstest-output mailing list under subjects like this:
Subject: [xen-unstable bisection] 132970: testing
test-amd64-amd64-amd64-pvgrub
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
>>> On 06.02.19 at 17:37, wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
>> - it marks problem sites clearly (one of Stefano's goals),
>> - it isolates future changes to how exactly the comparisons
>> are to be done to the comparison macro(s)
>> - it doesn't undermine type safety of the main (start-of-
>> whatever) symbols (one of my goals),
>> - it allows the end-of-whatever symbols to also be handed to
>> functions in a type-safe manner
>
> Yes. However...
>
>> (we could even have per-instance structure flavors, such that
>> unrelated "end" symbols can't be mixed up; for example the type
>> could simply be a structure wrapping a field of the original base
>> type).
>
> I think this would be difficult to achieve without writing a forbidden
> pointer comparison in the macro. It could perhaps be achieved with
> typeof() if that is available in hypervisor code.
I'm afraid I don't understand - you want to cast to an integer
type anyway for doing the comparison.
As to typeof() - this being a compiler construct, it is available
whenever the compiler supports it. We certainly use it
elsewhere in hypervisor code.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] SVM: make nested page-fault tracing and logging consistent
On 2/6/19 6:22 AM, Jan Beulich wrote: > Don't call __get_gfn_type_access() more than once, to make sure data > recorded for xentrace matches up with what gets logged in case of the > domain getting crashed. > > As a side effect this also eliminates a type mismatch reported by > Norbert Manthey, as the first call now also needs to update the local > variable "p2mt". > > Do a few cosmetics at the same time: Move a comment up a little, drop > the pointless "case 0" (seeing in particular the comment's wording), > and correct formatting. > > Signed-off-by: Jan Beulich Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> On 06.02.19 at 17:37, wrote:
> > Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> >> - it allows the end-of-whatever symbols to also be handed to
> >> functions in a type-safe manner
> >
> > Yes. However...
> >
> >> (we could even have per-instance structure flavors, such that
> >> unrelated "end" symbols can't be mixed up; for example the type
> >> could simply be a structure wrapping a field of the original base
> >> type).
> >
> > I think this would be difficult to achieve without writing a forbidden
> > pointer comparison in the macro. It could perhaps be achieved with
> > typeof() if that is available in hypervisor code.
>
> I'm afraid I don't understand - you want to cast to an integer
> type anyway for doing the comparison.
The usual approach to haking a macro perform an explicit typecheck
(ie, to have the macro check that the types of its arguments are
right) is to have the macro expansion contain a `spurious' comparison
whose result is ignored but which will yield a compile-type type
mismatch if the argument types were wrong. But this is only legal if
the provenance of the compared pointers is legal for a pointer
comparison. The bad effects of evaluating an UB expression are not
limited by within-program causality.
> As to typeof() - this being a compiler construct, it is available
> whenever the compiler supports it. We certainly use it
> elsewhere in hypervisor code.
I think then that
(typeof(X))0 == (typeof(Y))0
is the correct formulation of the type check - because it is legal no
matter the provenance of X and Y.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 7/8] x86/mm: handle foreign mappings in p2m_entry_modify
>>> On 30.01.19 at 11:36, wrote:
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -933,9 +933,12 @@ struct hvm_ioreq_server *p2m_get_ioreq_server(struct
> domain *d,
>unsigned int *flags);
>
> static inline void p2m_entry_modify(struct p2m_domain *p2m, p2m_type_t nt,
> -p2m_type_t ot, unsigned int level)
> +p2m_type_t ot, mfn_t nfn, mfn_t ofn,
> +unsigned int level)
> {
> -if ( level != 1 || nt == ot )
> +struct page_info *pg;
> +
> +if ( level != 1 || (nt == ot && mfn_eq(nfn, ofn)) )
> return;
>
> switch ( nt )
> @@ -948,6 +951,17 @@ static inline void p2m_entry_modify(struct p2m_domain
> *p2m, p2m_type_t nt,
> p2m->ioreq.entry_count++;
> break;
>
> +case p2m_map_foreign:
> +pg = mfn_to_page(nfn);
> +
> +if ( !pg || !page_get_owner_and_reference(pg) )
mfn_to_page() can't return NULL, can it? You may want to ASSERT()
beforehand that the MFN is not INVALID_MFN, though.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/shadow: adjust minimum allocation calculations
On Wed, Feb 06, 2019 at 05:53:16AM -0700, Jan Beulich wrote:
> >>> On 06.02.19 at 12:52, wrote:
> > On Wed, Feb 06, 2019 at 03:56:49AM -0700, Jan Beulich wrote:
> >> @@ -1373,9 +1377,15 @@ static unsigned int sh_min_allocation(co
> >> {
> >> /*
> >> * Don't allocate less than the minimum acceptable, plus one page per
> >> - * megabyte of RAM (for the p2m table).
> >> + * megabyte of RAM (for the p2m table, minimally enough for HVM's
> >> setting
> >> + * up of slot zero and VMX's setting up of the LAPIC page), plus one
> >> for
> >> + * HVM's 1-to-1 pagetable.
> >> */
> >> -return shadow_min_acceptable_pages(d) + (d->tot_pages / 256);
> >> +return shadow_min_acceptable_pages(d) +
> >> + max(d->tot_pages / 256,
> >> + is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + !!cpu_has_vmx * 2
> >> +: 0U) +
> >> + is_hvm_domain(d);
> >
> > Should the call to shadow_set_allocation be changed so it attempts to
> > allocate sh_min_allocation(d) + d->arch.paging.shadow.p2m_pages?
> >
> > It seems a little misleading to check whether there's a certain amount
> > of pages in the pool (sh_min_allocation(d) +
> > d->arch.paging.shadow.p2m_pages) and then set the allocation to 4M
> > unconditionally.
>
> Well, as said in the post-commit-message remark, I think we want to
> get rid of this, but only with it properly replaced (which would likely
> require hooking into paths increasing d->tot_pages). Right now all
> we're after is dealing with the regression.
Ack, with the changes requested by Andrew:
Reviewed-by: Roger Pau Monné
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 02/15] argo: introduce the argo_op hypercall boilerplate
Hi Stefano, On 2/5/19 9:34 PM, Stefano Stabellini wrote: On Tue, 5 Feb 2019, Julien Grall wrote: Sorry for the formatting. On Tue, 5 Feb 2019, 20:04 Stefano Stabellini, wrote: On Tue, 5 Feb 2019, Jan Beulich wrote: But I am afraid this is not correct. Upper 32-bit of the register will be zeroed when writing a 32-bit value. So we never rely on the register to be zeroed on boot. Hi Julien, Hi Stefano, Thank you for checking your emails. I found the reference in the ARM ARM, although it took me several minutes! "The upper 32 bits of the destination register are set to zero." from C6.1.1 (ID092916). Actually, you were right and I was wrong. This paragraph only talks about 32-bit access from AArch64. I found a paragraph on the Arm Arm (D1.20.2 in DDI 0487D.a) stating that the upper 32-bits can either "be zeroed, or hold the value that the same architectural register held before any AArch32". So we do rely the upper bits are zeroed when the vCPU is created. The registers are set by arch_set_info_guest via a context. The context can be set by either virtual PSCI call CPU_ON or via DOMCTL setvcpucontext (so the tools). We fully control PSCI call CPU_ON and zero the registers. So no question here. For the DOMCTL, per XSA-77, we trust how operation will be used. The toolstack (vcpu_arm32 libxc/xc_dom_arm.c) will zero the context before. So I think we should be safe here. However, I think we should add some sanity check in arch_set_info_guest for our peace of mind. For guest entry/exit, rather than zero the upper 32-bits I would also add sanity check in enter_hypervisor_head and leave_hypervisor_tail but only in debug build. Any opinions? FYI do_memory_op is declared as follows on the Linux side for arm32 and arm64: int HYPERVISOR_memory_op(unsigned int cmd, void *arg); When I went through all existing hypercalls to introduce them on arm32, I checked that we didn't actually need 64bit parameters, especially for cmd. I introduced them as int instead of long on the Linux side when possible (see include/xen/arm/hypercall.h), but I didn't attempt to modify all the existing Xen headers. I don't understand your concern with unsigned long. We use them in __DEFINE_XEN_GUEST_HANDLE that are in turn to describe guest pointer. __DEFINE_XEN_GUEST_HANDLE is for pointers inside memory structs, and we defined it as: * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes * aligned. You probably meant XEN_GUEST_HANDLE_PARAM which is the one for pointers when passed as hypercall parameters, that is defined as: * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64. Yes, I meant XEN_GUEST_HANDLE_PARAM. I should have waited to have my laptop in hand rather looking on my phone :). Yes, pointers as hypercalls parameters are the exception to the single-ABI rule and we introduced XEN_GUEST_HANDLE_PARAM purposely to handle them. However, I am not sure we took into account zero-extension when we discussed hypercalls parameters for arm back in the day when I wrote include/xen/arm/hypercall.h. I am not sure to follow your thoughts about taking into account zero-extension in the Linux header. Could you expand it? In the official public headers, I can't find anything telling you the size of each arguments and the number of arguments. Instead you have to look at Xen code to know the exact number of arguments and the size. Did I miss anything? Regarding Argo, there seem to have some kind of documentation per-hypercalls although some does not specify the size. But I can't find anything telling you the command number is in arg0. The mapping to from argN the hypercalls arguments is also not that clear. But I guess this kind of improvement can be done afterwards. The problem with explicitly sized (i.e 32-bit) is you ignore the top 32-bit. This means you can't check the upper bits are always 0 and would prevent extension. That is true. I implicitly assumed that our desire for a common 32-bit/64-bit ABI would not apply just to structs in memory (where we always define unsigned long and pointers as 64-bit) but also seamlessly apply to hypercalls parameters (except for pointers as per the above). There are no documentation in the public interface about the size of each arguments. When looking at traps.c, we assume that hypercalls arguments are the size of a register: typedef register_t (*arm_hypercall_fn_t)(register_t, register_t, register_t, register_t, register_t). So for 64-bit Xen, the hypercalls arguments will be 64-bit. If we want to impose 32-bit value, then we need to update the callback, add sanity check (?) and probably document it. There are still reasons for choosing unsigned int for cases like this where uns
Re: [Xen-devel] [PATCH v8 for-4.12 10/17] argo: implement the notify op
Hi, On 2/6/19 8:55 AM, Christopher Clark wrote: +/* + * XEN_ARGO_OP_notify + * + * Asks Xen for information about other rings in the system. + * + * ent->ring is the xen_argo_addr_t of the ring you want information on. + * Uses the same ring matching rules as XEN_ARGO_OP_sendv. + * + * ent->space_required : if this field is not null then Xen will check + * that there is space in the destination ring for this many bytes of payload. + * If the ring is too small for the requested space_required, it will set the + * XEN_ARGO_RING_EMSGSIZE flag on return. + * If sufficient space is available, it will set XEN_ARGO_RING_SUFFICIENT + * and CANCEL any pending notification for that ent->ring; otherwise it + * will schedule a notification event and the flag will not be set. + * + * These flags are set by Xen when notify replies: + * XEN_ARGO_RING_EXISTS ring exists + * XEN_ARGO_RING_SHARED ring is registered for wildcard partner + * XEN_ARGO_RING_EMPTY ring is empty + * XEN_ARGO_RING_SUFFICIENT sufficient space for space_required is there + * XEN_ARGO_RING_EMSGSIZE space_required is too large for the ring size + * XEN_ARGO_RING_EBUSY too many domains waiting for available space signals + * + * arg1: XEN_GUEST_HANDLE(xen_argo_ring_data_t) ring_data (may be NULL) + * arg2: NULL NIT: While looking at all the hypercall, I noticed you say NULL here. In most of the cases, NULL will be 0 but there are place where it might not be. So what is the exact value you expect here? + * arg3: 0 (ZERO) + * arg4: 0 (ZERO) NIT: I guess those to will be 0 in an unsigned long value? + */ +#define XEN_ARGO_OP_notify 4 + #endif diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst index 0d65f6a..e330f72 100644 --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -34,6 +34,8 @@ ! argo_iovargo.h ? argo_register_ring argo.h ? argo_ring argo.h +? argo_ring_data argo.h +? argo_ring_data_ent argo.h ? argo_ring_message_headerargo.h ? argo_send_addr argo.h ? argo_unregister_ringargo.h Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.11-testing test] 132938: tolerable FAIL - PUSHED
flight 132938 xen-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/132938/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-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-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-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-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-i386-libvirt 13 migrate-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail 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-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail 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-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail 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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 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-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 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 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail 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-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 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 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 87f51bf366ca79b98e1e201bf9bd7a9c164631e2 baseline version: xen e2e3a1d75798781a8031feec0050e6e1c98187ca Last test of basis 132779 2019-02-03 15:18:52 Z3 days Testing same since 132938 2019-02-05 12:06:45 Z1 days1 attempts People who touched revisions under test: Ian Jackson Jim Fehlig Juergen Gross Tamas K Lengyel Wei Liu jobs: build-amd64-xsm
Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
On Wed, 6 Feb 2019, Andrii Anisov wrote: > Hello Stefano, > > On 05.02.19 11:17, Andrii Anisov wrote: > > Thank you. > > I already referred this thread in the email to the vendor as the first step. > > But have no answer yet :( > > We've got an answer from the vendor. They agree with arguments and have > provided a patch to eliminate Set/Way usage. > We've checked and it does what is needed. So we do not need this WA any more. Great, best for everybody! ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 02/15] argo: introduce the argo_op hypercall boilerplate
On Wed, 6 Feb 2019, Julien Grall wrote: > Hi Stefano, > > On 2/5/19 9:34 PM, Stefano Stabellini wrote: > > On Tue, 5 Feb 2019, Julien Grall wrote: > > > Sorry for the formatting. > > > > > > On Tue, 5 Feb 2019, 20:04 Stefano Stabellini, > > > wrote: > > >On Tue, 5 Feb 2019, Jan Beulich wrote: > > > > But I am afraid this is not correct. Upper 32-bit of the register will be > > > zeroed when writing a 32-bit value. So we never rely on > > > the register to be zeroed on boot. > > > > Thank you for checking your emails. I found the reference in the ARM > > ARM, although it took me several minutes! > > > >"The upper 32 bits of the destination register are set to zero." > > > > from C6.1.1 (ID092916). > > Actually, you were right and I was wrong. This paragraph only talks about > 32-bit access from AArch64. I found a paragraph on the Arm Arm (D1.20.2 in DDI > 0487D.a) stating that the upper 32-bits can either "be zeroed, or hold the > value that the same architectural register held before any AArch32". Understanding the ARM ARM is really difficult, I am glad we clarified this (and that my memory didn't completely fail me yet). > So we do rely the upper bits are zeroed when the vCPU is created. The > registers are set by arch_set_info_guest via a context. The context can be set > by either virtual PSCI call CPU_ON or via DOMCTL setvcpucontext (so the > tools). > > We fully control PSCI call CPU_ON and zero the registers. So no question here. > > For the DOMCTL, per XSA-77, we trust how operation will be used. The toolstack > (vcpu_arm32 libxc/xc_dom_arm.c) will zero the context before. So I think we > should be safe here. > > However, I think we should add some sanity check in arch_set_info_guest for > our peace of mind. For guest entry/exit, rather than zero the upper 32-bits I > would also add sanity check in enter_hypervisor_head and leave_hypervisor_tail > but only in debug build. Any opinions? Definitely we should have sanity checks. > > >FYI do_memory_op is declared as follows on the Linux side for arm32 > > > and > > >arm64: > > > > > > int HYPERVISOR_memory_op(unsigned int cmd, void *arg); > > > > > >When I went through all existing hypercalls to introduce them on > > > arm32, > > >I checked that we didn't actually need 64bit parameters, especially > > > for > > >cmd. I introduced them as int instead of long on the Linux side > > > when > > >possible (see include/xen/arm/hypercall.h), but I didn't attempt to > > >modify all the existing Xen headers. > > > > > > > > > I don't understand your concern with unsigned long. We use them in > > > __DEFINE_XEN_GUEST_HANDLE that are in turn to describe guest > > > pointer. > > > > __DEFINE_XEN_GUEST_HANDLE is for pointers inside memory structs, and we > > defined it as: > > * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field > > * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes > > * aligned. > > > > You probably meant XEN_GUEST_HANDLE_PARAM which is the one for pointers > > when passed as hypercall parameters, that is defined as: > > * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an > > * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64. > > Yes, I meant XEN_GUEST_HANDLE_PARAM. I should have waited to have my laptop in > hand rather looking on my phone :). > > > > > Yes, pointers as hypercalls parameters are the exception to the > > single-ABI rule and we introduced XEN_GUEST_HANDLE_PARAM purposely to > > handle them. However, I am not sure we took into account zero-extension > > when we discussed hypercalls parameters for arm back in the day when I > > wrote include/xen/arm/hypercall.h. > > I am not sure to follow your thoughts about taking into account zero-extension > in the Linux header. Could you expand it? > > In the official public headers, I can't find anything telling you the size of > each arguments and the number of arguments. Instead you have to look at Xen > code to know the exact number of arguments and the size. Did I miss anything? No, it is like you wrote. I think I should have pushed the discussion further and added more information to the Xen public headers back in those days. > Regarding Argo, there seem to have some kind of documentation per-hypercalls > although some does not specify the size. But I can't find anything telling you > the command number is in arg0. The mapping to from argN the hypercalls > arguments is also not that clear. > > But I guess this kind of improvement can be done afterwards. > > > > The problem with explicitly sized (i.e 32-bit) is you ignore the top > > > 32-bit. This means you can't check the upper bits are always > > > 0 and would prevent extension. > > > > That is true. I implicitly assumed that our desire for a common > > 32-bit/64-bit ABI would not apply just to structs in memory (where we > > always define unsigned
[Xen-devel] [qemu-mainline baseline-only test] 83603: regressions - trouble: broken/fail/pass
This run is configured for baseline tests only. flight 83603 qemu-mainline real [real] http://osstest.xensource.com/osstest/logs/83603/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl broken test-armhf-armhf-xl-rtds broken test-armhf-armhf-xl-multivcpu broken test-armhf-armhf-xl-credit1 broken test-armhf-armhf-xl-midway broken test-armhf-armhf-libvirt broken test-armhf-armhf-xl-vhd broken test-armhf-armhf-xl-credit2 broken test-armhf-armhf-libvirt-raw broken test-armhf-armhf-xl 13 capture-logs(13)broken REGR. vs. 75595 test-armhf-armhf-libvirt 13 capture-logs(13)broken REGR. vs. 75595 test-armhf-armhf-xl-multivcpu 13 capture-logs(13) broken REGR. vs. 75595 test-armhf-armhf-xl-credit1 13 capture-logs(13)broken REGR. vs. 75595 test-armhf-armhf-xl-credit2 13 capture-logs(13)broken REGR. vs. 75595 test-armhf-armhf-xl-midway 13 capture-logs(13)broken REGR. vs. 75595 test-armhf-armhf-xl-vhd 11 capture-logs(11)broken REGR. vs. 75595 test-armhf-armhf-libvirt-raw 11 capture-logs(11)broken REGR. vs. 75595 test-amd64-i386-pair 21 guest-start/debianfail REGR. vs. 75595 test-amd64-amd64-xl-xsm 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-credit1 12 guest-start fail REGR. vs. 75595 test-amd64-i386-freebsd10-amd64 11 guest-startfail REGR. vs. 75595 test-amd64-i386-xl-shadow12 guest-start fail REGR. vs. 75595 test-amd64-i386-libvirt-pair 21 guest-start/debianfail REGR. vs. 75595 test-amd64-i386-xl-xsm 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-shadow 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-pair21 guest-start/debianfail REGR. vs. 75595 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-credit2 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 75595 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-multivcpu 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 75595 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 75595 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-installfail REGR. vs. 75595 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-i386-xl 12 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-qcow211 guest-start fail REGR. vs. 75595 test-amd64-i386-freebsd10-i386 11 guest-start fail REGR. vs. 75595 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 75595 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-amd64-pygrub 11 guest-start fail REGR. vs. 75595 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-amd64-amd64-pvgrub 11 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-amd64-libvirt-vhd 11 guest-start fail REGR. vs. 75595 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 75595 test-amd64-amd64-i386-pvgrub 10 debian-di-install fail REGR. vs. 75595 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-installfail REGR. vs. 75595 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 75595 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 75595 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-installfail REGR. vs. 75595 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 13 capture-logs(13)broken REGR. vs. 75595 test-amd64-amd64-xl-rtds 12 guest-start fail REGR. vs. 75595 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail like 75595 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail like 75595 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail like 75595 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-i
Re: [Xen-devel] RT Xen on ARM - R-Car series
Hello, On 05.02.19 10:40, Andrii Anisov wrote: On 04.02.19 23:58, Julien Grall wrote: Thank you for writing a patch. I am back in France this week for family reason and will not have time properly give a spin this week. Stefano, Andrii, can you test it? I'll do that today. I actually did it today. So that it is compiled and runs. I uncommented that print, and it does not appear. I see steal time slightly bouncing around 50% in a following configuration: - 4PCPUs - 2 VMs of 4 VCPUs - both VMs running `for i in 1 2 3 4; do dd if=/dev/zero of=/dev/null & done` from their consoles It seems that results are as expected. I also checked the build without the patch. I expected it would demonstrate more inconsistency in numbers, but no, it also shows 50% steal time pretty stable. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 132941: tolerable all pass - PUSHED
flight 132941 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/132941/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 132776 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 132776 test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 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-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-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-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 620d9dd598fde388f56ac37bcd3b31168c2f9fc6 baseline version: libvirt 7c9dcfed5ae6d5874ea0e67e47a6871707b8446a Last test of basis 132776 2019-02-03 13:45:39 Z3 days Failing since132846 2019-02-04 13:03:42 Z2 days2 attempts Testing same since 132941 2019-02-05 14:57:44 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani Cole Robinson Marc Hartmayer Michal Privoznik Peter Krempa 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 pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd 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/libvirt.git 7c9dcfed5a..620d9dd598 620d9dd598fde388f56ac37bcd3b31168c2f9fc6 -> xen-tested-master ___ Xen-devel
[Xen-devel] [PATCH] x86/pv: Fix construction of 32bit dom0's
dom0_construct_pv() has logic to transition dom0 into a compat domain when
booting an ELF32 image.
One aspect which is missing is the CPUID policy recalculation, meaning that a
32bit dom0 sees a 64bit policy, which differ by the Long Mode feature flag in
particular. Another missing item is the x87_fip_width initialisation.
Update dom0_construct_pv() to use switch_compat(), rather than retaining the
opencoding. Position the call to switch_compat() such that the compat32 local
variable can disappear entirely.
The 32bit monitor table is now created by setup_compat_l4(), avoiding the need
to for manual creation later. Furthermore, the L3 table creation is redundant
with the logic inside the main mapping loop, so can be dropped as well.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
Slightly RFC:
1) I've not worked out exactly what the
v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];
line is supposed to be doing and whether it is needed, but it doesn't
appear to matter. It is perhaps another redundant opencoding.
2) The reported
Dom0 alloc.: 3e80->3ec0 (240470 pages to be
allocated)
line changes by 1 page because of the alloc_domheap_page() moving ahead of
the printk(), but I'm fairly sure this is benign. There is a matching
reduction in the length of the constructed m2p which is perhaps less
benign.
---
xen/arch/x86/pv/dom0_build.c | 43 +--
xen/arch/x86/pv/domain.c | 2 +-
2 files changed, 14 insertions(+), 31 deletions(-)
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 837ef7b..c3d8ee7 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -285,7 +285,7 @@ int __init dom0_construct_pv(struct domain *d,
module_t *initrd,
char *cmdline)
{
-int i, cpu, rc, compatible, compat32, order, machine;
+int i, cpu, rc, compatible, order, machine;
struct cpu_user_regs *regs;
unsigned long pfn, mfn;
unsigned long nr_pages;
@@ -354,14 +354,18 @@ int __init dom0_construct_pv(struct domain *d,
/* compatibility check */
compatible = 0;
-compat32 = 0;
machine = elf_uval(&elf, elf.ehdr, e_machine);
printk(" Xen kernel: 64-bit, lsb, compat32\n");
if ( elf_32bit(&elf) && parms.pae == XEN_PAE_BIMODAL )
parms.pae = XEN_PAE_EXTCR3;
if ( elf_32bit(&elf) && parms.pae && machine == EM_386 )
{
-compat32 = 1;
+if ( unlikely(rc = switch_compat(d)) )
+{
+printk("Dom0 failed to switch to compat: %d\n", rc);
+return rc;
+}
+
compatible = 1;
}
if (elf_64bit(&elf) && machine == EM_X86_64)
@@ -392,16 +396,6 @@ int __init dom0_construct_pv(struct domain *d,
}
}
-if ( compat32 )
-{
-d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 1;
-d->arch.pv.xpti = false;
-d->arch.pv.pcid = false;
-v->vcpu_info = (void *)&d->shared_info->compat.vcpu_info[0];
-if ( setup_compat_arg_xlat(v) != 0 )
-BUG();
-}
-
nr_pages = dom0_compute_nr_pages(d, &parms, initrd_len);
if ( parms.pae == XEN_PAE_EXTCR3 )
@@ -425,8 +419,6 @@ int __init dom0_construct_pv(struct domain *d,
parms.p2m_base = UNSET_ADDR;
}
-domain_set_alloc_bitsize(d);
-
/*
* Why do we need this? The number of page-table frames depends on the
* size of the bootstrap address space. But the size of the address space
@@ -606,23 +598,14 @@ int __init dom0_construct_pv(struct domain *d,
{
maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l4_page_table;
l4start = l4tab = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
+clear_page(l4tab);
+init_xen_l4_slots(l4tab, _mfn(virt_to_mfn(l4start)),
+ d, INVALID_MFN, true);
+v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
}
else
-{
-page = alloc_domheap_page(d, MEMF_no_owner | MEMF_no_scrub);
-if ( !page )
-panic("Not enough RAM for domain 0 PML4\n");
-page->u.inuse.type_info = PGT_l4_page_table|PGT_validated|1;
-l4start = l4tab = page_to_virt(page);
-maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l3_page_table;
-l3start = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
-}
-clear_page(l4tab);
-init_xen_l4_slots(l4tab, _mfn(virt_to_mfn(l4start)),
- d, INVALID_MFN, true);
-v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
-if ( is_pv_32bit_domain(d) )
-v->arch.guest_table_user = v->arch.guest_table;
+/* Monitor table already created by switch_compat(). */
+l4start = l4tab = __va(pagetable_get_paddr(v->arch.guest_table));
l4tab += l4_table_offset(v_start);
pfn = alloc_spfn;
diff --git a/xen
Re: [Xen-devel] RT Xen on ARM - R-Car series
On Wed, 6 Feb 2019, Andrii Anisov wrote: > Hello, > > On 05.02.19 10:40, Andrii Anisov wrote: > > On 04.02.19 23:58, Julien Grall wrote: > > > Thank you for writing a patch. I am back in France this week for family > > > reason and will not have time properly give a spin this week. Stefano, > > > Andrii, can you test it? > > I'll do that today. > > I actually did it today. > So that it is compiled and runs. I uncommented that print, and it does not > appear. > I see steal time slightly bouncing around 50% in a following configuration: > - 4PCPUs > - 2 VMs of 4 VCPUs > - both VMs running `for i in 1 2 3 4; do dd if=/dev/zero of=/dev/null & done` > from their consoles > > It seems that results are as expected. > > I also checked the build without the patch. I expected it would demonstrate > more inconsistency in numbers, but no, it also shows 50% steal time pretty > stable. That's great. Could you or Roger take care of cleaning up the patch and properly submitting it to the list? And also double check that it won't break any guests (at least the ones we know about: Linux and Windows on x86). ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 132959: all pass - PUSHED
flight 132959 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/132959/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd 31147537ec123ed5f5eb12260430f1ef5f906324 baseline version: freebsd f2704b103eb396f8e9139b73406b504b837dfb61 Last test of basis 132832 2019-02-04 09:19:11 Z2 days Testing same since 132959 2019-02-06 09:19:27 Z0 days1 attempts People who touched revisions under test: andrew avos bde cem dim imp jah jchandra jhibbits kib luporl manu marius markj mav mmacy ngie tuexen vmaffione yuripv 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 f2704b103eb..31147537ec1 31147537ec123ed5f5eb12260430f1ef5f906324 -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 132977: tolerable all pass - PUSHED
flight 132977 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/132977/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: 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 13 migrate-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 0322e0db5b29a0d1ce4b452885e34023e3a4b00e baseline version: xen 6b6ff07fbd9ec17586d1f52fac61e87e2e58 Last test of basis 132967 2019-02-06 13:00:36 Z0 days Testing same since 132977 2019-02-06 20:00:50 Z0 days1 attempts People who touched revisions under test: Peng Fan 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 6b6ff07fbd..0322e0db5b 0322e0db5b29a0d1ce4b452885e34023e3a4b00e -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL
On Wed, 6 Feb 2019, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL"):
> > On 06.02.19 at 17:37, wrote:
> > > Jan Beulich writes ("Re: [Xen-devel] [PATCH v6 1/4] xen: introduce
> > > SYMBOL"):
> > >> - it allows the end-of-whatever symbols to also be handed to
> > >> functions in a type-safe manner
> > >
> > > Yes. However...
I am OK with this approach. Maybe not the best IMO, but good enough. It
should also satisfy the MISRAC guys, as they wrote "ideally cast to
uintptr_t only once": here we wouldn't be casting only once, but at
least we would do it inside a single well-defined macro.
I did manage to convert v4 of the series to this approach before writing
this answer -- everything looks plausible and compiles OK. Also, see
below.
> > >> (we could even have per-instance structure flavors, such that
> > >> unrelated "end" symbols can't be mixed up; for example the type
> > >> could simply be a structure wrapping a field of the original base
> > >> type).
> > >
> > > I think this would be difficult to achieve without writing a forbidden
> > > pointer comparison in the macro. It could perhaps be achieved with
> > > typeof() if that is available in hypervisor code.
> >
> > I'm afraid I don't understand - you want to cast to an integer
> > type anyway for doing the comparison.
>
> The usual approach to haking a macro perform an explicit typecheck
> (ie, to have the macro check that the types of its arguments are
> right) is to have the macro expansion contain a `spurious' comparison
> whose result is ignored but which will yield a compile-type type
> mismatch if the argument types were wrong. But this is only legal if
> the provenance of the compared pointers is legal for a pointer
> comparison. The bad effects of evaluating an UB expression are not
> limited by within-program causality.
>
> > As to typeof() - this being a compiler construct, it is available
> > whenever the compiler supports it. We certainly use it
> > elsewhere in hypervisor code.
>
> I think then that
>(typeof(X))0 == (typeof(Y))0
> is the correct formulation of the type check - because it is legal no
> matter the provenance of X and Y.
Thank you, Ian. I think I understand what you are saying. I'll probably
leave this out for the next iteration though, but I am happy to add it
afterwards.
I was thinking of going with two MACROs:
+/*
+ * Performs x - y, returns the original pointer type. To be used when
+ * either x or y or both are linker symbols.
+ */
+#define SYMBOLS_SUBTRACT(x, y) ({ \
+__typeof__(*(y)) *ptr_; \
+ptr_ = (typeof(ptr_)) (((uintptr_t)(x) - (uintptr_t)(y)) / sizeof(*(y))); \
+ptr_; \
+})
+
+/*
+ * Performs x - y, returns uintptr_t. To be used when either x or y or
+ * both are linker symbols.
+ */
+#define SYMBOLS_COMPARE(x, y) ({ \
+uintptr_t ptr_; \
+ptr_ = ((uintptr_t)(x) - (uintptr_t)(y)) / sizeof(*(y)); \
+ptr_; \
+})
Examples:
+new_ptr = SYMBOLS_SUBTRACT(func->old_addr, _start) + vmap_of_xen_text;
and:
+for ( alt = region->begin;
+ SYMBOLS_COMPARE(alt, region->end) < 0;
+ alt++ )
We could also define a third macro such as:
#define SYMBOLS_SUBTRACT_INT(x, y) SYMBOLS_COMPARE((x), (y))
because we have many places where we need the result of SYMBOLS_SUBTRACT
converted to an integer type. For instance:
paddr_t xen_size = (uintptr_t)SYMBOLS_SUBTRAC(_end, _start);
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 2/6] libxl: attach PCI device to qemu only after setting pciback/pcifront
When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.
Signed-off-by: Marek Marczykowski-Górecki
Acked-by: Wei Liu
---
Changes in v2:
- Fixed code style since previous version.
---
tools/libxl/libxl_pci.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 3b6b23c..1bde537 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1191,6 +1191,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcide
{
libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned int orig_vdev, pfunc_mask;
+char *be_path;
libxl_device_pci *assigned;
int num_assigned, i, rc;
int stubdomid = 0;
@@ -1245,6 +1246,14 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid,
libxl_device_pci *pcide
rc = do_pci_add(gc, stubdomid, &pcidev_s, 0);
if ( rc )
goto out;
+/* Wait for the device actually being connected, otherwise device model
+ * running there will fail to find the device. */
+be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0",
+ libxl__xs_get_dompath(gc, 0), stubdomid);
+rc = libxl__wait_for_backend(gc, be_path,
+ GCSPRINTF("%d", XenbusStateConnected));
+if (rc)
+goto out;
}
orig_vdev = pcidev->vdevfn & ~7U;
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 3/6] libxl: don't try to manipulate json config for stubdomain
Stubdomain do not have it's own config file - its configuration is
derived from target domains. Do not try to manipulate it when attaching
PCI device.
This bug prevented starting HVM with stubdomain and PCI passthrough
device attached.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- skip libxl__dm_check_start too, as stubdomain is guaranteed to be
running at this stage already
- do not init d_config at all, as it is used only for json manipulation
Changes in v4:
- adjust comment style
---
tools/libxl/libxl_pci.c | 50 ++
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 1bde537..0e70f0d 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -120,10 +120,14 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
libxl_domain_config d_config;
libxl_device_pci pcidev_saved;
libxl__domain_userdata_lock *lock = NULL;
+bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
-libxl_domain_config_init(&d_config);
-libxl_device_pci_init(&pcidev_saved);
-libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
+/* Stubdomain doesn't have own config. */
+if (!is_stubdomain) {
+libxl_domain_config_init(&d_config);
+libxl_device_pci_init(&pcidev_saved);
+libxl_device_pci_copy(CTX, &pcidev_saved, pcidev);
+}
be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
LIBXL__DEVICE_KIND_PCI);
@@ -152,27 +156,35 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
GCNEW(device);
libxl__device_from_pcidev(gc, domid, pcidev, device);
-lock = libxl__lock_domain_userdata(gc, domid);
-if (!lock) {
-rc = ERROR_LOCK_FAIL;
-goto out;
-}
+/*
+ * Stubdomin config is derived from its target domain, it doesn't have
+ * its own file.
+ */
+if (!is_stubdomain) {
+lock = libxl__lock_domain_userdata(gc, domid);
+if (!lock) {
+rc = ERROR_LOCK_FAIL;
+goto out;
+}
-rc = libxl__get_domain_configuration(gc, domid, &d_config);
-if (rc) goto out;
+rc = libxl__get_domain_configuration(gc, domid, &d_config);
+if (rc) goto out;
-device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
- &pcidev_saved);
+device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
+ &pcidev_saved);
-rc = libxl__dm_check_start(gc, &d_config, domid);
-if (rc) goto out;
+rc = libxl__dm_check_start(gc, &d_config, domid);
+if (rc) goto out;
+}
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
if (rc) goto out;
-rc = libxl__set_domain_configuration(gc, domid, &d_config);
-if (rc) goto out;
+if (lock) {
+rc = libxl__set_domain_configuration(gc, domid, &d_config);
+if (rc) goto out;
+}
libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc, back));
@@ -184,8 +196,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
uint32_t domid, libxl_d
out:
libxl__xs_transaction_abort(gc, &t);
if (lock) libxl__unlock_domain_userdata(lock);
-libxl_device_pci_dispose(&pcidev_saved);
-libxl_domain_config_dispose(&d_config);
+if (!is_stubdomain) {
+libxl_device_pci_dispose(&pcidev_saved);
+libxl_domain_config_dispose(&d_config);
+}
return rc;
}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 5/6] xen/x86: add PHYSDEVOP_msi_set_enable
Allow device model running in stubdomain to enable/disable MSI(-X),
bypassing pciback. While pciback is still used to access config space
from within stubdomain, it refuse to write to
PCI_MSI_FLAGS_ENABLE/PCI_MSIX_FLAGS_ENABLE in non-permissive mode. Which
is the right thing to do for PV domain (the main use case for pciback),
as PV domain should use XEN_PCI_OP_* commands for that. Unfortunately
those commands are not good for stubdomain use, as they configure MSI in
dom0's kernel too, which should not happen for HVM domain.
This new physdevop is allowed only for stubdomain controlling the domain
which own the device.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- new patch
Changes in v4:
- adjust code style
- s/msi_msix/msi/
- add msi_set_enable XSM hook
- flatten struct physdev_msi_set_enable
- add to include/xlat.lst
I'm not sure if XSM part is correct, compile-tested only, as I'm not
sure how to set the policy.
---
xen/arch/x86/msi.c| 24
xen/arch/x86/physdev.c| 24
xen/arch/x86/x86_64/physdev.c | 4
xen/include/asm-x86/msi.h | 1 +
xen/include/public/physdev.h | 15 +++
xen/include/xlat.lst | 1 +
xen/include/xsm/dummy.h | 7 +++
xen/include/xsm/xsm.h | 6 ++
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c | 25 +
10 files changed, 108 insertions(+)
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index babc414..c490c67 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -1474,6 +1474,30 @@ int pci_restore_msi_state(struct pci_dev *pdev)
return 0;
}
+int msi_msix_set_enable(struct pci_dev *pdev, int mode, int enable)
+{
+int ret;
+
+ret = xsm_msi_set_enable(XSM_DM_PRIV, pdev->domain,
+ (pdev->seg << 16) | (pdev->bus << 8) |
pdev->devfn,
+ mode, enable);
+if ( ret )
+return ret;
+
+switch ( mode )
+{
+case PHYSDEVOP_MSI_SET_ENABLE_MSI:
+msi_set_enable(pdev, enable);
+break;
+
+case PHYSDEVOP_MSI_SET_ENABLE_MSIX:
+msix_set_enable(pdev, enable);
+break;
+}
+
+return 0;
+}
+
void __init early_msi_init(void)
{
if ( use_msi < 0 )
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index de59e39..ead8af9 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -671,6 +671,30 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
arg)
break;
}
+case PHYSDEVOP_msi_set_enable: {
+struct physdev_msi_set_enable op;
+struct pci_dev *pdev;
+
+ret = -EFAULT;
+if ( copy_from_guest(&op, arg, 1) )
+break;
+
+ret = -EINVAL;
+if ( op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSI &&
+ op.mode != PHYSDEVOP_MSI_SET_ENABLE_MSIX )
+break;
+
+pcidevs_lock();
+pdev = pci_get_pdev(op.seg, op.bus, op.devfn);
+if ( pdev )
+ret = msi_msix_set_enable(pdev, op.mode, !!op.enable);
+else
+ret = -ENODEV;
+pcidevs_unlock();
+break;
+
+}
+
default:
ret = -ENOSYS;
break;
diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
index c5a00ea..cb26b1e 100644
--- a/xen/arch/x86/x86_64/physdev.c
+++ b/xen/arch/x86/x86_64/physdev.c
@@ -76,6 +76,10 @@ CHECK_physdev_pci_device_add
CHECK_physdev_pci_device
#undef xen_physdev_pci_device
+#define xen_physdev_msi_set_enable physdev_msi_set_enable
+CHECK_physdev_msi_set_enable
+#undef xen_physdev_msi_set_enable
+
#define COMPAT
#undef guest_handle_okay
#define guest_handle_okay compat_handle_okay
diff --git a/xen/include/asm-x86/msi.h b/xen/include/asm-x86/msi.h
index 10387dc..7a22595 100644
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -252,5 +252,6 @@ void guest_mask_msi_irq(struct irq_desc *, bool mask);
void ack_nonmaskable_msi_irq(struct irq_desc *);
void end_nonmaskable_msi_irq(struct irq_desc *, u8 vector);
void set_msi_affinity(struct irq_desc *, const cpumask_t *);
+int msi_msix_set_enable(struct pci_dev *pdev, int mode, int enable);
#endif /* __ASM_MSI_H */
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index b6faf83..187fc23 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -344,6 +344,21 @@ struct physdev_dbgp_op {
typedef struct physdev_dbgp_op physdev_dbgp_op_t;
DEFINE_XEN_GUEST_HANDLE(physdev_dbgp_op_t);
+#define PHYSDEVOP_MSI_SET_ENABLE_MSI 0
+#define PHYSDEVOP_MSI_SET_ENABLE_MSIX 1
+
+#define PHYSDEVOP_msi_set_enable 32
+struct physdev_msi_set_enable {
+/* IN */
+uint16_t seg;
+uint8_t bus;
+uint8_t devfn;
+uint8_t mode;
+uint8_t enable;
+};
+typedef struct physdev_msi_set_enable physdev_msi_set_enable_t;
+DEFINE_XEN_GUEST_HANDLE(physdev_msi_set_enable_t);
+
/*
* Not
[Xen-devel] [PATCH v4 4/6] xen/x86: Allow stubdom access to irq created for msi.
From: Simon Gaiser
Stubdomains need to be given sufficient privilege over the guest which it
provides emulation for in order for PCI passthrough to work correctly.
When a HVM domain try to enable MSI, QEMU in stubdomain calls
PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
part of xc_domain_update_msi_irq. Allow for that as part of
PHYSDEVOP_map_pirq.
This is not needed for PCI INTx, because IRQ in that case is known
beforehand and the stubdomain is given permissions over this IRQ by
libxl__device_pci_add (there's a do_pci_add against the stubdomain).
Based on
https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
by Eric Chanudet .
Signed-off-by: Simon Gaiser
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- extend commit message
Changes in v4:
- add missing destroy_irq on error path
With this patch, stubdomain will be able to create and map multiple irq
(DoS possibility?), as only target domain is validated in practice. Is
that ok? If not, what additional limits could be applied here?
In INTx case the problem doesn't apply, because toolstack grant access
to particular IRQ and no allocation happen on stubdomain request. But in
MSI case, it isn't that easy as IRQ number isn't known before (as
explained in the commit message).
---
xen/arch/x86/irq.c | 24
xen/arch/x86/physdev.c | 9 +
2 files changed, 33 insertions(+)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 8b44d6c..5e5dcac 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2674,6 +2674,22 @@ int allocate_and_map_msi_pirq(struct domain *d, int
index, int *pirq_p,
{
case MAP_PIRQ_TYPE_MULTI_MSI:
irq = create_irq(NUMA_NO_NODE);
+if ( !(irq < nr_irqs_gsi || irq >= nr_irqs) &&
+current->domain->target == d )
+{
+ret = irq_permit_access(current->domain, irq);
+if ( ret ) {
+dprintk(XENLOG_G_ERR,
+"dom%d: can't grant it's stubdom (%d) access to "
+"irq %d for msi: %d!\n",
+d->domain_id,
+current->domain->domain_id,
+irq,
+ret);
+destroy_irq(irq);
+return ret;
+}
+}
}
if ( irq < nr_irqs_gsi || irq >= nr_irqs )
@@ -2717,7 +2733,15 @@ int allocate_and_map_msi_pirq(struct domain *d, int
index, int *pirq_p,
case MAP_PIRQ_TYPE_MSI:
if ( index == -1 )
case MAP_PIRQ_TYPE_MULTI_MSI:
+{
+if ( current->domain->target == d &&
+irq_deny_access(current->domain, irq) )
+dprintk(XENLOG_G_ERR,
+"dom%d: can't revoke stubdom's access to irq
%d!\n",
+d->domain_id,
+irq);
destroy_irq(irq);
+}
break;
}
}
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 3a3c158..de59e39 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -164,6 +164,15 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
pcidevs_lock();
spin_lock(&d->event_lock);
+if ( current->domain->target == d)
+{
+int irq = domain_pirq_to_irq(d, pirq);
+if ( irq <= 0 || irq_deny_access(current->domain, irq) )
+dprintk(XENLOG_G_ERR,
+"dom%d: can't revoke stubdom's access to irq %d!\n",
+d->domain_id,
+irq);
+}
ret = unmap_domain_pirq(d, pirq);
spin_unlock(&d->event_lock);
pcidevs_unlock();
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 0/6] Fix PCI passthrough for HVM with stubdomain
In this version, I add PHYSDEVOP_msi_set_enable to allow stubdomain enabling MSI after mapping it. Related article: https://www.qubes-os.org/news/2017/10/18/msi-support/ Changes in v2: - new "xen/x86: Allow stubdom access to irq created for msi" patch - applied review comments from v1 Changes is v3: - apply suggestions by Roger - add PHYSDEVOP_msi_msix_set_enable Changes in v4: - implement suggestions by Wei, Roger, Jan - plug new physdevop into XSM Marek Marczykowski-Górecki (5): libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use libxl: attach PCI device to qemu only after setting pciback/pcifront libxl: don't try to manipulate json config for stubdomain xen/x86: add PHYSDEVOP_msi_set_enable tools/libxc: add wrapper for PHYSDEVOP_msi_set_enable Simon Gaiser (1): xen/x86: Allow stubdom access to irq created for msi. tools/libxc/include/xenctrl.h | 7 - tools/libxc/xc_physdev.c | 21 - tools/libxl/libxl_pci.c | 63 xen/arch/x86/irq.c| 24 ++- xen/arch/x86/msi.c| 24 ++- xen/arch/x86/physdev.c| 33 +++- xen/arch/x86/x86_64/physdev.c | 4 ++- xen/include/asm-x86/msi.h | 1 +- xen/include/public/physdev.h | 15 +- xen/include/xlat.lst | 1 +- xen/include/xsm/dummy.h | 7 - xen/include/xsm/xsm.h | 6 +++- xen/xsm/dummy.c | 1 +- xen/xsm/flask/hooks.c | 25 ++- 14 files changed, 212 insertions(+), 20 deletions(-) base-commit: 93a62c544e20ba9e141e411bbaae3d65259d13a3 -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 1/6] libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use
HVM domains use IOMMU and device model assistance for communicating with
PCI devices, xen-pcifront/pciback isn't directly needed by HVM domain.
But pciback serve also second function - it reset the device when it is
deassigned from the guest and for this reason pciback needs to be used
with HVM domain too.
When HVM domain has device model in stubdomain, attaching xen-pciback to
the target domain itself may prevent attaching xen-pciback to the
(PV) stubdomain, effectively breaking PCI passthrough.
Fix this by attaching pciback only to one domain: if PV stubdomain is in
use, let it be stubdomain (the commit prevents attaching device to target
HVM in this case); otherwise, attach it to the target domain.
Signed-off-by: Marek Marczykowski-Górecki
Acked-by: Wei Liu
---
Changes in v2:
- previously called "libxl: attach xen-pciback only to PV domains"
- instead of excluding all HVMs, change the condition to what actually
matters here - check if stubdomain is in use; this way xen-pciback is
always in use (either for the target domain, or it's stubdomain),
fixing PCI reset by xen-pciback concerns
Changes in v3:
- adjust commit message
---
tools/libxl/libxl_pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 87afa03..3b6b23c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1106,7 +1106,7 @@ out:
}
}
-if (!starting)
+if (!starting && !libxl_get_stubdom_id(CTX, domid))
rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
else
rc = 0;
@@ -1302,7 +1302,7 @@ static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao
*ao, uint32_t domid,
}
}
-if (d_config->num_pcidevs > 0) {
+if (d_config->num_pcidevs > 0 && !libxl_get_stubdom_id(CTX, domid)) {
rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
d_config->num_pcidevs);
if (rc < 0) {
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 6/6] tools/libxc: add wrapper for PHYSDEVOP_msi_set_enable
Add libxc wrapper for PHYSDEVOP_msi_set_enable introduced in
previous commit.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- new patch
Changes in v4:
- adjust for updated previous patch
---
tools/libxc/include/xenctrl.h | 7 +++
tools/libxc/xc_physdev.c | 21 +
2 files changed, 28 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 31cdda7..879e926 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1639,6 +1639,13 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
uint32_t domid,
int pirq);
+int xc_physdev_msi_set_enable(xc_interface *xch,
+ int seg,
+ int bus,
+ int devfn,
+ int mode,
+ int enable);
+
/*
* LOGGING AND ERROR REPORTING
*/
diff --git a/tools/libxc/xc_physdev.c b/tools/libxc/xc_physdev.c
index 460a8e7..af6116f 100644
--- a/tools/libxc/xc_physdev.c
+++ b/tools/libxc/xc_physdev.c
@@ -111,3 +111,24 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
return rc;
}
+int xc_physdev_msi_set_enable(xc_interface *xch,
+ int seg,
+ int bus,
+ int devfn,
+ int mode,
+ int enable)
+{
+int rc;
+struct physdev_msi_set_enable op;
+
+memset(&op, 0, sizeof(struct physdev_msi_set_enable));
+op.seg = seg;
+op.bus = bus;
+op.devfn = devfn;
+op.mode = mode;
+op.enable = enable;
+
+rc = do_physdev_op(xch, PHYSDEVOP_msi_set_enable, &op, sizeof(op));
+
+return rc;
+}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 bisection] complete test-amd64-i386-xl-raw
branch xen-unstable xenbranch xen-unstable job test-amd64-i386-xl-raw testid xen-boot Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.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/stable/linux-stable.git Bug introduced: 4c35624dcb3bce026bb08eb04085c187bafff863 Bug not present: ac35b66883e8330ffde609152e13c225b12de6a4 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/132983/ (Revision log too long, omitted.) For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-3.18/test-amd64-i386-xl-raw.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-3.18/test-amd64-i386-xl-raw.xen-boot --summary-out=tmp/132983.bisection-summary --basis-template=128858 --blessings=real,real-bisect linux-3.18 test-amd64-i386-xl-raw xen-boot Searching for failure / basis pass: 132798 fail [host=debina1] / 132456 [host=albana0] 131729 [host=baroque0] 131705 [host=rimava1] 131673 [host=fiano1] 131666 [host=huxelrebe0] 131641 [host=joubertin0] 131619 [host=albana0] 131593 [host=fiano0] 131580 [host=baroque1] 131563 [host=huxelrebe1] 131535 [host=debina0] 128858 [host=baroque0] 128841 [host=debina0] 128807 [host=fiano0] 128691 [host=albana1] 128258 [host=fiano0] 128232 [host=chardonnay0] 128177 [host=pinot0] 128096 [host=baroque0] 127486 [host=baroque0] 127472 [host=ital\ ia0] 127455 [host=rimava1] 127296 [host=fiano0] 127001 [host=pinot1] 126926 [host=pinot1] 126813 [host=pinot1] 126711 [host=pinot1] 126583 [host=pinot1] 126472 [host=pinot1] 126362 [host=pinot1] 126270 [host=pinot1] 126189 [host=pinot1] 126042 [host=albana0] 125899 [host=italia0] 125658 [host=rimava1] 125649 [host=italia0] 125641 [host=debina0] 125561 [host=pinot0] 125525 [host=joubertin1] 125505 [host=chardonnay1] 125138 ok. Failure / basis pass flights: 132798 / 125138 (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/stable/linux-stable.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 4c35624dcb3bce026bb08eb04085c187bafff863 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 de5b678ca4dcdfa83e322491d478d66df56c1986 f50dd67950ca9d5a517501af10de7c8d88d1a188 Basis pass ac35b66883e8330ffde609152e13c225b12de6a4 c530a75c1e6a472b0eb9558310b518f0dfcd8860 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a b4ac4bc410222d221dc46a74ac71efaa7b32d57c Generating revisions with ./adhoc-revtuple-generator git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git#ac35b66883e8330ffde609152e13c225b12de6a4-4c35624dcb3bce026bb08eb04085c187bafff863 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git\ #43139135a8938de44f66333831d3a8655d07663a-de5b678ca4dcdfa83e322491d478d66df56c1986 git://xenbits.xen.org/xen.git#b4ac4bc410222d221dc46a74ac71efaa7b32d57c-f50dd67950ca9d5a517501af10de7c8d88d1a188 adhoc-revtuple-generator: tree discontiguous: linux-stable adhoc-revtuple-generator: tree discontiguous: qemu-xen adhoc-revtuple-generator: tree discontiguous: xen Loaded 1007 nodes in revision graph Searching for test results: 113503 [host=nobling1] 113856 [host=nobling0] 113869 [host=baroque0] 114034 [host=nocera0] 114133 [host=pinot1] 114180 [host=pinot1] 114225 [host=chardonnay1] 114446 [host=baroque0] 114677 [host=baroque1] 114843 [host=merlot1] 115289 [host=pinot1] 115479 [host=baroque0] 115495 [host=nobling0] 115688 [host=nobling1] 115673 [host=nobling0] 115698 [host=elbling0] 115714 [host=fiano0] 115729 [host=fiano0] 116106 [host=chardonnay1] 116121 [host=pinot0] 116140 [host=merlot0] 116193 [host=merlot1] 116308 [host=baroque0] 116475 [host=italia0] 116501 [host=nocera0] 116728 [host=chardonnay0] 116760 [host=rimava0] 116862 [host=fiano0] 116890 [host=huxelrebe0] 116920 [host=baroque0] 117131 [host=pinot0] 117211 [host=elbling0] 117375 [host=baroque0] 117641 [host=italia1] 117702 [host=huxelrebe1] 118149 [host=pinot1] 118186 [host=fiano0] 118488 [host=chardonnay1] 118666 [host=huxelrebe0] 118730 [host=pinot
