Re: [PATCH v3 6/7] xen/mem_access: wrap memory access when VM_EVENT=n

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <jbeulich@xxxxxxxx>
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.

Jan