[Public] Hi, Tamas
May I ask a review on this commit? Many thanks, Penny Zheng > -----Original Message----- > From: Penny, Zheng <[email protected]> > Sent: Thursday, January 15, 2026 5:29 PM > To: [email protected]; Andryuk, Jason <[email protected]> > Cc: Huang, Ray <[email protected]>; Penny, Zheng > <[email protected]>; Jan Beulich <[email protected]>; Andrew Cooper > <[email protected]>; Roger Pau Monné <[email protected]>; Tamas > K Lengyel <[email protected]>; Alexandru Isaila <[email protected]>; > Petre Pircalabu <[email protected]> > Subject: [PATCH v4 5/6] xen/mem_access: wrap memory access when > VM_EVENT=n > > Feature memory access is based on vm event subsystem, and it could be disabled > in the future. So a few switch-blocks in do_altp2m_op() need > vm_event_is_enabled() condition check to pass compilation when ALTP2M=y and > VM_EVENT=n(, hence MEM_ACCESS=n), like > HVMOP_altp2m_set_mem_access, etc. > Function p2m_mem_access_check() still needs stub when VM_EVENT=n to pass > compilation. > Although local variable "req_ptr" still remains NULL throughout its lifetime, > with the > change of NULL assignment, we will face runtime undefined error only when > CONFIG_USBAN is on. So we strengthen the condition check via adding > vm_event_is_enabled() for the special case. > > Signed-off-by: Penny Zheng <[email protected]> > Acked-by: Jan Beulich <[email protected]> > --- > v1 -> v3: > - a comment next to the excessive condition > - use vm_event_is_enabled() instead > - avoid heavy churn by using the inverted condition plus break > --- > v3 - v4: > - refine comment > --- > xen/arch/x86/hvm/hvm.c | 26 +++++++++++++++++++++++++- > xen/arch/x86/include/asm/mem_access.h | 10 ++++++++++ > 2 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index > 07e54890d9..b34cd29629 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -52,6 +52,7 @@ > #include <asm/i387.h> > #include <asm/mc146818rtc.h> > #include <asm/mce.h> > +#include <asm/mem_access.h> > #include <asm/monitor.h> > #include <asm/msr.h> > #include <asm/mtrr.h> > @@ -2082,7 +2083,12 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned > long gla, #endif > } > > - if ( req_ptr ) > + /* > + * req_ptr being constant NULL when !CONFIG_VM_EVENT, > CONFIG_UBSAN=y > + * builds have been observed to still hit undefined-ness at runtime. > + * Hence do a seemingly redundant vm_event_is_enabled() check here. > + */ > + if ( req_ptr && vm_event_is_enabled(curr) ) > { > if ( monitor_traps(curr, sync, req_ptr) < 0 ) > rc = 0; > @@ -4804,6 +4810,12 @@ static int do_altp2m_op( > break; > > case HVMOP_altp2m_set_mem_access: > + if ( !vm_event_is_enabled(current) ) > + { > + rc = -EOPNOTSUPP; > + break; > + } > + > if ( a.u.mem_access.pad ) > rc = -EINVAL; > else > @@ -4813,6 +4825,12 @@ static int do_altp2m_op( > break; > > case HVMOP_altp2m_set_mem_access_multi: > + if ( !vm_event_is_enabled(current) ) > + { > + rc = -EOPNOTSUPP; > + break; > + } > + > if ( a.u.set_mem_access_multi.pad || > a.u.set_mem_access_multi.opaque > a.u.set_mem_access_multi.nr ) > { > @@ -4844,6 +4862,12 @@ static int do_altp2m_op( > break; > > case HVMOP_altp2m_get_mem_access: > + if ( !vm_event_is_enabled(current) ) > + { > + rc = -EOPNOTSUPP; > + break; > + } > + > if ( a.u.mem_access.pad ) > rc = -EINVAL; > else > diff --git a/xen/arch/x86/include/asm/mem_access.h > b/xen/arch/x86/include/asm/mem_access.h > index 257ed33de1..790bed81e8 100644 > --- a/xen/arch/x86/include/asm/mem_access.h > +++ b/xen/arch/x86/include/asm/mem_access.h > @@ -14,6 +14,7 @@ > #ifndef __ASM_X86_MEM_ACCESS_H__ > #define __ASM_X86_MEM_ACCESS_H__ > > +#ifdef CONFIG_VM_EVENT > /* > * Setup vm_event request based on the access (gla is -1ull if not > available). > * Handles the rw2rx conversion. Boolean return value indicates if event > type @@ - > 25,6 +26,15 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla, > struct npfec npfec, > struct vm_event_st **req_ptr); > +#else > +static inline bool p2m_mem_access_check(paddr_t gpa, unsigned long gla, > + struct npfec npfec, > + struct vm_event_st **req_ptr) { > + *req_ptr = NULL; > + return false; > +} > +#endif /* CONFIG_VM_EVENT */ > > /* Check for emulation and mark vcpu for skipping one instruction > * upon rescheduling if required. */ > -- > 2.34.1
