Discussion:
[edk2] MTRR setup in OVMF [was: PATCH v3 01/10 KVM: MMU: fix decoding cache type from MTRR]
Laszlo Ersek
2015-07-14 21:12:52 UTC
Permalink
Cross-posting to edk2-devel.

Original sub-thread starts here:
http://thread.gmane.org/gmane.linux.kernel/1952205/focus=1994315
+ /* MTRR is completely disabled, use UC for all of physical
memory. */
+ if (!(mtrr_state->enabled & 0x2))
+ return MTRR_TYPE_UNCACHABLE;
actually disappears in commit fa61213746a7 (KVM: MTRR: simplify
kvm_mtrr_get_guest_memory_type, 2015-06-15).
:(
Based on the SDM, UC is applied to all memory rather than default-type
if MTRR is disabled.
There are two issues I think. One is that I cannot find in the current
code that "UC is applied to all memory rather than default-type if MTRR
is disabled". mtrr_default_type unconditionally looks at
mtrr_state->deftype.
Yes... Will fix.
However, fast boot came back if "return 0xFF" here. So fast boot expects
that the memory type is WB.
Yes.
static u8 mtrr_default_type(struct kvm_mtrr *mtrr_state)
{
if (mtrr_is_enabled(mtrr_state))
return mtrr_state->deftype &
IA32_MTRR_DEF_TYPE_TYPE_MASK;
else
return MTRR_TYPE_UNCACHABLE;
}
? Then it's easy to add a quirk that makes the default WRITEBACK until
MTRRs are enabled.
It is the wrong configure in OVMF... shall we need to adjust KVM to
satisfy
OVMF?
That's what quirks are for... The firmware should still be fixed of
course.
I see, will do it.
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.

(Refer to "Firmware image structure" in the OVMF whitepaper,
<http://www.linux-kvm.org/page/OVMF>.)

Perhaps also significant, with this initial MTRR change: the x86_64
reset vector builds some page tables too. (Refer to "Select features |
X64-specific reset vector for OVMF" in the OVMF whitepaper.)

(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)

Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
(It is recommended to refer heavily to the OVMF whitepaper, or at least
to drink heavily.)

In the PEI phase, we do set up MTRRs sensibly, see
"OvmfPkg/PlatformPei/MemDetect.c", function QemuInitializeRam().
However, that's too late with regard this problem report, because PEI
modules run *from* one of the firmware volumes that SEC expands with LZMA.

Anyway, the logic in QemuInitializeRam() relies on the MtrrLib library
class, which OVMF resolves to the
"UefiCpuPkg/Library/MtrrLib/MtrrLib.inf" instance. The latter has no
client module type restriction (it's BASE), so it could be used in SEC
too, if someone were to write patches.

I'm sorry but I really can't take this on now. So, respectfully:
- please quirk it in KVM for now (SeaBIOS is also affected),
- "patches welcome" for OVMF, as always.

Thanks
Laszlo
Paolo Bonzini
2015-07-14 21:15:36 UTC
Permalink
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs
and set the default type to writeback.

In any case we're going to have to quirk it, because of the broken
guests in the wild.

Paolo
Laszlo Ersek
2015-07-14 21:29:11 UTC
Permalink
Post by Paolo Bonzini
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
That's an idea, yes, if someone feels sufficiently drawn to writing
assembly. Complications:
- the reset vector is specific to OvmfPkg only in the OvmfPkgX64.dsc
build
- it needs to be determined what memory to cover.
Post by Paolo Bonzini
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs
and set the default type to writeback.
Seems safe to me, off the top of my head (and testing could confirm /
disprove quickly).
Post by Paolo Bonzini
In any case we're going to have to quirk it, because of the broken
guests in the wild.
Thanks.
Laszlo
Jordan Justen
2015-07-14 22:37:04 UTC
Permalink
Post by Laszlo Ersek
Post by Paolo Bonzini
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
That's an idea, yes, if someone feels sufficiently drawn to writing
assembly.
Maybe we can use MtrrLib in the SEC C code?

-Jordan
Post by Laszlo Ersek
- the reset vector is specific to OvmfPkg only in the OvmfPkgX64.dsc
build
- it needs to be determined what memory to cover.
Post by Paolo Bonzini
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs
and set the default type to writeback.
Seems safe to me, off the top of my head (and testing could confirm /
disprove quickly).
Post by Paolo Bonzini
In any case we're going to have to quirk it, because of the broken
guests in the wild.
Thanks.
Laszlo
Laszlo Ersek
2015-07-15 09:57:30 UTC
Permalink
Post by Jordan Justen
Post by Laszlo Ersek
Post by Paolo Bonzini
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
That's an idea, yes, if someone feels sufficiently drawn to writing
assembly.
Maybe we can use MtrrLib in the SEC C code?
If the page table building in the reset vector is not too costly with
UC, then yes, it should suffice if MtrrLib was put to use first just
before the decompression in SEC.

Thanks
Laszlo
Post by Jordan Justen
-Jordan
Post by Laszlo Ersek
- the reset vector is specific to OvmfPkg only in the OvmfPkgX64.dsc
build
- it needs to be determined what memory to cover.
Post by Paolo Bonzini
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs
and set the default type to writeback.
Seems safe to me, off the top of my head (and testing could confirm /
disprove quickly).
Post by Paolo Bonzini
In any case we're going to have to quirk it, because of the broken
guests in the wild.
Thanks.
Laszlo
Fan, Jeff
2015-07-15 00:14:14 UTC
Permalink
Actually, MMIO will be used in OVMF SEC phase if local APIC is consumed. (SecPeiDebugAgentLib will consume local APIC timer for communication between debugger TARGET/HOST).

So, I suggest to keep MTRR default value to UC and set the code range to WB, or set default value to WB and set Local APIC space range to UC. (gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress|0xfee00000)

As Jordan's suggestion, you could make use of MTRR lib in UefiCpuPkg.

Jeff

-----Original Message-----
From: Paolo Bonzini [mailto:***@redhat.com]
Sent: Wednesday, July 15, 2015 5:16 AM
To: Laszlo Ersek
Cc: edk2-devel list; Xiao Guangrong; ***@vger.kernel.org; ***@kernel.org; ***@redhat.com; linux-***@vger.kernel.org
Subject: Re: [edk2] MTRR setup in OVMF [was: PATCH v3 01/10 KVM: MMU: fix decoding cache type from MTRR]
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due
to page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only
mentioning it here because it makes me appreciate the current problem
report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs and set the default type to writeback.

In any case we're going to have to quirk it, because of the broken guests in the wild.

Paolo

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-***@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Laszlo Ersek
2015-07-15 19:41:08 UTC
Permalink
Hi,
I have posted the pachset to make OVMF happy and have CCed you guys,
could you please check it if it works for you?
Sorry, I can't check it; I don't have an environment that exposes the
issue in the first place. Perhaps Alex can check it, or refer some of
the users that reported the problem to him to this patchset? (Those
users were using bleeding edge v4.2-rc1 anyway, unlike conservative me.)

Thanks!
Laszlo
Post by Paolo Bonzini
Post by Laszlo Ersek
The long delay that Alex reported (for the case when all guest memory
was set to UC up-front) is due to the fact that the SEC phase of OVMF
decompresses an approximately 1712 KB sized, LZMA-compressed blob, to
approx. 896 KB worth of PEI drivers and 8192 KB worth of DXE and UEFI
drivers -- and this decompression is extremely memory-intensive.
(When Jordan implemented that reset vector first, we saw similar
performance degradation on AMD hosts (albeit not due to MTRR but due to
page attributes). See
<https://github.com/tianocore/edk2/commit/98f378a7>. I'm only mentioning
it here because it makes me appreciate the current problem report.)
Anyway, the reset vector's page table building is implemented in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm". The decompression in SEC
can be found in "OvmfPkg/Sec/SecMain.c", function DecompressMemFvs().
Perhaps the OVMF reset vector should initialize the MTRRs for the BSP?
I think SEC doesn't do any MMIO, so it should be enough to enable MTRRs
and set the default type to writeback.
In any case we're going to have to quirk it, because of the broken
guests in the wild.
Paolo
Loading...