linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Reiji Watanabe <reijiw@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 29/43] KVM: SVM: Tweak order of cr0/cr4/efer writes at RESET/INIT
Date: Sun, 23 May 2021 16:04:03 -0700	[thread overview]
Message-ID: <CAAeT=FwjdQ768k5fJ2pCHbUwsYMdMdOPH3hXA9qKA+f+2CEQEg@mail.gmail.com> (raw)
In-Reply-To: <YKVt5XVIQMUCUIHd@google.com>

> > AMD's APM Vol2 (Table 14-1 in Revision 3.37) says CR0 After INIT will be:
> >
> >    CD and NW are unchanged
> >    Bit 4 (reserved) = 1
> >    All others = 0
> >
> > (CR0 will be 0x60000010 after RESET)
> >
> > So, it looks the CR0 value that init_vmcb() sets could be
> > different from what is indicated in the APM for INIT.
> >
> > BTW, Intel's SDM (April 2021 version) says CR0 for Power up/Reset/INIT
> > will be 0x60000010 with the following note.
> > -------------------------------------------------
> > The CD and NW flags are unchanged,
> > bit 4 is set to 1, all other bits are cleared.
> > -------------------------------------------------
> > The note is attached as '2' to all Power up/Reset/INIT cases
> > looking at the SDM.  I would guess it is erroneous that
> > the note is attached to Power up/Reset though.
>
> Agreed.  I'll double check that CD and NW are preserved by hardware on INIT,
> and will also ping Intel folks to fix the POWER-UP and RESET footnote.
>
> Hah!  Reading through that section yet again, there's another SDM bug.  It
> contradicts itself with respect to the TLBs after INIT.
>
>   9.1 INITIALIZATION OVERVIEW:
>     The major difference is that during an INIT, the internal caches, MSRs,
>     MTRRs, and x87 FPU state are left unchanged (although, the TLBs and BTB
>     are invalidated as with a hardware reset)
>
> while Table 9-1 says:
>
>   Register                    Power up    Reset      INIT
>   Data and Code Cache, TLBs:  Invalid[6]  Invalid[6] Unchanged
>
> I'm pretty sure that Intel CPUs are supposed to flush the TLB, i.e. Tabel 9-1 is
> wrong.  Back in my Intel validation days, I remember being involved in a Core2
> bug that manifested as a triple fault after INIT due to global TLB entries not
> being flushed.  Looks like that wasn't fixed:
>
> https://www.intel.com/content/dam/support/us/en/documents/processors/mobile/celeron/sb/320121.pdf
>
>   AZ28. INIT Does Not Clear Global Entries in the TLB
>   Problem: INIT may not flush a TLB entry when:
>     • The processor is in protected mode with paging enabled and the page global enable
>       flag is set (PGE bit of CR4 register)
>     • G bit for the page table entry is set
>     • TLB entry is present in TLB when INIT occurs
>     • Software may encounter unexpected page fault or incorrect address translation due
>       to a TLB entry erroneously left in TLB after INIT.
>
>   Workaround: Write to CR3, CR4 (setting bits PSE, PGE or PAE) or CR0 (setting
>               bits PG or PE) registers before writing to memory early in BIOS
>               code to clear all the global entries from TLB.
>
>   Status: For the steppings affected, see the Summary Tables of Changes.
>
> AMD's APM also appears to contradict itself, though that depends on one's
> interpretation of "external intialization".  Like the SDM, its table states that
> the TLBs are not flushed on INIT:
>
>   Table 14-1. Initial Processor State
>
>   Processor Resource         Value after RESET      Value after INIT
>   Instruction and Data TLBs  Invalidated            Unchanged
>
> but a blurb later on says:
>
>   5.5.3 TLB Management
>
>   Implicit Invalidations. The following operations cause the entire TLB to be
>   invalidated, including global pages:
>
>     • External initialization of the processor.

"Table 8-9. Simultaneous Interrupt Priorities" of AMD's APM has
the words "External Processor Initialization (INIT)", which make
me guess "the External initialization of the processor" in 5.5.3
TLB Management means INIT.


> All in all, that means KVM also has a bug in the form of a missing guest TLB
> flush on INIT, at least for VMX and probably for SVM.  I'll add a patch to flush
> the guest TLBs on INIT irrespective of vendor.  Even if AMD CPUs don't flush the
> TLB, I see no reason to bank on all guests being paranoid enough to flush the
> TLB immediately after INIT.

