Re: [PATCH 2/2] x86/vmx: implement Notify VM Exit

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.05.2022 15:21, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -67,6 +67,9 @@ integer_param("ple_gap", ple_gap);
>  static unsigned int __read_mostly ple_window = 4096;
>  integer_param("ple_window", ple_window);
>  
> +static int __read_mostly vm_notify_window;

__ro_after_init?

> @@ -1333,6 +1338,19 @@ static int construct_vmcs(struct vcpu *v)
>          rc = vmx_add_msr(v, MSR_FLUSH_CMD, FLUSH_CMD_L1D,
>                           VMX_MSR_GUEST_LOADONLY);
>  
> +    if ( cpu_has_vmx_notify_vm_exiting && vm_notify_window >= 0 )

The assumption then is that values >= 2^^31 are nonsense? Generally
I'd think we want to special case merely ~0u, giving the variable
unsigned type. However, I also don't see where you disable use of
the feature in that case: Merely skipping the VMCS update here isn't
enough, is it? The field itself doesn't know any special case
values (like ~0) as per the doc I'm looking at. So I guess the OR-ing
in of SECONDARY_EXEC_NOTIFY_VM_EXITING in vmx_init_vmcs_config()
wants to be conditional.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -4567,6 +4567,30 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>           */
>          break;
>  
> +    case EXIT_REASON_NOTIFY:
> +        __vmread(EXIT_QUALIFICATION, &exit_qualification);
> +
> +        if ( exit_qualification & NOTIFY_VM_CONTEXT_INVALID )
> +        {
> +            perfc_incr(vmnotify_crash);

Is this a useful event to count? We don't count other crash causes,
iirc.

Jan