mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dongli Zhang <dongli.zhang@oracle.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Josh Triplett <josh@joshtriplett.org>,
	kvm@vger.kernel.org, rcu@vger.kernel.org,
	linux-kernel@vger.kernel.org, Kevin Tian <kevin.tian@intel.com>,
	Yan Zhao <yan.y.zhao@intel.com>, Yiwei Zhang <zzyiwei@google.com>
Subject: Re: [PATCH 1/5] KVM: x86: Remove VMX support for virtualizing guest MTRR memtypes
Date: Thu, 14 Mar 2024 03:31:10 -0700	[thread overview]
Message-ID: <73c670d1-0301-49bf-472c-97ae8d1b6c7c@oracle.com> (raw)
In-Reply-To: <ZfCL8mCmmEx5wGwv@google.com>



On 3/12/24 10:08, Sean Christopherson wrote:
> On Mon, Mar 11, 2024, Dongli Zhang wrote:
>>
>>
>> On 3/8/24 17:09, Sean Christopherson wrote:
>>> Remove KVM's support for virtualizing guest MTRR memtypes, as full MTRR
>>> adds no value, negatively impacts guest performance, and is a maintenance
>>> burden due to it's complexity and oddities.
>>>
>>> KVM's approach to virtualizating MTRRs make no sense, at all.  KVM *only*
>>> honors guest MTRR memtypes if EPT is enabled *and* the guest has a device
>>> that may perform non-coherent DMA access.  From a hardware virtualization
>>> perspective of guest MTRRs, there is _nothing_ special about EPT.  Legacy
>>> shadowing paging doesn't magically account for guest MTRRs, nor does NPT.
>>
>> [snip]
>>
>>>  
>>> -bool __kvm_mmu_honors_guest_mtrrs(bool vm_has_noncoherent_dma)
>>> +bool kvm_mmu_may_ignore_guest_pat(void)
>>>  {
>>>  	/*
>>> -	 * If host MTRRs are ignored (shadow_memtype_mask is non-zero), and the
>>> -	 * VM has non-coherent DMA (DMA doesn't snoop CPU caches), KVM's ABI is
>>> -	 * to honor the memtype from the guest's MTRRs so that guest accesses
>>> -	 * to memory that is DMA'd aren't cached against the guest's wishes.
>>> -	 *
>>> -	 * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs,
>>> -	 * e.g. KVM will force UC memtype for host MMIO.
>>> +	 * When EPT is enabled (shadow_memtype_mask is non-zero), and the VM
>>> +	 * has non-coherent DMA (DMA doesn't snoop CPU caches), KVM's ABI is to
>>> +	 * honor the memtype from the guest's PAT so that guest accesses to
>>> +	 * memory that is DMA'd aren't cached against the guest's wishes.  As a
>>> +	 * result, KVM _may_ ignore guest PAT, whereas without non-coherent DMA,
>>> +	 * KVM _always_ ignores guest PAT (when EPT is enabled).
>>>  	 */
>>> -	return vm_has_noncoherent_dma && shadow_memtype_mask;
>>> +	return shadow_memtype_mask;
>>>  }
>>>  
>>
>> Any special reason to use the naming 'may_ignore_guest_pat', but not
>> 'may_honor_guest_pat'?
> 
> Because which (after this series) is would either be misleading or outright wrong.
> If KVM returns true from the helper based solely on shadow_memtype_mask, then it's
> misleading because KVM will *always* honors guest PAT for such CPUs.  I.e. that
> name would yield this misleading statement.
> 
>   If the CPU supports self-snoop, KVM may honor guest PAT.
> 
> If KVM returns true iff self-snoop is NOT available (as proposed in this series),
> then it's outright wrong as KVM would return false, i.e. would make this incorrect
> statement:
> 
>   If the CPU supports self-snoop, KVM never honors guest PAT.
> 
> As saying that KVM may not or cannot do something is saying that KVM will never
> do that thing.
> 
> And because the EPT flag is "ignore guest PAT", not "honor guest PAT", but that's
> as much coincidence as it is anything else.
> 
>> Since it is also controlled by other cases, e.g., kvm_arch_has_noncoherent_dma()
>> at vmx_get_mt_mask(), it can be 'may_honor_guest_pat' too?
>>
>> Therefore, why not directly use 'shadow_memtype_mask' (without the API), or some
>> naming like "ept_enabled_for_hardware".
> 
> Again, after this series, KVM will *always* honor guest PAT for CPUs with self-snoop,
> i.e. KVM will *never* ignore guest PAT.  But for CPUs without self-snoop (or with
> errata), KVM conditionally honors/ignores guest PAT.
> 
>> Even with the code from PATCH 5/5, we still have high chance that VM has
>> non-coherent DMA?
> 
> I don't follow.  On CPUs with self-snoop, whether or not the VM has non-coherent
> DMA (from VFIO!) is irrelevant.  If the CPU has self-snoop, then KVM can safely
> honor guest PAT at all times.


Thank you very much for the explanation.