Yes, I agree that would be better.
Thank you so much for all the helpful information !

Regards,
Reiji

  reply	other threads:[~2021-05-23 23:04 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-24  0:46 [PATCH 00/43] KVM: x86: vCPU RESET/INIT fixes and consolidation Sean Christopherson
2021-04-24  0:46 ` [PATCH 01/43] KVM: nVMX: Set LDTR to its architecturally defined value on nested VM-Exit Sean Christopherson
2021-05-19  5:30   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 02/43] KVM: VMX: Set EDX at INIT with CPUID.0x1, Family-Model-Stepping Sean Christopherson
2021-05-19  5:59   ` Reiji Watanabe
2021-05-19 18:47     ` Sean Christopherson
2021-05-21  7:07       ` Reiji Watanabe
2021-05-21 15:28         ` Sean Christopherson
2021-05-24  4:29           ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 03/43] KVM: SVM: Require exact CPUID.0x1 match when stuffing EDX at INIT Sean Christopherson
2021-05-19  5:30   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 04/43] KVM: SVM: Fall back to KVM's hardcoded value for EDX at RESET/INIT Sean Christopherson
2021-05-19  5:41   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 05/43] KVM: x86: Split out CR0/CR4 MMU role change detectors to separate helpers Sean Christopherson
2021-05-19  5:31   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 06/43] KVM: x86: Properly reset MMU context at vCPU RESET/INIT Sean Christopherson
2021-05-17 16:57   ` Reiji Watanabe
2021-05-18 20:23     ` Sean Christopherson
2021-05-18 22:42       ` Reiji Watanabe
2021-05-19 17:16         ` Sean Christopherson
2021-05-24  4:57           ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 07/43] KVM: VMX: Remove explicit MMU reset in enter_rmode() Sean Christopherson
2021-04-24  0:46 ` [PATCH 08/43] KVM: SVM: Drop explicit MMU reset at RESET/INIT Sean Christopherson
2021-05-19  5:32   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 09/43] KVM: SVM: Drop a redundant init_vmcb() from svm_create_vcpu() Sean Christopherson
2021-05-19  5:32   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 10/43] KVM: VMX: Move init_vmcs() invocation to vmx_vcpu_reset() Sean Christopherson
2021-05-19  5:33   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 11/43] KVM: x86: WARN if the APIC map is dirty without an in-kernel local APIC Sean Christopherson
2021-04-24  0:46 ` [PATCH 12/43] KVM: x86: Remove defunct BSP "update" in local APIC reset Sean Christopherson
2021-05-26  6:54   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 13/43] KVM: x86: Migrate the PIT only if vcpu0 is migrated, not any BSP Sean Christopherson
2021-04-24  0:46 ` [PATCH 14/43] KVM: x86: Don't force set BSP bit when local APIC is managed by userspace Sean Christopherson
2021-05-26  6:55   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 15/43] KVM: x86: Set BSP bit in reset BSP vCPU's APIC base by default Sean Christopherson
2021-05-26  6:55   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 16/43] KVM: VMX: Stuff vcpu->arch.apic_base directly at vCPU RESET Sean Christopherson
2021-05-26  6:55   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 17/43] KVM: x86: Open code necessary bits of kvm_lapic_set_base() " Sean Christopherson
2021-05-26  7:04   ` Reiji Watanabe
2021-05-26 15:15     ` Sean Christopherson
2021-04-24  0:46 ` [PATCH 18/43] KVM: x86: Consolidate APIC base RESET initialization code Sean Christopherson
2021-05-26  7:04   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 19/43] KVM: x86: Move EDX initialization at vCPU RESET to common code Sean Christopherson
2021-05-19  5:45   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 20/43] KVM: SVM: Don't bother writing vmcb->save.rip at vCPU RESET/INIT Sean Christopherson
2021-05-19  5:34   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 21/43] KVM: VMX: Invert handling of CR0.WP for EPT without unrestricted guest Sean Christopherson
2021-04-24  0:46 ` [PATCH 22/43] KVM: VMX: Remove direct write to vcpu->arch.cr0 during vCPU RESET/INIT Sean Christopherson
2021-05-19  5:34   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 23/43] KVM: VMX: Fold ept_update_paging_mode_cr0() back into vmx_set_cr0() Sean Christopherson
2021-04-24  0:46 ` [PATCH 24/43] KVM: nVMX: Do not clear CR3 load/store exiting bits if L1 wants 'em Sean Christopherson
2021-04-24  0:46 ` [PATCH 25/43] KVM: VMX: Pull GUEST_CR3 from the VMCS iff CR3 load exiting is disabled Sean Christopherson
2021-04-24  0:46 ` [PATCH 26/43] KVM: VMX: Process CR0.PG side effects after setting CR0 assets Sean Christopherson
2021-04-24  0:46 ` [PATCH 27/43] KVM: VMX: Skip emulation required checks during pmode/rmode transitions Sean Christopherson
2021-04-24  0:46 ` [PATCH 28/43] KVM: nVMX: Don't evaluate "emulation required" on VM-Exit Sean Christopherson
2021-04-24  0:46 ` [PATCH 29/43] KVM: SVM: Tweak order of cr0/cr4/efer writes at RESET/INIT Sean Christopherson
2021-05-19 18:16   ` Reiji Watanabe
2021-05-19 19:58     ` Sean Christopherson
2021-05-23 23:04       ` Reiji Watanabe [this message]
2021-04-24  0:46 ` [PATCH 30/43] KVM: SVM: Drop redundant writes to vmcb->save.cr4 " Sean Christopherson
2021-05-19  5:35   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 31/43] KVM: SVM: Stuff save->dr6 at during VMSA sync, not " Sean Christopherson
2021-04-24  0:46 ` [PATCH 32/43] KVM: VMX: Skip pointless MSR bitmap update when setting EFER Sean Christopherson
2021-04-24  0:46 ` [PATCH 33/43] KVM: VMX: Refresh list of user return MSRs after setting guest CPUID Sean Christopherson
2021-05-19  5:35   ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 34/43] KVM: VMX: Don't _explicitly_ reconfigure user return MSRs on vCPU INIT Sean Christopherson
2021-04-24  0:46 ` [PATCH 35/43] KVM: x86: Move setting of sregs during vCPU RESET/INIT to common x86 Sean Christopherson
2021-05-17 23:50   ` Reiji Watanabe
2021-05-18 21:45     ` Sean Christopherson
2021-05-21  5:19       ` Reiji Watanabe
2021-04-24  0:46 ` [PATCH 36/43] KVM: VMX: Remove obsolete MSR bitmap refresh at vCPU RESET/INIT Sean Christopherson
2021-04-24  0:46 ` [PATCH 37/43] KVM: nVMX: Remove obsolete MSR bitmap refresh at nested transitions Sean Christopherson
2021-04-24  0:46 ` [PATCH 38/43] KVM: VMX: Don't redo x2APIC MSR bitmaps when userspace filter is changed Sean Christopherson
2021-04-24  0:46 ` [PATCH 39/43] KVM: VMX: Remove unnecessary initialization of msr_bitmap_mode Sean Christopherson
2021-04-24  0:46 ` [PATCH 40/43] KVM: VMX: Smush x2APIC MSR bitmap adjustments into single function Sean Christopherson
2021-04-24  0:46 ` [PATCH 41/43] KVM: VMX: Remove redundant write to set vCPU as active at RESET/INIT Sean Christopherson
2021-04-24  0:46 ` [PATCH 42/43] KVM: VMX: Drop VMWRITEs to zero fields at vCPU RESET Sean Christopherson
2021-05-24 21:15   ` Paolo Bonzini
2021-05-24 22:21     ` Jim Mattson
2021-05-24 22:28     ` Sean Christopherson
2021-05-24 22:48       ` Jim Mattson
2021-05-25  1:02         ` Sean Christopherson
2021-04-24  0:46 ` [PATCH 43/43] KVM: x86: Drop pointless @reset_roots from kvm_init_mmu() Sean Christopherson
2021-05-27 19:11   ` Sean Christopherson
2021-05-27 19:25     ` Sean Christopherson
2021-06-10 16:54 ` [PATCH 00/43] KVM: x86: vCPU RESET/INIT fixes and consolidation Paolo Bonzini
2021-06-10 19:22   ` Sean Christopherson

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='CAAeT=FwjdQ768k5fJ2pCHbUwsYMdMdOPH3hXA9qKA+f+2CEQEg@mail.gmail.com' \
    --to=reijiw@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.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;
as well as URLs for NNTP newsgroup(s).