* [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable
@ 2024-05-15 8:57 Sergiy Kibrik
2024-05-15 8:59 ` [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options Sergiy Kibrik
` (14 more replies)
0 siblings, 15 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 8:57 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
George Dunlap, Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu,
Paul Durrant, Julien Grall, Stefano Stabellini, Xenia Ragiadakou
This is yet another attempt to provide a means to render the cpu virtualization
technology support in Xen configurable.
Currently, irrespectively of the target platform, both AMD-V and Intel VT-x
drivers are built.
The series adds three new Kconfig controls, ALT2PM, SVM and VMX, that can be
used to switch to a finer-grained configuration for a given platform, and
reduce dead code.
The code separation is done using the new config guards.
In this series, comparing to v1, new option CONFIG_ALT2PM introduced, that
can enable/disable altp2m support independently on VMX.
More specific changes are provided in per-patch changelog.
v1 series here:
https://lore.kernel.org/xen-devel/20240416061845.3467822-1-Sergiy_Kibrik@epam.com/
-Sergiy
Sergiy Kibrik (11):
x86/monitor: guard altp2m usage
x86/p2m: guard altp2m routines
x86/p2m: move altp2m-related code to separate file
x86: introduce CONFIG_ALTP2M Kconfig option
x86/p2m: guard altp2m code with CONFIG_ALTP2M option
x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
x86/traps: clean up superfluous #idef-s
x86/domain: clean up superfluous #idef-s
x86/vmx: guard access to cpu_has_vmx_* in common code
iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
Xenia Ragiadakou (4):
x86: introduce AMD-V and Intel VT-x Kconfig options
x86/oprofile: guard svm specific symbols with CONFIG_SVM
x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
x86/hvm: make AMD-V and Intel VT-x support configurable
xen/arch/x86/Kconfig | 25 +
xen/arch/x86/cpu/vpmu_amd.c | 8 +-
xen/arch/x86/cpu/vpmu_intel.c | 20 +-
xen/arch/x86/domain.c | 4 +-
xen/arch/x86/hvm/Makefile | 4 +-
xen/arch/x86/hvm/emulate.c | 2 +
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/hvm/ioreq.c | 2 +
xen/arch/x86/hvm/monitor.c | 4 +-
xen/arch/x86/hvm/viridian/viridian.c | 4 +-
xen/arch/x86/include/asm/altp2m.h | 5 +-
xen/arch/x86/include/asm/cpufeature.h | 6 +-
xen/arch/x86/include/asm/hvm/hvm.h | 2 +-
xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 4 +-
xen/arch/x86/include/asm/p2m.h | 17 +-
xen/arch/x86/mm/Makefile | 5 +-
xen/arch/x86/mm/altp2m.c | 630 +++++++++++++++++++++++
xen/arch/x86/mm/hap/Makefile | 2 +-
xen/arch/x86/mm/p2m-basic.c | 19 +-
xen/arch/x86/mm/p2m-ept.c | 2 +-
xen/arch/x86/mm/p2m.c | 632 +-----------------------
xen/arch/x86/mm/p2m.h | 3 +
xen/arch/x86/oprofile/op_model_athlon.c | 2 +-
xen/arch/x86/traps.c | 9 +-
xen/drivers/passthrough/vtd/iommu.c | 6 +-
xen/include/xen/sched.h | 2 +-
26 files changed, 738 insertions(+), 683 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 62+ messages in thread
* [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
@ 2024-05-15 8:59 ` Sergiy Kibrik
2024-05-16 10:09 ` Jan Beulich
2024-05-15 9:01 ` [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage Sergiy Kibrik
` (13 subsequent siblings)
14 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 8:59 UTC (permalink / raw
To: xen-devel
Cc: Xenia Ragiadakou, Andrew Cooper, Roger Pau Monné,
George Dunlap, Jan Beulich, Stefano Stabellini, Xenia Ragiadakou,
Sergiy Kibrik
From: Xenia Ragiadakou <burzalodowa@gmail.com>
Introduce two new Kconfig options, SVM and VMX, to allow code
specific to each virtualization technology to be separated and, when not
required, stripped.
CONFIG_SVM will be used to enable virtual machine extensions on platforms that
implement the AMD Virtualization Technology (AMD-V).
CONFIG_VMX will be used to enable virtual machine extensions on platforms that
implement the Intel Virtualization Technology (Intel VT-x).
Both features depend on HVM support.
Since, at this point, disabling any of them would cause Xen to not compile,
the options are enabled by default if HVM and are not selectable by the user.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v2:
- simplify kconfig expression to def_bool HVM
- keep file list in Makefile in alphabetical order
changes in v1:
- change kconfig option name AMD_SVM/INTEL_VMX -> SVM/VMX
---
xen/arch/x86/Kconfig | 6 ++++++
xen/arch/x86/hvm/Makefile | 4 ++--
xen/arch/x86/mm/Makefile | 3 ++-
xen/arch/x86/mm/hap/Makefile | 2 +-
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 7e03e4bc55..8c9f8431f0 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -122,6 +122,12 @@ config HVM
If unsure, say Y.
+config SVM
+ def_bool HVM
+
+config VMX
+ def_bool HVM
+
config XEN_SHSTK
bool "Supervisor Shadow Stacks"
depends on HAS_AS_CET_SS
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 3464191544..8434badc64 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -1,5 +1,5 @@
-obj-y += svm/
-obj-y += vmx/
+obj-$(CONFIG_SVM) += svm/
+obj-$(CONFIG_VMX) += vmx/
obj-y += viridian/
obj-y += asid.o
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 0803ac9297..0128ca7ab6 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MEM_SHARING) += mem_sharing.o
obj-$(CONFIG_HVM) += nested.o
obj-$(CONFIG_HVM) += p2m.o
obj-y += p2m-basic.o
-obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o p2m-pt.o
+obj-$(CONFIG_VMX) += p2m-ept.o
+obj-$(CONFIG_HVM) += p2m-pod.o p2m-pt.o
obj-y += paging.o
obj-y += physmap.o
diff --git a/xen/arch/x86/mm/hap/Makefile b/xen/arch/x86/mm/hap/Makefile
index 8ef54b1faa..98c8a87819 100644
--- a/xen/arch/x86/mm/hap/Makefile
+++ b/xen/arch/x86/mm/hap/Makefile
@@ -3,4 +3,4 @@ obj-y += guest_walk_2.o
obj-y += guest_walk_3.o
obj-y += guest_walk_4.o
obj-y += nested_hap.o
-obj-y += nested_ept.o
+obj-$(CONFIG_VMX) += nested_ept.o
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
2024-05-15 8:59 ` [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options Sergiy Kibrik
@ 2024-05-15 9:01 ` Sergiy Kibrik
2024-05-16 0:29 ` Stefano Stabellini
2024-05-16 10:20 ` Jan Beulich
2024-05-15 9:03 ` [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines Sergiy Kibrik
` (12 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:01 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Alexandru Isaila, Petre Pircalabu, Andrew Cooper,
Roger Pau Monné, Jan Beulich, Stefano Stabellini,
Xenia Ragiadakou, Tamas K Lengyel
Explicitly check whether altp2m is on for domain when getting altp2m index.
If explicit call to altp2m_active() always returns false, DCE will remove
call to altp2m_vcpu_idx().
The puspose of that is later to be able to disable altp2m support and
exclude its code from the build completely, when not supported by target
platform (as of now it's supported for VT-d only).
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v2:
- patch description changed, removed VMX mentioning
- guard by altp2m_active() instead of hvm_altp2m_supported()
---
xen/arch/x86/hvm/monitor.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 2a8ff07ec9..74621000b2 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -262,6 +262,8 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
struct vcpu *curr = current;
vm_event_request_t req = {};
paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
+ unsigned int altp2m_idx = altp2m_active(curr->domain) ?
+ altp2m_vcpu_idx(curr) : 0;
int rc;
ASSERT(curr->arch.vm_event->send_event);
@@ -270,7 +272,7 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
* p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
* in which case access must be restricted.
*/
- rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
+ rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);
if ( rc == -ESRCH )
access = XENMEM_access_n;
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
2024-05-15 8:59 ` [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options Sergiy Kibrik
2024-05-15 9:01 ` [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage Sergiy Kibrik
@ 2024-05-15 9:03 ` Sergiy Kibrik
2024-05-16 0:31 ` Stefano Stabellini
2024-05-16 10:26 ` Jan Beulich
2024-05-15 9:06 ` [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file Sergiy Kibrik
` (11 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:03 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, George Dunlap,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
Initialize and bring down altp2m only when it is supported by the platform,
e.g. VMX. Also guard p2m_altp2m_propagate_change().
The puspose of that is the possiblity to disable altp2m support and exclude its
code from the build completely, when it's not supported by the target platform.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
---
xen/arch/x86/mm/p2m-basic.c | 19 +++++++++++--------
xen/arch/x86/mm/p2m-ept.c | 2 +-
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/mm/p2m-basic.c b/xen/arch/x86/mm/p2m-basic.c
index 8599bd15c6..90106997d7 100644
--- a/xen/arch/x86/mm/p2m-basic.c
+++ b/xen/arch/x86/mm/p2m-basic.c
@@ -126,13 +126,15 @@ int p2m_init(struct domain *d)
return rc;
}
- rc = p2m_init_altp2m(d);
- if ( rc )
+ if ( hvm_altp2m_supported() )
{
- p2m_teardown_hostp2m(d);
- p2m_teardown_nestedp2m(d);
+ rc = p2m_init_altp2m(d);
+ if ( rc )
+ {
+ p2m_teardown_hostp2m(d);
+ p2m_teardown_nestedp2m(d);
+ }
}
-
return rc;
}
@@ -195,11 +197,12 @@ void p2m_final_teardown(struct domain *d)
{
if ( is_hvm_domain(d) )
{
+ if ( hvm_altp2m_supported() )
+ p2m_teardown_altp2m(d);
/*
- * We must tear down both of them unconditionally because
- * we initialise them unconditionally.
+ * We must tear down nestedp2m unconditionally because
+ * we initialise it unconditionally.
*/
- p2m_teardown_altp2m(d);
p2m_teardown_nestedp2m(d);
}
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index f83610cb8c..d264df5b14 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -986,7 +986,7 @@ out:
if ( is_epte_present(&old_entry) )
ept_free_entry(p2m, &old_entry, target);
- if ( entry_written && p2m_is_hostp2m(p2m) )
+ if ( entry_written && p2m_is_hostp2m(p2m) && hvm_altp2m_supported())
{
ret = p2m_altp2m_propagate_change(d, _gfn(gfn), mfn, order, p2mt, p2ma);
if ( !rc )
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (2 preceding siblings ...)
2024-05-15 9:03 ` [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines Sergiy Kibrik
@ 2024-05-15 9:06 ` Sergiy Kibrik
2024-05-16 0:35 ` Stefano Stabellini
2024-05-16 10:37 ` Jan Beulich
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
` (10 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:06 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Andrew Cooper, George Dunlap, Roger Pau Monné,
Jan Beulich, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
Move altp2m code from generic p2m.c file to altp2m.c, so it is kept separately
and can possibly be disabled in the build. We may want to disable it when
building for specific platform only, that doesn't support alternate p2m.
No functional change intended.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v2:
- no double blank lines
- no unrelated re-formatting
- header #include-s ordering
- changed patch description
---
xen/arch/x86/mm/altp2m.c | 630 ++++++++++++++++++++++++++++++++++++++
xen/arch/x86/mm/p2m.c | 632 +--------------------------------------
xen/arch/x86/mm/p2m.h | 3 +
3 files changed, 635 insertions(+), 630 deletions(-)
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index a04297b646..6fe1e9ed6b 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -7,6 +7,8 @@
#include <asm/hvm/hvm.h>
#include <asm/p2m.h>
#include <asm/altp2m.h>
+#include <public/hvm/hvm_op.h>
+#include <xen/event.h>
#include "mm-locks.h"
#include "p2m.h"
@@ -151,6 +153,634 @@ void p2m_teardown_altp2m(struct domain *d)
}
}
+int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
+ p2m_type_t *t, p2m_access_t *a,
+ bool prepopulate)
+{
+ *mfn = ap2m->get_entry(ap2m, gfn, t, a, 0, NULL, NULL);
+
+ /* Check host p2m if no valid entry in alternate */
+ if ( !mfn_valid(*mfn) && !p2m_is_hostp2m(ap2m) )
+ {
+ struct p2m_domain *hp2m = p2m_get_hostp2m(ap2m->domain);
+ unsigned int page_order;
+ int rc;
+
+ *mfn = p2m_get_gfn_type_access(hp2m, gfn, t, a, P2M_ALLOC | P2M_UNSHARE,
+ &page_order, 0);
+
+ rc = -ESRCH;
+ if ( !mfn_valid(*mfn) || *t != p2m_ram_rw )
+ return rc;
+
+ /* If this is a superpage, copy that first */
+ if ( prepopulate && page_order != PAGE_ORDER_4K )
+ {
+ unsigned long mask = ~((1UL << page_order) - 1);
+ gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
+ mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
+
+ rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
+ if ( rc )
+ return rc;
+ }
+ }
+
+ return 0;
+}
+
+void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
+{
+ if ( altp2m_active(v->domain) )
+ p2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
+bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
+{
+ struct domain *d = v->domain;
+ bool rc = false;
+
+ if ( idx >= MAX_ALTP2M )
+ return rc;
+
+ altp2m_list_lock(d);
+
+ if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
+ {
+ if ( p2m_set_altp2m(v, idx) )
+ altp2m_vcpu_update_p2m(v);
+ rc = 1;
+ }
+
+ altp2m_list_unlock(d);
+ return rc;
+}
+
+/*
+ * Read info about the gfn in an altp2m, locking the gfn.
+ *
+ * If the entry is valid, pass the results back to the caller.
+ *
+ * If the entry was invalid, and the host's entry is also invalid,
+ * return to the caller without any changes.
+ *
+ * If the entry is invalid, and the host entry was valid, propagate
+ * the host's entry to the altp2m (retaining page order), and indicate
+ * that the caller should re-try the faulting instruction.
+ */
+bool p2m_altp2m_get_or_propagate(struct p2m_domain *ap2m, unsigned long gfn_l,
+ mfn_t *mfn, p2m_type_t *p2mt,
+ p2m_access_t *p2ma, unsigned int *page_order)
+{
+ p2m_type_t ap2mt;
+ p2m_access_t ap2ma;
+ unsigned int cur_order;
+ unsigned long mask;
+ gfn_t gfn;
+ mfn_t amfn;
+ int rc;
+
+ /*
+ * NB we must get the full lock on the altp2m here, in addition to
+ * the lock on the individual gfn, since we may change a range of
+ * gfns below.
+ */
+ p2m_lock(ap2m);
+
+ amfn = get_gfn_type_access(ap2m, gfn_l, &ap2mt, &ap2ma, 0, &cur_order);
+
+ if ( cur_order > *page_order )
+ cur_order = *page_order;
+
+ if ( !mfn_eq(amfn, INVALID_MFN) )
+ {
+ p2m_unlock(ap2m);
+ *mfn = amfn;
+ *p2mt = ap2mt;
+ *p2ma = ap2ma;
+ *page_order = cur_order;
+ return false;
+ }
+
+ /* Host entry is also invalid; don't bother setting the altp2m entry. */
+ if ( mfn_eq(*mfn, INVALID_MFN) )
+ {
+ p2m_unlock(ap2m);
+ *page_order = cur_order;
+ return false;
+ }
+
+ /*
+ * If this is a superpage mapping, round down both frame numbers
+ * to the start of the superpage. NB that we repupose `amfn`
+ * here.
+ */
+ mask = ~((1UL << cur_order) - 1);
+ amfn = _mfn(mfn_x(*mfn) & mask);
+ gfn = _gfn(gfn_l & mask);
+
+ /* Override the altp2m entry with its default access. */
+ *p2ma = ap2m->default_access;
+
+ rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
+ p2m_unlock(ap2m);
+
+ if ( rc )
+ {
+ gprintk(XENLOG_ERR,
+ "failed to set entry for %"PRI_gfn" -> %"PRI_mfn" altp2m %u, rc %d\n",
+ gfn_l, mfn_x(amfn), vcpu_altp2m(current).p2midx, rc);
+ domain_crash(ap2m->domain);
+ }
+
+ return true;
+}
+
+enum altp2m_reset_type {
+ ALTP2M_RESET,
+ ALTP2M_DEACTIVATE
+};
+
+static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
+ enum altp2m_reset_type reset_type)
+{
+ struct p2m_domain *p2m;
+
+ ASSERT(idx < MAX_ALTP2M);
+ p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+
+ p2m_lock(p2m);
+
+ p2m_flush_table_locked(p2m);
+
+ if ( reset_type == ALTP2M_DEACTIVATE )
+ p2m_free_logdirty(p2m);
+
+ /* Uninit and reinit ept to force TLB shootdown */
+ ept_p2m_uninit(p2m);
+ ept_p2m_init(p2m);
+
+ p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
+ p2m->max_remapped_gfn = 0;
+
+ p2m_unlock(p2m);
+}
+
+void p2m_flush_altp2m(struct domain *d)
+{
+ unsigned int i;
+
+ altp2m_list_lock(d);
+
+ for ( i = 0; i < MAX_ALTP2M; i++ )
+ {
+ p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
+ d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
+ d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
+ }
+
+ altp2m_list_unlock(d);
+}
+
+static int p2m_activate_altp2m(struct domain *d, unsigned int idx,
+ p2m_access_t hvmmem_default_access)
+{
+ struct p2m_domain *hostp2m, *p2m;
+ int rc;
+
+ ASSERT(idx < MAX_ALTP2M);
+
+ p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+ hostp2m = p2m_get_hostp2m(d);
+
+ p2m_lock(p2m);
+
+ rc = p2m_init_logdirty(p2m);
+
+ if ( rc )
+ goto out;
+
+ /* The following is really just a rangeset copy. */
+ rc = rangeset_merge(p2m->logdirty_ranges, hostp2m->logdirty_ranges);
+
+ if ( rc )
+ {
+ p2m_free_logdirty(p2m);
+ goto out;
+ }
+
+ p2m->default_access = hvmmem_default_access;
+ p2m->domain = hostp2m->domain;
+ p2m->global_logdirty = hostp2m->global_logdirty;
+ p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
+ p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0;
+
+ p2m_init_altp2m_ept(d, idx);
+
+ out:
+ p2m_unlock(p2m);
+
+ return rc;
+}
+
+int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
+{
+ int rc = -EINVAL;
+ struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
+
+ if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
+ return rc;
+
+ altp2m_list_lock(d);
+
+ if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
+ mfn_x(INVALID_MFN) )
+ rc = p2m_activate_altp2m(d, idx, hostp2m->default_access);
+
+ altp2m_list_unlock(d);
+ return rc;
+}
+
+int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
+ xenmem_access_t hvmmem_default_access)
+{
+ int rc = -EINVAL;
+ unsigned int i;
+ p2m_access_t a;
+ struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
+
+ if ( hvmmem_default_access > XENMEM_access_default ||
+ !xenmem_access_to_p2m_access(hostp2m, hvmmem_default_access, &a) )
+ return rc;
+
+ altp2m_list_lock(d);
+
+ for ( i = 0; i < MAX_ALTP2M; i++ )
+ {
+ if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
+ continue;
+
+ rc = p2m_activate_altp2m(d, i, a);
+
+ if ( !rc )
+ *idx = i;
+
+ break;
+ }
+
+ altp2m_list_unlock(d);
+ return rc;
+}
+
+int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
+{
+ struct p2m_domain *p2m;
+ int rc = -EBUSY;
+
+ if ( !idx || idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
+ return rc;
+
+ rc = domain_pause_except_self(d);
+ if ( rc )
+ return rc;
+
+ rc = -EBUSY;
+ altp2m_list_lock(d);
+
+ if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
+ mfn_x(INVALID_MFN) )
+ {
+ p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+
+ if ( !_atomic_read(p2m->active_vcpus) )
+ {
+ p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
+ d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
+ mfn_x(INVALID_MFN);
+ d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)] =
+ mfn_x(INVALID_MFN);
+ rc = 0;
+ }
+ }
+
+ altp2m_list_unlock(d);
+
+ domain_unpause_except_self(d);
+
+ return rc;
+}
+
+int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
+{
+ struct vcpu *v;
+ int rc = -EINVAL;
+
+ if ( idx >= MAX_ALTP2M )
+ return rc;
+
+ rc = domain_pause_except_self(d);
+ if ( rc )
+ return rc;
+
+ rc = -EINVAL;
+ altp2m_list_lock(d);
+
+ if ( d->arch.altp2m_visible_eptp[idx] != mfn_x(INVALID_MFN) )
+ {
+ for_each_vcpu( d, v )
+ if ( p2m_set_altp2m(v, idx) )
+ altp2m_vcpu_update_p2m(v);
+
+ rc = 0;
+ }
+
+ altp2m_list_unlock(d);
+
+ domain_unpause_except_self(d);
+
+ return rc;
+}
+
+int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
+ gfn_t old_gfn, gfn_t new_gfn)
+{
+ struct p2m_domain *hp2m, *ap2m;
+ p2m_access_t a;
+ p2m_type_t t;
+ mfn_t mfn;
+ int rc = -EINVAL;
+
+ if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+ d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
+ mfn_x(INVALID_MFN) )
+ return rc;
+
+ hp2m = p2m_get_hostp2m(d);
+ ap2m = array_access_nospec(d->arch.altp2m_p2m, idx);
+
+ p2m_lock(hp2m);
+ p2m_lock(ap2m);
+
+ if ( gfn_eq(new_gfn, INVALID_GFN) )
+ {
+ mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
+ rc = mfn_valid(mfn)
+ ? p2m_remove_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K)
+ : 0;
+ goto out;
+ }
+
+ rc = altp2m_get_effective_entry(ap2m, old_gfn, &mfn, &t, &a,
+ AP2MGET_prepopulate);
+ if ( rc )
+ goto out;
+
+ rc = altp2m_get_effective_entry(ap2m, new_gfn, &mfn, &t, &a,
+ AP2MGET_query);
+ if ( rc )
+ goto out;
+
+ if ( !ap2m->set_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K, t, a,
+ (current->domain != d)) )
+ {
+ rc = 0;
+
+ if ( gfn_x(new_gfn) < ap2m->min_remapped_gfn )
+ ap2m->min_remapped_gfn = gfn_x(new_gfn);
+ if ( gfn_x(new_gfn) > ap2m->max_remapped_gfn )
+ ap2m->max_remapped_gfn = gfn_x(new_gfn);
+ }
+
+ out:
+ p2m_unlock(ap2m);
+ p2m_unlock(hp2m);
+ return rc;
+}
+
+int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
+ mfn_t mfn, unsigned int page_order,
+ p2m_type_t p2mt, p2m_access_t p2ma)
+{
+ struct p2m_domain *p2m;
+ unsigned int i;
+ unsigned int reset_count = 0;
+ unsigned int last_reset_idx = ~0;
+ int ret = 0;
+
+ if ( !altp2m_active(d) )
+ return 0;
+
+ altp2m_list_lock(d);
+
+ for ( i = 0; i < MAX_ALTP2M; i++ )
+ {
+ p2m_type_t t;
+ p2m_access_t a;
+
+ if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
+ continue;
+
+ p2m = d->arch.altp2m_p2m[i];
+
+ /* Check for a dropped page that may impact this altp2m */
+ if ( mfn_eq(mfn, INVALID_MFN) &&
+ gfn_x(gfn) + (1UL << page_order) > p2m->min_remapped_gfn &&
+ gfn_x(gfn) <= p2m->max_remapped_gfn )
+ {
+ if ( !reset_count++ )
+ {
+ p2m_reset_altp2m(d, i, ALTP2M_RESET);
+ last_reset_idx = i;
+ }
+ else
+ {
+ /* At least 2 altp2m's impacted, so reset everything */
+ for ( i = 0; i < MAX_ALTP2M; i++ )
+ {
+ if ( i == last_reset_idx ||
+ d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
+ continue;
+
+ p2m_reset_altp2m(d, i, ALTP2M_RESET);
+ }
+
+ ret = 0;
+ break;
+ }
+ }
+ else if ( !mfn_eq(get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0,
+ NULL), INVALID_MFN) )
+ {
+ int rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
+
+ /* Best effort: Don't bail on error. */
+ if ( !ret )
+ ret = rc;
+
+ p2m_put_gfn(p2m, gfn);
+ }
+ else
+ p2m_put_gfn(p2m, gfn);
+ }
+
+ altp2m_list_unlock(d);
+
+ return ret;
+}
+
+/*
+ * Set/clear the #VE suppress bit for a page. Only available on VMX.
+ */
+int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
+ unsigned int altp2m_idx)
+{
+ int rc;
+ struct xen_hvm_altp2m_suppress_ve_multi sve = {
+ altp2m_idx, suppress_ve, 0, 0, gfn_x(gfn), gfn_x(gfn), 0
+ };
+
+ if ( !(rc = p2m_set_suppress_ve_multi(d, &sve)) )
+ rc = sve.first_error;
+
+ return rc;
+}
+
+/*
+ * Set/clear the #VE suppress bit for multiple pages. Only available on VMX.
+ */
+int p2m_set_suppress_ve_multi(struct domain *d,
+ struct xen_hvm_altp2m_suppress_ve_multi *sve)
+{
+ struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
+ struct p2m_domain *ap2m = NULL;
+ struct p2m_domain *p2m = host_p2m;
+ uint64_t start = sve->first_gfn;
+ int rc = 0;
+
+ if ( sve->view > 0 )
+ {
+ if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+ d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
+ mfn_x(INVALID_MFN) )
+ return -EINVAL;
+
+ p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, sve->view);
+ }
+
+ p2m_lock(host_p2m);
+
+ if ( ap2m )
+ p2m_lock(ap2m);
+
+ while ( sve->last_gfn >= start )
+ {
+ p2m_access_t a;
+ p2m_type_t t;
+ mfn_t mfn;
+ int err = 0;
+
+ if ( (err = altp2m_get_effective_entry(p2m, _gfn(start), &mfn, &t, &a,
+ AP2MGET_query)) &&
+ !sve->first_error )
+ {
+ sve->first_error_gfn = start; /* Save the gfn of the first error */
+ sve->first_error = err; /* Save the first error code */
+ }
+
+ if ( !err && (err = p2m->set_entry(p2m, _gfn(start), mfn,
+ PAGE_ORDER_4K, t, a,
+ sve->suppress_ve)) &&
+ !sve->first_error )
+ {
+ sve->first_error_gfn = start; /* Save the gfn of the first error */
+ sve->first_error = err; /* Save the first error code */
+ }
+
+ /* Check for continuation if it's not the last iteration. */
+ if ( sve->last_gfn >= ++start && hypercall_preempt_check() )
+ {
+ rc = -ERESTART;
+ break;
+ }
+ }
+
+ sve->first_gfn = start;
+
+ if ( ap2m )
+ p2m_unlock(ap2m);
+
+ p2m_unlock(host_p2m);
+
+ return rc;
+}
+
+int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
+ unsigned int altp2m_idx)
+{
+ struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
+ struct p2m_domain *ap2m = NULL;
+ struct p2m_domain *p2m;
+ mfn_t mfn;
+ p2m_access_t a;
+ p2m_type_t t;
+ int rc = 0;
+
+ if ( altp2m_idx > 0 )
+ {
+ if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+ d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+ mfn_x(INVALID_MFN) )
+ return -EINVAL;
+
+ p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
+ }
+ else
+ p2m = host_p2m;
+
+ gfn_lock(host_p2m, gfn, 0);
+
+ if ( ap2m )
+ p2m_lock(ap2m);
+
+ mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, suppress_ve);
+ if ( !mfn_valid(mfn) )
+ rc = -ESRCH;
+
+ if ( ap2m )
+ p2m_unlock(ap2m);
+
+ gfn_unlock(host_p2m, gfn, 0);
+
+ return rc;
+}
+
+int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
+ uint8_t visible)
+{
+ int rc = 0;
+
+ altp2m_list_lock(d);
+
+ /*
+ * Eptp index is correlated with altp2m index and should not exceed
+ * min(MAX_ALTP2M, MAX_EPTP).
+ */
+ if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
+ d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
+ mfn_x(INVALID_MFN) )
+ rc = -EINVAL;
+ else if ( visible )
+ d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
+ d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)];
+ else
+ d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
+ mfn_x(INVALID_MFN);
+
+ altp2m_list_unlock(d);
+
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ce742c12e0..7c422a2d7e 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -500,7 +500,7 @@ int p2m_alloc_table(struct p2m_domain *p2m)
return 0;
}
-static int __must_check
+int __must_check
p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
unsigned int page_order)
{
@@ -1329,7 +1329,7 @@ p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
return p2m;
}
-static void
+void
p2m_flush_table_locked(struct p2m_domain *p2m)
{
struct page_info *top, *pg;
@@ -1729,481 +1729,6 @@ int unmap_mmio_regions(struct domain *d,
return i == nr ? 0 : i ?: ret;
}
-int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
- p2m_type_t *t, p2m_access_t *a,
- bool prepopulate)
-{
- *mfn = ap2m->get_entry(ap2m, gfn, t, a, 0, NULL, NULL);
-
- /* Check host p2m if no valid entry in alternate */
- if ( !mfn_valid(*mfn) && !p2m_is_hostp2m(ap2m) )
- {
- struct p2m_domain *hp2m = p2m_get_hostp2m(ap2m->domain);
- unsigned int page_order;
- int rc;
-
- *mfn = p2m_get_gfn_type_access(hp2m, gfn, t, a, P2M_ALLOC | P2M_UNSHARE,
- &page_order, 0);
-
- rc = -ESRCH;
- if ( !mfn_valid(*mfn) || *t != p2m_ram_rw )
- return rc;
-
- /* If this is a superpage, copy that first */
- if ( prepopulate && page_order != PAGE_ORDER_4K )
- {
- unsigned long mask = ~((1UL << page_order) - 1);
- gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
- mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
-
- rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
- if ( rc )
- return rc;
- }
- }
-
- return 0;
-}
-
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
- if ( altp2m_active(v->domain) )
- p2m_switch_vcpu_altp2m_by_id(v, idx);
-}
-
-bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
-{
- struct domain *d = v->domain;
- bool rc = false;
-
- if ( idx >= MAX_ALTP2M )
- return rc;
-
- altp2m_list_lock(d);
-
- if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
- {
- if ( p2m_set_altp2m(v, idx) )
- altp2m_vcpu_update_p2m(v);
- rc = 1;
- }
-
- altp2m_list_unlock(d);
- return rc;
-}
-
-/*
- * Read info about the gfn in an altp2m, locking the gfn.
- *
- * If the entry is valid, pass the results back to the caller.
- *
- * If the entry was invalid, and the host's entry is also invalid,
- * return to the caller without any changes.
- *
- * If the entry is invalid, and the host entry was valid, propagate
- * the host's entry to the altp2m (retaining page order), and indicate
- * that the caller should re-try the faulting instruction.
- */
-bool p2m_altp2m_get_or_propagate(struct p2m_domain *ap2m, unsigned long gfn_l,
- mfn_t *mfn, p2m_type_t *p2mt,
- p2m_access_t *p2ma, unsigned int *page_order)
-{
- p2m_type_t ap2mt;
- p2m_access_t ap2ma;
- unsigned int cur_order;
- unsigned long mask;
- gfn_t gfn;
- mfn_t amfn;
- int rc;
-
- /*
- * NB we must get the full lock on the altp2m here, in addition to
- * the lock on the individual gfn, since we may change a range of
- * gfns below.
- */
- p2m_lock(ap2m);
-
- amfn = get_gfn_type_access(ap2m, gfn_l, &ap2mt, &ap2ma, 0, &cur_order);
-
- if ( cur_order > *page_order )
- cur_order = *page_order;
-
- if ( !mfn_eq(amfn, INVALID_MFN) )
- {
- p2m_unlock(ap2m);
- *mfn = amfn;
- *p2mt = ap2mt;
- *p2ma = ap2ma;
- *page_order = cur_order;
- return false;
- }
-
- /* Host entry is also invalid; don't bother setting the altp2m entry. */
- if ( mfn_eq(*mfn, INVALID_MFN) )
- {
- p2m_unlock(ap2m);
- *page_order = cur_order;
- return false;
- }
-
- /*
- * If this is a superpage mapping, round down both frame numbers
- * to the start of the superpage. NB that we repupose `amfn`
- * here.
- */
- mask = ~((1UL << cur_order) - 1);
- amfn = _mfn(mfn_x(*mfn) & mask);
- gfn = _gfn(gfn_l & mask);
-
- /* Override the altp2m entry with its default access. */
- *p2ma = ap2m->default_access;
-
- rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
- p2m_unlock(ap2m);
-
- if ( rc )
- {
- gprintk(XENLOG_ERR,
- "failed to set entry for %"PRI_gfn" -> %"PRI_mfn" altp2m %u, rc %d\n",
- gfn_l, mfn_x(amfn), vcpu_altp2m(current).p2midx, rc);
- domain_crash(ap2m->domain);
- }
-
- return true;
-}
-
-enum altp2m_reset_type {
- ALTP2M_RESET,
- ALTP2M_DEACTIVATE
-};
-
-static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
- enum altp2m_reset_type reset_type)
-{
- struct p2m_domain *p2m;
-
- ASSERT(idx < MAX_ALTP2M);
- p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
-
- p2m_lock(p2m);
-
- p2m_flush_table_locked(p2m);
-
- if ( reset_type == ALTP2M_DEACTIVATE )
- p2m_free_logdirty(p2m);
-
- /* Uninit and reinit ept to force TLB shootdown */
- ept_p2m_uninit(p2m);
- ept_p2m_init(p2m);
-
- p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
- p2m->max_remapped_gfn = 0;
-
- p2m_unlock(p2m);
-}
-
-void p2m_flush_altp2m(struct domain *d)
-{
- unsigned int i;
-
- altp2m_list_lock(d);
-
- for ( i = 0; i < MAX_ALTP2M; i++ )
- {
- p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
- d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
- d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
- }
-
- altp2m_list_unlock(d);
-}
-
-static int p2m_activate_altp2m(struct domain *d, unsigned int idx,
- p2m_access_t hvmmem_default_access)
-{
- struct p2m_domain *hostp2m, *p2m;
- int rc;
-
- ASSERT(idx < MAX_ALTP2M);
-
- p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
- hostp2m = p2m_get_hostp2m(d);
-
- p2m_lock(p2m);
-
- rc = p2m_init_logdirty(p2m);
-
- if ( rc )
- goto out;
-
- /* The following is really just a rangeset copy. */
- rc = rangeset_merge(p2m->logdirty_ranges, hostp2m->logdirty_ranges);
-
- if ( rc )
- {
- p2m_free_logdirty(p2m);
- goto out;
- }
-
- p2m->default_access = hvmmem_default_access;
- p2m->domain = hostp2m->domain;
- p2m->global_logdirty = hostp2m->global_logdirty;
- p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
- p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0;
-
- p2m_init_altp2m_ept(d, idx);
-
- out:
- p2m_unlock(p2m);
-
- return rc;
-}
-
-int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
-{
- int rc = -EINVAL;
- struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
-
- if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
- return rc;
-
- altp2m_list_lock(d);
-
- if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
- mfn_x(INVALID_MFN) )
- rc = p2m_activate_altp2m(d, idx, hostp2m->default_access);
-
- altp2m_list_unlock(d);
- return rc;
-}
-
-int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
- xenmem_access_t hvmmem_default_access)
-{
- int rc = -EINVAL;
- unsigned int i;
- p2m_access_t a;
- struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
-
- if ( hvmmem_default_access > XENMEM_access_default ||
- !xenmem_access_to_p2m_access(hostp2m, hvmmem_default_access, &a) )
- return rc;
-
- altp2m_list_lock(d);
-
- for ( i = 0; i < MAX_ALTP2M; i++ )
- {
- if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
- continue;
-
- rc = p2m_activate_altp2m(d, i, a);
-
- if ( !rc )
- *idx = i;
-
- break;
- }
-
- altp2m_list_unlock(d);
- return rc;
-}
-
-int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
-{
- struct p2m_domain *p2m;
- int rc = -EBUSY;
-
- if ( !idx || idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
- return rc;
-
- rc = domain_pause_except_self(d);
- if ( rc )
- return rc;
-
- rc = -EBUSY;
- altp2m_list_lock(d);
-
- if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
- mfn_x(INVALID_MFN) )
- {
- p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
-
- if ( !_atomic_read(p2m->active_vcpus) )
- {
- p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
- d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
- mfn_x(INVALID_MFN);
- d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)] =
- mfn_x(INVALID_MFN);
- rc = 0;
- }
- }
-
- altp2m_list_unlock(d);
-
- domain_unpause_except_self(d);
-
- return rc;
-}
-
-int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
-{
- struct vcpu *v;
- int rc = -EINVAL;
-
- if ( idx >= MAX_ALTP2M )
- return rc;
-
- rc = domain_pause_except_self(d);
- if ( rc )
- return rc;
-
- rc = -EINVAL;
- altp2m_list_lock(d);
-
- if ( d->arch.altp2m_visible_eptp[idx] != mfn_x(INVALID_MFN) )
- {
- for_each_vcpu( d, v )
- if ( p2m_set_altp2m(v, idx) )
- altp2m_vcpu_update_p2m(v);
-
- rc = 0;
- }
-
- altp2m_list_unlock(d);
-
- domain_unpause_except_self(d);
-
- return rc;
-}
-
-int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
- gfn_t old_gfn, gfn_t new_gfn)
-{
- struct p2m_domain *hp2m, *ap2m;
- p2m_access_t a;
- p2m_type_t t;
- mfn_t mfn;
- int rc = -EINVAL;
-
- if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
- d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
- mfn_x(INVALID_MFN) )
- return rc;
-
- hp2m = p2m_get_hostp2m(d);
- ap2m = array_access_nospec(d->arch.altp2m_p2m, idx);
-
- p2m_lock(hp2m);
- p2m_lock(ap2m);
-
- if ( gfn_eq(new_gfn, INVALID_GFN) )
- {
- mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
- rc = mfn_valid(mfn)
- ? p2m_remove_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K)
- : 0;
- goto out;
- }
-
- rc = altp2m_get_effective_entry(ap2m, old_gfn, &mfn, &t, &a,
- AP2MGET_prepopulate);
- if ( rc )
- goto out;
-
- rc = altp2m_get_effective_entry(ap2m, new_gfn, &mfn, &t, &a,
- AP2MGET_query);
- if ( rc )
- goto out;
-
- if ( !ap2m->set_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K, t, a,
- (current->domain != d)) )
- {
- rc = 0;
-
- if ( gfn_x(new_gfn) < ap2m->min_remapped_gfn )
- ap2m->min_remapped_gfn = gfn_x(new_gfn);
- if ( gfn_x(new_gfn) > ap2m->max_remapped_gfn )
- ap2m->max_remapped_gfn = gfn_x(new_gfn);
- }
-
- out:
- p2m_unlock(ap2m);
- p2m_unlock(hp2m);
- return rc;
-}
-
-int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
- mfn_t mfn, unsigned int page_order,
- p2m_type_t p2mt, p2m_access_t p2ma)
-{
- struct p2m_domain *p2m;
- unsigned int i;
- unsigned int reset_count = 0;
- unsigned int last_reset_idx = ~0;
- int ret = 0;
-
- if ( !altp2m_active(d) )
- return 0;
-
- altp2m_list_lock(d);
-
- for ( i = 0; i < MAX_ALTP2M; i++ )
- {
- p2m_type_t t;
- p2m_access_t a;
-
- if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
- continue;
-
- p2m = d->arch.altp2m_p2m[i];
-
- /* Check for a dropped page that may impact this altp2m */
- if ( mfn_eq(mfn, INVALID_MFN) &&
- gfn_x(gfn) + (1UL << page_order) > p2m->min_remapped_gfn &&
- gfn_x(gfn) <= p2m->max_remapped_gfn )
- {
- if ( !reset_count++ )
- {
- p2m_reset_altp2m(d, i, ALTP2M_RESET);
- last_reset_idx = i;
- }
- else
- {
- /* At least 2 altp2m's impacted, so reset everything */
- for ( i = 0; i < MAX_ALTP2M; i++ )
- {
- if ( i == last_reset_idx ||
- d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
- continue;
-
- p2m_reset_altp2m(d, i, ALTP2M_RESET);
- }
-
- ret = 0;
- break;
- }
- }
- else if ( !mfn_eq(get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0,
- NULL), INVALID_MFN) )
- {
- int rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
-
- /* Best effort: Don't bail on error. */
- if ( !ret )
- ret = rc;
-
- p2m_put_gfn(p2m, gfn);
- }
- else
- p2m_put_gfn(p2m, gfn);
- }
-
- altp2m_list_unlock(d);
-
- return ret;
-}
-
/*** Audit ***/
#if P2M_AUDIT
@@ -2540,159 +2065,6 @@ int xenmem_add_to_physmap_one(
return rc;
}
-/*
- * Set/clear the #VE suppress bit for a page. Only available on VMX.
- */
-int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
- unsigned int altp2m_idx)
-{
- int rc;
- struct xen_hvm_altp2m_suppress_ve_multi sve = {
- altp2m_idx, suppress_ve, 0, 0, gfn_x(gfn), gfn_x(gfn), 0
- };
-
- if ( !(rc = p2m_set_suppress_ve_multi(d, &sve)) )
- rc = sve.first_error;
-
- return rc;
-}
-
-/*
- * Set/clear the #VE suppress bit for multiple pages. Only available on VMX.
- */
-int p2m_set_suppress_ve_multi(struct domain *d,
- struct xen_hvm_altp2m_suppress_ve_multi *sve)
-{
- struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
- struct p2m_domain *ap2m = NULL;
- struct p2m_domain *p2m = host_p2m;
- uint64_t start = sve->first_gfn;
- int rc = 0;
-
- if ( sve->view > 0 )
- {
- if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
- d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
- mfn_x(INVALID_MFN) )
- return -EINVAL;
-
- p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, sve->view);
- }
-
- p2m_lock(host_p2m);
-
- if ( ap2m )
- p2m_lock(ap2m);
-
- while ( sve->last_gfn >= start )
- {
- p2m_access_t a;
- p2m_type_t t;
- mfn_t mfn;
- int err = 0;
-
- if ( (err = altp2m_get_effective_entry(p2m, _gfn(start), &mfn, &t, &a,
- AP2MGET_query)) &&
- !sve->first_error )
- {
- sve->first_error_gfn = start; /* Save the gfn of the first error */
- sve->first_error = err; /* Save the first error code */
- }
-
- if ( !err && (err = p2m->set_entry(p2m, _gfn(start), mfn,
- PAGE_ORDER_4K, t, a,
- sve->suppress_ve)) &&
- !sve->first_error )
- {
- sve->first_error_gfn = start; /* Save the gfn of the first error */
- sve->first_error = err; /* Save the first error code */
- }
-
- /* Check for continuation if it's not the last iteration. */
- if ( sve->last_gfn >= ++start && hypercall_preempt_check() )
- {
- rc = -ERESTART;
- break;
- }
- }
-
- sve->first_gfn = start;
-
- if ( ap2m )
- p2m_unlock(ap2m);
-
- p2m_unlock(host_p2m);
-
- return rc;
-}
-
-int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
- unsigned int altp2m_idx)
-{
- struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
- struct p2m_domain *ap2m = NULL;
- struct p2m_domain *p2m;
- mfn_t mfn;
- p2m_access_t a;
- p2m_type_t t;
- int rc = 0;
-
- if ( altp2m_idx > 0 )
- {
- if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
- d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
- mfn_x(INVALID_MFN) )
- return -EINVAL;
-
- p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
- }
- else
- p2m = host_p2m;
-
- gfn_lock(host_p2m, gfn, 0);
-
- if ( ap2m )
- p2m_lock(ap2m);
-
- mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, suppress_ve);
- if ( !mfn_valid(mfn) )
- rc = -ESRCH;
-
- if ( ap2m )
- p2m_unlock(ap2m);
-
- gfn_unlock(host_p2m, gfn, 0);
-
- return rc;
-}
-
-int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
- uint8_t visible)
-{
- int rc = 0;
-
- altp2m_list_lock(d);
-
- /*
- * Eptp index is correlated with altp2m index and should not exceed
- * min(MAX_ALTP2M, MAX_EPTP).
- */
- if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
- d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
- mfn_x(INVALID_MFN) )
- rc = -EINVAL;
- else if ( visible )
- d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
- d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)];
- else
- d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
- mfn_x(INVALID_MFN);
-
- altp2m_list_unlock(d);
-
- return rc;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/arch/x86/mm/p2m.h b/xen/arch/x86/mm/p2m.h
index 04308cfb6d..635f5a7f45 100644
--- a/xen/arch/x86/mm/p2m.h
+++ b/xen/arch/x86/mm/p2m.h
@@ -22,6 +22,9 @@ static inline void p2m_free_logdirty(struct p2m_domain *p2m) {}
int p2m_init_altp2m(struct domain *d);
void p2m_teardown_altp2m(struct domain *d);
+void p2m_flush_table_locked(struct p2m_domain *p2m);
+int __must_check p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
+ unsigned int page_order);
void p2m_nestedp2m_init(struct p2m_domain *p2m);
int p2m_init_nestedp2m(struct domain *d);
void p2m_teardown_nestedp2m(struct domain *d);
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (3 preceding siblings ...)
2024-05-15 9:06 ` [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file Sergiy Kibrik
@ 2024-05-15 9:08 ` Sergiy Kibrik
2024-05-16 0:33 ` Stefano Stabellini
` (2 more replies)
2024-05-15 9:10 ` [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option Sergiy Kibrik
` (9 subsequent siblings)
14 siblings, 3 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:08 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel
Add new option to make altp2m code inclusion optional.
Currently altp2m support provided for VT-d only, so option is dependant on VMX.
No functional change intended.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
---
xen/arch/x86/Kconfig | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 8c9f8431f0..2872b031a7 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -358,6 +358,11 @@ config REQUIRE_NX
was unavailable. However, if enabled, Xen will no longer boot on
any CPU which is lacking NX support.
+config ALTP2M
+ bool "Alternate P2M support"
+ def_bool y
+ depends on VMX && EXPERT
+
endmenu
source "common/Kconfig"
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (4 preceding siblings ...)
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
@ 2024-05-15 9:10 ` Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 11:01 ` Jan Beulich
2024-05-15 9:12 ` [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX} Sergiy Kibrik
` (8 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:10 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
George Dunlap, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
Instead of using generic CONFIG_HVM option switch to a bit more specific
CONFIG_ALTP2M option for altp2m support. Also guard altp2m routines, so that
they can be disabled completely in the build -- when target platform does not
actually support altp2m (AMD-V & ARM as of now).
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
---
changes in v2:
- use separate CONFIG_ALTP2M option instead of CONFIG_VMX
---
xen/arch/x86/include/asm/altp2m.h | 5 ++++-
xen/arch/x86/include/asm/hvm/hvm.h | 2 +-
xen/arch/x86/include/asm/p2m.h | 17 ++++++++++++++++-
xen/arch/x86/mm/Makefile | 2 +-
4 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/include/asm/altp2m.h b/xen/arch/x86/include/asm/altp2m.h
index e5e59cbd68..092b13e231 100644
--- a/xen/arch/x86/include/asm/altp2m.h
+++ b/xen/arch/x86/include/asm/altp2m.h
@@ -7,7 +7,7 @@
#ifndef __ASM_X86_ALTP2M_H
#define __ASM_X86_ALTP2M_H
-#ifdef CONFIG_HVM
+#ifdef CONFIG_ALTP2M
#include <xen/types.h>
#include <xen/sched.h> /* for struct vcpu, struct domain */
@@ -38,7 +38,10 @@ static inline bool altp2m_active(const struct domain *d)
}
/* Only declaration is needed. DCE will optimise it out when linking. */
+void altp2m_vcpu_initialise(struct vcpu *v);
+void altp2m_vcpu_destroy(struct vcpu *v);
uint16_t altp2m_vcpu_idx(const struct vcpu *v);
+int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
void altp2m_vcpu_disable_ve(struct vcpu *v);
#endif
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 0c9e6f1564..4f03dd7af8 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
/* returns true if hardware supports alternate p2m's */
static inline bool hvm_altp2m_supported(void)
{
- return hvm_funcs.caps.altp2m;
+ return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
}
/* Returns true if we have the minimum hardware requirements for nested virt */
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..855e69d24a 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -581,9 +581,9 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, mfn_t mfn)
return _gfn(mfn_x(mfn));
}
-#ifdef CONFIG_HVM
#define AP2MGET_prepopulate true
#define AP2MGET_query false
+#ifdef CONFIG_ALTP2M
/*
* Looks up altp2m entry. If the entry is not found it looks up the entry in
@@ -593,6 +593,15 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, mfn_t mfn)
int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
p2m_type_t *t, p2m_access_t *a,
bool prepopulate);
+#else
+static inline int altp2m_get_effective_entry(struct p2m_domain *ap2m,
+ gfn_t gfn, mfn_t *mfn,
+ p2m_type_t *t, p2m_access_t *a,
+ bool prepopulate)
+{
+ ASSERT_UNREACHABLE();
+ return -EOPNOTSUPP;
+}
#endif
/* Init the datastructures for later use by the p2m code */
@@ -909,8 +918,14 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned int idx)
/* Switch alternate p2m for a single vcpu */
bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx);
+#ifdef CONFIG_ALTP2M
/* Check to see if vcpu should be switched to a different p2m. */
void p2m_altp2m_check(struct vcpu *v, uint16_t idx);
+#else
+static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
+{
+}
+#endif
/* Flush all the alternate p2m's for a domain */
void p2m_flush_altp2m(struct domain *d);
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 0128ca7ab6..d7d57b8190 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -1,7 +1,7 @@
obj-y += shadow/
obj-$(CONFIG_HVM) += hap/
-obj-$(CONFIG_HVM) += altp2m.o
+obj-$(CONFIG_ALTP2M) += altp2m.o
obj-$(CONFIG_HVM) += guest_walk_2.o guest_walk_3.o guest_walk_4.o
obj-$(CONFIG_SHADOW_PAGING) += guest_walk_4.o
obj-$(CONFIG_MEM_ACCESS) += mem_access.o
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (5 preceding siblings ...)
2024-05-15 9:10 ` [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option Sergiy Kibrik
@ 2024-05-15 9:12 ` Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 11:12 ` Jan Beulich
2024-05-15 9:14 ` [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm} Sergiy Kibrik
` (7 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:12 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Andrew Cooper, Roger Pau Monné, Jan Beulich,
Stefano Stabellini, Xenia Ragiadakou
As we now have SVM/VMX config options for enabling/disabling these features
completely in the build, it may be feasible to add build-time checks to
cpu_has_{svm,vmx} macros. These are used extensively thoughout HVM code, so
we won't have to add extra #ifdef-s to check whether svm/vmx has been enabled,
while DCE cleans up calls to vmx/svm functions, if their code not being built.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/include/asm/cpufeature.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index 9bc553681f..17f5aed000 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
#define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
#define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
#define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
-#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
+#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
+ boot_cpu_has(X86_FEATURE_VMX))
#define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
#define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
#define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
@@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
/* CPUID level 0x80000001.ecx */
#define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
-#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
+#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
+ boot_cpu_has(X86_FEATURE_SVM))
#define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
#define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
#define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (6 preceding siblings ...)
2024-05-15 9:12 ` [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX} Sergiy Kibrik
@ 2024-05-15 9:14 ` Sergiy Kibrik
2024-05-16 0:44 ` Stefano Stabellini
2024-05-16 11:23 ` Jan Beulich
2024-05-15 9:16 ` [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s Sergiy Kibrik
` (6 subsequent siblings)
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:14 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
If VMX/SVM disabled in the build, we may still want to have vPMU drivers for
PV guests. Yet some calls to vmx/svm-related routines needs to be guarded then.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
xen/arch/x86/cpu/vpmu_amd.c | 8 ++++----
xen/arch/x86/cpu/vpmu_intel.c | 20 ++++++++++----------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index db2fa420e1..40b0c8932f 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -290,7 +290,7 @@ static int cf_check amd_vpmu_save(struct vcpu *v, bool to_guest)
context_save(v);
if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
- is_msr_bitmap_on(vpmu) )
+ is_msr_bitmap_on(vpmu) && cpu_has_svm )
amd_vpmu_unset_msr_bitmap(v);
if ( to_guest )
@@ -363,7 +363,7 @@ static int cf_check amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
return 0;
vpmu_set(vpmu, VPMU_RUNNING);
- if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
+ if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
amd_vpmu_set_msr_bitmap(v);
}
@@ -372,7 +372,7 @@ static int cf_check amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
(is_pmu_enabled(msr_content) == 0) && vpmu_is_set(vpmu, VPMU_RUNNING) )
{
vpmu_reset(vpmu, VPMU_RUNNING);
- if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
+ if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
amd_vpmu_unset_msr_bitmap(v);
release_pmu_ownership(PMU_OWNER_HVM);
}
@@ -415,7 +415,7 @@ static void cf_check amd_vpmu_destroy(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);
- if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
+ if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
amd_vpmu_unset_msr_bitmap(v);
xfree(vpmu->context);
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index cd414165df..10c34a5691 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -269,7 +269,7 @@ static inline void __core2_vpmu_save(struct vcpu *v)
if ( !is_hvm_vcpu(v) )
rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS, core2_vpmu_cxt->global_status);
/* Save MSR to private context to make it fork-friendly */
- else if ( mem_sharing_enabled(v->domain) )
+ else if ( mem_sharing_enabled(v->domain) && cpu_has_vmx )
vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
&core2_vpmu_cxt->global_ctrl);
}
@@ -288,7 +288,7 @@ static int cf_check core2_vpmu_save(struct vcpu *v, bool to_guest)
/* Unset PMU MSR bitmap to trap lazy load. */
if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
- cpu_has_vmx_msr_bitmap )
+ cpu_has_vmx && cpu_has_vmx_msr_bitmap )
core2_vpmu_unset_msr_bitmap(v);
if ( to_guest )
@@ -333,7 +333,7 @@ static inline void __core2_vpmu_load(struct vcpu *v)
wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, core2_vpmu_cxt->global_ctrl);
}
/* Restore MSR from context when used with a fork */
- else if ( mem_sharing_is_fork(v->domain) )
+ else if ( mem_sharing_is_fork(v->domain) && cpu_has_vmx )
vmx_write_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
core2_vpmu_cxt->global_ctrl);
}
@@ -442,7 +442,7 @@ static int cf_check core2_vpmu_alloc_resource(struct vcpu *v)
if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
return 0;
- if ( is_hvm_vcpu(v) )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx )
{
if ( vmx_add_host_load_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, 0) )
goto out_err;
@@ -513,7 +513,7 @@ static int core2_vpmu_msr_common_check(u32 msr_index, int *type, int *index)
__core2_vpmu_load(current);
vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
- if ( is_hvm_vcpu(current) && cpu_has_vmx_msr_bitmap )
+ if ( is_hvm_vcpu(current) && cpu_has_vmx && cpu_has_vmx_msr_bitmap )
core2_vpmu_set_msr_bitmap(current);
}
return 1;
@@ -584,7 +584,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
if ( msr_content & fixed_ctrl_mask )
return -EINVAL;
- if ( is_hvm_vcpu(v) )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx )
vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
&core2_vpmu_cxt->global_ctrl);
else
@@ -653,7 +653,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
if ( blocked )
return -EINVAL;
- if ( is_hvm_vcpu(v) )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx)
vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
&core2_vpmu_cxt->global_ctrl);
else
@@ -672,7 +672,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
wrmsrl(msr, msr_content);
else
{
- if ( is_hvm_vcpu(v) )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx )
vmx_write_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
else
wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
@@ -706,7 +706,7 @@ static int cf_check core2_vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
*msr_content = core2_vpmu_cxt->global_status;
break;
case MSR_CORE_PERF_GLOBAL_CTRL:
- if ( is_hvm_vcpu(v) )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx )
vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
else
rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, *msr_content);
@@ -808,7 +808,7 @@ static void cf_check core2_vpmu_destroy(struct vcpu *v)
vpmu->context = NULL;
xfree(vpmu->priv_context);
vpmu->priv_context = NULL;
- if ( is_hvm_vcpu(v) && cpu_has_vmx_msr_bitmap )
+ if ( is_hvm_vcpu(v) && cpu_has_vmx && cpu_has_vmx_msr_bitmap )
core2_vpmu_unset_msr_bitmap(v);
release_pmu_ownership(PMU_OWNER_HVM);
vpmu_clear(vpmu);
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (7 preceding siblings ...)
2024-05-15 9:14 ` [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm} Sergiy Kibrik
@ 2024-05-15 9:16 ` Sergiy Kibrik
2024-05-16 0:45 ` Stefano Stabellini
2024-05-15 9:18 ` [XEN PATCH v2 10/15] x86/domain: " Sergiy Kibrik
` (5 subsequent siblings)
14 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:16 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
Remove preprocessor checks for CONFIG_HVM option, because expressions covered
by these checks are already guarded by cpu_has_vmx, which itself depends
on CONFIG_HVM option (via CONFIG_VMX).
No functional change intended.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
xen/arch/x86/traps.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d554c9d41e..7b8ee45edf 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -676,7 +676,6 @@ void vcpu_show_execution_state(struct vcpu *v)
vcpu_pause(v); /* acceptably dangerous */
-#ifdef CONFIG_HVM
/*
* For VMX special care is needed: Reading some of the register state will
* require VMCS accesses. Engaging foreign VMCSes involves acquiring of a
@@ -689,7 +688,6 @@ void vcpu_show_execution_state(struct vcpu *v)
ASSERT(!in_irq());
vmx_vmcs_enter(v);
}
-#endif
/* Prevent interleaving of output. */
flags = console_lock_recursive_irqsave();
@@ -714,10 +712,8 @@ void vcpu_show_execution_state(struct vcpu *v)
console_unlock_recursive_irqrestore(flags);
}
-#ifdef CONFIG_HVM
if ( cpu_has_vmx && is_hvm_vcpu(v) )
vmx_vmcs_exit(v);
-#endif
vcpu_unpause(v);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 10/15] x86/domain: clean up superfluous #idef-s
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (8 preceding siblings ...)
2024-05-15 9:16 ` [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s Sergiy Kibrik
@ 2024-05-15 9:18 ` Sergiy Kibrik
2024-05-16 0:46 ` Stefano Stabellini
2024-05-15 9:20 ` [XEN PATCH v2 11/15] x86/oprofile: guard svm specific symbols with CONFIG_SVM Sergiy Kibrik
` (4 subsequent siblings)
14 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:18 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
Remove preprocessor checks for CONFIG_HVM option, because expressions covered
by these checks are already guarded by cpu_has_svm, which itself depends
on CONFIG_HVM option (via CONFIG_SVM).
No functional change intended.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
xen/arch/x86/domain.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 20e83cf38b..5c7fb7fc73 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1708,11 +1708,9 @@ static void load_segments(struct vcpu *n)
if ( !(n->arch.flags & TF_kernel_mode) )
SWAP(gsb, gss);
-#ifdef CONFIG_HVM
if ( cpu_has_svm && (uregs->fs | uregs->gs) <= 3 )
fs_gs_done = svm_load_segs(n->arch.pv.ldt_ents, LDT_VIRT_START(n),
n->arch.pv.fs_base, gsb, gss);
-#endif
}
if ( !fs_gs_done )
@@ -2025,7 +2023,7 @@ static void __context_switch(void)
write_ptbase(n);
-#if defined(CONFIG_PV) && defined(CONFIG_HVM)
+#if defined(CONFIG_PV)
/* Prefetch the VMCB if we expect to use it later in the context switch */
if ( cpu_has_svm && is_pv_64bit_domain(nd) && !is_idle_domain(nd) )
svm_load_segs_prefetch();
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 11/15] x86/oprofile: guard svm specific symbols with CONFIG_SVM
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (9 preceding siblings ...)
2024-05-15 9:18 ` [XEN PATCH v2 10/15] x86/domain: " Sergiy Kibrik
@ 2024-05-15 9:20 ` Sergiy Kibrik
2024-05-15 9:22 ` [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code Sergiy Kibrik
` (3 subsequent siblings)
14 siblings, 0 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:20 UTC (permalink / raw
To: xen-devel
Cc: Xenia Ragiadakou, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou,
Sergiy Kibrik
From: Xenia Ragiadakou <burzalodowa@gmail.com>
The symbol svm_stgi_label is AMD-V specific so guard its usage in common code
with CONFIG_SVM.
Since SVM depends on HVM, it can be used alone.
Also, use #ifdef instead of #if.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/oprofile/op_model_athlon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
index 69fd3fcc86..a9c7b87d67 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -320,7 +320,7 @@ static int cf_check athlon_check_ctrs(
struct vcpu *v = current;
unsigned int const nr_ctrs = model->num_counters;
-#if CONFIG_HVM
+#ifdef CONFIG_SVM
struct cpu_user_regs *guest_regs = guest_cpu_user_regs();
if (!guest_mode(regs) &&
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (10 preceding siblings ...)
2024-05-15 9:20 ` [XEN PATCH v2 11/15] x86/oprofile: guard svm specific symbols with CONFIG_SVM Sergiy Kibrik
@ 2024-05-15 9:22 ` Sergiy Kibrik
2024-05-16 0:50 ` Stefano Stabellini
2024-05-15 9:24 ` [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX Sergiy Kibrik
` (2 subsequent siblings)
14 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:22 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Roger Pau Monné, Paul Durrant, Jan Beulich,
Stefano Stabellini, Xenia Ragiadakou, Andrew Cooper
There're several places in common code, outside of arch/x86/hvm/vmx,
where cpu_has_vmx_* get accessed without checking if VMX present first.
We may want to guard these macros, as they read global variables defined
inside vmx-specific files -- so VMX can be made optional later on.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
Here I've tried a different approach from prev.patches [1,2] -- instead of
modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
1) do not touch SVM part at all, because just as Andrew pointed out they're
used inside arch/x86/hvm/svm only.
2) track several places in common code where cpu_has_vmx_* features are
checked out and guard them using cpu_has_vmx condition
3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
more tricky way, so instead of making complex conditionals even more complicated,
we can instead integrate cpu_has_vmx condition inside these two macros.
This patch aims to replace [1,2] from v1 series by doing steps above.
1. https://lore.kernel.org/xen-devel/20240416064402.3469959-1-Sergiy_Kibrik@epam.com/
2. https://lore.kernel.org/xen-devel/20240416064606.3470052-1-Sergiy_Kibrik@epam.com/
---
changes in v2:
- do not touch SVM code and macros
- drop vmx_ctrl_has_feature()
- guard cpu_has_vmx_* macros in common code instead
changes in v1:
- introduced helper routine vmx_ctrl_has_feature() and used it for all
cpu_has_vmx_* macros
---
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/hvm/viridian/viridian.c | 4 ++--
xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 4 ++--
xen/arch/x86/traps.c | 5 +++--
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9594e0a5c5..ab75de9779 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5180,7 +5180,7 @@ int hvm_debug_op(struct vcpu *v, int32_t op)
{
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
- if ( !cpu_has_monitor_trap_flag )
+ if ( !cpu_has_vmx || !cpu_has_monitor_trap_flag )
return -EOPNOTSUPP;
break;
default:
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 0496c52ed5..657c6a3ea7 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -196,7 +196,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
res->a = CPUID4A_RELAX_TIMER_INT;
if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush )
res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
- if ( !cpu_has_vmx_apic_reg_virt )
+ if ( !cpu_has_vmx || !cpu_has_vmx_apic_reg_virt )
res->a |= CPUID4A_MSR_BASED_APIC;
if ( viridian_feature_mask(d) & HVMPV_hcall_ipi )
res->a |= CPUID4A_SYNTHETIC_CLUSTER_IPI;
@@ -236,7 +236,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
case 6:
/* Detected and in use hardware features. */
- if ( cpu_has_vmx_virtualize_apic_accesses )
+ if ( cpu_has_vmx && cpu_has_vmx_virtualize_apic_accesses )
res->a |= CPUID6A_APIC_OVERLAY;
if ( cpu_has_vmx_msr_bitmap || (read_efer() & EFER_SVME) )
res->a |= CPUID6A_MSR_BITMAPS;
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
index 58140af691..aa05f9cf6e 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -306,7 +306,7 @@ extern u64 vmx_ept_vpid_cap;
#define cpu_has_vmx_vnmi \
(vmx_pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
#define cpu_has_vmx_msr_bitmap \
- (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
+ (cpu_has_vmx && vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
#define cpu_has_vmx_secondary_exec_control \
(vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
#define cpu_has_vmx_tertiary_exec_control \
@@ -347,7 +347,7 @@ extern u64 vmx_ept_vpid_cap;
#define cpu_has_vmx_vmfunc \
(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS)
#define cpu_has_vmx_virt_exceptions \
- (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
+ (cpu_has_vmx && vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
#define cpu_has_vmx_pml \
(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
#define cpu_has_vmx_mpx \
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 7b8ee45edf..3595bb379a 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1130,7 +1130,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
if ( !is_hvm_domain(d) || subleaf != 0 )
break;
- if ( cpu_has_vmx_apic_reg_virt )
+ if ( cpu_has_vmx && cpu_has_vmx_apic_reg_virt )
res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
/*
@@ -1139,7 +1139,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
* and wrmsr in the guest will run without VMEXITs (see
* vmx_vlapic_msr_changed()).
*/
- if ( cpu_has_vmx_virtualize_x2apic_mode &&
+ if ( cpu_has_vmx &&
+ cpu_has_vmx_virtualize_x2apic_mode &&
cpu_has_vmx_apic_reg_virt &&
cpu_has_vmx_virtual_intr_delivery )
res->a |= XEN_HVM_CPUID_X2APIC_VIRT;
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (11 preceding siblings ...)
2024-05-15 9:22 ` [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code Sergiy Kibrik
@ 2024-05-15 9:24 ` Sergiy Kibrik
2024-05-16 0:51 ` Stefano Stabellini
2024-05-16 12:11 ` Jan Beulich
2024-05-15 9:26 ` [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx Sergiy Kibrik
2024-05-15 9:28 ` [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable Sergiy Kibrik
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:24 UTC (permalink / raw
To: xen-devel
Cc: Xenia Ragiadakou, Jan Beulich, Andrew Cooper,
Roger Pau Monné, George Dunlap, Julien Grall,
Stefano Stabellini, Xenia Ragiadakou, Sergiy Kibrik
From: Xenia Ragiadakou <burzalodowa@gmail.com>
VIO_realmode_completion is specific to vmx realmode, so guard the completion
handling code with CONFIG_VMX. Also, guard VIO_realmode_completion itself by
CONFIG_VMX, instead of generic CONFIG_X86.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
changes in v1:
- put VIO_realmode_completion enum under #ifdef CONFIG_VMX
---
xen/arch/x86/hvm/emulate.c | 2 ++
xen/arch/x86/hvm/ioreq.c | 2 ++
xen/include/xen/sched.h | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index ab1bc51683..d60b1f6f4d 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
break;
case VIO_mmio_completion:
+#ifdef CONFIG_VMX
case VIO_realmode_completion:
+#endif
BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf));
hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes;
memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes);
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 4eb7a70182..b37bbd660b 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
{
switch ( completion )
{
+#ifdef CONFIG_VMX
case VIO_realmode_completion:
{
struct hvm_emulate_ctxt ctxt;
@@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
break;
}
+#endif
default:
ASSERT_UNREACHABLE();
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 132b841995..50a58fe428 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -152,7 +152,7 @@ enum vio_completion {
VIO_no_completion,
VIO_mmio_completion,
VIO_pio_completion,
-#ifdef CONFIG_X86
+#ifdef CONFIG_VMX
VIO_realmode_completion,
#endif
};
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (12 preceding siblings ...)
2024-05-15 9:24 ` [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX Sergiy Kibrik
@ 2024-05-15 9:26 ` Sergiy Kibrik
2024-05-16 0:54 ` Stefano Stabellini
2024-05-16 12:15 ` Jan Beulich
2024-05-15 9:28 ` [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable Sergiy Kibrik
14 siblings, 2 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:26 UTC (permalink / raw
To: xen-devel
Cc: Sergiy Kibrik, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
VMX posted interrupts support can now be excluded from x86 build along with
VMX code itself, but still we may want to keep the possibility to use
VT-d IOMMU driver in non-HVM setups.
So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
No functional change intended here.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
xen/drivers/passthrough/vtd/iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index e13be244c1..ad78282250 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2772,7 +2772,7 @@ static int cf_check reassign_device_ownership(
if ( !QUARANTINE_SKIP(target, pdev->arch.vtd.pgd_maddr) )
{
- if ( !has_arch_pdevs(target) )
+ if ( cpu_has_vmx && !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
#ifdef CONFIG_PV
@@ -2806,7 +2806,7 @@ static int cf_check reassign_device_ownership(
}
if ( ret )
{
- if ( !has_arch_pdevs(target) )
+ if ( cpu_has_vmx && !has_arch_pdevs(target) )
vmx_pi_hooks_deassign(target);
return ret;
}
@@ -2824,7 +2824,7 @@ static int cf_check reassign_device_ownership(
write_unlock(&target->pci_lock);
}
- if ( !has_arch_pdevs(source) )
+ if ( cpu_has_vmx && !has_arch_pdevs(source) )
vmx_pi_hooks_deassign(source);
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
` (13 preceding siblings ...)
2024-05-15 9:26 ` [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx Sergiy Kibrik
@ 2024-05-15 9:28 ` Sergiy Kibrik
2024-05-16 0:54 ` Stefano Stabellini
14 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-15 9:28 UTC (permalink / raw
To: xen-devel
Cc: Xenia Ragiadakou, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou,
Sergiy Kibrik
From: Xenia Ragiadakou <burzalodowa@gmail.com>
Provide the user with configuration control over the cpu virtualization support
in Xen by making SVM and VMX options user selectable.
To preserve the current default behavior, both options depend on HVM and
default to value of HVM.
To prevent users from unknowingly disabling virtualization support, make the
controls user selectable only if EXPERT is enabled.
No functional change intended.
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
changes in v2:
- remove dependency of build options IOMMU/AMD_IOMMU on VMX/SVM options
---
xen/arch/x86/Kconfig | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 2872b031a7..62621c7271 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -123,10 +123,24 @@ config HVM
If unsure, say Y.
config SVM
- def_bool HVM
+ bool "AMD-V" if EXPERT
+ depends on HVM
+ default HVM
+ help
+ Enables virtual machine extensions on platforms that implement the
+ AMD Virtualization Technology (AMD-V).
+ If your system includes a processor with AMD-V support, say Y.
+ If in doubt, say Y.
config VMX
- def_bool HVM
+ bool "Intel VT-x" if EXPERT
+ depends on HVM
+ default HVM
+ help
+ Enables virtual machine extensions on platforms that implement the
+ Intel Virtualization Technology (Intel VT-x).
+ If your system includes a processor with Intel VT-x support, say Y.
+ If in doubt, say Y.
config XEN_SHSTK
bool "Supervisor Shadow Stacks"
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage
2024-05-15 9:01 ` [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage Sergiy Kibrik
@ 2024-05-16 0:29 ` Stefano Stabellini
2024-05-16 10:20 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:29 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Alexandru Isaila, Petre Pircalabu, Andrew Cooper,
Roger Pau Monné, Jan Beulich, Stefano Stabellini,
Xenia Ragiadakou, Tamas K Lengyel
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Explicitly check whether altp2m is on for domain when getting altp2m index.
> If explicit call to altp2m_active() always returns false, DCE will remove
> call to altp2m_vcpu_idx().
>
> The puspose of that is later to be able to disable altp2m support and
> exclude its code from the build completely, when not supported by target
> platform (as of now it's supported for VT-d only).
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> CC: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> changes in v2:
> - patch description changed, removed VMX mentioning
> - guard by altp2m_active() instead of hvm_altp2m_supported()
> ---
> xen/arch/x86/hvm/monitor.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index 2a8ff07ec9..74621000b2 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -262,6 +262,8 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
> struct vcpu *curr = current;
> vm_event_request_t req = {};
> paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
> + unsigned int altp2m_idx = altp2m_active(curr->domain) ?
> + altp2m_vcpu_idx(curr) : 0;
> int rc;
>
> ASSERT(curr->arch.vm_event->send_event);
> @@ -270,7 +272,7 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
> * p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
> * in which case access must be restricted.
> */
> - rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
> + rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);
>
> if ( rc == -ESRCH )
> access = XENMEM_access_n;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines
2024-05-15 9:03 ` [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines Sergiy Kibrik
@ 2024-05-16 0:31 ` Stefano Stabellini
2024-05-16 10:26 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:31 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, George Dunlap,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Initialize and bring down altp2m only when it is supported by the platform,
> e.g. VMX. Also guard p2m_altp2m_propagate_change().
> The puspose of that is the possiblity to disable altp2m support and exclude its
> code from the build completely, when it's not supported by the target platform.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> xen/arch/x86/mm/p2m-basic.c | 19 +++++++++++--------
> xen/arch/x86/mm/p2m-ept.c | 2 +-
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/x86/mm/p2m-basic.c b/xen/arch/x86/mm/p2m-basic.c
> index 8599bd15c6..90106997d7 100644
> --- a/xen/arch/x86/mm/p2m-basic.c
> +++ b/xen/arch/x86/mm/p2m-basic.c
> @@ -126,13 +126,15 @@ int p2m_init(struct domain *d)
> return rc;
> }
>
> - rc = p2m_init_altp2m(d);
> - if ( rc )
> + if ( hvm_altp2m_supported() )
> {
> - p2m_teardown_hostp2m(d);
> - p2m_teardown_nestedp2m(d);
> + rc = p2m_init_altp2m(d);
> + if ( rc )
> + {
> + p2m_teardown_hostp2m(d);
> + p2m_teardown_nestedp2m(d);
> + }
> }
> -
> return rc;
> }
>
> @@ -195,11 +197,12 @@ void p2m_final_teardown(struct domain *d)
> {
> if ( is_hvm_domain(d) )
> {
> + if ( hvm_altp2m_supported() )
> + p2m_teardown_altp2m(d);
> /*
> - * We must tear down both of them unconditionally because
> - * we initialise them unconditionally.
> + * We must tear down nestedp2m unconditionally because
> + * we initialise it unconditionally.
> */
> - p2m_teardown_altp2m(d);
> p2m_teardown_nestedp2m(d);
> }
>
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index f83610cb8c..d264df5b14 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -986,7 +986,7 @@ out:
> if ( is_epte_present(&old_entry) )
> ept_free_entry(p2m, &old_entry, target);
>
> - if ( entry_written && p2m_is_hostp2m(p2m) )
> + if ( entry_written && p2m_is_hostp2m(p2m) && hvm_altp2m_supported())
> {
> ret = p2m_altp2m_propagate_change(d, _gfn(gfn), mfn, order, p2mt, p2ma);
> if ( !rc )
This is a matter of taste but I would put hvm_altp2m_supported() first.
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
@ 2024-05-16 0:33 ` Stefano Stabellini
2024-05-16 10:41 ` Jan Beulich
2024-05-18 1:07 ` Tamas K Lengyel
2 siblings, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:33 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Add new option to make altp2m code inclusion optional.
> Currently altp2m support provided for VT-d only, so option is dependant on VMX.
>
> No functional change intended.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> xen/arch/x86/Kconfig | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 8c9f8431f0..2872b031a7 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -358,6 +358,11 @@ config REQUIRE_NX
> was unavailable. However, if enabled, Xen will no longer boot on
> any CPU which is lacking NX support.
>
> +config ALTP2M
> + bool "Alternate P2M support"
> + def_bool y
> + depends on VMX && EXPERT
I think we need a small help text here, something like
Alternate-p2m allows a guest to manage multiple p2m guest physical "memory
views" (as opposed to a single p2m). Useful for memory introspection.
> endmenu
>
> source "common/Kconfig"
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file
2024-05-15 9:06 ` [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file Sergiy Kibrik
@ 2024-05-16 0:35 ` Stefano Stabellini
2024-05-16 11:46 ` Jan Beulich
2024-05-16 10:37 ` Jan Beulich
1 sibling, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:35 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Andrew Cooper, George Dunlap, Roger Pau Monné,
Jan Beulich, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Move altp2m code from generic p2m.c file to altp2m.c, so it is kept separately
> and can possibly be disabled in the build. We may want to disable it when
> building for specific platform only, that doesn't support alternate p2m.
>
> No functional change intended.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> CC: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> changes in v2:
> - no double blank lines
> - no unrelated re-formatting
> - header #include-s ordering
> - changed patch description
> ---
> xen/arch/x86/mm/altp2m.c | 630 ++++++++++++++++++++++++++++++++++++++
> xen/arch/x86/mm/p2m.c | 632 +--------------------------------------
> xen/arch/x86/mm/p2m.h | 3 +
> 3 files changed, 635 insertions(+), 630 deletions(-)
>
> diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
> index a04297b646..6fe1e9ed6b 100644
> --- a/xen/arch/x86/mm/altp2m.c
> +++ b/xen/arch/x86/mm/altp2m.c
> @@ -7,6 +7,8 @@
> #include <asm/hvm/hvm.h>
> #include <asm/p2m.h>
> #include <asm/altp2m.h>
> +#include <public/hvm/hvm_op.h>
> +#include <xen/event.h>
> #include "mm-locks.h"
> #include "p2m.h"
>
> @@ -151,6 +153,634 @@ void p2m_teardown_altp2m(struct domain *d)
> }
> }
>
> +int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
> + p2m_type_t *t, p2m_access_t *a,
> + bool prepopulate)
> +{
> + *mfn = ap2m->get_entry(ap2m, gfn, t, a, 0, NULL, NULL);
> +
> + /* Check host p2m if no valid entry in alternate */
> + if ( !mfn_valid(*mfn) && !p2m_is_hostp2m(ap2m) )
> + {
> + struct p2m_domain *hp2m = p2m_get_hostp2m(ap2m->domain);
> + unsigned int page_order;
> + int rc;
> +
> + *mfn = p2m_get_gfn_type_access(hp2m, gfn, t, a, P2M_ALLOC | P2M_UNSHARE,
> + &page_order, 0);
> +
> + rc = -ESRCH;
> + if ( !mfn_valid(*mfn) || *t != p2m_ram_rw )
> + return rc;
> +
> + /* If this is a superpage, copy that first */
> + if ( prepopulate && page_order != PAGE_ORDER_4K )
> + {
> + unsigned long mask = ~((1UL << page_order) - 1);
> + gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
> + mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
> +
> + rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
> + if ( rc )
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> + if ( altp2m_active(v->domain) )
> + p2m_switch_vcpu_altp2m_by_id(v, idx);
> +}
> +
> +bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
> +{
> + struct domain *d = v->domain;
> + bool rc = false;
> +
> + if ( idx >= MAX_ALTP2M )
> + return rc;
> +
> + altp2m_list_lock(d);
> +
> + if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
> + {
> + if ( p2m_set_altp2m(v, idx) )
> + altp2m_vcpu_update_p2m(v);
> + rc = 1;
> + }
> +
> + altp2m_list_unlock(d);
> + return rc;
> +}
> +
> +/*
> + * Read info about the gfn in an altp2m, locking the gfn.
> + *
> + * If the entry is valid, pass the results back to the caller.
> + *
> + * If the entry was invalid, and the host's entry is also invalid,
> + * return to the caller without any changes.
> + *
> + * If the entry is invalid, and the host entry was valid, propagate
> + * the host's entry to the altp2m (retaining page order), and indicate
> + * that the caller should re-try the faulting instruction.
> + */
> +bool p2m_altp2m_get_or_propagate(struct p2m_domain *ap2m, unsigned long gfn_l,
> + mfn_t *mfn, p2m_type_t *p2mt,
> + p2m_access_t *p2ma, unsigned int *page_order)
> +{
> + p2m_type_t ap2mt;
> + p2m_access_t ap2ma;
> + unsigned int cur_order;
> + unsigned long mask;
> + gfn_t gfn;
> + mfn_t amfn;
> + int rc;
> +
> + /*
> + * NB we must get the full lock on the altp2m here, in addition to
> + * the lock on the individual gfn, since we may change a range of
> + * gfns below.
> + */
> + p2m_lock(ap2m);
> +
> + amfn = get_gfn_type_access(ap2m, gfn_l, &ap2mt, &ap2ma, 0, &cur_order);
> +
> + if ( cur_order > *page_order )
> + cur_order = *page_order;
> +
> + if ( !mfn_eq(amfn, INVALID_MFN) )
> + {
> + p2m_unlock(ap2m);
> + *mfn = amfn;
> + *p2mt = ap2mt;
> + *p2ma = ap2ma;
> + *page_order = cur_order;
> + return false;
> + }
> +
> + /* Host entry is also invalid; don't bother setting the altp2m entry. */
> + if ( mfn_eq(*mfn, INVALID_MFN) )
> + {
> + p2m_unlock(ap2m);
> + *page_order = cur_order;
> + return false;
> + }
> +
> + /*
> + * If this is a superpage mapping, round down both frame numbers
> + * to the start of the superpage. NB that we repupose `amfn`
> + * here.
> + */
> + mask = ~((1UL << cur_order) - 1);
> + amfn = _mfn(mfn_x(*mfn) & mask);
> + gfn = _gfn(gfn_l & mask);
> +
> + /* Override the altp2m entry with its default access. */
> + *p2ma = ap2m->default_access;
> +
> + rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
> + p2m_unlock(ap2m);
> +
> + if ( rc )
> + {
> + gprintk(XENLOG_ERR,
> + "failed to set entry for %"PRI_gfn" -> %"PRI_mfn" altp2m %u, rc %d\n",
> + gfn_l, mfn_x(amfn), vcpu_altp2m(current).p2midx, rc);
> + domain_crash(ap2m->domain);
> + }
> +
> + return true;
> +}
> +
> +enum altp2m_reset_type {
> + ALTP2M_RESET,
> + ALTP2M_DEACTIVATE
> +};
> +
> +static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
> + enum altp2m_reset_type reset_type)
> +{
> + struct p2m_domain *p2m;
> +
> + ASSERT(idx < MAX_ALTP2M);
> + p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> +
> + p2m_lock(p2m);
> +
> + p2m_flush_table_locked(p2m);
> +
> + if ( reset_type == ALTP2M_DEACTIVATE )
> + p2m_free_logdirty(p2m);
> +
> + /* Uninit and reinit ept to force TLB shootdown */
> + ept_p2m_uninit(p2m);
> + ept_p2m_init(p2m);
> +
> + p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> + p2m->max_remapped_gfn = 0;
> +
> + p2m_unlock(p2m);
> +}
> +
> +void p2m_flush_altp2m(struct domain *d)
> +{
> + unsigned int i;
> +
> + altp2m_list_lock(d);
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + {
> + p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
> + d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
> + d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
> + }
> +
> + altp2m_list_unlock(d);
> +}
> +
> +static int p2m_activate_altp2m(struct domain *d, unsigned int idx,
> + p2m_access_t hvmmem_default_access)
> +{
> + struct p2m_domain *hostp2m, *p2m;
> + int rc;
> +
> + ASSERT(idx < MAX_ALTP2M);
> +
> + p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> + hostp2m = p2m_get_hostp2m(d);
> +
> + p2m_lock(p2m);
> +
> + rc = p2m_init_logdirty(p2m);
> +
> + if ( rc )
> + goto out;
> +
> + /* The following is really just a rangeset copy. */
> + rc = rangeset_merge(p2m->logdirty_ranges, hostp2m->logdirty_ranges);
> +
> + if ( rc )
> + {
> + p2m_free_logdirty(p2m);
> + goto out;
> + }
> +
> + p2m->default_access = hvmmem_default_access;
> + p2m->domain = hostp2m->domain;
> + p2m->global_logdirty = hostp2m->global_logdirty;
> + p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> + p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0;
> +
> + p2m_init_altp2m_ept(d, idx);
> +
> + out:
> + p2m_unlock(p2m);
> +
> + return rc;
> +}
> +
> +int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
> +{
> + int rc = -EINVAL;
> + struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> +
> + if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
> + return rc;
> +
> + altp2m_list_lock(d);
> +
> + if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + rc = p2m_activate_altp2m(d, idx, hostp2m->default_access);
> +
> + altp2m_list_unlock(d);
> + return rc;
> +}
> +
> +int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
> + xenmem_access_t hvmmem_default_access)
> +{
> + int rc = -EINVAL;
> + unsigned int i;
> + p2m_access_t a;
> + struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> +
> + if ( hvmmem_default_access > XENMEM_access_default ||
> + !xenmem_access_to_p2m_access(hostp2m, hvmmem_default_access, &a) )
> + return rc;
> +
> + altp2m_list_lock(d);
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + {
> + if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> + continue;
> +
> + rc = p2m_activate_altp2m(d, i, a);
> +
> + if ( !rc )
> + *idx = i;
> +
> + break;
> + }
> +
> + altp2m_list_unlock(d);
> + return rc;
> +}
> +
> +int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
> +{
> + struct p2m_domain *p2m;
> + int rc = -EBUSY;
> +
> + if ( !idx || idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
> + return rc;
> +
> + rc = domain_pause_except_self(d);
> + if ( rc )
> + return rc;
> +
> + rc = -EBUSY;
> + altp2m_list_lock(d);
> +
> + if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
> + mfn_x(INVALID_MFN) )
> + {
> + p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> +
> + if ( !_atomic_read(p2m->active_vcpus) )
> + {
> + p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
> + d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
> + mfn_x(INVALID_MFN);
> + d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)] =
> + mfn_x(INVALID_MFN);
> + rc = 0;
> + }
> + }
> +
> + altp2m_list_unlock(d);
> +
> + domain_unpause_except_self(d);
> +
> + return rc;
> +}
> +
> +int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
> +{
> + struct vcpu *v;
> + int rc = -EINVAL;
> +
> + if ( idx >= MAX_ALTP2M )
> + return rc;
> +
> + rc = domain_pause_except_self(d);
> + if ( rc )
> + return rc;
> +
> + rc = -EINVAL;
> + altp2m_list_lock(d);
> +
> + if ( d->arch.altp2m_visible_eptp[idx] != mfn_x(INVALID_MFN) )
> + {
> + for_each_vcpu( d, v )
> + if ( p2m_set_altp2m(v, idx) )
> + altp2m_vcpu_update_p2m(v);
> +
> + rc = 0;
> + }
> +
> + altp2m_list_unlock(d);
> +
> + domain_unpause_except_self(d);
> +
> + return rc;
> +}
> +
> +int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
> + gfn_t old_gfn, gfn_t new_gfn)
> +{
> + struct p2m_domain *hp2m, *ap2m;
> + p2m_access_t a;
> + p2m_type_t t;
> + mfn_t mfn;
> + int rc = -EINVAL;
> +
> + if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + return rc;
> +
> + hp2m = p2m_get_hostp2m(d);
> + ap2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> +
> + p2m_lock(hp2m);
> + p2m_lock(ap2m);
> +
> + if ( gfn_eq(new_gfn, INVALID_GFN) )
> + {
> + mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
> + rc = mfn_valid(mfn)
> + ? p2m_remove_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K)
> + : 0;
> + goto out;
> + }
> +
> + rc = altp2m_get_effective_entry(ap2m, old_gfn, &mfn, &t, &a,
> + AP2MGET_prepopulate);
> + if ( rc )
> + goto out;
> +
> + rc = altp2m_get_effective_entry(ap2m, new_gfn, &mfn, &t, &a,
> + AP2MGET_query);
> + if ( rc )
> + goto out;
> +
> + if ( !ap2m->set_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K, t, a,
> + (current->domain != d)) )
> + {
> + rc = 0;
> +
> + if ( gfn_x(new_gfn) < ap2m->min_remapped_gfn )
> + ap2m->min_remapped_gfn = gfn_x(new_gfn);
> + if ( gfn_x(new_gfn) > ap2m->max_remapped_gfn )
> + ap2m->max_remapped_gfn = gfn_x(new_gfn);
> + }
> +
> + out:
> + p2m_unlock(ap2m);
> + p2m_unlock(hp2m);
> + return rc;
> +}
> +
> +int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
> + mfn_t mfn, unsigned int page_order,
> + p2m_type_t p2mt, p2m_access_t p2ma)
> +{
> + struct p2m_domain *p2m;
> + unsigned int i;
> + unsigned int reset_count = 0;
> + unsigned int last_reset_idx = ~0;
> + int ret = 0;
> +
> + if ( !altp2m_active(d) )
> + return 0;
> +
> + altp2m_list_lock(d);
> +
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + {
> + p2m_type_t t;
> + p2m_access_t a;
> +
> + if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> + continue;
> +
> + p2m = d->arch.altp2m_p2m[i];
> +
> + /* Check for a dropped page that may impact this altp2m */
> + if ( mfn_eq(mfn, INVALID_MFN) &&
> + gfn_x(gfn) + (1UL << page_order) > p2m->min_remapped_gfn &&
> + gfn_x(gfn) <= p2m->max_remapped_gfn )
> + {
> + if ( !reset_count++ )
> + {
> + p2m_reset_altp2m(d, i, ALTP2M_RESET);
> + last_reset_idx = i;
> + }
> + else
> + {
> + /* At least 2 altp2m's impacted, so reset everything */
> + for ( i = 0; i < MAX_ALTP2M; i++ )
> + {
> + if ( i == last_reset_idx ||
> + d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> + continue;
> +
> + p2m_reset_altp2m(d, i, ALTP2M_RESET);
> + }
> +
> + ret = 0;
> + break;
> + }
> + }
> + else if ( !mfn_eq(get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0,
> + NULL), INVALID_MFN) )
> + {
> + int rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
> +
> + /* Best effort: Don't bail on error. */
> + if ( !ret )
> + ret = rc;
> +
> + p2m_put_gfn(p2m, gfn);
> + }
> + else
> + p2m_put_gfn(p2m, gfn);
> + }
> +
> + altp2m_list_unlock(d);
> +
> + return ret;
> +}
> +
> +/*
> + * Set/clear the #VE suppress bit for a page. Only available on VMX.
> + */
> +int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
> + unsigned int altp2m_idx)
> +{
> + int rc;
> + struct xen_hvm_altp2m_suppress_ve_multi sve = {
> + altp2m_idx, suppress_ve, 0, 0, gfn_x(gfn), gfn_x(gfn), 0
> + };
> +
> + if ( !(rc = p2m_set_suppress_ve_multi(d, &sve)) )
> + rc = sve.first_error;
> +
> + return rc;
> +}
> +
> +/*
> + * Set/clear the #VE suppress bit for multiple pages. Only available on VMX.
> + */
> +int p2m_set_suppress_ve_multi(struct domain *d,
> + struct xen_hvm_altp2m_suppress_ve_multi *sve)
> +{
> + struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> + struct p2m_domain *ap2m = NULL;
> + struct p2m_domain *p2m = host_p2m;
> + uint64_t start = sve->first_gfn;
> + int rc = 0;
> +
> + if ( sve->view > 0 )
> + {
> + if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + return -EINVAL;
> +
> + p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, sve->view);
> + }
> +
> + p2m_lock(host_p2m);
> +
> + if ( ap2m )
> + p2m_lock(ap2m);
> +
> + while ( sve->last_gfn >= start )
> + {
> + p2m_access_t a;
> + p2m_type_t t;
> + mfn_t mfn;
> + int err = 0;
> +
> + if ( (err = altp2m_get_effective_entry(p2m, _gfn(start), &mfn, &t, &a,
> + AP2MGET_query)) &&
> + !sve->first_error )
> + {
> + sve->first_error_gfn = start; /* Save the gfn of the first error */
> + sve->first_error = err; /* Save the first error code */
> + }
> +
> + if ( !err && (err = p2m->set_entry(p2m, _gfn(start), mfn,
> + PAGE_ORDER_4K, t, a,
> + sve->suppress_ve)) &&
> + !sve->first_error )
> + {
> + sve->first_error_gfn = start; /* Save the gfn of the first error */
> + sve->first_error = err; /* Save the first error code */
> + }
> +
> + /* Check for continuation if it's not the last iteration. */
> + if ( sve->last_gfn >= ++start && hypercall_preempt_check() )
> + {
> + rc = -ERESTART;
> + break;
> + }
> + }
> +
> + sve->first_gfn = start;
> +
> + if ( ap2m )
> + p2m_unlock(ap2m);
> +
> + p2m_unlock(host_p2m);
> +
> + return rc;
> +}
> +
> +int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
> + unsigned int altp2m_idx)
> +{
> + struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> + struct p2m_domain *ap2m = NULL;
> + struct p2m_domain *p2m;
> + mfn_t mfn;
> + p2m_access_t a;
> + p2m_type_t t;
> + int rc = 0;
> +
> + if ( altp2m_idx > 0 )
> + {
> + if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + return -EINVAL;
> +
> + p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
> + }
> + else
> + p2m = host_p2m;
> +
> + gfn_lock(host_p2m, gfn, 0);
> +
> + if ( ap2m )
> + p2m_lock(ap2m);
> +
> + mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, suppress_ve);
> + if ( !mfn_valid(mfn) )
> + rc = -ESRCH;
> +
> + if ( ap2m )
> + p2m_unlock(ap2m);
> +
> + gfn_unlock(host_p2m, gfn, 0);
> +
> + return rc;
> +}
> +
> +int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
> + uint8_t visible)
> +{
> + int rc = 0;
> +
> + altp2m_list_lock(d);
> +
> + /*
> + * Eptp index is correlated with altp2m index and should not exceed
> + * min(MAX_ALTP2M, MAX_EPTP).
> + */
> + if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> + mfn_x(INVALID_MFN) )
> + rc = -EINVAL;
> + else if ( visible )
> + d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)];
> + else
> + d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
> + mfn_x(INVALID_MFN);
> +
> + altp2m_list_unlock(d);
> +
> + return rc;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index ce742c12e0..7c422a2d7e 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -500,7 +500,7 @@ int p2m_alloc_table(struct p2m_domain *p2m)
> return 0;
> }
>
> -static int __must_check
> +int __must_check
> p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
> unsigned int page_order)
> {
> @@ -1329,7 +1329,7 @@ p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
> return p2m;
> }
>
> -static void
> +void
> p2m_flush_table_locked(struct p2m_domain *p2m)
> {
> struct page_info *top, *pg;
> @@ -1729,481 +1729,6 @@ int unmap_mmio_regions(struct domain *d,
> return i == nr ? 0 : i ?: ret;
> }
>
> -int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
> - p2m_type_t *t, p2m_access_t *a,
> - bool prepopulate)
> -{
> - *mfn = ap2m->get_entry(ap2m, gfn, t, a, 0, NULL, NULL);
> -
> - /* Check host p2m if no valid entry in alternate */
> - if ( !mfn_valid(*mfn) && !p2m_is_hostp2m(ap2m) )
> - {
> - struct p2m_domain *hp2m = p2m_get_hostp2m(ap2m->domain);
> - unsigned int page_order;
> - int rc;
> -
> - *mfn = p2m_get_gfn_type_access(hp2m, gfn, t, a, P2M_ALLOC | P2M_UNSHARE,
> - &page_order, 0);
> -
> - rc = -ESRCH;
> - if ( !mfn_valid(*mfn) || *t != p2m_ram_rw )
> - return rc;
> -
> - /* If this is a superpage, copy that first */
> - if ( prepopulate && page_order != PAGE_ORDER_4K )
> - {
> - unsigned long mask = ~((1UL << page_order) - 1);
> - gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
> - mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
> -
> - rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
> - if ( rc )
> - return rc;
> - }
> - }
> -
> - return 0;
> -}
> -
> -void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> -{
> - if ( altp2m_active(v->domain) )
> - p2m_switch_vcpu_altp2m_by_id(v, idx);
> -}
> -
> -bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
> -{
> - struct domain *d = v->domain;
> - bool rc = false;
> -
> - if ( idx >= MAX_ALTP2M )
> - return rc;
> -
> - altp2m_list_lock(d);
> -
> - if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
> - {
> - if ( p2m_set_altp2m(v, idx) )
> - altp2m_vcpu_update_p2m(v);
> - rc = 1;
> - }
> -
> - altp2m_list_unlock(d);
> - return rc;
> -}
> -
> -/*
> - * Read info about the gfn in an altp2m, locking the gfn.
> - *
> - * If the entry is valid, pass the results back to the caller.
> - *
> - * If the entry was invalid, and the host's entry is also invalid,
> - * return to the caller without any changes.
> - *
> - * If the entry is invalid, and the host entry was valid, propagate
> - * the host's entry to the altp2m (retaining page order), and indicate
> - * that the caller should re-try the faulting instruction.
> - */
> -bool p2m_altp2m_get_or_propagate(struct p2m_domain *ap2m, unsigned long gfn_l,
> - mfn_t *mfn, p2m_type_t *p2mt,
> - p2m_access_t *p2ma, unsigned int *page_order)
> -{
> - p2m_type_t ap2mt;
> - p2m_access_t ap2ma;
> - unsigned int cur_order;
> - unsigned long mask;
> - gfn_t gfn;
> - mfn_t amfn;
> - int rc;
> -
> - /*
> - * NB we must get the full lock on the altp2m here, in addition to
> - * the lock on the individual gfn, since we may change a range of
> - * gfns below.
> - */
> - p2m_lock(ap2m);
> -
> - amfn = get_gfn_type_access(ap2m, gfn_l, &ap2mt, &ap2ma, 0, &cur_order);
> -
> - if ( cur_order > *page_order )
> - cur_order = *page_order;
> -
> - if ( !mfn_eq(amfn, INVALID_MFN) )
> - {
> - p2m_unlock(ap2m);
> - *mfn = amfn;
> - *p2mt = ap2mt;
> - *p2ma = ap2ma;
> - *page_order = cur_order;
> - return false;
> - }
> -
> - /* Host entry is also invalid; don't bother setting the altp2m entry. */
> - if ( mfn_eq(*mfn, INVALID_MFN) )
> - {
> - p2m_unlock(ap2m);
> - *page_order = cur_order;
> - return false;
> - }
> -
> - /*
> - * If this is a superpage mapping, round down both frame numbers
> - * to the start of the superpage. NB that we repupose `amfn`
> - * here.
> - */
> - mask = ~((1UL << cur_order) - 1);
> - amfn = _mfn(mfn_x(*mfn) & mask);
> - gfn = _gfn(gfn_l & mask);
> -
> - /* Override the altp2m entry with its default access. */
> - *p2ma = ap2m->default_access;
> -
> - rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
> - p2m_unlock(ap2m);
> -
> - if ( rc )
> - {
> - gprintk(XENLOG_ERR,
> - "failed to set entry for %"PRI_gfn" -> %"PRI_mfn" altp2m %u, rc %d\n",
> - gfn_l, mfn_x(amfn), vcpu_altp2m(current).p2midx, rc);
> - domain_crash(ap2m->domain);
> - }
> -
> - return true;
> -}
> -
> -enum altp2m_reset_type {
> - ALTP2M_RESET,
> - ALTP2M_DEACTIVATE
> -};
> -
> -static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
> - enum altp2m_reset_type reset_type)
> -{
> - struct p2m_domain *p2m;
> -
> - ASSERT(idx < MAX_ALTP2M);
> - p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> -
> - p2m_lock(p2m);
> -
> - p2m_flush_table_locked(p2m);
> -
> - if ( reset_type == ALTP2M_DEACTIVATE )
> - p2m_free_logdirty(p2m);
> -
> - /* Uninit and reinit ept to force TLB shootdown */
> - ept_p2m_uninit(p2m);
> - ept_p2m_init(p2m);
> -
> - p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> - p2m->max_remapped_gfn = 0;
> -
> - p2m_unlock(p2m);
> -}
> -
> -void p2m_flush_altp2m(struct domain *d)
> -{
> - unsigned int i;
> -
> - altp2m_list_lock(d);
> -
> - for ( i = 0; i < MAX_ALTP2M; i++ )
> - {
> - p2m_reset_altp2m(d, i, ALTP2M_DEACTIVATE);
> - d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
> - d->arch.altp2m_visible_eptp[i] = mfn_x(INVALID_MFN);
> - }
> -
> - altp2m_list_unlock(d);
> -}
> -
> -static int p2m_activate_altp2m(struct domain *d, unsigned int idx,
> - p2m_access_t hvmmem_default_access)
> -{
> - struct p2m_domain *hostp2m, *p2m;
> - int rc;
> -
> - ASSERT(idx < MAX_ALTP2M);
> -
> - p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> - hostp2m = p2m_get_hostp2m(d);
> -
> - p2m_lock(p2m);
> -
> - rc = p2m_init_logdirty(p2m);
> -
> - if ( rc )
> - goto out;
> -
> - /* The following is really just a rangeset copy. */
> - rc = rangeset_merge(p2m->logdirty_ranges, hostp2m->logdirty_ranges);
> -
> - if ( rc )
> - {
> - p2m_free_logdirty(p2m);
> - goto out;
> - }
> -
> - p2m->default_access = hvmmem_default_access;
> - p2m->domain = hostp2m->domain;
> - p2m->global_logdirty = hostp2m->global_logdirty;
> - p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> - p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0;
> -
> - p2m_init_altp2m_ept(d, idx);
> -
> - out:
> - p2m_unlock(p2m);
> -
> - return rc;
> -}
> -
> -int p2m_init_altp2m_by_id(struct domain *d, unsigned int idx)
> -{
> - int rc = -EINVAL;
> - struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> -
> - if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
> - return rc;
> -
> - altp2m_list_lock(d);
> -
> - if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
> - mfn_x(INVALID_MFN) )
> - rc = p2m_activate_altp2m(d, idx, hostp2m->default_access);
> -
> - altp2m_list_unlock(d);
> - return rc;
> -}
> -
> -int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
> - xenmem_access_t hvmmem_default_access)
> -{
> - int rc = -EINVAL;
> - unsigned int i;
> - p2m_access_t a;
> - struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> -
> - if ( hvmmem_default_access > XENMEM_access_default ||
> - !xenmem_access_to_p2m_access(hostp2m, hvmmem_default_access, &a) )
> - return rc;
> -
> - altp2m_list_lock(d);
> -
> - for ( i = 0; i < MAX_ALTP2M; i++ )
> - {
> - if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
> - continue;
> -
> - rc = p2m_activate_altp2m(d, i, a);
> -
> - if ( !rc )
> - *idx = i;
> -
> - break;
> - }
> -
> - altp2m_list_unlock(d);
> - return rc;
> -}
> -
> -int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx)
> -{
> - struct p2m_domain *p2m;
> - int rc = -EBUSY;
> -
> - if ( !idx || idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) )
> - return rc;
> -
> - rc = domain_pause_except_self(d);
> - if ( rc )
> - return rc;
> -
> - rc = -EBUSY;
> - altp2m_list_lock(d);
> -
> - if ( d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] !=
> - mfn_x(INVALID_MFN) )
> - {
> - p2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> -
> - if ( !_atomic_read(p2m->active_vcpus) )
> - {
> - p2m_reset_altp2m(d, idx, ALTP2M_DEACTIVATE);
> - d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] =
> - mfn_x(INVALID_MFN);
> - d->arch.altp2m_visible_eptp[array_index_nospec(idx, MAX_EPTP)] =
> - mfn_x(INVALID_MFN);
> - rc = 0;
> - }
> - }
> -
> - altp2m_list_unlock(d);
> -
> - domain_unpause_except_self(d);
> -
> - return rc;
> -}
> -
> -int p2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
> -{
> - struct vcpu *v;
> - int rc = -EINVAL;
> -
> - if ( idx >= MAX_ALTP2M )
> - return rc;
> -
> - rc = domain_pause_except_self(d);
> - if ( rc )
> - return rc;
> -
> - rc = -EINVAL;
> - altp2m_list_lock(d);
> -
> - if ( d->arch.altp2m_visible_eptp[idx] != mfn_x(INVALID_MFN) )
> - {
> - for_each_vcpu( d, v )
> - if ( p2m_set_altp2m(v, idx) )
> - altp2m_vcpu_update_p2m(v);
> -
> - rc = 0;
> - }
> -
> - altp2m_list_unlock(d);
> -
> - domain_unpause_except_self(d);
> -
> - return rc;
> -}
> -
> -int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
> - gfn_t old_gfn, gfn_t new_gfn)
> -{
> - struct p2m_domain *hp2m, *ap2m;
> - p2m_access_t a;
> - p2m_type_t t;
> - mfn_t mfn;
> - int rc = -EINVAL;
> -
> - if ( idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> - d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
> - mfn_x(INVALID_MFN) )
> - return rc;
> -
> - hp2m = p2m_get_hostp2m(d);
> - ap2m = array_access_nospec(d->arch.altp2m_p2m, idx);
> -
> - p2m_lock(hp2m);
> - p2m_lock(ap2m);
> -
> - if ( gfn_eq(new_gfn, INVALID_GFN) )
> - {
> - mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
> - rc = mfn_valid(mfn)
> - ? p2m_remove_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K)
> - : 0;
> - goto out;
> - }
> -
> - rc = altp2m_get_effective_entry(ap2m, old_gfn, &mfn, &t, &a,
> - AP2MGET_prepopulate);
> - if ( rc )
> - goto out;
> -
> - rc = altp2m_get_effective_entry(ap2m, new_gfn, &mfn, &t, &a,
> - AP2MGET_query);
> - if ( rc )
> - goto out;
> -
> - if ( !ap2m->set_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K, t, a,
> - (current->domain != d)) )
> - {
> - rc = 0;
> -
> - if ( gfn_x(new_gfn) < ap2m->min_remapped_gfn )
> - ap2m->min_remapped_gfn = gfn_x(new_gfn);
> - if ( gfn_x(new_gfn) > ap2m->max_remapped_gfn )
> - ap2m->max_remapped_gfn = gfn_x(new_gfn);
> - }
> -
> - out:
> - p2m_unlock(ap2m);
> - p2m_unlock(hp2m);
> - return rc;
> -}
> -
> -int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
> - mfn_t mfn, unsigned int page_order,
> - p2m_type_t p2mt, p2m_access_t p2ma)
> -{
> - struct p2m_domain *p2m;
> - unsigned int i;
> - unsigned int reset_count = 0;
> - unsigned int last_reset_idx = ~0;
> - int ret = 0;
> -
> - if ( !altp2m_active(d) )
> - return 0;
> -
> - altp2m_list_lock(d);
> -
> - for ( i = 0; i < MAX_ALTP2M; i++ )
> - {
> - p2m_type_t t;
> - p2m_access_t a;
> -
> - if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> - continue;
> -
> - p2m = d->arch.altp2m_p2m[i];
> -
> - /* Check for a dropped page that may impact this altp2m */
> - if ( mfn_eq(mfn, INVALID_MFN) &&
> - gfn_x(gfn) + (1UL << page_order) > p2m->min_remapped_gfn &&
> - gfn_x(gfn) <= p2m->max_remapped_gfn )
> - {
> - if ( !reset_count++ )
> - {
> - p2m_reset_altp2m(d, i, ALTP2M_RESET);
> - last_reset_idx = i;
> - }
> - else
> - {
> - /* At least 2 altp2m's impacted, so reset everything */
> - for ( i = 0; i < MAX_ALTP2M; i++ )
> - {
> - if ( i == last_reset_idx ||
> - d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> - continue;
> -
> - p2m_reset_altp2m(d, i, ALTP2M_RESET);
> - }
> -
> - ret = 0;
> - break;
> - }
> - }
> - else if ( !mfn_eq(get_gfn_type_access(p2m, gfn_x(gfn), &t, &a, 0,
> - NULL), INVALID_MFN) )
> - {
> - int rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
> -
> - /* Best effort: Don't bail on error. */
> - if ( !ret )
> - ret = rc;
> -
> - p2m_put_gfn(p2m, gfn);
> - }
> - else
> - p2m_put_gfn(p2m, gfn);
> - }
> -
> - altp2m_list_unlock(d);
> -
> - return ret;
> -}
> -
> /*** Audit ***/
>
> #if P2M_AUDIT
> @@ -2540,159 +2065,6 @@ int xenmem_add_to_physmap_one(
> return rc;
> }
>
> -/*
> - * Set/clear the #VE suppress bit for a page. Only available on VMX.
> - */
> -int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
> - unsigned int altp2m_idx)
> -{
> - int rc;
> - struct xen_hvm_altp2m_suppress_ve_multi sve = {
> - altp2m_idx, suppress_ve, 0, 0, gfn_x(gfn), gfn_x(gfn), 0
> - };
> -
> - if ( !(rc = p2m_set_suppress_ve_multi(d, &sve)) )
> - rc = sve.first_error;
> -
> - return rc;
> -}
> -
> -/*
> - * Set/clear the #VE suppress bit for multiple pages. Only available on VMX.
> - */
> -int p2m_set_suppress_ve_multi(struct domain *d,
> - struct xen_hvm_altp2m_suppress_ve_multi *sve)
> -{
> - struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> - struct p2m_domain *ap2m = NULL;
> - struct p2m_domain *p2m = host_p2m;
> - uint64_t start = sve->first_gfn;
> - int rc = 0;
> -
> - if ( sve->view > 0 )
> - {
> - if ( sve->view >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> - d->arch.altp2m_eptp[array_index_nospec(sve->view, MAX_EPTP)] ==
> - mfn_x(INVALID_MFN) )
> - return -EINVAL;
> -
> - p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, sve->view);
> - }
> -
> - p2m_lock(host_p2m);
> -
> - if ( ap2m )
> - p2m_lock(ap2m);
> -
> - while ( sve->last_gfn >= start )
> - {
> - p2m_access_t a;
> - p2m_type_t t;
> - mfn_t mfn;
> - int err = 0;
> -
> - if ( (err = altp2m_get_effective_entry(p2m, _gfn(start), &mfn, &t, &a,
> - AP2MGET_query)) &&
> - !sve->first_error )
> - {
> - sve->first_error_gfn = start; /* Save the gfn of the first error */
> - sve->first_error = err; /* Save the first error code */
> - }
> -
> - if ( !err && (err = p2m->set_entry(p2m, _gfn(start), mfn,
> - PAGE_ORDER_4K, t, a,
> - sve->suppress_ve)) &&
> - !sve->first_error )
> - {
> - sve->first_error_gfn = start; /* Save the gfn of the first error */
> - sve->first_error = err; /* Save the first error code */
> - }
> -
> - /* Check for continuation if it's not the last iteration. */
> - if ( sve->last_gfn >= ++start && hypercall_preempt_check() )
> - {
> - rc = -ERESTART;
> - break;
> - }
> - }
> -
> - sve->first_gfn = start;
> -
> - if ( ap2m )
> - p2m_unlock(ap2m);
> -
> - p2m_unlock(host_p2m);
> -
> - return rc;
> -}
> -
> -int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
> - unsigned int altp2m_idx)
> -{
> - struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> - struct p2m_domain *ap2m = NULL;
> - struct p2m_domain *p2m;
> - mfn_t mfn;
> - p2m_access_t a;
> - p2m_type_t t;
> - int rc = 0;
> -
> - if ( altp2m_idx > 0 )
> - {
> - if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> - d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> - mfn_x(INVALID_MFN) )
> - return -EINVAL;
> -
> - p2m = ap2m = array_access_nospec(d->arch.altp2m_p2m, altp2m_idx);
> - }
> - else
> - p2m = host_p2m;
> -
> - gfn_lock(host_p2m, gfn, 0);
> -
> - if ( ap2m )
> - p2m_lock(ap2m);
> -
> - mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, suppress_ve);
> - if ( !mfn_valid(mfn) )
> - rc = -ESRCH;
> -
> - if ( ap2m )
> - p2m_unlock(ap2m);
> -
> - gfn_unlock(host_p2m, gfn, 0);
> -
> - return rc;
> -}
> -
> -int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
> - uint8_t visible)
> -{
> - int rc = 0;
> -
> - altp2m_list_lock(d);
> -
> - /*
> - * Eptp index is correlated with altp2m index and should not exceed
> - * min(MAX_ALTP2M, MAX_EPTP).
> - */
> - if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> - d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> - mfn_x(INVALID_MFN) )
> - rc = -EINVAL;
> - else if ( visible )
> - d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
> - d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)];
> - else
> - d->arch.altp2m_visible_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] =
> - mfn_x(INVALID_MFN);
> -
> - altp2m_list_unlock(d);
> -
> - return rc;
> -}
> -
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/arch/x86/mm/p2m.h b/xen/arch/x86/mm/p2m.h
> index 04308cfb6d..635f5a7f45 100644
> --- a/xen/arch/x86/mm/p2m.h
> +++ b/xen/arch/x86/mm/p2m.h
> @@ -22,6 +22,9 @@ static inline void p2m_free_logdirty(struct p2m_domain *p2m) {}
> int p2m_init_altp2m(struct domain *d);
> void p2m_teardown_altp2m(struct domain *d);
>
> +void p2m_flush_table_locked(struct p2m_domain *p2m);
> +int __must_check p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
> + unsigned int page_order);
> void p2m_nestedp2m_init(struct p2m_domain *p2m);
> int p2m_init_nestedp2m(struct domain *d);
> void p2m_teardown_nestedp2m(struct domain *d);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-15 9:10 ` [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option Sergiy Kibrik
@ 2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 10:55 ` Jan Beulich
2024-05-16 11:01 ` Jan Beulich
1 sibling, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:38 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
George Dunlap, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Instead of using generic CONFIG_HVM option switch to a bit more specific
> CONFIG_ALTP2M option for altp2m support. Also guard altp2m routines, so that
> they can be disabled completely in the build -- when target platform does not
> actually support altp2m (AMD-V & ARM as of now).
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> ---
> changes in v2:
> - use separate CONFIG_ALTP2M option instead of CONFIG_VMX
> ---
> xen/arch/x86/include/asm/altp2m.h | 5 ++++-
> xen/arch/x86/include/asm/hvm/hvm.h | 2 +-
> xen/arch/x86/include/asm/p2m.h | 17 ++++++++++++++++-
> xen/arch/x86/mm/Makefile | 2 +-
> 4 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/altp2m.h b/xen/arch/x86/include/asm/altp2m.h
> index e5e59cbd68..092b13e231 100644
> --- a/xen/arch/x86/include/asm/altp2m.h
> +++ b/xen/arch/x86/include/asm/altp2m.h
> @@ -7,7 +7,7 @@
> #ifndef __ASM_X86_ALTP2M_H
> #define __ASM_X86_ALTP2M_H
>
> -#ifdef CONFIG_HVM
> +#ifdef CONFIG_ALTP2M
>
> #include <xen/types.h>
> #include <xen/sched.h> /* for struct vcpu, struct domain */
> @@ -38,7 +38,10 @@ static inline bool altp2m_active(const struct domain *d)
> }
>
> /* Only declaration is needed. DCE will optimise it out when linking. */
> +void altp2m_vcpu_initialise(struct vcpu *v);
> +void altp2m_vcpu_destroy(struct vcpu *v);
> uint16_t altp2m_vcpu_idx(const struct vcpu *v);
> +int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
> void altp2m_vcpu_disable_ve(struct vcpu *v);
>
> #endif
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
> index 0c9e6f1564..4f03dd7af8 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
> /* returns true if hardware supports alternate p2m's */
> static inline bool hvm_altp2m_supported(void)
> {
> - return hvm_funcs.caps.altp2m;
> + return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
> }
>
> /* Returns true if we have the minimum hardware requirements for nested virt */
> diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
> index 111badf89a..855e69d24a 100644
> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -581,9 +581,9 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, mfn_t mfn)
> return _gfn(mfn_x(mfn));
> }
>
> -#ifdef CONFIG_HVM
> #define AP2MGET_prepopulate true
> #define AP2MGET_query false
> +#ifdef CONFIG_ALTP2M
Is it necessary? Can't we just replace CONFIG_HVM with CONFIG_ALTP2M on
the same line
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-15 9:12 ` [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX} Sergiy Kibrik
@ 2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 11:12 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:38 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Andrew Cooper, Roger Pau Monné, Jan Beulich,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> As we now have SVM/VMX config options for enabling/disabling these features
> completely in the build, it may be feasible to add build-time checks to
> cpu_has_{svm,vmx} macros. These are used extensively thoughout HVM code, so
> we won't have to add extra #ifdef-s to check whether svm/vmx has been enabled,
> while DCE cleans up calls to vmx/svm functions, if their code not being built.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
2024-05-15 9:14 ` [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm} Sergiy Kibrik
@ 2024-05-16 0:44 ` Stefano Stabellini
2024-05-16 7:25 ` Jan Beulich
2024-05-16 11:23 ` Jan Beulich
1 sibling, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:44 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> If VMX/SVM disabled in the build, we may still want to have vPMU drivers for
> PV guests. Yet some calls to vmx/svm-related routines needs to be guarded then.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Question to the x86 maintainers: are we sure we want to support the case
where VMX/SVM is disabled in the build but still we want to run PV
guests with vPMU?
If the question is not, could we simplify this simply by making vpmu_amd
dependent on CONFIG_SVM and vpmu_intel dependent on CONFIG_VMX?
I realize that it is possible and technically correct to disable
CONFIG_SVM (or VMX) to run on AMD hardware (or Intel) with plain PV
guests only. But do we want to support it? I wonder if we could make
things easier by avoiding to support this configuration until somebody
asks for it.
> ---
> xen/arch/x86/cpu/vpmu_amd.c | 8 ++++----
> xen/arch/x86/cpu/vpmu_intel.c | 20 ++++++++++----------
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
> index db2fa420e1..40b0c8932f 100644
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -290,7 +290,7 @@ static int cf_check amd_vpmu_save(struct vcpu *v, bool to_guest)
> context_save(v);
>
> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
> - is_msr_bitmap_on(vpmu) )
> + is_msr_bitmap_on(vpmu) && cpu_has_svm )
> amd_vpmu_unset_msr_bitmap(v);
>
> if ( to_guest )
> @@ -363,7 +363,7 @@ static int cf_check amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> return 0;
> vpmu_set(vpmu, VPMU_RUNNING);
>
> - if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
> + if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
> amd_vpmu_set_msr_bitmap(v);
> }
>
> @@ -372,7 +372,7 @@ static int cf_check amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> (is_pmu_enabled(msr_content) == 0) && vpmu_is_set(vpmu, VPMU_RUNNING) )
> {
> vpmu_reset(vpmu, VPMU_RUNNING);
> - if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
> + if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
> amd_vpmu_unset_msr_bitmap(v);
> release_pmu_ownership(PMU_OWNER_HVM);
> }
> @@ -415,7 +415,7 @@ static void cf_check amd_vpmu_destroy(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> - if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) )
> + if ( is_hvm_vcpu(v) && is_msr_bitmap_on(vpmu) && cpu_has_svm )
> amd_vpmu_unset_msr_bitmap(v);
>
> xfree(vpmu->context);
> diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
> index cd414165df..10c34a5691 100644
> --- a/xen/arch/x86/cpu/vpmu_intel.c
> +++ b/xen/arch/x86/cpu/vpmu_intel.c
> @@ -269,7 +269,7 @@ static inline void __core2_vpmu_save(struct vcpu *v)
> if ( !is_hvm_vcpu(v) )
> rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS, core2_vpmu_cxt->global_status);
> /* Save MSR to private context to make it fork-friendly */
> - else if ( mem_sharing_enabled(v->domain) )
> + else if ( mem_sharing_enabled(v->domain) && cpu_has_vmx )
> vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
> &core2_vpmu_cxt->global_ctrl);
> }
> @@ -288,7 +288,7 @@ static int cf_check core2_vpmu_save(struct vcpu *v, bool to_guest)
>
> /* Unset PMU MSR bitmap to trap lazy load. */
> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
> - cpu_has_vmx_msr_bitmap )
> + cpu_has_vmx && cpu_has_vmx_msr_bitmap )
> core2_vpmu_unset_msr_bitmap(v);
>
> if ( to_guest )
> @@ -333,7 +333,7 @@ static inline void __core2_vpmu_load(struct vcpu *v)
> wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, core2_vpmu_cxt->global_ctrl);
> }
> /* Restore MSR from context when used with a fork */
> - else if ( mem_sharing_is_fork(v->domain) )
> + else if ( mem_sharing_is_fork(v->domain) && cpu_has_vmx )
> vmx_write_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
> core2_vpmu_cxt->global_ctrl);
> }
> @@ -442,7 +442,7 @@ static int cf_check core2_vpmu_alloc_resource(struct vcpu *v)
> if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
> return 0;
>
> - if ( is_hvm_vcpu(v) )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx )
> {
> if ( vmx_add_host_load_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, 0) )
> goto out_err;
> @@ -513,7 +513,7 @@ static int core2_vpmu_msr_common_check(u32 msr_index, int *type, int *index)
> __core2_vpmu_load(current);
> vpmu_set(vpmu, VPMU_CONTEXT_LOADED);
>
> - if ( is_hvm_vcpu(current) && cpu_has_vmx_msr_bitmap )
> + if ( is_hvm_vcpu(current) && cpu_has_vmx && cpu_has_vmx_msr_bitmap )
> core2_vpmu_set_msr_bitmap(current);
> }
> return 1;
> @@ -584,7 +584,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> if ( msr_content & fixed_ctrl_mask )
> return -EINVAL;
>
> - if ( is_hvm_vcpu(v) )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx )
> vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
> &core2_vpmu_cxt->global_ctrl);
> else
> @@ -653,7 +653,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> if ( blocked )
> return -EINVAL;
>
> - if ( is_hvm_vcpu(v) )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx)
> vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
> &core2_vpmu_cxt->global_ctrl);
> else
> @@ -672,7 +672,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> wrmsrl(msr, msr_content);
> else
> {
> - if ( is_hvm_vcpu(v) )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx )
> vmx_write_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
> else
> wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
> @@ -706,7 +706,7 @@ static int cf_check core2_vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
> *msr_content = core2_vpmu_cxt->global_status;
> break;
> case MSR_CORE_PERF_GLOBAL_CTRL:
> - if ( is_hvm_vcpu(v) )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx )
> vmx_read_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL, msr_content);
> else
> rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, *msr_content);
> @@ -808,7 +808,7 @@ static void cf_check core2_vpmu_destroy(struct vcpu *v)
> vpmu->context = NULL;
> xfree(vpmu->priv_context);
> vpmu->priv_context = NULL;
> - if ( is_hvm_vcpu(v) && cpu_has_vmx_msr_bitmap )
> + if ( is_hvm_vcpu(v) && cpu_has_vmx && cpu_has_vmx_msr_bitmap )
> core2_vpmu_unset_msr_bitmap(v);
> release_pmu_ownership(PMU_OWNER_HVM);
> vpmu_clear(vpmu);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s
2024-05-15 9:16 ` [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s Sergiy Kibrik
@ 2024-05-16 0:45 ` Stefano Stabellini
0 siblings, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:45 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Remove preprocessor checks for CONFIG_HVM option, because expressions covered
> by these checks are already guarded by cpu_has_vmx, which itself depends
> on CONFIG_HVM option (via CONFIG_VMX).
>
> No functional change intended.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Nice!
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 10/15] x86/domain: clean up superfluous #idef-s
2024-05-15 9:18 ` [XEN PATCH v2 10/15] x86/domain: " Sergiy Kibrik
@ 2024-05-16 0:46 ` Stefano Stabellini
0 siblings, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:46 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Remove preprocessor checks for CONFIG_HVM option, because expressions covered
> by these checks are already guarded by cpu_has_svm, which itself depends
> on CONFIG_HVM option (via CONFIG_SVM).
>
> No functional change intended.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code
2024-05-15 9:22 ` [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code Sergiy Kibrik
@ 2024-05-16 0:50 ` Stefano Stabellini
2024-05-16 7:32 ` Jan Beulich
0 siblings, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:50 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Roger Pau Monné, Paul Durrant, Jan Beulich,
Stefano Stabellini, Xenia Ragiadakou, Andrew Cooper
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> There're several places in common code, outside of arch/x86/hvm/vmx,
> where cpu_has_vmx_* get accessed without checking if VMX present first.
> We may want to guard these macros, as they read global variables defined
> inside vmx-specific files -- so VMX can be made optional later on.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> ---
> Here I've tried a different approach from prev.patches [1,2] -- instead of
> modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
> 1) do not touch SVM part at all, because just as Andrew pointed out they're
> used inside arch/x86/hvm/svm only.
> 2) track several places in common code where cpu_has_vmx_* features are
> checked out and guard them using cpu_has_vmx condition
> 3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
> more tricky way, so instead of making complex conditionals even more complicated,
> we can instead integrate cpu_has_vmx condition inside these two macros.
>
> This patch aims to replace [1,2] from v1 series by doing steps above.
>
> 1. https://lore.kernel.org/xen-devel/20240416064402.3469959-1-Sergiy_Kibrik@epam.com/
> 2. https://lore.kernel.org/xen-devel/20240416064606.3470052-1-Sergiy_Kibrik@epam.com/
I am missing some of the previous discussions but why can't we just fix
all of the cpu_has_vmx_* #defines in vmcs.h to also check for
cpu_has_vmx?
That seems easier and simpler than to add add-hoc checks at the invocations?
> ---
> changes in v2:
> - do not touch SVM code and macros
> - drop vmx_ctrl_has_feature()
> - guard cpu_has_vmx_* macros in common code instead
> changes in v1:
> - introduced helper routine vmx_ctrl_has_feature() and used it for all
> cpu_has_vmx_* macros
> ---
> xen/arch/x86/hvm/hvm.c | 2 +-
> xen/arch/x86/hvm/viridian/viridian.c | 4 ++--
> xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 4 ++--
> xen/arch/x86/traps.c | 5 +++--
> 4 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 9594e0a5c5..ab75de9779 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5180,7 +5180,7 @@ int hvm_debug_op(struct vcpu *v, int32_t op)
> {
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON:
> case XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF:
> - if ( !cpu_has_monitor_trap_flag )
> + if ( !cpu_has_vmx || !cpu_has_monitor_trap_flag )
> return -EOPNOTSUPP;
> break;
> default:
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index 0496c52ed5..657c6a3ea7 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -196,7 +196,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
> res->a = CPUID4A_RELAX_TIMER_INT;
> if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush )
> res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH;
> - if ( !cpu_has_vmx_apic_reg_virt )
> + if ( !cpu_has_vmx || !cpu_has_vmx_apic_reg_virt )
> res->a |= CPUID4A_MSR_BASED_APIC;
> if ( viridian_feature_mask(d) & HVMPV_hcall_ipi )
> res->a |= CPUID4A_SYNTHETIC_CLUSTER_IPI;
> @@ -236,7 +236,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t leaf,
>
> case 6:
> /* Detected and in use hardware features. */
> - if ( cpu_has_vmx_virtualize_apic_accesses )
> + if ( cpu_has_vmx && cpu_has_vmx_virtualize_apic_accesses )
> res->a |= CPUID6A_APIC_OVERLAY;
> if ( cpu_has_vmx_msr_bitmap || (read_efer() & EFER_SVME) )
> res->a |= CPUID6A_MSR_BITMAPS;
> diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> index 58140af691..aa05f9cf6e 100644
> --- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> +++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
> @@ -306,7 +306,7 @@ extern u64 vmx_ept_vpid_cap;
> #define cpu_has_vmx_vnmi \
> (vmx_pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
> #define cpu_has_vmx_msr_bitmap \
> - (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
> + (cpu_has_vmx && vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
> #define cpu_has_vmx_secondary_exec_control \
> (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
> #define cpu_has_vmx_tertiary_exec_control \
> @@ -347,7 +347,7 @@ extern u64 vmx_ept_vpid_cap;
> #define cpu_has_vmx_vmfunc \
> (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS)
> #define cpu_has_vmx_virt_exceptions \
> - (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
> + (cpu_has_vmx && vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
> #define cpu_has_vmx_pml \
> (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
> #define cpu_has_vmx_mpx \
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 7b8ee45edf..3595bb379a 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1130,7 +1130,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
> if ( !is_hvm_domain(d) || subleaf != 0 )
> break;
>
> - if ( cpu_has_vmx_apic_reg_virt )
> + if ( cpu_has_vmx && cpu_has_vmx_apic_reg_virt )
> res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>
> /*
> @@ -1139,7 +1139,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
> * and wrmsr in the guest will run without VMEXITs (see
> * vmx_vlapic_msr_changed()).
> */
> - if ( cpu_has_vmx_virtualize_x2apic_mode &&
> + if ( cpu_has_vmx &&
> + cpu_has_vmx_virtualize_x2apic_mode &&
> cpu_has_vmx_apic_reg_virt &&
> cpu_has_vmx_virtual_intr_delivery )
> res->a |= XEN_HVM_CPUID_X2APIC_VIRT;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
2024-05-15 9:24 ` [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX Sergiy Kibrik
@ 2024-05-16 0:51 ` Stefano Stabellini
2024-05-16 12:11 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:51 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Xenia Ragiadakou, Jan Beulich, Andrew Cooper,
Roger Pau Monné, George Dunlap, Julien Grall,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>
> VIO_realmode_completion is specific to vmx realmode, so guard the completion
> handling code with CONFIG_VMX. Also, guard VIO_realmode_completion itself by
> CONFIG_VMX, instead of generic CONFIG_X86.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-15 9:26 ` [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx Sergiy Kibrik
@ 2024-05-16 0:54 ` Stefano Stabellini
2024-05-16 7:37 ` Jan Beulich
2024-05-16 12:15 ` Jan Beulich
1 sibling, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:54 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> VMX posted interrupts support can now be excluded from x86 build along with
> VMX code itself, but still we may want to keep the possibility to use
> VT-d IOMMU driver in non-HVM setups.
> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
>
> No functional change intended here.
>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
I know that Andrew was keep on having a separate Kconfig option for
VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
option depending on CONFIG_VMX?
To me, VT-D should require VMX, without VMX it should not be possible to
enable VT-D.
This comment goes in the same direction of my previous comment regarding
the vpmu: we are trying to make things more configurable and flexible
and that's good, but we don't necessary need to make all possible
combination work. VT-D without VMX is another one of those combination
that I would only enable after a customer asks.
> ---
> xen/drivers/passthrough/vtd/iommu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index e13be244c1..ad78282250 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2772,7 +2772,7 @@ static int cf_check reassign_device_ownership(
>
> if ( !QUARANTINE_SKIP(target, pdev->arch.vtd.pgd_maddr) )
> {
> - if ( !has_arch_pdevs(target) )
> + if ( cpu_has_vmx && !has_arch_pdevs(target) )
> vmx_pi_hooks_assign(target);
>
> #ifdef CONFIG_PV
> @@ -2806,7 +2806,7 @@ static int cf_check reassign_device_ownership(
> }
> if ( ret )
> {
> - if ( !has_arch_pdevs(target) )
> + if ( cpu_has_vmx && !has_arch_pdevs(target) )
> vmx_pi_hooks_deassign(target);
> return ret;
> }
> @@ -2824,7 +2824,7 @@ static int cf_check reassign_device_ownership(
> write_unlock(&target->pci_lock);
> }
>
> - if ( !has_arch_pdevs(source) )
> + if ( cpu_has_vmx && !has_arch_pdevs(source) )
> vmx_pi_hooks_deassign(source);
>
> /*
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable
2024-05-15 9:28 ` [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable Sergiy Kibrik
@ 2024-05-16 0:54 ` Stefano Stabellini
2024-05-16 12:17 ` Jan Beulich
0 siblings, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 0:54 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Xenia Ragiadakou, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>
> Provide the user with configuration control over the cpu virtualization support
> in Xen by making SVM and VMX options user selectable.
>
> To preserve the current default behavior, both options depend on HVM and
> default to value of HVM.
>
> To prevent users from unknowingly disabling virtualization support, make the
> controls user selectable only if EXPERT is enabled.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
2024-05-16 0:44 ` Stefano Stabellini
@ 2024-05-16 7:25 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 7:25 UTC (permalink / raw
To: Stefano Stabellini
Cc: xen-devel, Andrew Cooper, Roger Pau Monné, Xenia Ragiadakou,
Sergiy Kibrik
On 16.05.2024 02:44, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> If VMX/SVM disabled in the build, we may still want to have vPMU drivers for
>> PV guests. Yet some calls to vmx/svm-related routines needs to be guarded then.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>
> Question to the x86 maintainers: are we sure we want to support the case
> where VMX/SVM is disabled in the build but still we want to run PV
> guests with vPMU?
>
> If the question is not, could we simplify this simply by making vpmu_amd
> dependent on CONFIG_SVM and vpmu_intel dependent on CONFIG_VMX?
>
> I realize that it is possible and technically correct to disable
> CONFIG_SVM (or VMX) to run on AMD hardware (or Intel) with plain PV
> guests only. But do we want to support it? I wonder if we could make
> things easier by avoiding to support this configuration until somebody
> asks for it.
I think we want to allow for such a configuration; whether that's deemed
a supported one is an orthogonal question. Much like you can set PV=n and
HVM=n at the same time, yielding a largely useless hypervisor (where
perhaps even the question of whether it's support may raise eyebrows).
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code
2024-05-16 0:50 ` Stefano Stabellini
@ 2024-05-16 7:32 ` Jan Beulich
2024-05-29 10:58 ` Sergiy Kibrik
0 siblings, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 7:32 UTC (permalink / raw
To: Stefano Stabellini, Sergiy Kibrik
Cc: xen-devel, Roger Pau Monné, Paul Durrant, Xenia Ragiadakou,
Andrew Cooper
On 16.05.2024 02:50, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> There're several places in common code, outside of arch/x86/hvm/vmx,
>> where cpu_has_vmx_* get accessed without checking if VMX present first.
>> We may want to guard these macros, as they read global variables defined
>> inside vmx-specific files -- so VMX can be made optional later on.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>> ---
>> Here I've tried a different approach from prev.patches [1,2] -- instead of
>> modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
>> 1) do not touch SVM part at all, because just as Andrew pointed out they're
>> used inside arch/x86/hvm/svm only.
>> 2) track several places in common code where cpu_has_vmx_* features are
>> checked out and guard them using cpu_has_vmx condition
>> 3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
>> more tricky way, so instead of making complex conditionals even more complicated,
>> we can instead integrate cpu_has_vmx condition inside these two macros.
>>
>> This patch aims to replace [1,2] from v1 series by doing steps above.
>>
>> 1. https://lore.kernel.org/xen-devel/20240416064402.3469959-1-Sergiy_Kibrik@epam.com/
>> 2. https://lore.kernel.org/xen-devel/20240416064606.3470052-1-Sergiy_Kibrik@epam.com/
>
> I am missing some of the previous discussions but why can't we just fix
> all of the cpu_has_vmx_* #defines in vmcs.h to also check for
> cpu_has_vmx?
>
> That seems easier and simpler than to add add-hoc checks at the invocations?
I'd like to take the question on step further: Following 0b5f149338e3
("x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware"),
is this change needed at all? IOW is there a path left where cpu_has_vmx
may be false, by any cpu_has_vmx_* may still yield true?
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-16 0:54 ` Stefano Stabellini
@ 2024-05-16 7:37 ` Jan Beulich
2024-05-16 23:41 ` Stefano Stabellini
0 siblings, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 7:37 UTC (permalink / raw
To: Stefano Stabellini, Sergiy Kibrik
Cc: xen-devel, Andrew Cooper, Roger Pau Monné, Xenia Ragiadakou
On 16.05.2024 02:54, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> VMX posted interrupts support can now be excluded from x86 build along with
>> VMX code itself, but still we may want to keep the possibility to use
>> VT-d IOMMU driver in non-HVM setups.
>> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
>>
>> No functional change intended here.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>
> I know that Andrew was keep on having a separate Kconfig option for
> VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
> option depending on CONFIG_VMX?
>
> To me, VT-D should require VMX, without VMX it should not be possible to
> enable VT-D.
>
> This comment goes in the same direction of my previous comment regarding
> the vpmu: we are trying to make things more configurable and flexible
> and that's good, but we don't necessary need to make all possible
> combination work. VT-D without VMX is another one of those combination
> that I would only enable after a customer asks.
Well. Imo again the configuration should be permitted. VMX and INTEL_IOMMU
ought to be default to INTEL, but permit being turned on/off in all cases.
(That's btw part of the reason why I continue to be unhappy with it being
INTEL where really INTEL_CPU was meant. If what is INTEL now would be
INTEL_CPU, INTEL could be an umbrella option for all three, or two if we
were to tie VMX to INTEL_CPU.)
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options
2024-05-15 8:59 ` [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options Sergiy Kibrik
@ 2024-05-16 10:09 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:09 UTC (permalink / raw
To: Sergiy Kibrik, Andrew Cooper, Roger Pau Monné
Cc: Xenia Ragiadakou, George Dunlap, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 15.05.2024 10:59, Sergiy Kibrik wrote:
> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>
> Introduce two new Kconfig options, SVM and VMX, to allow code
> specific to each virtualization technology to be separated and, when not
> required, stripped.
> CONFIG_SVM will be used to enable virtual machine extensions on platforms that
> implement the AMD Virtualization Technology (AMD-V).
> CONFIG_VMX will be used to enable virtual machine extensions on platforms that
> implement the Intel Virtualization Technology (Intel VT-x).
>
> Both features depend on HVM support.
>
> Since, at this point, disabling any of them would cause Xen to not compile,
> the options are enabled by default if HVM and are not selectable by the user.
>
> No functional change intended.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
In principle I'm okay with this, so
Acked-by: Jan Beulich <jbeulich@suse.com>
Nevertheless, ...
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -122,6 +122,12 @@ config HVM
>
> If unsure, say Y.
>
> +config SVM
> + def_bool HVM
> +
> +config VMX
> + def_bool HVM
... VT-x having a sub-technology abbreviated as SVM (Secure Virtual Memory),
I wonder if we wouldn't be better off using AMD_SVM and INTEL_VMX. Andrew,
Roger?
> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_MEM_SHARING) += mem_sharing.o
> obj-$(CONFIG_HVM) += nested.o
> obj-$(CONFIG_HVM) += p2m.o
> obj-y += p2m-basic.o
> -obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o p2m-pt.o
> +obj-$(CONFIG_VMX) += p2m-ept.o
> +obj-$(CONFIG_HVM) += p2m-pod.o p2m-pt.o
> obj-y += paging.o
> obj-y += physmap.o
Just as a remark, with no request for you to change anything right here:
With SVM=n and SHADOW_PAGING=n, I don't think we really need to build
p2m-pt.c. Yet of course this may require adjustments outside of the
Makefile to then actually build correctly.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage
2024-05-15 9:01 ` [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage Sergiy Kibrik
2024-05-16 0:29 ` Stefano Stabellini
@ 2024-05-16 10:20 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:20 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Alexandru Isaila, Petre Pircalabu, Andrew Cooper,
Roger Pau Monné, Stefano Stabellini, Xenia Ragiadakou,
Tamas K Lengyel, xen-devel
On 15.05.2024 11:01, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -262,6 +262,8 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
> struct vcpu *curr = current;
> vm_event_request_t req = {};
> paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
> + unsigned int altp2m_idx = altp2m_active(curr->domain) ?
> + altp2m_vcpu_idx(curr) : 0;
> int rc;
>
> ASSERT(curr->arch.vm_event->send_event);
> @@ -270,7 +272,7 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
> * p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
> * in which case access must be restricted.
> */
> - rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
> + rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);
I think something like this wants taking care of by the function itself.
asm/altp2m.h already has certain stubs (the conditional of which I expect
a later change will switch to CONFIG_ALTP2M); you'd need to just add one
more. Then the other (and any future) users of the function would be
covered as well.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines
2024-05-15 9:03 ` [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines Sergiy Kibrik
2024-05-16 0:31 ` Stefano Stabellini
@ 2024-05-16 10:26 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:26 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, George Dunlap, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel, xen-devel
On 15.05.2024 11:03, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/mm/p2m-basic.c
> +++ b/xen/arch/x86/mm/p2m-basic.c
> @@ -126,13 +126,15 @@ int p2m_init(struct domain *d)
> return rc;
> }
>
> - rc = p2m_init_altp2m(d);
> - if ( rc )
> + if ( hvm_altp2m_supported() )
> {
> - p2m_teardown_hostp2m(d);
> - p2m_teardown_nestedp2m(d);
> + rc = p2m_init_altp2m(d);
> + if ( rc )
> + {
> + p2m_teardown_hostp2m(d);
> + p2m_teardown_nestedp2m(d);
> + }
With less code churn and less indentation:
rc = hvm_altp2m_supported() ? p2m_init_altp2m(d) : 0;
if ( rc )
{
p2m_teardown_hostp2m(d);
p2m_teardown_nestedp2m(d);
}
> }
> -
> return rc;
> }
Please don't remove the blank line ahead of the main return of a function.
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -986,7 +986,7 @@ out:
> if ( is_epte_present(&old_entry) )
> ept_free_entry(p2m, &old_entry, target);
>
> - if ( entry_written && p2m_is_hostp2m(p2m) )
> + if ( entry_written && p2m_is_hostp2m(p2m) && hvm_altp2m_supported())
I agree with Stefano's ordering comment here, btw.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file
2024-05-15 9:06 ` [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file Sergiy Kibrik
2024-05-16 0:35 ` Stefano Stabellini
@ 2024-05-16 10:37 ` Jan Beulich
1 sibling, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:37 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, George Dunlap, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel, xen-devel
On 15.05.2024 11:06, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -500,7 +500,7 @@ int p2m_alloc_table(struct p2m_domain *p2m)
> return 0;
> }
>
> -static int __must_check
> +int __must_check
> p2m_remove_entry(struct p2m_domain *p2m, gfn_t gfn, mfn_t mfn,
> unsigned int page_order)
With there now being a separate declaration, __must_check doesn't need
repeating here. Then
Acked-by: Jan Beulich <jbeulich@suse.com>
I'll see whether this applies okay ahead of the earlier patches; if it
does, I'll commit it with said adjustment.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
2024-05-16 0:33 ` Stefano Stabellini
@ 2024-05-16 10:41 ` Jan Beulich
2024-05-18 1:07 ` Tamas K Lengyel
2 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:41 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, Tamas K Lengyel, xen-devel
On 15.05.2024 11:08, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -358,6 +358,11 @@ config REQUIRE_NX
> was unavailable. However, if enabled, Xen will no longer boot on
> any CPU which is lacking NX support.
>
> +config ALTP2M
> + bool "Alternate P2M support"
> + def_bool y
Since you have "bool" already, here you mean "default".
> + depends on VMX && EXPERT
This doesn't look right. Typical distro builds will be with EXPERT=n. Such
builds would suddenly end up without altp2m support then. What I think you
mean is a conditional prompt. I.e. overall
config ALTP2M
bool "Alternate P2M support" if EXPERT
default y
depends on VMX
help
...
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-16 0:38 ` Stefano Stabellini
@ 2024-05-16 10:55 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 10:55 UTC (permalink / raw
To: Stefano Stabellini, Sergiy Kibrik
Cc: xen-devel, Andrew Cooper, Roger Pau Monné, George Dunlap,
Xenia Ragiadakou, Tamas K Lengyel
On 16.05.2024 02:38, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/include/asm/p2m.h
>> +++ b/xen/arch/x86/include/asm/p2m.h
>> @@ -581,9 +581,9 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, mfn_t mfn)
>> return _gfn(mfn_x(mfn));
>> }
>>
>> -#ifdef CONFIG_HVM
>> #define AP2MGET_prepopulate true
>> #define AP2MGET_query false
>> +#ifdef CONFIG_ALTP2M
>
> Is it necessary? Can't we just replace CONFIG_HVM with CONFIG_ALTP2M on
> the same line
No, because of where those constants are used.
Sergiy, you want to move the new #ifdef down by a line though, such that
the #define-s are separated from it by the blank line that presently (with
your change applied) follows the #ifdef line.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-15 9:10 ` [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
@ 2024-05-16 11:01 ` Jan Beulich
2024-05-23 10:44 ` Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 11:01 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, George Dunlap,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel, xen-devel
On 15.05.2024 11:10, Sergiy Kibrik wrote:
> @@ -38,7 +38,10 @@ static inline bool altp2m_active(const struct domain *d)
> }
>
> /* Only declaration is needed. DCE will optimise it out when linking. */
> +void altp2m_vcpu_initialise(struct vcpu *v);
> +void altp2m_vcpu_destroy(struct vcpu *v);
> uint16_t altp2m_vcpu_idx(const struct vcpu *v);
> +int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
> void altp2m_vcpu_disable_ve(struct vcpu *v);
These additions look unrelated, as long as the description says nothing in
this regard.
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
> /* returns true if hardware supports alternate p2m's */
> static inline bool hvm_altp2m_supported(void)
> {
> - return hvm_funcs.caps.altp2m;
> + return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
Which in turn raises the question whether the altp2m struct field shouldn't
become conditional upon CONFIG_ALTP2M too (or rather: instead, as the change
here then would need to be done differently). Yet maybe that would entail
further changes elsewhere, so may well better be left for later.
> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -1,7 +1,7 @@
> obj-y += shadow/
> obj-$(CONFIG_HVM) += hap/
>
> -obj-$(CONFIG_HVM) += altp2m.o
> +obj-$(CONFIG_ALTP2M) += altp2m.o
This change I think wants to move to patch 5.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-15 9:12 ` [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX} Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
@ 2024-05-16 11:12 ` Jan Beulich
2024-05-23 13:07 ` Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 11:12 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 15.05.2024 11:12, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
> + boot_cpu_has(X86_FEATURE_VMX))
> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>
> /* CPUID level 0x80000001.ecx */
> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
> + boot_cpu_has(X86_FEATURE_SVM))
> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
Hmm, leaving aside the style issue (stray blanks after opening parentheses,
and as a result one-off indentation on the wrapped lines) I'm not really
certain we can do this. The description goes into detail why we would want
this, but it doesn't cover at all why it is safe for all present (and
ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
just to derive further knowledge from that, without them being directly
related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
for example. While it looks to be okay there, it may give you an idea of
what I mean.
Things might become better separated if instead for such checks we used
host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
that's still pretty far out, I'm afraid.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
2024-05-15 9:14 ` [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm} Sergiy Kibrik
2024-05-16 0:44 ` Stefano Stabellini
@ 2024-05-16 11:23 ` Jan Beulich
2024-06-03 9:02 ` Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 11:23 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 15.05.2024 11:14, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -290,7 +290,7 @@ static int cf_check amd_vpmu_save(struct vcpu *v, bool to_guest)
> context_save(v);
>
> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
> - is_msr_bitmap_on(vpmu) )
> + is_msr_bitmap_on(vpmu) && cpu_has_svm )
> amd_vpmu_unset_msr_bitmap(v);
Assuming the change in the earlier patch can actually be established to be
safe, along the lines of an earlier comment from Stefano the addition may
want to move earlier in the overall conditionals (here and below). In fact
I wonder whether it wouldn't be neater to have
#define is_svm_vcpu(v) (cpu_has_svm && is_hvm_vcpu(v))
at the top of the file, and then use that throughout to replace is_hvm_vcpu().
Same on the Intel side then, obviously.
> @@ -288,7 +288,7 @@ static int cf_check core2_vpmu_save(struct vcpu *v, bool to_guest)
>
> /* Unset PMU MSR bitmap to trap lazy load. */
> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
> - cpu_has_vmx_msr_bitmap )
> + cpu_has_vmx && cpu_has_vmx_msr_bitmap )
Aren't you elsewhere adding IS_ENABLED() to cpu_has_vmx_*, rendering this (and
similar changes further down) redundant?
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file
2024-05-16 0:35 ` Stefano Stabellini
@ 2024-05-16 11:46 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 11:46 UTC (permalink / raw
To: Stefano Stabellini
Cc: xen-devel, Andrew Cooper, George Dunlap, Roger Pau Monné,
Xenia Ragiadakou, Tamas K Lengyel, Sergiy Kibrik
On 16.05.2024 02:35, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> Move altp2m code from generic p2m.c file to altp2m.c, so it is kept separately
>> and can possibly be disabled in the build. We may want to disable it when
>> building for specific platform only, that doesn't support alternate p2m.
>>
>> No functional change intended.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>> CC: Tamas K Lengyel <tamas@tklengyel.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Ouch, I'm sorry, Stefano, I failed to put this in while committing.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
2024-05-15 9:24 ` [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX Sergiy Kibrik
2024-05-16 0:51 ` Stefano Stabellini
@ 2024-05-16 12:11 ` Jan Beulich
2024-05-31 8:05 ` Sergiy Kibrik
2024-05-31 8:06 ` [XEN PATCH RFC] ioreq: make arch_vcpu_ioreq_completion() an optional callback Sergiy Kibrik
1 sibling, 2 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 12:11 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Xenia Ragiadakou, Andrew Cooper, Roger Pau Monné,
George Dunlap, Julien Grall, Stefano Stabellini, Xenia Ragiadakou,
xen-devel
On 15.05.2024 11:24, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
> break;
>
> case VIO_mmio_completion:
> +#ifdef CONFIG_VMX
> case VIO_realmode_completion:
> +#endif
> BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf));
> hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes;
> memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes);
This change doesn't buy us anything, does it?
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
> {
> switch ( completion )
> {
> +#ifdef CONFIG_VMX
> case VIO_realmode_completion:
> {
> struct hvm_emulate_ctxt ctxt;
> @@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
>
> break;
> }
> +#endif
>
> default:
> ASSERT_UNREACHABLE();
And while this change is needed for the goal of the series, I wonder whether
it wouldn't better be arch_vcpu_ioreq_completion() as whole that then gets
stubbed out.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-15 9:26 ` [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx Sergiy Kibrik
2024-05-16 0:54 ` Stefano Stabellini
@ 2024-05-16 12:15 ` Jan Beulich
2024-06-03 9:34 ` Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 12:15 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 15.05.2024 11:26, Sergiy Kibrik wrote:
> VMX posted interrupts support can now be excluded from x86 build along with
> VMX code itself, but still we may want to keep the possibility to use
> VT-d IOMMU driver in non-HVM setups.
> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
But both function already have a stub each. Isn't is merely a matter of
changing when those stubs come into play?
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable
2024-05-16 0:54 ` Stefano Stabellini
@ 2024-05-16 12:17 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-16 12:17 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Xenia Ragiadakou, Andrew Cooper, Roger Pau Monné,
Xenia Ragiadakou, Stefano Stabellini
On 16.05.2024 02:54, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> From: Xenia Ragiadakou <burzalodowa@gmail.com>
>>
>> Provide the user with configuration control over the cpu virtualization support
>> in Xen by making SVM and VMX options user selectable.
>>
>> To preserve the current default behavior, both options depend on HVM and
>> default to value of HVM.
>>
>> To prevent users from unknowingly disabling virtualization support, make the
>> controls user selectable only if EXPERT is enabled.
>>
>> No functional change intended.
>>
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-16 7:37 ` Jan Beulich
@ 2024-05-16 23:41 ` Stefano Stabellini
0 siblings, 0 replies; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-16 23:41 UTC (permalink / raw
To: Jan Beulich
Cc: Stefano Stabellini, Sergiy Kibrik, xen-devel, Andrew Cooper,
Roger Pau Monné, Xenia Ragiadakou
On Thu, 16 May 2024, Jan Beulich wrote:
> On 16.05.2024 02:54, Stefano Stabellini wrote:
> > On Wed, 15 May 2024, Sergiy Kibrik wrote:
> >> VMX posted interrupts support can now be excluded from x86 build along with
> >> VMX code itself, but still we may want to keep the possibility to use
> >> VT-d IOMMU driver in non-HVM setups.
> >> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
> >>
> >> No functional change intended here.
> >>
> >> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> >
> > I know that Andrew was keep on having a separate Kconfig option for
> > VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
> > option depending on CONFIG_VMX?
> >
> > To me, VT-D should require VMX, without VMX it should not be possible to
> > enable VT-D.
> >
> > This comment goes in the same direction of my previous comment regarding
> > the vpmu: we are trying to make things more configurable and flexible
> > and that's good, but we don't necessary need to make all possible
> > combination work. VT-D without VMX is another one of those combination
> > that I would only enable after a customer asks.
>
> Well. Imo again the configuration should be permitted.
FYI Andrew said the same thing as you on Matrix, so I withdraw my
suggestion.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
2024-05-16 0:33 ` Stefano Stabellini
2024-05-16 10:41 ` Jan Beulich
@ 2024-05-18 1:07 ` Tamas K Lengyel
2 siblings, 0 replies; 62+ messages in thread
From: Tamas K Lengyel @ 2024-05-18 1:07 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou
> Currently altp2m support provided for VT-d only, so option is dependant on VMX.
No clue what is meant by "support provided for VT-d only". Altp2m has
nothing to do with VT-d. It would be more accurate to say it's only
implemented for Intel EPT.
Tamas
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-16 11:01 ` Jan Beulich
@ 2024-05-23 10:44 ` Sergiy Kibrik
2024-05-23 14:43 ` Jan Beulich
0 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-23 10:44 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, George Dunlap,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel, xen-devel
16.05.24 14:01, Jan Beulich:
> On 15.05.2024 11:10, Sergiy Kibrik wrote:
>> @@ -38,7 +38,10 @@ static inline bool altp2m_active(const struct domain *d)
>> }
>>
>> /* Only declaration is needed. DCE will optimise it out when linking. */
>> +void altp2m_vcpu_initialise(struct vcpu *v);
>> +void altp2m_vcpu_destroy(struct vcpu *v);
>> uint16_t altp2m_vcpu_idx(const struct vcpu *v);
>> +int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
>> void altp2m_vcpu_disable_ve(struct vcpu *v);
>
> These additions look unrelated, as long as the description says nothing in
> this regard.
agree, I'll update description on why these declarations are added
>
>> --- a/xen/arch/x86/include/asm/hvm/hvm.h
>> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
>> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
>> /* returns true if hardware supports alternate p2m's */
>> static inline bool hvm_altp2m_supported(void)
>> {
>> - return hvm_funcs.caps.altp2m;
>> + return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
>
> Which in turn raises the question whether the altp2m struct field shouldn't
> become conditional upon CONFIG_ALTP2M too (or rather: instead, as the change
> here then would need to be done differently). Yet maybe that would entail
> further changes elsewhere, so may well better be left for later.
>
but hvm_funcs.caps.altp2m is only a capability bit -- is it worth to
become conditional?
>> --- a/xen/arch/x86/mm/Makefile
>> +++ b/xen/arch/x86/mm/Makefile
>> @@ -1,7 +1,7 @@
>> obj-y += shadow/
>> obj-$(CONFIG_HVM) += hap/
>>
>> -obj-$(CONFIG_HVM) += altp2m.o
>> +obj-$(CONFIG_ALTP2M) += altp2m.o
>
> This change I think wants to move to patch 5.
>
If this moves to patch 5 then HVM=y && ALTP2M=n configuration
combination will break the build in between patch 5 and 6, so I've
decided to put it together with fixes of these build failures in patch 6.
Maybe I can merge patch 5 & 6 together then ?
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-16 11:12 ` Jan Beulich
@ 2024-05-23 13:07 ` Sergiy Kibrik
2024-05-23 14:50 ` Jan Beulich
0 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-23 13:07 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
16.05.24 14:12, Jan Beulich:
> On 15.05.2024 11:12, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
>> + boot_cpu_has(X86_FEATURE_VMX))
>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>
>> /* CPUID level 0x80000001.ecx */
>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
>> + boot_cpu_has(X86_FEATURE_SVM))
>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
>
> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
> and as a result one-off indentation on the wrapped lines) I'm not really
> certain we can do this. The description goes into detail why we would want
> this, but it doesn't cover at all why it is safe for all present (and
> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
> just to derive further knowledge from that, without them being directly
> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
> for example. While it looks to be okay there, it may give you an idea of
> what I mean.
>
> Things might become better separated if instead for such checks we used
> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
> that's still pretty far out, I'm afraid.
>
I've followed a suggestion you made for patch in previous series:
https://lore.kernel.org/xen-devel/8fbd604e-5e5d-410c-880f-2ad257bbe08a@suse.com/
yet if this approach can potentially be unsafe (I'm not completely sure
it's safe), should we instead fallback to the way it was done in v1
series? I.e. guard calls to vmx/svm-specific calls where needed, like in
these 3 patches:
1)
https://lore.kernel.org/xen-devel/20240416063328.3469386-1-Sergiy_Kibrik@epam.com/
2)
https://lore.kernel.org/xen-devel/20240416063740.3469592-1-Sergiy_Kibrik@epam.com/
3)
https://lore.kernel.org/xen-devel/20240416063947.3469718-1-Sergiy_Kibrik@epam.com/
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option
2024-05-23 10:44 ` Sergiy Kibrik
@ 2024-05-23 14:43 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-23 14:43 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, George Dunlap,
Stefano Stabellini, Xenia Ragiadakou, Tamas K Lengyel, xen-devel
On 23.05.2024 12:44, Sergiy Kibrik wrote:
> 16.05.24 14:01, Jan Beulich:
>> On 15.05.2024 11:10, Sergiy Kibrik wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/hvm.h
>>> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
>>> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
>>> /* returns true if hardware supports alternate p2m's */
>>> static inline bool hvm_altp2m_supported(void)
>>> {
>>> - return hvm_funcs.caps.altp2m;
>>> + return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
>>
>> Which in turn raises the question whether the altp2m struct field shouldn't
>> become conditional upon CONFIG_ALTP2M too (or rather: instead, as the change
>> here then would need to be done differently). Yet maybe that would entail
>> further changes elsewhere, so may well better be left for later.
>
> but hvm_funcs.caps.altp2m is only a capability bit -- is it worth to
> become conditional?
Well, the comment was more based on the overall principle than the actual
space savings that might result. Plus as said - likely that would not work
anyway without further changes elsewhere. So perhaps okay to leave as you
have it.
>>> --- a/xen/arch/x86/mm/Makefile
>>> +++ b/xen/arch/x86/mm/Makefile
>>> @@ -1,7 +1,7 @@
>>> obj-y += shadow/
>>> obj-$(CONFIG_HVM) += hap/
>>>
>>> -obj-$(CONFIG_HVM) += altp2m.o
>>> +obj-$(CONFIG_ALTP2M) += altp2m.o
>>
>> This change I think wants to move to patch 5.
>>
>
> If this moves to patch 5 then HVM=y && ALTP2M=n configuration
> combination will break the build in between patch 5 and 6, so I've
> decided to put it together with fixes of these build failures in patch 6.
Hmm, yes, I think I see what you mean.
> Maybe I can merge patch 5 & 6 together then ?
Perhaps more consistent that way, yes.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-23 13:07 ` Sergiy Kibrik
@ 2024-05-23 14:50 ` Jan Beulich
2024-05-23 23:36 ` Stefano Stabellini
2024-05-27 10:27 ` Sergiy Kibrik
0 siblings, 2 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-23 14:50 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 23.05.2024 15:07, Sergiy Kibrik wrote:
> 16.05.24 14:12, Jan Beulich:
>> On 15.05.2024 11:12, Sergiy Kibrik wrote:
>>> --- a/xen/arch/x86/include/asm/cpufeature.h
>>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
>>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
>>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
>>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
>>> + boot_cpu_has(X86_FEATURE_VMX))
>>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
>>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
>>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
>>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>
>>> /* CPUID level 0x80000001.ecx */
>>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
>>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
>>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
>>> + boot_cpu_has(X86_FEATURE_SVM))
>>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
>>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
>>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
>>
>> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
>> and as a result one-off indentation on the wrapped lines) I'm not really
>> certain we can do this. The description goes into detail why we would want
>> this, but it doesn't cover at all why it is safe for all present (and
>> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
>> just to derive further knowledge from that, without them being directly
>> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
>> for example. While it looks to be okay there, it may give you an idea of
>> what I mean.
>>
>> Things might become better separated if instead for such checks we used
>> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
>> that's still pretty far out, I'm afraid.
>
> I've followed a suggestion you made for patch in previous series:
>
> https://lore.kernel.org/xen-devel/8fbd604e-5e5d-410c-880f-2ad257bbe08a@suse.com/
See the "If not, ..." that I had put there. Doing the change just mechanically
isn't enough, you also need to make clear (in the description) that you
verified it's safe to have this way.
> yet if this approach can potentially be unsafe (I'm not completely sure
> it's safe), should we instead fallback to the way it was done in v1
> series? I.e. guard calls to vmx/svm-specific calls where needed, like in
> these 3 patches:
>
> 1)
> https://lore.kernel.org/xen-devel/20240416063328.3469386-1-Sergiy_Kibrik@epam.com/
>
> 2)
> https://lore.kernel.org/xen-devel/20240416063740.3469592-1-Sergiy_Kibrik@epam.com/
>
> 3)
> https://lore.kernel.org/xen-devel/20240416063947.3469718-1-Sergiy_Kibrik@epam.com/
I don't like this sprinkling around of IS_ENABLED() very much. Maybe we want
to have two new helpers (say using_svm() and using_vmx()), to be used in place
of most but possibly not all cpu_has_{svm,vmx}? Doing such a transformation
would then kind of implicitly answer the safety question above, as at every
use site you'd need to judge whether the replacement is correct. If it's
correct everywhere, the construct(s) as proposed in this version could then be
considered to be used in this very shape (instead of introducing the two new
helpers). But of course the transition could also be done gradually then,
touching only those uses that previously you touched in 1), 2), and 3).
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-23 14:50 ` Jan Beulich
@ 2024-05-23 23:36 ` Stefano Stabellini
2024-05-24 6:49 ` Jan Beulich
2024-05-27 10:27 ` Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Stefano Stabellini @ 2024-05-23 23:36 UTC (permalink / raw
To: Jan Beulich
Cc: Sergiy Kibrik, Andrew Cooper, Roger Pau Monné,
Stefano Stabellini, Xenia Ragiadakou, xen-devel
On Thu, 23 May 2024, Jan Beulich wrote:
> On 23.05.2024 15:07, Sergiy Kibrik wrote:
> > 16.05.24 14:12, Jan Beulich:
> >> On 15.05.2024 11:12, Sergiy Kibrik wrote:
> >>> --- a/xen/arch/x86/include/asm/cpufeature.h
> >>> +++ b/xen/arch/x86/include/asm/cpufeature.h
> >>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
> >>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
> >>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
> >>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
> >>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
> >>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
> >>> + boot_cpu_has(X86_FEATURE_VMX))
> >>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
> >>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
> >>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
> >>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
> >>>
> >>> /* CPUID level 0x80000001.ecx */
> >>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
> >>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
> >>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
> >>> + boot_cpu_has(X86_FEATURE_SVM))
> >>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
> >>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
> >>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
> >>
> >> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
> >> and as a result one-off indentation on the wrapped lines) I'm not really
> >> certain we can do this. The description goes into detail why we would want
> >> this, but it doesn't cover at all why it is safe for all present (and
> >> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
> >> just to derive further knowledge from that, without them being directly
> >> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
> >> for example. While it looks to be okay there, it may give you an idea of
> >> what I mean.
> >>
> >> Things might become better separated if instead for such checks we used
> >> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
> >> that's still pretty far out, I'm afraid.
> >
> > I've followed a suggestion you made for patch in previous series:
> >
> > https://lore.kernel.org/xen-devel/8fbd604e-5e5d-410c-880f-2ad257bbe08a@suse.com/
>
> See the "If not, ..." that I had put there. Doing the change just mechanically
> isn't enough, you also need to make clear (in the description) that you
> verified it's safe to have this way.
What does it mean to "verified it's safe to have this way"? "Safe" in
what way?
> > yet if this approach can potentially be unsafe (I'm not completely sure
> > it's safe), should we instead fallback to the way it was done in v1
> > series? I.e. guard calls to vmx/svm-specific calls where needed, like in
> > these 3 patches:
> >
> > 1)
> > https://lore.kernel.org/xen-devel/20240416063328.3469386-1-Sergiy_Kibrik@epam.com/
> >
> > 2)
> > https://lore.kernel.org/xen-devel/20240416063740.3469592-1-Sergiy_Kibrik@epam.com/
> >
> > 3)
> > https://lore.kernel.org/xen-devel/20240416063947.3469718-1-Sergiy_Kibrik@epam.com/
>
> I don't like this sprinkling around of IS_ENABLED() very much. Maybe we want
> to have two new helpers (say using_svm() and using_vmx()), to be used in place
> of most but possibly not all cpu_has_{svm,vmx}? Doing such a transformation
> would then kind of implicitly answer the safety question above, as at every
> use site you'd need to judge whether the replacement is correct. If it's
> correct everywhere, the construct(s) as proposed in this version could then be
> considered to be used in this very shape (instead of introducing the two new
> helpers). But of course the transition could also be done gradually then,
> touching only those uses that previously you touched in 1), 2), and 3).
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-23 23:36 ` Stefano Stabellini
@ 2024-05-24 6:49 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-24 6:49 UTC (permalink / raw
To: Stefano Stabellini
Cc: Sergiy Kibrik, Andrew Cooper, Roger Pau Monné,
Xenia Ragiadakou, xen-devel
On 24.05.2024 01:36, Stefano Stabellini wrote:
> On Thu, 23 May 2024, Jan Beulich wrote:
>> On 23.05.2024 15:07, Sergiy Kibrik wrote:
>>> 16.05.24 14:12, Jan Beulich:
>>>> On 15.05.2024 11:12, Sergiy Kibrik wrote:
>>>>> --- a/xen/arch/x86/include/asm/cpufeature.h
>>>>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>>>>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
>>>>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>>>>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
>>>>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
>>>>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
>>>>> + boot_cpu_has(X86_FEATURE_VMX))
>>>>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
>>>>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
>>>>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
>>>>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>>>
>>>>> /* CPUID level 0x80000001.ecx */
>>>>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
>>>>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
>>>>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
>>>>> + boot_cpu_has(X86_FEATURE_SVM))
>>>>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
>>>>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
>>>>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
>>>>
>>>> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
>>>> and as a result one-off indentation on the wrapped lines) I'm not really
>>>> certain we can do this. The description goes into detail why we would want
>>>> this, but it doesn't cover at all why it is safe for all present (and
>>>> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
>>>> just to derive further knowledge from that, without them being directly
>>>> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
>>>> for example. While it looks to be okay there, it may give you an idea of
>>>> what I mean.
>>>>
>>>> Things might become better separated if instead for such checks we used
>>>> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
>>>> that's still pretty far out, I'm afraid.
>>>
>>> I've followed a suggestion you made for patch in previous series:
>>>
>>> https://lore.kernel.org/xen-devel/8fbd604e-5e5d-410c-880f-2ad257bbe08a@suse.com/
>>
>> See the "If not, ..." that I had put there. Doing the change just mechanically
>> isn't enough, you also need to make clear (in the description) that you
>> verified it's safe to have this way.
>
> What does it mean to "verified it's safe to have this way"? "Safe" in
> what way?
"Safe" as in "not breaking existing logic", anywhere.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-23 14:50 ` Jan Beulich
2024-05-23 23:36 ` Stefano Stabellini
@ 2024-05-27 10:27 ` Sergiy Kibrik
2024-05-27 10:49 ` Jan Beulich
1 sibling, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-27 10:27 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
23.05.24 17:50, Jan Beulich:
> On 23.05.2024 15:07, Sergiy Kibrik wrote:
>> 16.05.24 14:12, Jan Beulich:
>>> On 15.05.2024 11:12, Sergiy Kibrik wrote:
>>>> --- a/xen/arch/x86/include/asm/cpufeature.h
>>>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>>>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
>>>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>>>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
>>>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
>>>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
>>>> + boot_cpu_has(X86_FEATURE_VMX))
>>>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
>>>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
>>>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
>>>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>>
>>>> /* CPUID level 0x80000001.ecx */
>>>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
>>>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
>>>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
>>>> + boot_cpu_has(X86_FEATURE_SVM))
>>>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
>>>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
>>>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
>>>
>>> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
>>> and as a result one-off indentation on the wrapped lines) I'm not really
>>> certain we can do this. The description goes into detail why we would want
>>> this, but it doesn't cover at all why it is safe for all present (and
>>> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
>>> just to derive further knowledge from that, without them being directly
>>> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
>>> for example. While it looks to be okay there, it may give you an idea of
>>> what I mean.
>>>
>>> Things might become better separated if instead for such checks we used
>>> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
>>> that's still pretty far out, I'm afraid.
>>
>> I've followed a suggestion you made for patch in previous series:
>>
[..]
>
> See the "If not, ..." that I had put there. Doing the change just mechanically
> isn't enough, you also need to make clear (in the description) that you
> verified it's safe to have this way.
>
>> yet if this approach can potentially be unsafe (I'm not completely sure
>> it's safe), should we instead fallback to the way it was done in v1
>> series? I.e. guard calls to vmx/svm-specific calls where needed, like in
>> these 3 patches:
>>
[..]
>
> I don't like this sprinkling around of IS_ENABLED() very much. Maybe we want
> to have two new helpers (say using_svm() and using_vmx()), to be used in place
> of most but possibly not all cpu_has_{svm,vmx}? Doing such a transformation
> would then kind of implicitly answer the safety question above, as at every
> use site you'd need to judge whether the replacement is correct. If it's
> correct everywhere, the construct(s) as proposed in this version could then be
> considered to be used in this very shape (instead of introducing the two new
> helpers). But of course the transition could also be done gradually then,
> touching only those uses that previously you touched in 1), 2), and 3).
>
now I might be seeing your concerns, if I understood correctly,
situation is the following.
As an example of cpu_has_vmx macro, it can be used to prove either of
following two statements: 1) VMX features can be used or 2) CPU provides
VMX features.
Currently they're the same for Xen, yet after this patch series they're
not, as the situation possible when non-vmx build would be able to get
executed on vmx-enabled machine. E.g. the case of PV guest, or (if that
makes any sense) at least hypervisor's code is still able to run until
HVM guest has to be created. Changes in this patch makes
indistinguishable for a user whether VMX support is absent in code or in
hardware -- hence we may need two separate macros for these.
Still the question remains whether a separate macro to check if CPU
provides VMX/SVM is really needed at all at this point.
I've counted only 16 uses of cpu_has_vmx in the code, not that much to
check every one of them, so I did that.
Most of uses are obviously checks before using vmx features, so logic
not broken.
As for the others, the surrounding context presumes that HVM domain
required there or had already been created. But non-vmx build can't
create HVM VMX domain anyway, so the logic not broken either.
As for cpu_has_svm only 8 uses I've counted, all but one also don't seem
to break logic as described above. One check of cpu_has_svm in
init_speculation_mitigations(), where default speculation control flag
gets set, not uses SVM features directly. Yet from the comment I can
assume that it's also related to running HVM domain and usage of VMX
features at later time.
With all above, at the moment there doesn't seem to be uses of
cpu_has_{svm,vmx} macros without subsequent usage of svm/vmx features,
so this patch should be quite safe.
Please let me know whether the above reasoning makes sense.
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX}
2024-05-27 10:27 ` Sergiy Kibrik
@ 2024-05-27 10:49 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-27 10:49 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
On 27.05.2024 12:27, Sergiy Kibrik wrote:
> 23.05.24 17:50, Jan Beulich:
>> On 23.05.2024 15:07, Sergiy Kibrik wrote:
>>> 16.05.24 14:12, Jan Beulich:
>>>> On 15.05.2024 11:12, Sergiy Kibrik wrote:
>>>>> --- a/xen/arch/x86/include/asm/cpufeature.h
>>>>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>>>>> @@ -81,7 +81,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>>> #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3)
>>>>> #define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
>>>>> #define cpu_has_monitor boot_cpu_has(X86_FEATURE_MONITOR)
>>>>> -#define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMX)
>>>>> +#define cpu_has_vmx ( IS_ENABLED(CONFIG_VMX) && \
>>>>> + boot_cpu_has(X86_FEATURE_VMX))
>>>>> #define cpu_has_eist boot_cpu_has(X86_FEATURE_EIST)
>>>>> #define cpu_has_ssse3 boot_cpu_has(X86_FEATURE_SSSE3)
>>>>> #define cpu_has_fma boot_cpu_has(X86_FEATURE_FMA)
>>>>> @@ -109,7 +110,8 @@ static inline bool boot_cpu_has(unsigned int feat)
>>>>>
>>>>> /* CPUID level 0x80000001.ecx */
>>>>> #define cpu_has_cmp_legacy boot_cpu_has(X86_FEATURE_CMP_LEGACY)
>>>>> -#define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM)
>>>>> +#define cpu_has_svm ( IS_ENABLED(CONFIG_SVM) && \
>>>>> + boot_cpu_has(X86_FEATURE_SVM))
>>>>> #define cpu_has_sse4a boot_cpu_has(X86_FEATURE_SSE4A)
>>>>> #define cpu_has_xop boot_cpu_has(X86_FEATURE_XOP)
>>>>> #define cpu_has_skinit boot_cpu_has(X86_FEATURE_SKINIT)
>>>>
>>>> Hmm, leaving aside the style issue (stray blanks after opening parentheses,
>>>> and as a result one-off indentation on the wrapped lines) I'm not really
>>>> certain we can do this. The description goes into detail why we would want
>>>> this, but it doesn't cover at all why it is safe for all present (and
>>>> ideally also future) uses. I wouldn't be surprised if we had VMX/SVM checks
>>>> just to derive further knowledge from that, without them being directly
>>>> related to the use of VMX/SVM. Take a look at calculate_hvm_max_policy(),
>>>> for example. While it looks to be okay there, it may give you an idea of
>>>> what I mean.
>>>>
>>>> Things might become better separated if instead for such checks we used
>>>> host and raw CPU policies instead of cpuinfo_x86.x86_capability[]. But
>>>> that's still pretty far out, I'm afraid.
>>>
>>> I've followed a suggestion you made for patch in previous series:
>>>
> [..]
>>
>> See the "If not, ..." that I had put there. Doing the change just mechanically
>> isn't enough, you also need to make clear (in the description) that you
>> verified it's safe to have this way.
>>
>>> yet if this approach can potentially be unsafe (I'm not completely sure
>>> it's safe), should we instead fallback to the way it was done in v1
>>> series? I.e. guard calls to vmx/svm-specific calls where needed, like in
>>> these 3 patches:
>>>
> [..]
>>
>> I don't like this sprinkling around of IS_ENABLED() very much. Maybe we want
>> to have two new helpers (say using_svm() and using_vmx()), to be used in place
>> of most but possibly not all cpu_has_{svm,vmx}? Doing such a transformation
>> would then kind of implicitly answer the safety question above, as at every
>> use site you'd need to judge whether the replacement is correct. If it's
>> correct everywhere, the construct(s) as proposed in this version could then be
>> considered to be used in this very shape (instead of introducing the two new
>> helpers). But of course the transition could also be done gradually then,
>> touching only those uses that previously you touched in 1), 2), and 3).
>>
>
> now I might be seeing your concerns, if I understood correctly,
> situation is the following.
>
> As an example of cpu_has_vmx macro, it can be used to prove either of
> following two statements: 1) VMX features can be used or 2) CPU provides
> VMX features.
> Currently they're the same for Xen, yet after this patch series they're
> not, as the situation possible when non-vmx build would be able to get
> executed on vmx-enabled machine. E.g. the case of PV guest, or (if that
> makes any sense) at least hypervisor's code is still able to run until
> HVM guest has to be created. Changes in this patch makes
> indistinguishable for a user whether VMX support is absent in code or in
> hardware -- hence we may need two separate macros for these.
>
> Still the question remains whether a separate macro to check if CPU
> provides VMX/SVM is really needed at all at this point.
>
> I've counted only 16 uses of cpu_has_vmx in the code, not that much to
> check every one of them, so I did that.
> Most of uses are obviously checks before using vmx features, so logic
> not broken.
> As for the others, the surrounding context presumes that HVM domain
> required there or had already been created. But non-vmx build can't
> create HVM VMX domain anyway, so the logic not broken either.
>
> As for cpu_has_svm only 8 uses I've counted, all but one also don't seem
> to break logic as described above. One check of cpu_has_svm in
> init_speculation_mitigations(), where default speculation control flag
> gets set, not uses SVM features directly. Yet from the comment I can
> assume that it's also related to running HVM domain and usage of VMX
> features at later time.
Note how to comment (necessarily) mixes SVM and VT-x. The !cpu_have_svm
really looks to mean cpu_has_vmx there (i.e. absence of SVM on a HVM-only
path implying VT-x). This would be broken if cpu_has_svm had
IS_ENABLED() added to it. You'll want to consult the commit introducing
the construct as well as perhaps Andrew directly as to whether switching
to cpu_has_vmx here would be appropriate. I'm pretty sure there was a
reason why this was written using the negative (!SVM) form.
> With all above, at the moment there doesn't seem to be uses of
> cpu_has_{svm,vmx} macros without subsequent usage of svm/vmx features,
> so this patch should be quite safe.
>
> Please let me know whether the above reasoning makes sense.
Fundamentally it does, with said one exception. Yet even in the absence
of that exception the question would then remain whether we really want
to close the road to one of the two possible uses of these constructs.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code
2024-05-16 7:32 ` Jan Beulich
@ 2024-05-29 10:58 ` Sergiy Kibrik
2024-05-29 12:16 ` Jan Beulich
0 siblings, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-29 10:58 UTC (permalink / raw
To: Jan Beulich, Stefano Stabellini
Cc: xen-devel, Roger Pau Monné, Paul Durrant, Xenia Ragiadakou,
Andrew Cooper
16.05.24 10:32, Jan Beulich:
> On 16.05.2024 02:50, Stefano Stabellini wrote:
>> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>>> There're several places in common code, outside of arch/x86/hvm/vmx,
>>> where cpu_has_vmx_* get accessed without checking if VMX present first.
>>> We may want to guard these macros, as they read global variables defined
>>> inside vmx-specific files -- so VMX can be made optional later on.
>>>
>>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>>> CC: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> Here I've tried a different approach from prev.patches [1,2] -- instead of
>>> modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
>>> 1) do not touch SVM part at all, because just as Andrew pointed out they're
>>> used inside arch/x86/hvm/svm only.
>>> 2) track several places in common code where cpu_has_vmx_* features are
>>> checked out and guard them using cpu_has_vmx condition
>>> 3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
>>> more tricky way, so instead of making complex conditionals even more complicated,
>>> we can instead integrate cpu_has_vmx condition inside these two macros.
>>>
>>> This patch aims to replace [1,2] from v1 series by doing steps above.
>>>
[..]
>>
>> I am missing some of the previous discussions but why can't we just fix
>> all of the cpu_has_vmx_* #defines in vmcs.h to also check for
>> cpu_has_vmx?
>>
>> That seems easier and simpler than to add add-hoc checks at the invocations?
>
> I'd like to take the question on step further: Following 0b5f149338e3
> ("x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware"),
> is this change needed at all? IOW is there a path left where cpu_has_vmx
> may be false, by any cpu_has_vmx_* may still yield true?
>
This change is about exec control variables (vmx_secondary_exec_control,
vmx_pin_based_exec_control etc) not been built, because they're in vmx
code, but accessed in common code. The description is probably unclear
about that.
Also build issues related to VMX can be solved differently, without
touching cpu_has_vmx_* macros and related logic at all.
I can move exec control variables from vmcs.c to common hvm.c, this
would be simpler change and directly related to problem that I'm having.
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code
2024-05-29 10:58 ` Sergiy Kibrik
@ 2024-05-29 12:16 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-29 12:16 UTC (permalink / raw
To: Sergiy Kibrik
Cc: xen-devel, Roger Pau Monné, Paul Durrant, Xenia Ragiadakou,
Andrew Cooper, Stefano Stabellini
On 29.05.2024 12:58, Sergiy Kibrik wrote:
> 16.05.24 10:32, Jan Beulich:
>> On 16.05.2024 02:50, Stefano Stabellini wrote:
>>> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>>>> There're several places in common code, outside of arch/x86/hvm/vmx,
>>>> where cpu_has_vmx_* get accessed without checking if VMX present first.
>>>> We may want to guard these macros, as they read global variables defined
>>>> inside vmx-specific files -- so VMX can be made optional later on.
>>>>
>>>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>>>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> CC: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> Here I've tried a different approach from prev.patches [1,2] -- instead of
>>>> modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
>>>> 1) do not touch SVM part at all, because just as Andrew pointed out they're
>>>> used inside arch/x86/hvm/svm only.
>>>> 2) track several places in common code where cpu_has_vmx_* features are
>>>> checked out and guard them using cpu_has_vmx condition
>>>> 3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
>>>> more tricky way, so instead of making complex conditionals even more complicated,
>>>> we can instead integrate cpu_has_vmx condition inside these two macros.
>>>>
>>>> This patch aims to replace [1,2] from v1 series by doing steps above.
>>>>
> [..]
>>>
>>> I am missing some of the previous discussions but why can't we just fix
>>> all of the cpu_has_vmx_* #defines in vmcs.h to also check for
>>> cpu_has_vmx?
>>>
>>> That seems easier and simpler than to add add-hoc checks at the invocations?
>>
>> I'd like to take the question on step further: Following 0b5f149338e3
>> ("x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware"),
>> is this change needed at all? IOW is there a path left where cpu_has_vmx
>> may be false, by any cpu_has_vmx_* may still yield true?
>>
>
> This change is about exec control variables (vmx_secondary_exec_control,
> vmx_pin_based_exec_control etc) not been built, because they're in vmx
> code, but accessed in common code. The description is probably unclear
> about that.
> Also build issues related to VMX can be solved differently, without
> touching cpu_has_vmx_* macros and related logic at all.
> I can move exec control variables from vmcs.c to common hvm.c, this
> would be simpler change and directly related to problem that I'm having.
That would be moving them one layer too high. Proper disentangling then
will need to wait until that data is actually part of the (host) CPU
policy. For the time being your change may thus be acceptable, assuming
that we won't be very quick in doing said move.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
2024-05-16 12:11 ` Jan Beulich
@ 2024-05-31 8:05 ` Sergiy Kibrik
2024-05-31 8:31 ` Jan Beulich
2024-05-31 8:06 ` [XEN PATCH RFC] ioreq: make arch_vcpu_ioreq_completion() an optional callback Sergiy Kibrik
1 sibling, 1 reply; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-31 8:05 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, George Dunlap, Julien Grall,
Stefano Stabellini, Xenia Ragiadakou, xen-devel
16.05.24 15:11, Jan Beulich:
> On 15.05.2024 11:24, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
>> break;
>>
>> case VIO_mmio_completion:
>> +#ifdef CONFIG_VMX
>> case VIO_realmode_completion:
>> +#endif
>> BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf));
>> hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes;
>> memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes);
>
> This change doesn't buy us anything, does it?
why not? Code won't compile w/o it.
Or do you mean hiding the whole VIO_realmode_completion enum under
CONFIG_VMX wasn't really useful?
>> --- a/xen/arch/x86/hvm/ioreq.c
>> +++ b/xen/arch/x86/hvm/ioreq.c
>> @@ -33,6 +33,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
>> {
>> switch ( completion )
>> {
>> +#ifdef CONFIG_VMX
>> case VIO_realmode_completion:
>> {
>> struct hvm_emulate_ctxt ctxt;
>> @@ -43,6 +44,7 @@ bool arch_vcpu_ioreq_completion(enum vio_completion completion)
>>
>> break;
>> }
>> +#endif
>>
>> default:
>> ASSERT_UNREACHABLE();
>
> And while this change is needed for the goal of the series, I wonder whether
> it wouldn't better be arch_vcpu_ioreq_completion() as whole that then gets
> stubbed out.
>
I'll post a patch to this thread to confirm whether I got your point
correctly.
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* [XEN PATCH RFC] ioreq: make arch_vcpu_ioreq_completion() an optional callback
2024-05-16 12:11 ` Jan Beulich
2024-05-31 8:05 ` Sergiy Kibrik
@ 2024-05-31 8:06 ` Sergiy Kibrik
1 sibling, 0 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-05-31 8:06 UTC (permalink / raw
To: jbeulich
Cc: andrew.cooper3, george.dunlap, julien, roger.pau, sstabellini,
xen-devel, xenia.ragiadakou, Sergiy Kibrik
For the most cases arch_vcpu_ioreq_completion() routine is just an empty stub,
except when handling VIO_realmode_completion, which only happens on HVM
domains running on VT-x machine. When VT-x is disabled in build configuration,
both x86 & arm version of routine become empty stubs.
To dispose of these useless stubs we can do optional call to arch-specific
ioreq completion handler, if it's present, and drop arm and generic x86 handlers.
Actual handling of VIO_realmore_completion can be done by VMX code then.
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
xen/arch/arm/ioreq.c | 6 ------
xen/arch/x86/hvm/ioreq.c | 25 -------------------------
xen/arch/x86/hvm/vmx/vmx.c | 15 +++++++++++++++
xen/common/ioreq.c | 5 ++++-
xen/include/xen/ioreq.h | 2 +-
5 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index 5df755b48b..2e829d2e7f 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -135,12 +135,6 @@ bool arch_ioreq_complete_mmio(void)
return false;
}
-bool arch_vcpu_ioreq_completion(enum vio_completion completion)
-{
- ASSERT_UNREACHABLE();
- return true;
-}
-
/*
* The "legacy" mechanism of mapping magic pages for the IOREQ servers
* is x86 specific, so the following hooks don't need to be implemented on Arm:
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index b37bbd660b..088650e007 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -29,31 +29,6 @@ bool arch_ioreq_complete_mmio(void)
return handle_mmio();
}
-bool arch_vcpu_ioreq_completion(enum vio_completion completion)
-{
- switch ( completion )
- {
-#ifdef CONFIG_VMX
- case VIO_realmode_completion:
- {
- struct hvm_emulate_ctxt ctxt;
-
- hvm_emulate_init_once(&ctxt, NULL, guest_cpu_user_regs());
- vmx_realmode_emulate_one(&ctxt);
- hvm_emulate_writeback(&ctxt);
-
- break;
- }
-#endif
-
- default:
- ASSERT_UNREACHABLE();
- break;
- }
-
- return true;
-}
-
static gfn_t hvm_alloc_legacy_ioreq_gfn(struct ioreq_server *s)
{
struct domain *d = s->target;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 8ba996546f..4f9be50fe7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2749,6 +2749,20 @@ static void cf_check vmx_set_reg(struct vcpu *v, unsigned int reg, uint64_t val)
vmx_vmcs_exit(v);
}
+bool realmode_vcpu_ioreq_completion(enum vio_completion completion)
+{
+ struct hvm_emulate_ctxt ctxt;
+
+ if ( completion != VIO_realmode_completion )
+ ASSERT_UNREACHABLE();
+
+ hvm_emulate_init_once(&ctxt, NULL, guest_cpu_user_regs());
+ vmx_realmode_emulate_one(&ctxt);
+ hvm_emulate_writeback(&ctxt);
+
+ return true;
+}
+
static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
.name = "VMX",
.cpu_up_prepare = vmx_cpu_up_prepare,
@@ -3070,6 +3084,7 @@ const struct hvm_function_table * __init start_vmx(void)
lbr_tsx_fixup_check();
ler_to_fixup_check();
+ arch_vcpu_ioreq_completion = realmode_vcpu_ioreq_completion;
return &vmx_function_table;
}
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 1257a3d972..94fde97ece 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -33,6 +33,8 @@
#include <public/hvm/ioreq.h>
#include <public/hvm/params.h>
+bool (*arch_vcpu_ioreq_completion)(enum vio_completion completion) = NULL;
+
void ioreq_request_mapcache_invalidate(const struct domain *d)
{
struct vcpu *v = current;
@@ -244,7 +246,8 @@ bool vcpu_ioreq_handle_completion(struct vcpu *v)
break;
default:
- res = arch_vcpu_ioreq_completion(completion);
+ if ( arch_vcpu_ioreq_completion )
+ res = arch_vcpu_ioreq_completion(completion);
break;
}
diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h
index cd399adf17..880214ec41 100644
--- a/xen/include/xen/ioreq.h
+++ b/xen/include/xen/ioreq.h
@@ -111,7 +111,7 @@ void ioreq_domain_init(struct domain *d);
int ioreq_server_dm_op(struct xen_dm_op *op, struct domain *d, bool *const_op);
bool arch_ioreq_complete_mmio(void);
-bool arch_vcpu_ioreq_completion(enum vio_completion completion);
+extern bool (*arch_vcpu_ioreq_completion)(enum vio_completion completion);
int arch_ioreq_server_map_pages(struct ioreq_server *s);
void arch_ioreq_server_unmap_pages(struct ioreq_server *s);
void arch_ioreq_server_enable(struct ioreq_server *s);
--
2.25.1
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX
2024-05-31 8:05 ` Sergiy Kibrik
@ 2024-05-31 8:31 ` Jan Beulich
0 siblings, 0 replies; 62+ messages in thread
From: Jan Beulich @ 2024-05-31 8:31 UTC (permalink / raw
To: Sergiy Kibrik
Cc: Andrew Cooper, Roger Pau Monné, George Dunlap, Julien Grall,
Stefano Stabellini, Xenia Ragiadakou, xen-devel
On 31.05.2024 10:05, Sergiy Kibrik wrote:
> 16.05.24 15:11, Jan Beulich:
>> On 15.05.2024 11:24, Sergiy Kibrik wrote:
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -2667,7 +2667,9 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt,
>>> break;
>>>
>>> case VIO_mmio_completion:
>>> +#ifdef CONFIG_VMX
>>> case VIO_realmode_completion:
>>> +#endif
>>> BUILD_BUG_ON(sizeof(hvio->mmio_insn) < sizeof(hvmemul_ctxt->insn_buf));
>>> hvio->mmio_insn_bytes = hvmemul_ctxt->insn_buf_bytes;
>>> memcpy(hvio->mmio_insn, hvmemul_ctxt->insn_buf, hvio->mmio_insn_bytes);
>>
>> This change doesn't buy us anything, does it?
>
> why not? Code won't compile w/o it.
> Or do you mean hiding the whole VIO_realmode_completion enum under
> CONFIG_VMX wasn't really useful?
That's what I meant, by implication. To me it's extra #ifdef-ary without
real gain.
Jan
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm}
2024-05-16 11:23 ` Jan Beulich
@ 2024-06-03 9:02 ` Sergiy Kibrik
0 siblings, 0 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-06-03 9:02 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
16.05.24 14:23, Jan Beulich:
> On 15.05.2024 11:14, Sergiy Kibrik wrote:
>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>> @@ -290,7 +290,7 @@ static int cf_check amd_vpmu_save(struct vcpu *v, bool to_guest)
>> context_save(v);
>>
>> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
>> - is_msr_bitmap_on(vpmu) )
>> + is_msr_bitmap_on(vpmu) && cpu_has_svm )
>> amd_vpmu_unset_msr_bitmap(v);
>
> Assuming the change in the earlier patch can actually be established to be
> safe, along the lines of an earlier comment from Stefano the addition may
> want to move earlier in the overall conditionals (here and below). In fact
> I wonder whether it wouldn't be neater to have
>
> #define is_svm_vcpu(v) (cpu_has_svm && is_hvm_vcpu(v))
>
> at the top of the file, and then use that throughout to replace is_hvm_vcpu().
> Same on the Intel side then, obviously.
>
sure, will do
>> @@ -288,7 +288,7 @@ static int cf_check core2_vpmu_save(struct vcpu *v, bool to_guest)
>>
>> /* Unset PMU MSR bitmap to trap lazy load. */
>> if ( !vpmu_is_set(vpmu, VPMU_RUNNING) && is_hvm_vcpu(v) &&
>> - cpu_has_vmx_msr_bitmap )
>> + cpu_has_vmx && cpu_has_vmx_msr_bitmap )
>
> Aren't you elsewhere adding IS_ENABLED() to cpu_has_vmx_*, rendering this (and
> similar changes further down) redundant?
>
indeed, I can move this change later in the series & reuse checks
provided by cpu_has_vmx_msr_bitmap
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx
2024-05-16 12:15 ` Jan Beulich
@ 2024-06-03 9:34 ` Sergiy Kibrik
0 siblings, 0 replies; 62+ messages in thread
From: Sergiy Kibrik @ 2024-06-03 9:34 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Stefano Stabellini,
Xenia Ragiadakou, xen-devel
16.05.24 15:15, Jan Beulich:
> On 15.05.2024 11:26, Sergiy Kibrik wrote:
>> VMX posted interrupts support can now be excluded from x86 build along with
>> VMX code itself, but still we may want to keep the possibility to use
>> VT-d IOMMU driver in non-HVM setups.
>> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
>
> But both function already have a stub each. Isn't is merely a matter of
> changing when those stubs come into play?
>
indeed, we've got stubs already provided. Then this becomes a nice one
line change.
-Sergiy
^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2024-06-03 9:35 UTC | newest]
Thread overview: 62+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-15 8:57 [XEN PATCH v2 00/15] x86: make cpu virtualization support configurable Sergiy Kibrik
2024-05-15 8:59 ` [XEN PATCH v2 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options Sergiy Kibrik
2024-05-16 10:09 ` Jan Beulich
2024-05-15 9:01 ` [XEN PATCH v2 02/15] x86/monitor: guard altp2m usage Sergiy Kibrik
2024-05-16 0:29 ` Stefano Stabellini
2024-05-16 10:20 ` Jan Beulich
2024-05-15 9:03 ` [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines Sergiy Kibrik
2024-05-16 0:31 ` Stefano Stabellini
2024-05-16 10:26 ` Jan Beulich
2024-05-15 9:06 ` [XEN PATCH v2 04/15] x86/p2m: move altp2m-related code to separate file Sergiy Kibrik
2024-05-16 0:35 ` Stefano Stabellini
2024-05-16 11:46 ` Jan Beulich
2024-05-16 10:37 ` Jan Beulich
2024-05-15 9:08 ` [XEN PATCH v2 05/15] x86: introduce CONFIG_ALTP2M Kconfig option Sergiy Kibrik
2024-05-16 0:33 ` Stefano Stabellini
2024-05-16 10:41 ` Jan Beulich
2024-05-18 1:07 ` Tamas K Lengyel
2024-05-15 9:10 ` [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 10:55 ` Jan Beulich
2024-05-16 11:01 ` Jan Beulich
2024-05-23 10:44 ` Sergiy Kibrik
2024-05-23 14:43 ` Jan Beulich
2024-05-15 9:12 ` [XEN PATCH v2 07/15] x86: guard cpu_has_{svm/vmx} macros with CONFIG_{SVM/VMX} Sergiy Kibrik
2024-05-16 0:38 ` Stefano Stabellini
2024-05-16 11:12 ` Jan Beulich
2024-05-23 13:07 ` Sergiy Kibrik
2024-05-23 14:50 ` Jan Beulich
2024-05-23 23:36 ` Stefano Stabellini
2024-05-24 6:49 ` Jan Beulich
2024-05-27 10:27 ` Sergiy Kibrik
2024-05-27 10:49 ` Jan Beulich
2024-05-15 9:14 ` [XEN PATCH v2 08/15] x86/vpmu: guard vmx/svm calls with cpu_has_{vmx,svm} Sergiy Kibrik
2024-05-16 0:44 ` Stefano Stabellini
2024-05-16 7:25 ` Jan Beulich
2024-05-16 11:23 ` Jan Beulich
2024-06-03 9:02 ` Sergiy Kibrik
2024-05-15 9:16 ` [XEN PATCH v2 09/15] x86/traps: clean up superfluous #idef-s Sergiy Kibrik
2024-05-16 0:45 ` Stefano Stabellini
2024-05-15 9:18 ` [XEN PATCH v2 10/15] x86/domain: " Sergiy Kibrik
2024-05-16 0:46 ` Stefano Stabellini
2024-05-15 9:20 ` [XEN PATCH v2 11/15] x86/oprofile: guard svm specific symbols with CONFIG_SVM Sergiy Kibrik
2024-05-15 9:22 ` [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code Sergiy Kibrik
2024-05-16 0:50 ` Stefano Stabellini
2024-05-16 7:32 ` Jan Beulich
2024-05-29 10:58 ` Sergiy Kibrik
2024-05-29 12:16 ` Jan Beulich
2024-05-15 9:24 ` [XEN PATCH v2 13/15] x86/ioreq: guard VIO_realmode_completion with CONFIG_VMX Sergiy Kibrik
2024-05-16 0:51 ` Stefano Stabellini
2024-05-16 12:11 ` Jan Beulich
2024-05-31 8:05 ` Sergiy Kibrik
2024-05-31 8:31 ` Jan Beulich
2024-05-31 8:06 ` [XEN PATCH RFC] ioreq: make arch_vcpu_ioreq_completion() an optional callback Sergiy Kibrik
2024-05-15 9:26 ` [XEN PATCH v2 14/15] iommu/vt-d: guard vmx_pi_hooks_* calls with cpu_has_vmx Sergiy Kibrik
2024-05-16 0:54 ` Stefano Stabellini
2024-05-16 7:37 ` Jan Beulich
2024-05-16 23:41 ` Stefano Stabellini
2024-05-16 12:15 ` Jan Beulich
2024-06-03 9:34 ` Sergiy Kibrik
2024-05-15 9:28 ` [XEN PATCH v2 15/15] x86/hvm: make AMD-V and Intel VT-x support configurable Sergiy Kibrik
2024-05-16 0:54 ` Stefano Stabellini
2024-05-16 12:17 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).