On 03.11.25 11:47, Teddy Astie wrote:
Le 31/10/2025 à 22:24, Grygorii Strashko a écrit :From: Grygorii Strashko <[email protected]> The VMTRACE feature depends on Platform/Arch HW and code support and now can be used only on x86 HVM with Intel VT-x (INTEL_VMX) enabled. This makes VMTRACE support optional by introducing two Kconfig options: - CONFIG_HAS_VMTRACE which allows Platform/Arch to advertise VMTRACE support readiness - CONFIG_VMTRACE to enable/disable the feature.I like the idea of making it optional since it's only used in specific contexts.Signed-off-by: Grygorii Strashko <[email protected]> --- Based on top of patch "domain: adjust soft-reset arch dependency" [1] [1] https://patchwork.kernel.org/project/xen-devel/patch/[email protected]/ xen/Kconfig.debug | 15 +++++++++++++++ xen/arch/x86/domctl.c | 4 ++++ xen/arch/x86/hvm/Kconfig | 1 + xen/arch/x86/hvm/vmx/vmcs.c | 2 ++ xen/arch/x86/hvm/vmx/vmx.c | 10 ++++++++++ xen/arch/x86/include/asm/guest-msr.h | 2 ++ xen/arch/x86/include/asm/hvm/hvm.h | 9 +++++++++ xen/arch/x86/include/asm/hvm/vmx/vmcs.h | 2 ++ xen/arch/x86/mm/mem_sharing.c | 2 ++ xen/arch/x86/vm_event.c | 6 ++++-- xen/common/domain.c | 10 ++++++++++ xen/common/memory.c | 6 ++++++ xen/common/vm_event.c | 3 ++- xen/include/xen/domain.h | 4 ++++ xen/include/xen/sched.h | 4 ++++ 15 files changed, 77 insertions(+), 3 deletions(-) diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug index d900d926c555..70ec4f0d14a5 100644 --- a/xen/Kconfig.debug +++ b/xen/Kconfig.debug @@ -155,4 +155,19 @@ config DEBUG_INFO "make install-xen" for installing xen.efi, stripping needs to be done outside the Xen build environment).+config HAS_VMTRACE+ bool + +config VMTRACE + bool "HW VM tracing support" + depends on HAS_VMTRACE + default y + help + Enables HW VM tracing support which allows to configure HW processor + features (vmtrace_op) to enable capturing information about software + execution using dedicated hardware facilities with minimal interference + to the software being traced. The trace date can be retrieved using buffer + shared between Xen and domain + (see XENMEM_acquire_resource(XENMEM_resource_vmtrace_buf)). + endmenu
[...]
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.cindex fc349270b9c5..f4c8696ce54e 100644 --- a/xen/arch/x86/vm_event.c +++ b/xen/arch/x86/vm_event.c @@ -253,7 +253,9 @@ void vm_event_fill_regs(vm_event_request_t *req) req->data.regs.x86.shadow_gs = ctxt.shadow_gs; req->data.regs.x86.dr6 = ctxt.dr6;+#ifdef CONFIG_VMTRACEif ( hvm_vmtrace_output_position(curr, &req->data.regs.x86.vmtrace_pos) != 1 ) +#endif req->data.regs.x86.vmtrace_pos = ~0;This if-def looks very oddly placed.
I assume you are asking for hvm_vmtrace_output_position() stub, Right?
#endif } @@ -303,12 +305,12 @@ void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) #endif }+#ifdef CONFIG_VMTRACEvoid vm_event_reset_vmtrace(struct vcpu *v) { -#ifdef CONFIG_HVM hvm_vmtrace_reset(v); -#endif } +#endif/** Local variables: diff --git a/xen/common/domain.c b/xen/common/domain.c index 7dcd466e5a12..2be6ee03d004 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -136,7 +136,9 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly;vcpu_info_t dummy_vcpu_info; +#ifdef CONFIG_VMTRACEbool __read_mostly vmtrace_available; +#endifbool __read_mostly vpmu_is_available; @@ -318,6 +320,7 @@ static void vcpu_info_reset(struct vcpu *v) static void vmtrace_free_buffer(struct vcpu *v){ +#ifdef CONFIG_VMTRACE const struct domain *d = v->domain; struct page_info *pg = v->vmtrace.pg; unsigned int i; @@ -332,10 +335,12 @@ static void vmtrace_free_buffer(struct vcpu *v) put_page_alloc_ref(&pg[i]); put_page_and_type(&pg[i]); } +#endif }static int vmtrace_alloc_buffer(struct vcpu *v){ +#ifdef CONFIG_VMTRACE struct domain *d = v->domain; struct page_info *pg; unsigned int i; @@ -377,6 +382,9 @@ static int vmtrace_alloc_buffer(struct vcpu *v) }return -ENODATA;+#else + return 0; +#endif }/*@@ -825,7 +833,9 @@ struct domain *domain_create(domid_t domid, ASSERT(!config->altp2m.nr); #endif+#ifdef CONFIG_VMTRACEd->vmtrace_size = config->vmtrace_size; +#endif }/* Sort out our idea of is_control_domain(). */diff --git a/xen/common/memory.c b/xen/common/memory.c index 3688e6dd5032..66dc7f7a0a41 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -1155,8 +1155,10 @@ static unsigned int resource_max_frames(const struct domain *d, case XENMEM_resource_ioreq_server: return ioreq_server_max_frames(d);+#ifdef CONFIG_VMTRACEcase XENMEM_resource_vmtrace_buf: return d->vmtrace_size >> PAGE_SHIFT; +#endifdefault:return 0; @@ -1198,6 +1200,7 @@ static int acquire_ioreq_server(struct domain *d, #endif }+#ifdef CONFIG_VMTRACEstatic int acquire_vmtrace_buf( struct domain *d, unsigned int id, unsigned int frame, unsigned int nr_frames, xen_pfn_t mfn_list[]) @@ -1220,6 +1223,7 @@ static int acquire_vmtrace_buf(return nr_frames;} +#endifGiven that vmtrace feature is fairly isolated in VMX code, wouldn't it be better to move all vmtrace related functions (including vmx_init_ipt) in a separate vmtrace.c file and make such file gated behind CONFIG_VMTRACE ?
Are you thinking about: xen/common/ |- vmtrace.c xen/arch/x86/hvm/vmx/ |- vmtrace.c ? or smth else? [...] -- Best regards, -grygorii
