All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.