[Public] > -----Original Message----- > From: Jan Beulich <[email protected]> > Sent: Monday, November 24, 2025 10:10 PM > To: Penny, Zheng <[email protected]> > Cc: Huang, Ray <[email protected]>; [email protected]; Andrew > Cooper <[email protected]>; Roger Pau Monné <[email protected]>; > Tamas K Lengyel <[email protected]>; Alexandru Isaila > <[email protected]>; Petre Pircalabu <[email protected]>; xen- > [email protected] > Subject: Re: [PATCH v3 6/7] xen/mem_access: wrap memory access when > VM_EVENT=n > > On 21.11.2025 10:15, Penny Zheng wrote: > > @@ -2080,7 +2081,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, > > unsigned long gla, #endif > > } > > > > - if ( req_ptr ) > > + /* > > + * Excessive condition is to avoid runtime undefined error only > > + * when CONFIG_USBAN=y > > + */ > > + if ( req_ptr && vm_event_is_enabled(curr) ) > > { > > I fear the comment isn't really helpful this way. What's "excessive" here may > be > clear from patch context, but it won't be clear when looking at the code > later. Nor > would it then be immediately clear why the vm_event_is_enabled() check is > (seemingly) unnecessary. How about this: > > "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." > > With this or any other suitable improvement to the comment: > Acked-by: Jan Beulich <[email protected]> If we want to go with the suggestion > above, I'd be happy to do the replacement while committing. But of course > first the > necessary 2nd ack will want collecting.
Thx! > > Jan
