[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 7 Jun 2022 12:05:48 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=sP+J3hksyJu9gy2PUY/QvzBqSYuyjyjHyxqnlzQqoQg=; b=IBEdBmlAYfaBpEp9PBlo5nAsj/2eacy6z1vRJxGCBIyijg6LvalSodq/F82SRbbMf/UX1pjtCZS3Dh/UaQ7KIyO2Wu44uXAWwgP0fhD7BzyrCv1jdROiLA961f2JxCNG5bS78fStG6cCn0KEpzb6pEZWyS+VJdlt5QBInGVKl7Ff+mBIRDMy9jtmKVOIf83n3GOn7i36i07f74Sf5vT3iaIeyTle3TS8VZ/q5qwoCaRJpUMn6mlBRF7oYfAlSqgpEEtgEcWFFchLsKx46xcfXhvPjg8jF+GXxDcOHN7WHdQheguLzjq0UpM1zm7LxA9P/6GteiilLjdzGkeReLXyuA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=n/68THZIkbldY52Ydlp7L5jnS75pZd2qc67VXvUBvFJMG+ion+a3Adf0mECLKmRJ8cfM+ovvSpe0AlQfIl/bHZoqUevSBeEItt91K2hrZq0Me19pYeIZl31JpFK+nVSKPJ1fphL3jTHjQUqRAVipl8Gsy9N7b6gopGpSnN/UHeBb/CWtR+vY/zZ85S+/Jc+T4cQAqTtINTnRU/oYU7rEt/rUJhAebVv2cm3wiAnXZCA6y4FyH9rcejjcqNb9hwpMxa7VpLGBRt4rSaFFt4+e3t0vDVL+AFUlbZu8qUq9FoX1bwgXax8LFyohECQ9RKIDpKrzWfV1p9xMX3Ururzf4A==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 07 Jun 2022 10:06:00 +0000
  • Ironport-data: A9a23:pgBkT6uThtzclnqZ2gd8AL+pvufnVJRfMUV32f8akzHdYApBsoF/q tZmKW7UPfeOYWr0L9okOYqzo05Sv8PXzNYxSAc6/380EylA+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZhSAgk/nOHNIQMcacUsxLbVYMpBwJ1FQywobVvqYy2YLjW13V5 ouryyHiEATNNwBcYzp8B52r8HuDjNyq0N/PlgVjDRzjlAa2e0g9VPrzF4noR5fLatA88tqBb /TC1NmEElbxpH/BPD8HfoHTKSXmSpaKVeSHZ+E/t6KK2nCurQRquko32WZ1he66RFxlkvgoo Oihu6BcRi80M+7Ro8U8QSBYHiplIIFk+pj/fl+W5Jn7I03uKxMAwt1IJWRvZ8gy3LYyBmtDs /sFNDoKcxaPwfqsx662QfVtgcJlK9T3OIQYuTdryjSx4fQOGMifBfmVo4IFmm5v2qiiHt6HD yYdQSBoYxnaJQVGJ38cCY4knffujX76G9FdgA3O/fZvvzaMpOB3+OT9Od3rW8GTfNV+3UDC5 WjY4lbeHh5PYbRzzhLAqBpAnNTnnyn2RYYTH72Q7eNxjRuYwWl7IB8LUVq2p9Gph0j4XMhQQ 2QP4TYnp6U28E2tT/H+Uge+rXrCuQQTM/JPF8Uq5QfLzbDbiy6aC3YFSHhdadUgnM4wWTEuk FSOmrvBByFp9rucSnuf97KdhTK0JSURa2QFYEcsXQYDptXuvow3phbOVcp4Vr64iMXvHjP9y CzMqzIx74j/luYO3qS/uFrB0zSlo8GTShZvv1qLGGW48gl+eYipIZSy7kTW5upBK4DfSUSdu H8DmI6V6+Vm4YyxqRFhid4lRNmBj8tp+hWB6bKzN/HNLwiQxkM=
  • Ironport-hdrordr: A9a23:Yt5HfauqsTK5i/QQ06oGv/sc7skC/4Mji2hC6mlwRA09TyXGra 2TdaUgvyMc1gx7ZJhBo7+90We7MBbhHLpOkPEs1NCZLXLbUQqTXfhfBO7ZrwEIdBefygcw79 YCT0E6MqyLMbEYt7eE3ODbKadG/DDvysnB64bjJjVWPGdXgslbnntE422gYylLrWd9dPgE/M 323Ls7m9PsQwVeUiz9bUN1LNTrlpnurtbLcBQGDxko5E2nii6p0qfzF1y90g0FWz1C7L8++S yd+jaJrJmLgrWe8FvxxmXT55NZlJ/IzcZCPtWFjowwJi/3ggilSYx9U/mpvSwzosuo9FE2+e O86CsIDoBW0Tf8b2u1qRzi103J1ysv0WbrzRuijX7qsaXCNUUHIvsEobgcXgrS6kImst05+r lMxXilu51eCg6FtDjh5vDTPisa2HackD4Hq6o+nnZfWYwRZPt6tooE5n5YF58GAWbT9J0nKu 9zF8vRjcwmPm9yV0qp/lWH/ebcHUjaRny9Mwo/U42uonRrdUlCvgolLJd1pAZEyHo/I6M0k9 gsfJ4Y0I2mdfVmHJ6VNN1xP/dfNVa9MS4kEFjiV2gPR5t3ck4klfbMkccIzdDvXqA0570Pv7 mEeG9klAcJCjfT4Iu1rdB2ziw=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Jun 07, 2022 at 09:43:25AM +0200, Jan Beulich wrote:
> On 03.06.2022 16:46, Roger Pau Monné wrote:
> > On Fri, Jun 03, 2022 at 02:49:54PM +0200, Jan Beulich wrote:
> >> On 26.05.2022 13:11, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/hvm/vmx/vmx.c
> >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >>> @@ -1419,10 +1419,19 @@ static void cf_check vmx_update_host_cr3(struct 
> >>> vcpu *v)
> >>>  
> >>>  void vmx_update_debug_state(struct vcpu *v)
> >>>  {
> >>> +    unsigned int mask = 1u << TRAP_int3;
> >>> +
> >>> +    if ( !cpu_has_monitor_trap_flag && cpu_has_vmx_notify_vm_exiting )
> >>
> >> I'm puzzled by the lack of symmetry between this and ...
> >>
> >>> +        /*
> >>> +         * Only allow toggling TRAP_debug if notify VM exit is enabled, 
> >>> as
> >>> +         * unconditionally setting TRAP_debug is part of the XSA-156 fix.
> >>> +         */
> >>> +        mask |= 1u << TRAP_debug;
> >>> +
> >>>      if ( v->arch.hvm.debug_state_latch )
> >>> -        v->arch.hvm.vmx.exception_bitmap |= 1U << TRAP_int3;
> >>> +        v->arch.hvm.vmx.exception_bitmap |= mask;
> >>>      else
> >>> -        v->arch.hvm.vmx.exception_bitmap &= ~(1U << TRAP_int3);
> >>> +        v->arch.hvm.vmx.exception_bitmap &= ~mask;
> >>>  
> >>>      vmx_vmcs_enter(v);
> >>>      vmx_update_exception_bitmap(v);
> >>> @@ -4155,6 +4164,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> >>>          switch ( vector )
> >>>          {
> >>>          case TRAP_debug:
> >>> +            if ( cpu_has_monitor_trap_flag && 
> >>> cpu_has_vmx_notify_vm_exiting )
> >>> +                goto exit_and_crash;
> >>
> >> ... this condition. Shouldn't one be the inverse of the other (and
> >> then it's the one down here which wants adjusting)?
> > 
> > The condition in vmx_update_debug_state() sets the mask so that
> > TRAP_debug will only be added or removed from the bitmap if
> > !cpu_has_monitor_trap_flag && cpu_has_vmx_notify_vm_exiting (note that
> > otherwise TRAP_debug is unconditionally set if
> > !cpu_has_vmx_notify_vm_exiting).
> > 
> > Hence it's impossible to get a VMExit TRAP_debug with
> > cpu_has_monitor_trap_flag && cpu_has_vmx_notify_vm_exiting because
> > TRAP_debug will never be set by vmx_update_debug_state() in that
> > case.
> 
> Hmm, yes, I've been misguided by you not altering the existing setting
> of v->arch.hvm.vmx.exception_bitmap in construct_vmcs(). Instead you
> add an entirely new block of code near the bottom of the function. Is
> there any chance you could move up that adjustment, perhaps along the
> lines of
> 
>     v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
>               | (paging_mode_hap(d) ? 0 : (1U << TRAP_page_fault))
>               | (v->arch.fully_eager_fpu ? 0 : (1U << TRAP_no_device));
>     if ( cpu_has_vmx_notify_vm_exiting )
>     {
>         __vmwrite(NOTIFY_WINDOW, vm_notify_window);
>         /*
>          * Disable #AC and #DB interception: by using VM Notify Xen is
>          * guaranteed to get a VM exit even if the guest manages to lock the
>          * CPU.
>          */
>         v->arch.hvm.vmx.exception_bitmap &= ~((1U << TRAP_debug) |
>                                               (1U << TRAP_alignment_check));
>     }
>     vmx_update_exception_bitmap(v);

Sure, will move up when posting a new version then.  I will wait for
feedback from Jun or Kevin regarding the default window size before
doing so.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.