* [PATCH] x86/kvm/nVMX: tweak shadow fields
@ 2018-10-19 14:16 Vitaly Kuznetsov
2018-10-19 16:45 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Vitaly Kuznetsov @ 2018-10-19 14:16 UTC (permalink / raw)
To: kvm
Cc: Paolo Bonzini, Radim Krčmář,
Jim Mattson, Liran Alon, linux-kernel
It seems we have some leftovers from times when 'unrestricted guest'
wasn't exposed to L1. Stop shadowing GUEST_CS_{BASE,LIMIT,AR_SELECTOR}
and GUEST_ES_BASE, shadow GUEST_SS_AR_BYTES as it was found that some
hypervisors (e.g. Hyper-V without Enlightened VMCS) access it pretty
often.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
arch/x86/kvm/vmx.c | 10 +++++-----
arch/x86/kvm/vmx_shadow_fields.h | 5 +----
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index abeeb45d1c33..641a65b30685 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -12715,6 +12715,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
if (!hv_evmcs || !(hv_evmcs->hv_clean_fields &
HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) {
vmcs_write16(GUEST_ES_SELECTOR, vmcs12->guest_es_selector);
+ vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector);
vmcs_write16(GUEST_SS_SELECTOR, vmcs12->guest_ss_selector);
vmcs_write16(GUEST_DS_SELECTOR, vmcs12->guest_ds_selector);
vmcs_write16(GUEST_FS_SELECTOR, vmcs12->guest_fs_selector);
@@ -12722,6 +12723,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
vmcs_write16(GUEST_LDTR_SELECTOR, vmcs12->guest_ldtr_selector);
vmcs_write16(GUEST_TR_SELECTOR, vmcs12->guest_tr_selector);
vmcs_write32(GUEST_ES_LIMIT, vmcs12->guest_es_limit);
+ vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit);
vmcs_write32(GUEST_SS_LIMIT, vmcs12->guest_ss_limit);
vmcs_write32(GUEST_DS_LIMIT, vmcs12->guest_ds_limit);
vmcs_write32(GUEST_FS_LIMIT, vmcs12->guest_fs_limit);
@@ -12731,12 +12733,13 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
vmcs_write32(GUEST_GDTR_LIMIT, vmcs12->guest_gdtr_limit);
vmcs_write32(GUEST_IDTR_LIMIT, vmcs12->guest_idtr_limit);
vmcs_write32(GUEST_ES_AR_BYTES, vmcs12->guest_es_ar_bytes);
- vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes);
vmcs_write32(GUEST_DS_AR_BYTES, vmcs12->guest_ds_ar_bytes);
vmcs_write32(GUEST_FS_AR_BYTES, vmcs12->guest_fs_ar_bytes);
vmcs_write32(GUEST_GS_AR_BYTES, vmcs12->guest_gs_ar_bytes);
vmcs_write32(GUEST_LDTR_AR_BYTES, vmcs12->guest_ldtr_ar_bytes);
vmcs_write32(GUEST_TR_AR_BYTES, vmcs12->guest_tr_ar_bytes);
+ vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base);
+ vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base);
vmcs_writel(GUEST_SS_BASE, vmcs12->guest_ss_base);
vmcs_writel(GUEST_DS_BASE, vmcs12->guest_ds_base);
vmcs_writel(GUEST_FS_BASE, vmcs12->guest_fs_base);
@@ -12838,11 +12841,8 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
*/
if (!hv_evmcs || !(hv_evmcs->hv_clean_fields &
HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) {
- vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector);
- vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit);
vmcs_write32(GUEST_CS_AR_BYTES, vmcs12->guest_cs_ar_bytes);
- vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base);
- vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base);
+ vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes);
}
if (vmx->nested.nested_run_pending &&
diff --git a/arch/x86/kvm/vmx_shadow_fields.h b/arch/x86/kvm/vmx_shadow_fields.h
index cd0c75f6d037..132432f375c2 100644
--- a/arch/x86/kvm/vmx_shadow_fields.h
+++ b/arch/x86/kvm/vmx_shadow_fields.h
@@ -28,7 +28,6 @@
*/
/* 16-bits */
-SHADOW_FIELD_RW(GUEST_CS_SELECTOR)
SHADOW_FIELD_RW(GUEST_INTR_STATUS)
SHADOW_FIELD_RW(GUEST_PML_INDEX)
SHADOW_FIELD_RW(HOST_FS_SELECTOR)
@@ -47,8 +46,8 @@ SHADOW_FIELD_RW(VM_ENTRY_EXCEPTION_ERROR_CODE)
SHADOW_FIELD_RW(VM_ENTRY_INTR_INFO_FIELD)
SHADOW_FIELD_RW(VM_ENTRY_INSTRUCTION_LEN)
SHADOW_FIELD_RW(TPR_THRESHOLD)
-SHADOW_FIELD_RW(GUEST_CS_LIMIT)
SHADOW_FIELD_RW(GUEST_CS_AR_BYTES)
+SHADOW_FIELD_RW(GUEST_SS_AR_BYTES)
SHADOW_FIELD_RW(GUEST_INTERRUPTIBILITY_INFO)
SHADOW_FIELD_RW(VMX_PREEMPTION_TIMER_VALUE)
@@ -61,8 +60,6 @@ SHADOW_FIELD_RW(GUEST_CR0)
SHADOW_FIELD_RW(GUEST_CR3)
SHADOW_FIELD_RW(GUEST_CR4)
SHADOW_FIELD_RW(GUEST_RFLAGS)
-SHADOW_FIELD_RW(GUEST_CS_BASE)
-SHADOW_FIELD_RW(GUEST_ES_BASE)
SHADOW_FIELD_RW(CR0_GUEST_HOST_MASK)
SHADOW_FIELD_RW(CR0_READ_SHADOW)
SHADOW_FIELD_RW(CR4_READ_SHADOW)
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] x86/kvm/nVMX: tweak shadow fields 2018-10-19 14:16 [PATCH] x86/kvm/nVMX: tweak shadow fields Vitaly Kuznetsov @ 2018-10-19 16:45 ` Paolo Bonzini 2018-11-09 22:11 ` Jim Mattson 0 siblings, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2018-10-19 16:45 UTC (permalink / raw) To: Vitaly Kuznetsov, kvm Cc: Radim Krčmář, Jim Mattson, Liran Alon, linux-kernel On 19/10/2018 16:16, Vitaly Kuznetsov wrote: > It seems we have some leftovers from times when 'unrestricted guest' > wasn't exposed to L1. Stop shadowing GUEST_CS_{BASE,LIMIT,AR_SELECTOR} > and GUEST_ES_BASE, shadow GUEST_SS_AR_BYTES as it was found that some > hypervisors (e.g. Hyper-V without Enlightened VMCS) access it pretty > often. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Queued, thanks. Paolo > --- > arch/x86/kvm/vmx.c | 10 +++++----- > arch/x86/kvm/vmx_shadow_fields.h | 5 +---- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index abeeb45d1c33..641a65b30685 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -12715,6 +12715,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) > if (!hv_evmcs || !(hv_evmcs->hv_clean_fields & > HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) { > vmcs_write16(GUEST_ES_SELECTOR, vmcs12->guest_es_selector); > + vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector); > vmcs_write16(GUEST_SS_SELECTOR, vmcs12->guest_ss_selector); > vmcs_write16(GUEST_DS_SELECTOR, vmcs12->guest_ds_selector); > vmcs_write16(GUEST_FS_SELECTOR, vmcs12->guest_fs_selector); > @@ -12722,6 +12723,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) > vmcs_write16(GUEST_LDTR_SELECTOR, vmcs12->guest_ldtr_selector); > vmcs_write16(GUEST_TR_SELECTOR, vmcs12->guest_tr_selector); > vmcs_write32(GUEST_ES_LIMIT, vmcs12->guest_es_limit); > + vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit); > vmcs_write32(GUEST_SS_LIMIT, vmcs12->guest_ss_limit); > vmcs_write32(GUEST_DS_LIMIT, vmcs12->guest_ds_limit); > vmcs_write32(GUEST_FS_LIMIT, vmcs12->guest_fs_limit); > @@ -12731,12 +12733,13 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) > vmcs_write32(GUEST_GDTR_LIMIT, vmcs12->guest_gdtr_limit); > vmcs_write32(GUEST_IDTR_LIMIT, vmcs12->guest_idtr_limit); > vmcs_write32(GUEST_ES_AR_BYTES, vmcs12->guest_es_ar_bytes); > - vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes); > vmcs_write32(GUEST_DS_AR_BYTES, vmcs12->guest_ds_ar_bytes); > vmcs_write32(GUEST_FS_AR_BYTES, vmcs12->guest_fs_ar_bytes); > vmcs_write32(GUEST_GS_AR_BYTES, vmcs12->guest_gs_ar_bytes); > vmcs_write32(GUEST_LDTR_AR_BYTES, vmcs12->guest_ldtr_ar_bytes); > vmcs_write32(GUEST_TR_AR_BYTES, vmcs12->guest_tr_ar_bytes); > + vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base); > + vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base); > vmcs_writel(GUEST_SS_BASE, vmcs12->guest_ss_base); > vmcs_writel(GUEST_DS_BASE, vmcs12->guest_ds_base); > vmcs_writel(GUEST_FS_BASE, vmcs12->guest_fs_base); > @@ -12838,11 +12841,8 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > */ > if (!hv_evmcs || !(hv_evmcs->hv_clean_fields & > HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) { > - vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector); > - vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit); > vmcs_write32(GUEST_CS_AR_BYTES, vmcs12->guest_cs_ar_bytes); > - vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base); > - vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base); > + vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes); > } > > if (vmx->nested.nested_run_pending && > diff --git a/arch/x86/kvm/vmx_shadow_fields.h b/arch/x86/kvm/vmx_shadow_fields.h > index cd0c75f6d037..132432f375c2 100644 > --- a/arch/x86/kvm/vmx_shadow_fields.h > +++ b/arch/x86/kvm/vmx_shadow_fields.h > @@ -28,7 +28,6 @@ > */ > > /* 16-bits */ > -SHADOW_FIELD_RW(GUEST_CS_SELECTOR) > SHADOW_FIELD_RW(GUEST_INTR_STATUS) > SHADOW_FIELD_RW(GUEST_PML_INDEX) > SHADOW_FIELD_RW(HOST_FS_SELECTOR) > @@ -47,8 +46,8 @@ SHADOW_FIELD_RW(VM_ENTRY_EXCEPTION_ERROR_CODE) > SHADOW_FIELD_RW(VM_ENTRY_INTR_INFO_FIELD) > SHADOW_FIELD_RW(VM_ENTRY_INSTRUCTION_LEN) > SHADOW_FIELD_RW(TPR_THRESHOLD) > -SHADOW_FIELD_RW(GUEST_CS_LIMIT) > SHADOW_FIELD_RW(GUEST_CS_AR_BYTES) > +SHADOW_FIELD_RW(GUEST_SS_AR_BYTES) > SHADOW_FIELD_RW(GUEST_INTERRUPTIBILITY_INFO) > SHADOW_FIELD_RW(VMX_PREEMPTION_TIMER_VALUE) > > @@ -61,8 +60,6 @@ SHADOW_FIELD_RW(GUEST_CR0) > SHADOW_FIELD_RW(GUEST_CR3) > SHADOW_FIELD_RW(GUEST_CR4) > SHADOW_FIELD_RW(GUEST_RFLAGS) > -SHADOW_FIELD_RW(GUEST_CS_BASE) > -SHADOW_FIELD_RW(GUEST_ES_BASE) > SHADOW_FIELD_RW(CR0_GUEST_HOST_MASK) > SHADOW_FIELD_RW(CR0_READ_SHADOW) > SHADOW_FIELD_RW(CR4_READ_SHADOW) > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/nVMX: tweak shadow fields 2018-10-19 16:45 ` Paolo Bonzini @ 2018-11-09 22:11 ` Jim Mattson 2018-11-12 14:39 ` Vitaly Kuznetsov 0 siblings, 1 reply; 5+ messages in thread From: Jim Mattson @ 2018-11-09 22:11 UTC (permalink / raw) To: Paolo Bonzini Cc: Vitaly Kuznetsov, kvm list, Radim Krčmář, Liran Alon, LKML I'm not convinced that the "one size fits all" and "context-free" approaches to VMCS shadowing are terribly effective. For example, we never shadow VMX_INSTRUCTION_INFO, but if we just reflected an exit to L1 for which that field is defined, there's probably a good chance that L1 will use it. We always shadow VM_EXIT_INTR_INFO, but if we didn't just reflect exit reason 0 to L1, it's not likely to be read. If the L2 guest is in legacy mode or compatibility mode, L1 is much more likely to be interested in the contents of the descriptor cache than if the guest is in 64-bit mode. Some hypervisors write TSC_OFFSET quite frequently. Others rarely. Last time I checked (it's been a while), VirtualBox was always interested in everything. :-) Kvm, Hyper-V, VMware, VirtualBox, Parallels...they all have different patterns, and they change from release to release. Is it worth having a set of VMCS shadowing bitmaps per-vCPU, in order to make better use of this feature? On Fri, Oct 19, 2018 at 9:45 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 19/10/2018 16:16, Vitaly Kuznetsov wrote: >> It seems we have some leftovers from times when 'unrestricted guest' >> wasn't exposed to L1. Stop shadowing GUEST_CS_{BASE,LIMIT,AR_SELECTOR} >> and GUEST_ES_BASE, shadow GUEST_SS_AR_BYTES as it was found that some >> hypervisors (e.g. Hyper-V without Enlightened VMCS) access it pretty >> often. >> >> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > Queued, thanks. > > Paolo > >> --- >> arch/x86/kvm/vmx.c | 10 +++++----- >> arch/x86/kvm/vmx_shadow_fields.h | 5 +---- >> 2 files changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index abeeb45d1c33..641a65b30685 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -12715,6 +12715,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) >> if (!hv_evmcs || !(hv_evmcs->hv_clean_fields & >> HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) { >> vmcs_write16(GUEST_ES_SELECTOR, vmcs12->guest_es_selector); >> + vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector); >> vmcs_write16(GUEST_SS_SELECTOR, vmcs12->guest_ss_selector); >> vmcs_write16(GUEST_DS_SELECTOR, vmcs12->guest_ds_selector); >> vmcs_write16(GUEST_FS_SELECTOR, vmcs12->guest_fs_selector); >> @@ -12722,6 +12723,7 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) >> vmcs_write16(GUEST_LDTR_SELECTOR, vmcs12->guest_ldtr_selector); >> vmcs_write16(GUEST_TR_SELECTOR, vmcs12->guest_tr_selector); >> vmcs_write32(GUEST_ES_LIMIT, vmcs12->guest_es_limit); >> + vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit); >> vmcs_write32(GUEST_SS_LIMIT, vmcs12->guest_ss_limit); >> vmcs_write32(GUEST_DS_LIMIT, vmcs12->guest_ds_limit); >> vmcs_write32(GUEST_FS_LIMIT, vmcs12->guest_fs_limit); >> @@ -12731,12 +12733,13 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12) >> vmcs_write32(GUEST_GDTR_LIMIT, vmcs12->guest_gdtr_limit); >> vmcs_write32(GUEST_IDTR_LIMIT, vmcs12->guest_idtr_limit); >> vmcs_write32(GUEST_ES_AR_BYTES, vmcs12->guest_es_ar_bytes); >> - vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes); >> vmcs_write32(GUEST_DS_AR_BYTES, vmcs12->guest_ds_ar_bytes); >> vmcs_write32(GUEST_FS_AR_BYTES, vmcs12->guest_fs_ar_bytes); >> vmcs_write32(GUEST_GS_AR_BYTES, vmcs12->guest_gs_ar_bytes); >> vmcs_write32(GUEST_LDTR_AR_BYTES, vmcs12->guest_ldtr_ar_bytes); >> vmcs_write32(GUEST_TR_AR_BYTES, vmcs12->guest_tr_ar_bytes); >> + vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base); >> + vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base); >> vmcs_writel(GUEST_SS_BASE, vmcs12->guest_ss_base); >> vmcs_writel(GUEST_DS_BASE, vmcs12->guest_ds_base); >> vmcs_writel(GUEST_FS_BASE, vmcs12->guest_fs_base); >> @@ -12838,11 +12841,8 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, >> */ >> if (!hv_evmcs || !(hv_evmcs->hv_clean_fields & >> HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP2)) { >> - vmcs_write16(GUEST_CS_SELECTOR, vmcs12->guest_cs_selector); >> - vmcs_write32(GUEST_CS_LIMIT, vmcs12->guest_cs_limit); >> vmcs_write32(GUEST_CS_AR_BYTES, vmcs12->guest_cs_ar_bytes); >> - vmcs_writel(GUEST_ES_BASE, vmcs12->guest_es_base); >> - vmcs_writel(GUEST_CS_BASE, vmcs12->guest_cs_base); >> + vmcs_write32(GUEST_SS_AR_BYTES, vmcs12->guest_ss_ar_bytes); >> } >> >> if (vmx->nested.nested_run_pending && >> diff --git a/arch/x86/kvm/vmx_shadow_fields.h b/arch/x86/kvm/vmx_shadow_fields.h >> index cd0c75f6d037..132432f375c2 100644 >> --- a/arch/x86/kvm/vmx_shadow_fields.h >> +++ b/arch/x86/kvm/vmx_shadow_fields.h >> @@ -28,7 +28,6 @@ >> */ >> >> /* 16-bits */ >> -SHADOW_FIELD_RW(GUEST_CS_SELECTOR) >> SHADOW_FIELD_RW(GUEST_INTR_STATUS) >> SHADOW_FIELD_RW(GUEST_PML_INDEX) >> SHADOW_FIELD_RW(HOST_FS_SELECTOR) >> @@ -47,8 +46,8 @@ SHADOW_FIELD_RW(VM_ENTRY_EXCEPTION_ERROR_CODE) >> SHADOW_FIELD_RW(VM_ENTRY_INTR_INFO_FIELD) >> SHADOW_FIELD_RW(VM_ENTRY_INSTRUCTION_LEN) >> SHADOW_FIELD_RW(TPR_THRESHOLD) >> -SHADOW_FIELD_RW(GUEST_CS_LIMIT) >> SHADOW_FIELD_RW(GUEST_CS_AR_BYTES) >> +SHADOW_FIELD_RW(GUEST_SS_AR_BYTES) >> SHADOW_FIELD_RW(GUEST_INTERRUPTIBILITY_INFO) >> SHADOW_FIELD_RW(VMX_PREEMPTION_TIMER_VALUE) >> >> @@ -61,8 +60,6 @@ SHADOW_FIELD_RW(GUEST_CR0) >> SHADOW_FIELD_RW(GUEST_CR3) >> SHADOW_FIELD_RW(GUEST_CR4) >> SHADOW_FIELD_RW(GUEST_RFLAGS) >> -SHADOW_FIELD_RW(GUEST_CS_BASE) >> -SHADOW_FIELD_RW(GUEST_ES_BASE) >> SHADOW_FIELD_RW(CR0_GUEST_HOST_MASK) >> SHADOW_FIELD_RW(CR0_READ_SHADOW) >> SHADOW_FIELD_RW(CR4_READ_SHADOW) >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/nVMX: tweak shadow fields 2018-11-09 22:11 ` Jim Mattson @ 2018-11-12 14:39 ` Vitaly Kuznetsov 2018-11-14 11:34 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Vitaly Kuznetsov @ 2018-11-12 14:39 UTC (permalink / raw) To: Jim Mattson, Paolo Bonzini Cc: kvm list, Radim Krčmář, Liran Alon, LKML Jim Mattson <jmattson@google.com> writes: > I'm not convinced that the "one size fits all" and "context-free" > approaches to VMCS shadowing are terribly effective. > > For example, we never shadow VMX_INSTRUCTION_INFO, but if we just > reflected an exit to L1 for which that field is defined, there's > probably a good chance that L1 will use it. We always shadow > VM_EXIT_INTR_INFO, but if we didn't just reflect exit reason 0 to L1, > it's not likely to be read. If the L2 guest is in legacy mode or > compatibility mode, L1 is much more likely to be interested in the > contents of the descriptor cache than if the guest is in 64-bit mode. > > Some hypervisors write TSC_OFFSET quite frequently. Others rarely. > Last time I checked (it's been a while), VirtualBox was always > interested in everything. :-) Kvm, Hyper-V, VMware, VirtualBox, > Parallels...they all have different patterns, and they change from > release to release. > > Is it worth having a set of VMCS shadowing bitmaps per-vCPU, in order > to make better use of this feature? Per CPU or not, to improve the feature we'll probably need some sort of an 'adaptive' algorithm picking which fields to shadow. I haven't thought this through, especially read/write shadowing, but we can probably start with an empty bitmap and later shadow it when we get over some threshold of vmread/vmwrite exits we enabling shadowing. The question is when we un-shadow it. For example, we can un-shadow a field for writing every time we see it was not changed between two exits to L0 (so we're trying to write the same value to vmcs12). -- Vitaly ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/nVMX: tweak shadow fields 2018-11-12 14:39 ` Vitaly Kuznetsov @ 2018-11-14 11:34 ` Paolo Bonzini 0 siblings, 0 replies; 5+ messages in thread From: Paolo Bonzini @ 2018-11-14 11:34 UTC (permalink / raw) To: Vitaly Kuznetsov, Jim Mattson Cc: kvm list, Radim Krčmář, Liran Alon, LKML On 12/11/2018 15:39, Vitaly Kuznetsov wrote: >> Is it worth having a set of VMCS shadowing bitmaps per-vCPU, in order >> to make better use of this feature? > Per CPU or not, to improve the feature we'll probably need some sort of > an 'adaptive' algorithm picking which fields to shadow. I agree, making it per-VCPU is not useful alone. The question is to balance. The complexity and the number of fields that have to be copied between the VMCSes. If a vmexit type is rare, it makes sense not to shadow a field that would be always defined by that vmexit type, rather than pay a fixed price (even if it is loop overhead only) on all vmexits; this is the case VMX_INSTRUCTION_INFO. One thing that would make sense is to have separate shadow bitmaps for 32- and 64-bit L2. 32-bit L2 probably will need to shadow at least the segment bases. But for 64-bit L2, the current set is small and nice. There are still a few things that can be refined, but it's small things: 1) EXCEPTION_BITMAP which can go because everyone is probably using eager FPU these days---and has always been if you have shadow VMCS; 2) CR0_READ_SHADOW/CR4_READ_SHADOW/GUEST_CR0/GUEST_CR4 were needed on old KVM and would need to be tested on other hypervisors, but are probably unnecessary; 3) I would be surprised if HOST_FS_BASE/HOST_GS_BASE are needed too, though again you'd need testing on other hypervisors Overall, I prefer simple code that optimizes the common case very well, rather than complex code that tries to cover all bases... Paolo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-14 11:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-19 14:16 [PATCH] x86/kvm/nVMX: tweak shadow fields Vitaly Kuznetsov 2018-10-19 16:45 ` Paolo Bonzini 2018-11-09 22:11 ` Jim Mattson 2018-11-12 14:39 ` Vitaly Kuznetsov 2018-11-14 11:34 ` Paolo Bonzini
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).