According to my understanding of the explanation (after this series):

1. When static_cpu_has(X86_FEATURE_SELFSNOOP) == true, it is 100% to "honor
guest PAT".

2. When static_cpu_has(X86_FEATURE_SELFSNOOP) == false (and
shadow_memtype_mask), although only 50% chance (depending on where there is
non-coherent DMA), at least now it is NOT 100% (to honor guest PAT) any longer.

Due to the fact it is not 100% (to honor guest PAT) any longer, there starts the
trend (from 100% to 50%) to "ignore guest PAT", that is:
kvm_mmu_may_ignore_guest_pat().

Dongli Zhang

  reply	other threads:[~2024-03-14 10:31 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-09  1:09 [PATCH 0/5] KVM: VMX: Drop MTRR virtualization, honor guest PAT Sean Christopherson
2024-03-09  1:09 ` [PATCH 1/5] KVM: x86: Remove VMX support for virtualizing guest MTRR memtypes Sean Christopherson
2024-03-11  7:44   ` Yan Zhao
2024-03-12  0:08     ` Sean Christopherson
2024-03-12  1:10   ` Dongli Zhang
2024-03-12 17:08     ` Sean Christopherson
2024-03-14 10:31       ` Dongli Zhang [this message]
2024-03-14 14:47         ` Sean Christopherson
2024-03-09  1:09 ` [PATCH 2/5] KVM: VMX: Drop support for forcing UC memory when guest CR0.CD=1 Sean Christopherson
2024-03-09  1:09 ` [PATCH 3/5] srcu: Add an API for a memory barrier after SRCU read lock Sean Christopherson
2024-03-09  1:09 ` [PATCH 4/5] KVM: x86: Ensure a full memory barrier is emitted in the VM-Exit path Sean Christopherson
2024-06-20 22:38   ` Paolo Bonzini
2024-06-20 23:42     ` Paul E. McKenney
2024-06-21  0:52     ` Yan Zhao
2024-03-09  1:09 ` [PATCH 5/5] KVM: VMX: Always honor guest PAT on CPUs that support self-snoop Sean Christopherson
2024-03-11  1:16   ` Yan Zhao
2024-03-12  0:25     ` Sean Christopherson
2024-03-12  7:30       ` Tian, Kevin
2024-03-12 16:07         ` Sean Christopherson
2024-03-13  1:18           ` Yan Zhao
2024-03-13  8:52             ` Tian, Kevin
2024-03-13  8:55               ` Yan Zhao
2024-03-13 15:09                 ` Sean Christopherson
2024-03-14  0:12                   ` Yan Zhao
2024-03-14  1:00                     ` Sean Christopherson
2024-03-25  3:43   ` Chao Gao
2024-04-01 22:29     ` Sean Christopherson
2024-08-30  9:35   ` Vitaly Kuznetsov
2024-08-30 11:05     ` Gerd Hoffmann
2024-08-30 13:47       ` Vitaly Kuznetsov
2024-08-30 13:52         ` Sean Christopherson
2024-08-30 14:06           ` Vitaly Kuznetsov
2024-08-30 14:37             ` Vitaly Kuznetsov
2024-08-30 16:13               ` Sean Christopherson
2024-09-02  8:23                 ` Gerd Hoffmann
2024-09-02  1:44         ` Yan Zhao
2024-09-02  9:49           ` Vitaly Kuznetsov
2024-09-03  0:25             ` Yan Zhao
2024-09-03 15:30             ` Sean Christopherson
2024-09-03 16:20               ` Vitaly Kuznetsov
2024-09-04  2:28                 ` Yan Zhao
2024-09-04 12:17                   ` Yan Zhao
2024-09-05  0:41                     ` Sean Christopherson
2024-09-05  9:43                       ` Yan Zhao
2024-09-09  5:30                         ` Yan Zhao
2024-09-09 13:24                           ` Paolo Bonzini
2024-09-09 16:04                             ` Sean Christopherson
2024-09-10  1:05                             ` Yan Zhao
2024-09-04 11:47                 ` Vitaly Kuznetsov
2024-10-07 13:28     ` Linux regression tracking (Thorsten Leemhuis)
2024-10-07 13:38       ` Vitaly Kuznetsov
2024-10-07 14:04         ` Linux regression tracking (Thorsten Leemhuis)
2024-03-22  9:29 ` [PATCH 0/5] KVM: VMX: Drop MTRR virtualization, honor guest PAT Ma, Yongwei
2024-03-22 13:08 ` Yan Zhao
2024-03-25  6:56   ` Ma, XiangfeiX
2024-03-25  8:02     ` Ma, XiangfeiX
2024-06-05 23:20 ` Sean Christopherson
2024-06-06  0:03   ` Paul E. McKenney

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=73c670d1-0301-49bf-472c-97ae8d1b6c7c@oracle.com \
    --to=dongli.zhang@oracle.com \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rcu@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=yan.y.zhao@intel.com \
    --cc=zzyiwei@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®