linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jim Mattson <jmattson@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Vitaly Kuznetsov" <vkuznets@redhat.com>,
	"kvm list" <kvm@vger.kernel.org>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	"Liran Alon" <liran.alon@oracle.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x86/kvm/nVMX: tweak shadow fields
Date: Fri, 9 Nov 2018 14:11:35 -0800	[thread overview]
Message-ID: <CALMp9eQ4493qH4sA4e5UibaHhatEKnztGNBO2s40jc2jXXCdUQ@mail.gmail.com> (raw)
In-Reply-To: <31279dfd-d0a1-3720-46a2-52395a124057@redhat.com>

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)
>>
>

  reply	other threads:[~2018-11-09 22:11 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19 14:16 Vitaly Kuznetsov
2018-10-19 16:45 ` Paolo Bonzini
2018-11-09 22:11   ` Jim Mattson [this message]
2018-11-12 14:39     ` Vitaly Kuznetsov
2018-11-14 11:34       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALMp9eQ4493qH4sA4e5UibaHhatEKnztGNBO2s40jc2jXXCdUQ@mail.gmail.com \
    --to=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liran.alon@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --cc=vkuznets@redhat.com \
    --subject='Re: [PATCH] x86/kvm/nVMX: tweak shadow fields' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).