diff mbox series

[v4,07/12] KVM: VMX: drop IPAT in memtype when CD=1 for KVM_X86_QUIRK_CD_NW_CLEARED

Message ID 20230714065326.20557-1-yan.y.zhao@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86/mmu: refine memtype related mmu zap | expand

Commit Message

Yan Zhao July 14, 2023, 6:53 a.m. UTC
For KVM_X86_QUIRK_CD_NW_CLEARED is on, remove the IPAT (ignore PAT) bit in
EPT memory types when cache is disabled and non-coherent DMA are present.

To correctly emulate CR0.CD=1, UC + IPAT are required as memtype in EPT.
However, as with commit
fb279950ba02 ("KVM: vmx: obey KVM_QUIRK_CD_NW_CLEARED"), WB + IPAT are
now returned to workaround a BIOS issue that guest MTRRs are enabled too
late. Without this workaround, a super slow guest boot-up is expected
during the pre-guest-MTRR-enabled period due to UC as the effective memory
type for all guest memory.

Absent emulating CR0.CD=1 with UC, it makes no sense to set IPAT when KVM
is honoring the guest memtype.
Removing the IPAT bit in this patch allows effective memory type to honor
PAT values as well, as WB is the weakest memtype. It means if a guest
explicitly claims UC as the memtype in PAT, the effective memory is UC
instead of previous WB. If, for some unknown reason, a guest meets a slow
boot-up issue with the removal of IPAT, it's desired to fix the blamed PAT
in the guest.

Besides, this patch is also a preparation patch for later fine-grained gfn
zap when guest MTRRs are honored, because it allows zapping only non-WB
ranges when CR0.CD toggles.

BTW, returning guest MTRR type as if CR0.CD=0 is also not preferred because
it still has to hardcode the MTRR type to WB during the
pre-guest-MTRR-enabled period to workaround the slow guest boot-up issue
(guest MTRR type when guest MTRRs are disabled is UC).
In addition, it will make the quirk unnecessarily complexer .

The change of removing IPAT has been verified with normal boot-up time
on old OVMF of commit c9e5618f84b0cb54a9ac2d7604f7b7e7859b45a7 as well,
dated back to Apr 14 2015.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Sean Christopherson Aug. 25, 2023, 9:43 p.m. UTC | #1
On Fri, Jul 14, 2023, Yan Zhao wrote:
> For KVM_X86_QUIRK_CD_NW_CLEARED is on, remove the IPAT (ignore PAT) bit in
> EPT memory types when cache is disabled and non-coherent DMA are present.
> 
> To correctly emulate CR0.CD=1, UC + IPAT are required as memtype in EPT.
> However, as with commit
> fb279950ba02 ("KVM: vmx: obey KVM_QUIRK_CD_NW_CLEARED"), WB + IPAT are
> now returned to workaround a BIOS issue that guest MTRRs are enabled too
> late. Without this workaround, a super slow guest boot-up is expected
> during the pre-guest-MTRR-enabled period due to UC as the effective memory
> type for all guest memory.
> 
> Absent emulating CR0.CD=1 with UC, it makes no sense to set IPAT when KVM
> is honoring the guest memtype.
> Removing the IPAT bit in this patch allows effective memory type to honor
> PAT values as well, as WB is the weakest memtype. It means if a guest
> explicitly claims UC as the memtype in PAT, the effective memory is UC
> instead of previous WB. If, for some unknown reason, a guest meets a slow
> boot-up issue with the removal of IPAT, it's desired to fix the blamed PAT
> in the guest.
> 
> Besides, this patch is also a preparation patch for later fine-grained gfn
> zap when guest MTRRs are honored, because it allows zapping only non-WB
> ranges when CR0.CD toggles.
> 
> BTW, returning guest MTRR type as if CR0.CD=0 is also not preferred because
> it still has to hardcode the MTRR type to WB during the

Please use full names instead of prononous, I found the "it still has to hardcode"
part really hard to grok.  I think this is what you're saying?

  BTW, returning guest MTRR type as if CR0.CD=0 is also not preferred because
  KVMs ABI for the quirk also requires KVM to force WB memtype regardless of
  guest MTRRs to workaround the slow guest boot-up issue.

> pre-guest-MTRR-enabled period to workaround the slow guest boot-up issue
> (guest MTRR type when guest MTRRs are disabled is UC).
> In addition, it will make the quirk unnecessarily complexer .
Yan Zhao Sept. 4, 2023, 7:41 a.m. UTC | #2
...
> > BTW, returning guest MTRR type as if CR0.CD=0 is also not preferred because
> > it still has to hardcode the MTRR type to WB during the
> 
> Please use full names instead of prononous, I found the "it still has to hardcode"
Thanks! yes, will avoid this kind of prononous in future.

> part really hard to grok.  I think this is what you're saying?
> 
>   BTW, returning guest MTRR type as if CR0.CD=0 is also not preferred because
>   KVMs ABI for the quirk also requires KVM to force WB memtype regardless of
>   guest MTRRs to workaround the slow guest boot-up issue.

Yes, exactly :)
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 0ecf4be2c6af..c1e93678cea4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7548,8 +7548,6 @@  static int vmx_vm_init(struct kvm *kvm)
 
 static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 {
-	u8 cache;
-
 	/* We wanted to honor guest CD/MTRR/PAT, but doing so could result in
 	 * memory aliases with conflicting memory types and sometimes MCEs.
 	 * We have to be careful as to what are honored and when.
@@ -7576,11 +7574,10 @@  static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 
 	if (kvm_read_cr0_bits(vcpu, X86_CR0_CD)) {
 		if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_CD_NW_CLEARED))
-			cache = MTRR_TYPE_WRBACK;
+			return MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT;
 		else
-			cache = MTRR_TYPE_UNCACHABLE;
-
-		return (cache << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT;
+			return (MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT) |
+				VMX_EPT_IPAT_BIT;
 	}
 
 	return kvm_mtrr_get_guest_memory_type(vcpu, gfn) << VMX_EPT_MT_EPTE_SHIFT;