* [PATCH v2 0/2] vmx/hap: optimize CR4 trapping @ 2018-02-16 12:10 Roger Pau Monne 2018-02-16 12:10 ` [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses Roger Pau Monne 2018-02-16 12:10 ` [PATCH v2 2/2] vmx/hap: optimize CR4 trapping Roger Pau Monne 0 siblings, 2 replies; 10+ messages in thread From: Roger Pau Monne @ 2018-02-16 12:10 UTC (permalink / raw) To: xen-devel; +Cc: Roger Pau Monne Hello, Following two-patch series optimize CR4 access for vmx/hap. I couldn't figure out a similar approach for AMD, since according to section 15.9 (Instruction Intercepts), you either intercept all access to CR4 or none. In any case a similar optimization for AMD can always be added later. Thanks, Roger. Roger Pau Monne (2): x86/hvm: introduce cr_mask to store trapped bits of CR accesses vmx/hap: optimize CR4 trapping xen/arch/x86/hvm/svm/vmcb.c | 1 + xen/arch/x86/hvm/vmx/vmcs.c | 1 + xen/arch/x86/hvm/vmx/vmx.c | 39 +++++++++++++++++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vvmx.c | 2 ++ xen/arch/x86/monitor.c | 5 +++-- xen/include/asm-x86/hvm/vcpu.h | 3 +++ 6 files changed, 49 insertions(+), 2 deletions(-) -- 2.16.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses 2018-02-16 12:10 [PATCH v2 0/2] vmx/hap: optimize CR4 trapping Roger Pau Monne @ 2018-02-16 12:10 ` Roger Pau Monne 2018-02-16 12:49 ` Andrew Cooper 2018-02-16 12:10 ` [PATCH v2 2/2] vmx/hap: optimize CR4 trapping Roger Pau Monne 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monne @ 2018-02-16 12:10 UTC (permalink / raw) To: xen-devel Cc: Kevin Tian, Jan Beulich, Andrew Cooper, Jun Nakajima, Boris Ostrovsky, Suravee Suthikulpanit, Roger Pau Monne At the moment this is currently set at VMCS creation and not changed, but further patches are going to change the CR4 mask at runtime. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Jun Nakajima <jun.nakajima@intel.com> Cc: Kevin Tian <kevin.tian@intel.com> --- Changes since v1: - New in this version. --- xen/arch/x86/hvm/svm/vmcb.c | 1 + xen/arch/x86/hvm/vmx/vmcs.c | 1 + xen/include/asm-x86/hvm/vcpu.h | 3 +++ 3 files changed, 5 insertions(+) diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c index 0e6cba5b7b..fdb796ade1 100644 --- a/xen/arch/x86/hvm/svm/vmcb.c +++ b/xen/arch/x86/hvm/svm/vmcb.c @@ -169,6 +169,7 @@ static int construct_vmcb(struct vcpu *v) vmcb->tr.base = 0; vmcb->tr.limit = 0xff; + v->arch.hvm_vcpu.mask_cr[0] = v->arch.hvm_vcpu.mask_cr[4] = ~0UL; v->arch.hvm_vcpu.guest_cr[0] = X86_CR0_PE | X86_CR0_ET; hvm_update_guest_cr(v, 0); diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index e7818caed0..e3328742e0 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -1165,6 +1165,7 @@ static int construct_vmcs(struct vcpu *v) __vmwrite(CR0_GUEST_HOST_MASK, ~0UL); __vmwrite(CR4_GUEST_HOST_MASK, ~0UL); + v->arch.hvm_vcpu.mask_cr[0] = v->arch.hvm_vcpu.mask_cr[4] = ~0UL; __vmwrite(PAGE_FAULT_ERROR_CODE_MASK, 0); __vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, 0); diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h index d93166fb92..811d4c10ae 100644 --- a/xen/include/asm-x86/hvm/vcpu.h +++ b/xen/include/asm-x86/hvm/vcpu.h @@ -156,6 +156,9 @@ struct hvm_vcpu { */ unsigned long hw_cr[5]; + /* Cached copy of the trapped bits of CRs. Used for CR0 and CR4. */ + unsigned long mask_cr[5]; + struct vlapic vlapic; s64 cache_tsc_offset; u64 guest_time; -- 2.16.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses 2018-02-16 12:10 ` [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses Roger Pau Monne @ 2018-02-16 12:49 ` Andrew Cooper 2018-02-19 15:04 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Andrew Cooper @ 2018-02-16 12:49 UTC (permalink / raw) To: Roger Pau Monne, xen-devel Cc: Jun Nakajima, Kevin Tian, Boris Ostrovsky, Jan Beulich, Suravee Suthikulpanit On 16/02/18 12:10, Roger Pau Monne wrote: > diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h > index d93166fb92..811d4c10ae 100644 > --- a/xen/include/asm-x86/hvm/vcpu.h > +++ b/xen/include/asm-x86/hvm/vcpu.h > @@ -156,6 +156,9 @@ struct hvm_vcpu { > */ > unsigned long hw_cr[5]; > > + /* Cached copy of the trapped bits of CRs. Used for CR0 and CR4. */ > + unsigned long mask_cr[5]; We only need masks for cr0 and cr4, and I don't expect this to change for the foreseeable future (CR4 still has plenty of available bits in it). I'd recommend: unsigned long cr0_host_mask, cr4_host_mask; ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses 2018-02-16 12:49 ` Andrew Cooper @ 2018-02-19 15:04 ` Roger Pau Monné 2018-02-19 15:07 ` Andrew Cooper 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2018-02-19 15:04 UTC (permalink / raw) To: Andrew Cooper Cc: Kevin Tian, Jan Beulich, Jun Nakajima, xen-devel, Boris Ostrovsky, Suravee Suthikulpanit On Fri, Feb 16, 2018 at 12:49:57PM +0000, Andrew Cooper wrote: > On 16/02/18 12:10, Roger Pau Monne wrote: > > diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h > > index d93166fb92..811d4c10ae 100644 > > --- a/xen/include/asm-x86/hvm/vcpu.h > > +++ b/xen/include/asm-x86/hvm/vcpu.h > > @@ -156,6 +156,9 @@ struct hvm_vcpu { > > */ > > unsigned long hw_cr[5]; > > > > + /* Cached copy of the trapped bits of CRs. Used for CR0 and CR4. */ > > + unsigned long mask_cr[5]; > > We only need masks for cr0 and cr4, and I don't expect this to change > for the foreseeable future (CR4 still has plenty of available bits in > it). I'd recommend: > > unsigned long cr0_host_mask, cr4_host_mask; OK, I've used an array to keep consistency with the other CR array fields, but I guess here it makes less sense because only two of them are going to be used. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses 2018-02-19 15:04 ` Roger Pau Monné @ 2018-02-19 15:07 ` Andrew Cooper 0 siblings, 0 replies; 10+ messages in thread From: Andrew Cooper @ 2018-02-19 15:07 UTC (permalink / raw) To: Roger Pau Monné Cc: Kevin Tian, Jan Beulich, Jun Nakajima, xen-devel, Boris Ostrovsky, Suravee Suthikulpanit On 19/02/18 15:04, Roger Pau Monné wrote: > On Fri, Feb 16, 2018 at 12:49:57PM +0000, Andrew Cooper wrote: >> On 16/02/18 12:10, Roger Pau Monne wrote: >>> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h >>> index d93166fb92..811d4c10ae 100644 >>> --- a/xen/include/asm-x86/hvm/vcpu.h >>> +++ b/xen/include/asm-x86/hvm/vcpu.h >>> @@ -156,6 +156,9 @@ struct hvm_vcpu { >>> */ >>> unsigned long hw_cr[5]; >>> >>> + /* Cached copy of the trapped bits of CRs. Used for CR0 and CR4. */ >>> + unsigned long mask_cr[5]; >> We only need masks for cr0 and cr4, and I don't expect this to change >> for the foreseeable future (CR4 still has plenty of available bits in >> it). I'd recommend: >> >> unsigned long cr0_host_mask, cr4_host_mask; > OK, I've used an array to keep consistency with the other CR array > fields, but I guess here it makes less sense because only two of them > are going to be used. I'll be dropping the arrays in due course anyway. Some of the value aren't used at all, and it will simplify code which is common between PV and HVM. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] vmx/hap: optimize CR4 trapping 2018-02-16 12:10 [PATCH v2 0/2] vmx/hap: optimize CR4 trapping Roger Pau Monne 2018-02-16 12:10 ` [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses Roger Pau Monne @ 2018-02-16 12:10 ` Roger Pau Monne 2018-02-16 12:30 ` Razvan Cojocaru 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monne @ 2018-02-16 12:10 UTC (permalink / raw) To: xen-devel Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Razvan Cojocaru, Andrew Cooper, Jan Beulich, Roger Pau Monne There a bunch of bits in CR4 that should be allowed to be set directly by the guest without requiring Xen intervention, currently this is already done by passing through guest writes into the CR4 used when running in non-root mode, but taking an expensive vmexit in order to do so. xenalyze reports the following when running a PV guest in shim mode: CR_ACCESS 3885950 6.41s 17.04% 3957 cyc { 2361| 3378| 7920} cr4 3885940 6.41s 17.04% 3957 cyc { 2361| 3378| 7920} cr3 1 0.00s 0.00% 3480 cyc { 3480| 3480| 3480} *[ 0] 1 0.00s 0.00% 3480 cyc { 3480| 3480| 3480} cr0 7 0.00s 0.00% 7112 cyc { 3248| 5960|17480} clts 2 0.00s 0.00% 4588 cyc { 3456| 5720| 5720} After this change this turns into: CR_ACCESS 12 0.00s 0.00% 9972 cyc { 3680|11024|24032} cr4 2 0.00s 0.00% 17528 cyc {11024|24032|24032} cr3 1 0.00s 0.00% 3680 cyc { 3680| 3680| 3680} *[ 0] 1 0.00s 0.00% 3680 cyc { 3680| 3680| 3680} cr0 7 0.00s 0.00% 9209 cyc { 4184| 7848|17488} clts 2 0.00s 0.00% 8232 cyc { 5352|11112|11112} Note that this optimized trapping is currently only applied to guests running with HAP on Intel hardware. If using shadow paging more CR4 bits need to be unconditionally trapped, which makes this approach unlikely to yield any important performance improvements. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jun Nakajima <jun.nakajima@intel.com> Cc: Kevin Tian <kevin.tian@intel.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com> Cc: Tamas K Lengyel <tamas@tklengyel.com> --- Changes since v1: - Use the mask_cr variable in order to cache the cr4 mask. - Take into account write_ctrlreg_mask when introspection is enabled. --- xen/arch/x86/hvm/vmx/vmx.c | 39 +++++++++++++++++++++++++++++++++++++++ xen/arch/x86/hvm/vmx/vvmx.c | 2 ++ xen/arch/x86/monitor.c | 5 +++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index d35cf55982..108f251bb9 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1684,6 +1684,36 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr) } __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]); + + if ( !paging_mode_hap(v->domain) ) + /* + * Shadow path has not been optimized because it requires + * unconditionally trapping more CR4 bits, at which point the + * performance benefit of doing this is quite dubious. + */ + v->arch.hvm_vcpu.mask_cr[4] = ~0UL; + else + { + /* + * Update CR4 host mask to only trap when the guest tries to set + * bits that are controlled by the hypervisor. + */ + v->arch.hvm_vcpu.mask_cr[4] = HVM_CR4_HOST_MASK | X86_CR4_PKE | + ~hvm_cr4_guest_valid_bits(v, 0); + v->arch.hvm_vcpu.mask_cr[4] |= v->arch.hvm_vmx.vmx_realmode ? + X86_CR4_VME : 0; + v->arch.hvm_vcpu.mask_cr[4] |= !hvm_paging_enabled(v) ? + (X86_CR4_PSE | X86_CR4_SMEP | + X86_CR4_SMAP) + : 0; + if ( v->domain->arch.monitor.write_ctrlreg_enabled & + monitor_ctrlreg_bitmask(VM_EVENT_X86_CR4) ) + v->arch.hvm_vcpu.mask_cr[4] |= + ~v->domain->arch.monitor.write_ctrlreg_mask[4]; + + } + __vmwrite(CR4_GUEST_HOST_MASK, v->arch.hvm_vcpu.mask_cr[4]); + break; case 2: @@ -3512,6 +3542,15 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) if ( paging_mode_hap(v->domain) ) { + /* + * Xen allows the guest to modify some CR4 bits directly, update cached + * values to match. + */ + __vmread(GUEST_CR4, &v->arch.hvm_vcpu.hw_cr[4]); + v->arch.hvm_vcpu.guest_cr[4] &= v->arch.hvm_vcpu.mask_cr[4]; + v->arch.hvm_vcpu.guest_cr[4] |= v->arch.hvm_vcpu.hw_cr[4] & + ~v->arch.hvm_vcpu.mask_cr[4]; + __vmread(GUEST_CR3, &v->arch.hvm_vcpu.hw_cr[3]); if ( vmx_unrestricted_guest(v) || hvm_paging_enabled(v) ) v->arch.hvm_vcpu.guest_cr[3] = v->arch.hvm_vcpu.hw_cr[3]; diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index dfe97b9705..54608e0011 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1100,6 +1100,8 @@ static void load_shadow_guest_state(struct vcpu *v) cr_read_shadow = (get_vvmcs(v, GUEST_CR4) & ~cr_gh_mask) | (get_vvmcs(v, CR4_READ_SHADOW) & cr_gh_mask); __vmwrite(CR4_READ_SHADOW, cr_read_shadow); + /* Add the nested host mask to the one set by vmx_update_guest_cr. */ + __vmwrite(CR4_GUEST_HOST_MASK, cr_gh_mask | v->arch.hvm_vcpu.mask_cr[4]); /* TODO: CR3 target control */ } diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index f229e69948..4317658c56 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -189,10 +189,11 @@ int arch_monitor_domctl_event(struct domain *d, ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; } - if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) + if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index || + VM_EVENT_X86_CR4 == mop->u.mov_to_cr.index ) { struct vcpu *v; - /* Latches new CR3 mask through CR0 code. */ + /* Latches new CR3 or CR4 mask through CR0 code. */ for_each_vcpu ( d, v ) hvm_update_guest_cr(v, 0); } -- 2.16.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] vmx/hap: optimize CR4 trapping 2018-02-16 12:10 ` [PATCH v2 2/2] vmx/hap: optimize CR4 trapping Roger Pau Monne @ 2018-02-16 12:30 ` Razvan Cojocaru 2018-02-16 12:37 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Razvan Cojocaru @ 2018-02-16 12:30 UTC (permalink / raw) To: Roger Pau Monne, xen-devel Cc: Andrew Cooper, Kevin Tian, Tamas K Lengyel, Jan Beulich, Jun Nakajima On 02/16/2018 02:10 PM, Roger Pau Monne wrote: > diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c > index f229e69948..4317658c56 100644 > --- a/xen/arch/x86/monitor.c > +++ b/xen/arch/x86/monitor.c > @@ -189,10 +189,11 @@ int arch_monitor_domctl_event(struct domain *d, > ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; > } > > - if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) > + if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index || > + VM_EVENT_X86_CR4 == mop->u.mov_to_cr.index ) > { > struct vcpu *v; > - /* Latches new CR3 mask through CR0 code. */ > + /* Latches new CR3 or CR4 mask through CR0 code. */ > for_each_vcpu ( d, v ) > hvm_update_guest_cr(v, 0); > } Did you, by any chance, test this code with xen-access.c (it already has a test for CR4 for the PGE stuff)? I'm not convinced the hvm_update_guest_cr(v, 0); call suffices to enable CR4 load exits. Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] vmx/hap: optimize CR4 trapping 2018-02-16 12:30 ` Razvan Cojocaru @ 2018-02-16 12:37 ` Roger Pau Monné 2018-02-16 12:39 ` Razvan Cojocaru 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2018-02-16 12:37 UTC (permalink / raw) To: Razvan Cojocaru Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Andrew Cooper, Jan Beulich, xen-devel On Fri, Feb 16, 2018 at 02:30:55PM +0200, Razvan Cojocaru wrote: > On 02/16/2018 02:10 PM, Roger Pau Monne wrote: > > diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c > > index f229e69948..4317658c56 100644 > > --- a/xen/arch/x86/monitor.c > > +++ b/xen/arch/x86/monitor.c > > @@ -189,10 +189,11 @@ int arch_monitor_domctl_event(struct domain *d, > > ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; > > } > > > > - if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) > > + if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index || > > + VM_EVENT_X86_CR4 == mop->u.mov_to_cr.index ) > > { > > struct vcpu *v; > > - /* Latches new CR3 mask through CR0 code. */ > > + /* Latches new CR3 or CR4 mask through CR0 code. */ > > for_each_vcpu ( d, v ) > > hvm_update_guest_cr(v, 0); > > } > > Did you, by any chance, test this code with xen-access.c (it already has > a test for CR4 for the PGE stuff)? I'm not convinced the > hvm_update_guest_cr(v, 0); call suffices to enable CR4 load exits. hvm_update_guest_cr is just a wrapper to vmx_update_guest_cr when using vmx, which will unconditionally re-calculate the CR4 mask when called with cr == 0 or cr == 4. I have not tested it with xen-access, but it seems quite straightforward to me. Are you seeing any other path that could enable CR4 load accesses without calling hvm_update_guest_cr? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] vmx/hap: optimize CR4 trapping 2018-02-16 12:37 ` Roger Pau Monné @ 2018-02-16 12:39 ` Razvan Cojocaru 2018-02-16 15:03 ` Razvan Cojocaru 0 siblings, 1 reply; 10+ messages in thread From: Razvan Cojocaru @ 2018-02-16 12:39 UTC (permalink / raw) To: Roger Pau Monné Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Andrew Cooper, Jan Beulich, xen-devel On 02/16/2018 02:37 PM, Roger Pau Monné wrote: > On Fri, Feb 16, 2018 at 02:30:55PM +0200, Razvan Cojocaru wrote: >> On 02/16/2018 02:10 PM, Roger Pau Monne wrote: >>> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c >>> index f229e69948..4317658c56 100644 >>> --- a/xen/arch/x86/monitor.c >>> +++ b/xen/arch/x86/monitor.c >>> @@ -189,10 +189,11 @@ int arch_monitor_domctl_event(struct domain *d, >>> ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; >>> } >>> >>> - if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) >>> + if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index || >>> + VM_EVENT_X86_CR4 == mop->u.mov_to_cr.index ) >>> { >>> struct vcpu *v; >>> - /* Latches new CR3 mask through CR0 code. */ >>> + /* Latches new CR3 or CR4 mask through CR0 code. */ >>> for_each_vcpu ( d, v ) >>> hvm_update_guest_cr(v, 0); >>> } >> >> Did you, by any chance, test this code with xen-access.c (it already has >> a test for CR4 for the PGE stuff)? I'm not convinced the >> hvm_update_guest_cr(v, 0); call suffices to enable CR4 load exits. > > hvm_update_guest_cr is just a wrapper to vmx_update_guest_cr when > using vmx, which will unconditionally re-calculate the CR4 mask when > called with cr == 0 or cr == 4. > > I have not tested it with xen-access, but it seems quite > straightforward to me. Are you seeing any other path that could > enable CR4 load accesses without calling hvm_update_guest_cr? No, I thought I did but as it turns out it wasn't. I'll run a quick test on the patches just to make sure though. They should be alright. Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] vmx/hap: optimize CR4 trapping 2018-02-16 12:39 ` Razvan Cojocaru @ 2018-02-16 15:03 ` Razvan Cojocaru 0 siblings, 0 replies; 10+ messages in thread From: Razvan Cojocaru @ 2018-02-16 15:03 UTC (permalink / raw) To: Roger Pau Monné Cc: Kevin Tian, Tamas K Lengyel, Jun Nakajima, Andrew Cooper, Jan Beulich, xen-devel On 02/16/2018 02:39 PM, Razvan Cojocaru wrote: > On 02/16/2018 02:37 PM, Roger Pau Monné wrote: >> On Fri, Feb 16, 2018 at 02:30:55PM +0200, Razvan Cojocaru wrote: >>> On 02/16/2018 02:10 PM, Roger Pau Monne wrote: >>>> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c >>>> index f229e69948..4317658c56 100644 >>>> --- a/xen/arch/x86/monitor.c >>>> +++ b/xen/arch/x86/monitor.c >>>> @@ -189,10 +189,11 @@ int arch_monitor_domctl_event(struct domain *d, >>>> ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask; >>>> } >>>> >>>> - if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index ) >>>> + if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index || >>>> + VM_EVENT_X86_CR4 == mop->u.mov_to_cr.index ) >>>> { >>>> struct vcpu *v; >>>> - /* Latches new CR3 mask through CR0 code. */ >>>> + /* Latches new CR3 or CR4 mask through CR0 code. */ >>>> for_each_vcpu ( d, v ) >>>> hvm_update_guest_cr(v, 0); >>>> } >>> >>> Did you, by any chance, test this code with xen-access.c (it already has >>> a test for CR4 for the PGE stuff)? I'm not convinced the >>> hvm_update_guest_cr(v, 0); call suffices to enable CR4 load exits. >> >> hvm_update_guest_cr is just a wrapper to vmx_update_guest_cr when >> using vmx, which will unconditionally re-calculate the CR4 mask when >> called with cr == 0 or cr == 4. >> >> I have not tested it with xen-access, but it seems quite >> straightforward to me. Are you seeing any other path that could >> enable CR4 load accesses without calling hvm_update_guest_cr? > > No, I thought I did but as it turns out it wasn't. I'll run a quick test > on the patches just to make sure though. They should be alright. Ran a few tests, and everything appears to be in order. For the monitor / vm_event parts: Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com> Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-02-19 15:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-16 12:10 [PATCH v2 0/2] vmx/hap: optimize CR4 trapping Roger Pau Monne 2018-02-16 12:10 ` [PATCH v2 1/2] x86/hvm: introduce cr_mask to store trapped bits of CR accesses Roger Pau Monne 2018-02-16 12:49 ` Andrew Cooper 2018-02-19 15:04 ` Roger Pau Monné 2018-02-19 15:07 ` Andrew Cooper 2018-02-16 12:10 ` [PATCH v2 2/2] vmx/hap: optimize CR4 trapping Roger Pau Monne 2018-02-16 12:30 ` Razvan Cojocaru 2018-02-16 12:37 ` Roger Pau Monné 2018-02-16 12:39 ` Razvan Cojocaru 2018-02-16 15:03 ` Razvan Cojocaru
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).