On Wed, Jan 24, 2024 at 05:14:10PM +0100, Laszlo Ersek wrote:
> On 1/24/24 16:15, Gerd Hoffmann wrote:
> > Specifically before running lzma uncompress of the main firmware volume.
> > This is needed to make sure caching is enabled, otherwise the uncompress
> > can be extremely slow.
> >
> > Adapt the ASSERTs in PlatformInitLib to the changes.
> >
> > Background: In some virtual machine configurations with assigned
> > devices kvm uses EPT memory types to apply guest MTRR settings.
> > In case MTRRs are disabled kvm will use the uncachable memory type
> > for all mappings.
>
> I suggest mentioning mdev and noncoherent DMA here (the linux code you
> quoted elsewhere would be welcome too).
Ok, I guess it makes sense to quote it completely in the
commit message then ...
static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
{
/* 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.
*
* For MMIO, guest CD/MTRR are ignored. The EPT memory type is set to
* UC. The effective memory type is UC or WC depending on guest PAT.
* This was historically the source of MCEs and we want to be
* conservative.
*
* When there is no need to deal with noncoherent DMA (e.g., no VT-d
* or VT-d has snoop control), guest CD/MTRR/PAT are all ignored. The
* EPT memory type is set to WB. The effective memory type is forced
* WB.
*
* Otherwise, we trust guest. Guest CD/MTRR/PAT are all honored. The
* EPT memory type is used to emulate guest CD/MTRR.
*/
if (is_mmio)
return MTRR_TYPE_UNCACHABLE << VMX_EPT_MT_EPTE_SHIFT;
if (!kvm_arch_has_noncoherent_dma(vcpu->kvm))
return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) |
VMX_EPT_IPAT_BIT;
if (kvm_read_cr0_bits(vcpu, X86_CR0_CD)) {
if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_CD_NW_CLEARED))
return MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT;
else
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;
}
> The original report <https://edk2.groups.io/g/devel/message/113517>
> claims that commit e8aa4c6546ad ("UefiCpuPkg/ResetVector: Cache Disable
> should not be set by default in CR0", 2023-08-30) triggered the symptom.
> I don't understand how *clearing* CD in CR0, could make guest memory
> *less* cacheable. I understand the point about MTRR, but exactly the
> MTRR's currently upstream state should have masked any changes to CD.
... because it also explains that question: I think with CD set we
take the KVM_X86_QUIRK_CD_NW_CLEARED code path and run with memory in
write-back mode.
> (2) We already assume (minimally since 2015) that MTRRs are supported by
> the processor.
No. The whole MTRR setup is gated by "if (IsMtrrSupported ())".
Also note that qemu allows to turn off MTRRs (-cpu host,mtrr=off),
which btw can be used as workaround for this bug. With MTRR support
disabled kvm takes yet another code path ...
> > InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
> > DisableApicTimerInterrupt ();
> >
> > + //
> > + // Initialize MTRR
> > + //
> > + SecMtrrSetup ();
> (7) If you have a particular reason for doing it here, please capture
> that in the comment.
Placing it near other hardware init (timers) looked somewhat logical to
me. Any other place in that function should work equally well though.
> (8) Just for symmetry -- I'm noticing there are two
> SecCoreStartupWithStack() functions; the other being in
> "OvmfPkg/IntelTdx/Sec/SecMain.c".
>
> Also, Min's original QEMU command line included TDVF references.
>
> Does that mean this patch should be reflected to the TDX platform's modules?
Probably, I'll have a look.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114367): https://edk2.groups.io/g/devel/message/114367
Mute This Topic: https://groups.io/mt/103933443/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
