[edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[xueshengfeng via groups.io]

From: "Wu, MingliangX" <mingliangx.wu@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511

With 64 bit build we are seeing the CD in control register CR 0 set.
This causes the NEM to disabled for some specific bios profiles.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Wu, Mingliang <mingliangx.wu@intel.com>
---
 UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
index f59fc6ead4ba..4af2e875c31c 100644
--- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
+++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
@@ -7,7 +7,7 @@
 ;
 ;------------------------------------------------------------------------------
 
-%define SEC_DEFAULT_CR0  0x40000023
+%define SEC_DEFAULT_CR0  0x00000023
 %define SEC_DEFAULT_CR4  0x640
 
 BITS    16
-- 
2.26.2.windows.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107303): https://edk2.groups.io/g/devel/message/107303
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Ni, Ray]

This patch is not right.

Intel SDM explicitly says the initial CR0 value is 6000_0010. CD bit is set.

So the ResetVector code that still sets CD bit should be good.

If you are facing NEM enable failure, can you change your NEM enable logic to explicitly clear CD bit instead of changing here?

Thanks,
Ray


> -----Original Message-----
> From: xueshengfeng <xueshengfeng@byosoft.com.cn>
> Sent: Wednesday, July 26, 2023 5:48 PM
> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Cc: Wu, MingliangX <mingliangx.wu@intel.com>; Wu
> Subject: [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be
> set by default in CR0
> 
> From: "Wu, MingliangX" <mingliangx.wu@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511
> 
> With 64 bit build we are seeing the CD in control register CR 0 set.
> This causes the NEM to disabled for some specific bios profiles.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Wu, Mingliang <mingliangx.wu@intel.com>
> ---
>  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> index f59fc6ead4ba..4af2e875c31c 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> @@ -7,7 +7,7 @@
>  ;
>  ;------------------------------------------------------------------------------
> 
> -%define SEC_DEFAULT_CR0  0x40000023
> +%define SEC_DEFAULT_CR0  0x00000023
>  %define SEC_DEFAULT_CR4  0x640
> 
>  BITS    16
> --
> 2.26.2.windows.1
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107267): https://edk2.groups.io/g/devel/message/107267
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Ni, Ray]

The patch resolves an issue in Boot Guard enabled system that NEM is already enabled by Boot Guard, disabling cache evicts all cache content which is unexpected.

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Wednesday, July 26, 2023 5:56 PM
> To: Xue, Shengfeng <xueshengfeng@byosoft.com.cn>; devel@edk2.groups.io;
> Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West, Catharine
> <catharine.west@intel.com>
> Cc: Wu, MingliangX <mingliangx.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> Disable should not be set by default in CR0
> 
> This patch is not right.
> 
> Intel SDM explicitly says the initial CR0 value is 6000_0010. CD bit is set.
> 
> So the ResetVector code that still sets CD bit should be good.
> 
> If you are facing NEM enable failure, can you change your NEM enable logic to
> explicitly clear CD bit instead of changing here?
> 
> Thanks,
> Ray
> 
> 
> > -----Original Message-----
> > From: xueshengfeng <xueshengfeng@byosoft.com.cn>
> > Sent: Wednesday, July 26, 2023 5:48 PM
> > To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> > kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West, Catharine
> > <catharine.west@intel.com>
> > Cc: Wu, MingliangX <mingliangx.wu@intel.com>; Wu
> > Subject: [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be
> > set by default in CR0
> >
> > From: "Wu, MingliangX" <mingliangx.wu@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511
> >
> > With 64 bit build we are seeing the CD in control register CR 0 set.
> > This causes the NEM to disabled for some specific bios profiles.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Debkumar De <debkumar.de@intel.com>
> > Cc: Catharine West <catharine.west@intel.com>
> > Signed-off-by: Wu, Mingliang <mingliangx.wu@intel.com>
> > ---
> >  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > index f59fc6ead4ba..4af2e875c31c 100644
> > --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > @@ -7,7 +7,7 @@
> >  ;
> >  ;------------------------------------------------------------------------------
> >
> > -%define SEC_DEFAULT_CR0  0x40000023
> > +%define SEC_DEFAULT_CR0  0x00000023
> >  %define SEC_DEFAULT_CR4  0x640
> >
> >  BITS    16
> > --
> > 2.26.2.windows.1
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107527): https://edk2.groups.io/g/devel/message/107527
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Min Xu]

This patch causes a regression when launching a vm guest with below command:

$ /usr/libexec/qemu-kvm  \
-name guestVM1 -machine q35 -accel kvm -m 10240 -smp 8 -cpu host -monitor pty \
-drive format=raw,file=/home/tdvf/centos-stream-9.img  \
-bios /home/tdvf/OVMF.fd \
-nic user,hostfwd=tcp::2222-:22 -nographic \
-object iommufd,id=iommufd0 \
-device intel-iommu,caching-mode=on,dma-drain=on,x-scalable-mode="modern",x-pasid-mode=true,device-iotlb=on,iommufd=iommufd0 \
-device vfio-pci,sysfsdev=/sys/bus/dsa/devices/vdev0.0,iommufd=iommufd0,bypass-iommu=false

Commit e8aa4c6546 (this patch has been merged) clear the CD bit in CR0 when transferring from real16 mode to 32bit protect mode. After the patch is applied,  it costs about 60s in DecompressMemFvs@SecMain.c. 

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, August 3, 2023 4:14 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Xue, Shengfeng
> <xueshengfeng@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; kraxel@redhat.com; De, Debkumar
> <debkumar.de@intel.com>; West, Catharine <catharine.west@intel.com>
> Cc: Wu, MingliangX <mingliangx.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> Disable should not be set by default in CR0
> 
> The patch resolves an issue in Boot Guard enabled system that NEM is already
> enabled by Boot Guard, disabling cache evicts all cache content which is
> unexpected.
> 
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> > Sent: Wednesday, July 26, 2023 5:56 PM
> > To: Xue, Shengfeng <xueshengfeng@byosoft.com.cn>;
> > devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R
> > <rahul.r.kumar@intel.com>; kraxel@redhat.com; De, Debkumar
> > <debkumar.de@intel.com>; West, Catharine <catharine.west@intel.com>
> > Cc: Wu, MingliangX <mingliangx.wu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> > Disable should not be set by default in CR0
> >
> > This patch is not right.
> >
> > Intel SDM explicitly says the initial CR0 value is 6000_0010. CD bit is set.
> >
> > So the ResetVector code that still sets CD bit should be good.
> >
> > If you are facing NEM enable failure, can you change your NEM enable
> > logic to explicitly clear CD bit instead of changing here?
> >
> > Thanks,
> > Ray
> >
> >
> > > -----Original Message-----
> > > From: xueshengfeng <xueshengfeng@byosoft.com.cn>
> > > Sent: Wednesday, July 26, 2023 5:48 PM
> > > To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> > > kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West,
> > > Catharine <catharine.west@intel.com>
> > > Cc: Wu, MingliangX <mingliangx.wu@intel.com>; Wu
> > > Subject: [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should
> > > not be set by default in CR0
> > >
> > > From: "Wu, MingliangX" <mingliangx.wu@intel.com>
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511
> > >
> > > With 64 bit build we are seeing the CD in control register CR 0 set.
> > > This causes the NEM to disabled for some specific bios profiles.
> > >
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: Debkumar De <debkumar.de@intel.com>
> > > Cc: Catharine West <catharine.west@intel.com>
> > > Signed-off-by: Wu, Mingliang <mingliangx.wu@intel.com>
> > > ---
> > >  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > index f59fc6ead4ba..4af2e875c31c 100644
> > > --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > @@ -7,7 +7,7 @@
> > >  ;
> > >
> > > ;-------------------------------------------------------------------
> > > -----------
> > >
> > > -%define SEC_DEFAULT_CR0  0x40000023
> > > +%define SEC_DEFAULT_CR0  0x00000023
> > >  %define SEC_DEFAULT_CR4  0x640
> > >
> > >  BITS    16
> > > --
> > > 2.26.2.windows.1
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113517): https://edk2.groups.io/g/devel/message/113517
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[West, Catharine]

Disabling cache by default results in violation of BTG protections (if BTG enabled).
 
BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.

Whatever solution needs to be done here cannot evict ACM-enabled NEM.

Why is boot time increasing?  In this failing case was ACM executed / cache enabled by ACM?  If not, then CD should be 0 by hardware default right?

thanks
Catharine

-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com> 
Sent: Tuesday, January 9, 2024 11:52 PM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Wu, MingliangX <mingliangx.wu@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Xue, Shengfeng <xueshengfeng@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West, Catharine <catharine.west@intel.com>; Xu, Min M <min.m.xu@intel.com>
Subject: RE: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

This patch causes a regression when launching a vm guest with below command:

$ /usr/libexec/qemu-kvm  \
-name guestVM1 -machine q35 -accel kvm -m 10240 -smp 8 -cpu host -monitor pty \ -drive format=raw,file=/home/tdvf/centos-stream-9.img  \ -bios /home/tdvf/OVMF.fd \ -nic user,hostfwd=tcp::2222-:22 -nographic \ -object iommufd,id=iommufd0 \ -device intel-iommu,caching-mode=on,dma-drain=on,x-scalable-mode="modern",x-pasid-mode=true,device-iotlb=on,iommufd=iommufd0 \ -device vfio-pci,sysfsdev=/sys/bus/dsa/devices/vdev0.0,iommufd=iommufd0,bypass-iommu=false

Commit e8aa4c6546 (this patch has been merged) clear the CD bit in CR0 when transferring from real16 mode to 32bit protect mode. After the patch is applied,  it costs about 60s in DecompressMemFvs@SecMain.c. 

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, August 3, 2023 4:14 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Xue, Shengfeng 
> <xueshengfeng@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; 
> Kumar, Rahul R <rahul.r.kumar@intel.com>; kraxel@redhat.com; De, 
> Debkumar <debkumar.de@intel.com>; West, Catharine 
> <catharine.west@intel.com>
> Cc: Wu, MingliangX <mingliangx.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache 
> Disable should not be set by default in CR0
> 
> The patch resolves an issue in Boot Guard enabled system that NEM is 
> already enabled by Boot Guard, disabling cache evicts all cache 
> content which is unexpected.
> 
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, 
> > Ray
> > Sent: Wednesday, July 26, 2023 5:56 PM
> > To: Xue, Shengfeng <xueshengfeng@byosoft.com.cn>; 
> > devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul 
> > R <rahul.r.kumar@intel.com>; kraxel@redhat.com; De, Debkumar 
> > <debkumar.de@intel.com>; West, Catharine <catharine.west@intel.com>
> > Cc: Wu, MingliangX <mingliangx.wu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: 
> > Cache Disable should not be set by default in CR0
> >
> > This patch is not right.
> >
> > Intel SDM explicitly says the initial CR0 value is 6000_0010. CD bit is set.
> >
> > So the ResetVector code that still sets CD bit should be good.
> >
> > If you are facing NEM enable failure, can you change your NEM enable 
> > logic to explicitly clear CD bit instead of changing here?
> >
> > Thanks,
> > Ray
> >
> >
> > > -----Original Message-----
> > > From: xueshengfeng <xueshengfeng@byosoft.com.cn>
> > > Sent: Wednesday, July 26, 2023 5:48 PM
> > > To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>; Ni, 
> > > Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; 
> > > kraxel@redhat.com; De, Debkumar <debkumar.de@intel.com>; West, 
> > > Catharine <catharine.west@intel.com>
> > > Cc: Wu, MingliangX <mingliangx.wu@intel.com>; Wu
> > > Subject: [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable 
> > > should not be set by default in CR0
> > >
> > > From: "Wu, MingliangX" <mingliangx.wu@intel.com>
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4511
> > >
> > > With 64 bit build we are seeing the CD in control register CR 0 set.
> > > This causes the NEM to disabled for some specific bios profiles.
> > >
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: Debkumar De <debkumar.de@intel.com>
> > > Cc: Catharine West <catharine.west@intel.com>
> > > Signed-off-by: Wu, Mingliang <mingliangx.wu@intel.com>
> > > ---
> > >  UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > index f59fc6ead4ba..4af2e875c31c 100644
> > > --- a/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > +++ b/UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
> > > @@ -7,7 +7,7 @@
> > >  ;
> > >
> > > ;-----------------------------------------------------------------
> > > --
> > > -----------
> > >
> > > -%define SEC_DEFAULT_CR0  0x40000023
> > > +%define SEC_DEFAULT_CR0  0x00000023
> > >  %define SEC_DEFAULT_CR4  0x640
> > >
> > >  BITS    16
> > > --
> > > 2.26.2.windows.1
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113536): https://edk2.groups.io/g/devel/message/113536
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Gerd Hoffmann]

On Wed, Jan 10, 2024 at 04:43:47PM +0000, West, Catharine wrote:
> Disabling cache by default results in violation of BTG protections (if BTG enabled).
>  
> BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.
> 
> Whatever solution needs to be done here cannot evict ACM-enabled NEM.

Well, it's OVMF in a virtual machine.  No boot guard involved.
So we could probably go for a OVMF-specific patch here.

But I'd prefer to figure what exactly is happening here before going
down that route.  An extreme slowdown just because we flip that bit
doesn't make sense to me.

> Why is boot time increasing?

Not clear.  It seems to be the lzma uncompress of the firmware volume
in rom / pflash which is very slow.  Also it is apparently only
triggered in case pci device assignment is used.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113996): https://edk2.groups.io/g/devel/message/113996
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Brian J. Johnson]

On 1/18/24 09:46, Gerd Hoffmann wrote:
> On Wed, Jan 10, 2024 at 04:43:47PM +0000, West, Catharine wrote:
>> Disabling cache by default results in violation of BTG protections (if BTG enabled).
>>   
>> BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.
>>
>> Whatever solution needs to be done here cannot evict ACM-enabled NEM.
> 
> Well, it's OVMF in a virtual machine.  No boot guard involved.
> So we could probably go for a OVMF-specific patch here.
> 
> But I'd prefer to figure what exactly is happening here before going
> down that route.  An extreme slowdown just because we flip that bit
> doesn't make sense to me.
> 
>> Why is boot time increasing?
> 
> Not clear.  It seems to be the lzma uncompress of the firmware volume
> in rom / pflash which is very slow.  Also it is apparently only
> triggered in case pci device assignment is used.

I've seen extreme slowness on physical platforms when we've mixed up the 
MTRRs or page tables, causing code to be mapped uncached.

Lzma uncompress of ROM could be pretty slow as well, if the ROM is being 
read uncached.  Lzma probably reads the data a byte at a time, which is 
the worst case for uncached accesses.  Since this is a VM, it's not 
actually uncached at the hardware level, but I don't know how QEMU/KVM 
handles uncached guest mappings.... It may be doing a VMEXIT for every byte.

Anyway, I suggest double-checking your page tables and MTRRs.
-- 
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114145): https://edk2.groups.io/g/devel/message/114145
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Min Xu]

Thanks much Johnson! We will investigate it based on your comments.

> -----Original Message-----
> From: Brian J. Johnson <brian.johnson@hpe.com>
> Sent: Tuesday, January 23, 2024 3:12 AM
> To: devel@edk2.groups.io; kraxel@redhat.com; West, Catharine
> <catharine.west@intel.com>
> Cc: Xu, Min M <min.m.xu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu,
> MingliangX <mingliangx.wu@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Xue, Shengfeng <xueshengfeng@byosoft.com.cn>;
> Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; De, Debkumar <debkumar.de@intel.com>
> Subject: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> Disable should not be set by default in CR0
> 
> On 1/18/24 09:46, Gerd Hoffmann wrote:
> > On Wed, Jan 10, 2024 at 04:43:47PM +0000, West, Catharine wrote:
> >> Disabling cache by default results in violation of BTG protections (if BTG
> enabled).
> >>
> >> BIOS cannot assume that cache is disabled before it executes as ACM may
> be required to enable NEM.
> >>
> >> Whatever solution needs to be done here cannot evict ACM-enabled
> NEM.
> >
> > Well, it's OVMF in a virtual machine.  No boot guard involved.
> > So we could probably go for a OVMF-specific patch here.
> >
> > But I'd prefer to figure what exactly is happening here before going
> > down that route.  An extreme slowdown just because we flip that bit
> > doesn't make sense to me.
> >
> >> Why is boot time increasing?
> >
> > Not clear.  It seems to be the lzma uncompress of the firmware volume
> > in rom / pflash which is very slow.  Also it is apparently only
> > triggered in case pci device assignment is used.
> 
> I've seen extreme slowness on physical platforms when we've mixed up the
> MTRRs or page tables, causing code to be mapped uncached.
> 
> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
> read uncached.  Lzma probably reads the data a byte at a time, which is the
> worst case for uncached accesses.  Since this is a VM, it's not actually
> uncached at the hardware level, but I don't know how QEMU/KVM handles
> uncached guest mappings.... It may be doing a VMEXIT for every byte.
> 
> Anyway, I suggest double-checking your page tables and MTRRs.
> --
> Brian J. Johnson
> Enterprise X86 Lab
> Hewlett Packard Enterprise


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114173): https://edk2.groups.io/g/devel/message/114173
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Gerd Hoffmann]

On Mon, Jan 22, 2024 at 01:11:52PM -0600, Brian J. Johnson wrote:
> On 1/18/24 09:46, Gerd Hoffmann wrote:
> > On Wed, Jan 10, 2024 at 04:43:47PM +0000, West, Catharine wrote:
> > > Disabling cache by default results in violation of BTG protections (if BTG enabled).
> > > BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.
> > > 
> > > Whatever solution needs to be done here cannot evict ACM-enabled NEM.
> > 
> > Well, it's OVMF in a virtual machine.  No boot guard involved.
> > So we could probably go for a OVMF-specific patch here.
> > 
> > But I'd prefer to figure what exactly is happening here before going
> > down that route.  An extreme slowdown just because we flip that bit
> > doesn't make sense to me.
> > 
> > > Why is boot time increasing?
> > 
> > Not clear.  It seems to be the lzma uncompress of the firmware volume
> > in rom / pflash which is very slow.  Also it is apparently only
> > triggered in case pci device assignment is used.
> 
> I've seen extreme slowness on physical platforms when we've mixed up the
> MTRRs or page tables, causing code to be mapped uncached.
> 
> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
> read uncached.  Lzma probably reads the data a byte at a time, which is the
> worst case for uncached accesses.  Since this is a VM, it's not actually
> uncached at the hardware level, but I don't know how QEMU/KVM handles
> uncached guest mappings.... It may be doing a VMEXIT for every byte.
> 
> Anyway, I suggest double-checking your page tables and MTRRs.

It happens very early at boot, before MTRRs are setup, running on the
initial page tables created by the OVMF reset vector.  The initial page
tables have just 'accessed', 'dirty', 'read/write' and 'present' bits
set for the 0-4G identity mapping.

It seems to have something to do with EPT.  It does not happen on AMD
processors.  It also does not happen when disabling EPT support in kvm
on the host machine.

looked at kvm kernel traces, I don't see excessive vmexits.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114193): https://edk2.groups.io/g/devel/message/114193
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Laszlo Ersek]

On 1/23/24 11:52, Gerd Hoffmann wrote:
> On Mon, Jan 22, 2024 at 01:11:52PM -0600, Brian J. Johnson wrote:
>> On 1/18/24 09:46, Gerd Hoffmann wrote:
>>> On Wed, Jan 10, 2024 at 04:43:47PM +0000, West, Catharine wrote:
>>>> Disabling cache by default results in violation of BTG protections (if BTG enabled).
>>>> BIOS cannot assume that cache is disabled before it executes as ACM may be required to enable NEM.
>>>>
>>>> Whatever solution needs to be done here cannot evict ACM-enabled NEM.
>>>
>>> Well, it's OVMF in a virtual machine.  No boot guard involved.
>>> So we could probably go for a OVMF-specific patch here.
>>>
>>> But I'd prefer to figure what exactly is happening here before going
>>> down that route.  An extreme slowdown just because we flip that bit
>>> doesn't make sense to me.
>>>
>>>> Why is boot time increasing?
>>>
>>> Not clear.  It seems to be the lzma uncompress of the firmware volume
>>> in rom / pflash which is very slow.  Also it is apparently only
>>> triggered in case pci device assignment is used.
>>
>> I've seen extreme slowness on physical platforms when we've mixed up the
>> MTRRs or page tables, causing code to be mapped uncached.
>>
>> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
>> read uncached.  Lzma probably reads the data a byte at a time, which is the
>> worst case for uncached accesses.  Since this is a VM, it's not actually
>> uncached at the hardware level, but I don't know how QEMU/KVM handles
>> uncached guest mappings.... It may be doing a VMEXIT for every byte.
>>
>> Anyway, I suggest double-checking your page tables and MTRRs.
> 
> It happens very early at boot, before MTRRs are setup, running on the
> initial page tables created by the OVMF reset vector.  The initial page
> tables have just 'accessed', 'dirty', 'read/write' and 'present' bits
> set for the 0-4G identity mapping.
> 
> It seems to have something to do with EPT.  It does not happen on AMD
> processors.  It also does not happen when disabling EPT support in kvm
> on the host machine.
> 
> looked at kvm kernel traces, I don't see excessive vmexits.

This discussion evokes vague memories in me. I'll dump them here, but I
have no idea if they will be useful. (They probably won't.)

- edk2 commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in
initial page tables", 2013-09-24)

- Linux (host) commit 879ae1880449 ("KVM: x86: obey
KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04)

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114202): https://edk2.groups.io/g/devel/message/114202
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Gerd Hoffmann]

  Hi,

> >>> Well, it's OVMF in a virtual machine.  No boot guard involved.
> >>> So we could probably go for a OVMF-specific patch here.
> >>>
> >>> But I'd prefer to figure what exactly is happening here before going
> >>> down that route.  An extreme slowdown just because we flip that bit
> >>> doesn't make sense to me.
> >>>
> >>>> Why is boot time increasing?
> >>>
> >>> Not clear.  It seems to be the lzma uncompress of the firmware volume
> >>> in rom / pflash which is very slow.  Also it is apparently only
> >>> triggered in case pci device assignment is used.
> >>
> >> I've seen extreme slowness on physical platforms when we've mixed up the
> >> MTRRs or page tables, causing code to be mapped uncached.
> >>
> >> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
> >> read uncached.  Lzma probably reads the data a byte at a time, which is the
> >> worst case for uncached accesses.  Since this is a VM, it's not actually
> >> uncached at the hardware level, but I don't know how QEMU/KVM handles
> >> uncached guest mappings.... It may be doing a VMEXIT for every byte.
> >>
> >> Anyway, I suggest double-checking your page tables and MTRRs.
> > 
> > It happens very early at boot, before MTRRs are setup, running on the
> > initial page tables created by the OVMF reset vector.  The initial page
> > tables have just 'accessed', 'dirty', 'read/write' and 'present' bits
> > set for the 0-4G identity mapping.
> > 
> > It seems to have something to do with EPT.  It does not happen on AMD
> > processors.  It also does not happen when disabling EPT support in kvm
> > on the host machine.
> > 
> > looked at kvm kernel traces, I don't see excessive vmexits.
> 
> This discussion evokes vague memories in me. I'll dump them here, but I
> have no idea if they will be useful. (They probably won't.)
> 
> - edk2 commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in
> initial page tables", 2013-09-24)
> 
> - Linux (host) commit 879ae1880449 ("KVM: x86: obey
> KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04)

I actually waded through the source code in both places ;)

Turned out kvm propagates guest MTRR settings to EPT memory types,
but only in case kvm_arch_has_noncoherent_dma() is true, which why
this triggers only with a mdev device assigned.

MTRR disabled gets translated to UNCACHABLE, this is where the
slowdown comes from.  Test patch below fixes it for me.

take care,
  Gerd

----------------------------- cut here ----------------------------
commit eb9f40ffd8afad03ac1fb6ac0e2a9af12ae78152
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Tue Jan 23 15:33:51 2024 +0100

    OvmfPkg/Sec: early mtrr setup
    
    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
index f042517bb64a..14f39236e44d 100644
--- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
@@ -1081,12 +1081,14 @@ PlatformQemuInitializeRam (
   if (IsMtrrSupported () && (PlatformInfoHob->HostBridgeDevId != CLOUDHV_DEVICE_ID)) {
     MtrrGetAllMtrrs (&MtrrSettings);
 
+#if 0
     //
     // MTRRs disabled, fixed MTRRs disabled, default type is uncached
     //
     ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
     ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
     ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
+#endif
 
     //
     // flip default type to writeback
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9bd1b9c95227..2820be1bab7c 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -30,6 +30,7 @@
 #include <Ppi/MpInitLibDep.h>
 #include <Library/TdxHelperLib.h>
 #include <Library/CcProbeLib.h>
+#include <Register/Intel/ArchitecturalMsr.h>
 #include "AmdSev.h"
 
 #define SEC_IDT_ENTRY_COUNT  34
@@ -956,6 +957,19 @@ SecCoreStartupWithStack (
   InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
   DisableApicTimerInterrupt ();
 
+  //
+  // Early MTRR setup (enable + set sefault)
+  //
+  {
+    MSR_IA32_MTRR_DEF_TYPE_REGISTER  DefType;
+
+    DefType.Uint64    = 0;
+    DefType.Bits.Type = 6; /* write back */
+    DefType.Bits.E    = 1; /* enable */
+    AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
+    DEBUG ((DEBUG_ERROR, "%a:%d early mtrr: %lx\n", __func__, __LINE__, DefType.Uint64));
+  }
+
   //
   // Initialize Debug Agent to support source level debug in SEC/PEI phases before memory ready.
   //



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114214): https://edk2.groups.io/g/devel/message/114214
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Min Xu]

Add intel linux guys in CC list.

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Wednesday, January 24, 2024 12:12 AM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: devel@edk2.groups.io; Johnson, Brian <brian.johnson@hpe.com>; West,
> Catharine <catharine.west@intel.com>; Xu, Min M <min.m.xu@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Wu, MingliangX <mingliangx.wu@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Xue, Shengfeng
> <xueshengfeng@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; De, Debkumar
> <debkumar.de@intel.com>
> Subject: Re: Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache
> Disable should not be set by default in CR0
> 
>   Hi,
> 
> > >>> Well, it's OVMF in a virtual machine.  No boot guard involved.
> > >>> So we could probably go for a OVMF-specific patch here.
> > >>>
> > >>> But I'd prefer to figure what exactly is happening here before
> > >>> going down that route.  An extreme slowdown just because we flip
> > >>> that bit doesn't make sense to me.
> > >>>
> > >>>> Why is boot time increasing?
> > >>>
> > >>> Not clear.  It seems to be the lzma uncompress of the firmware
> > >>> volume in rom / pflash which is very slow.  Also it is apparently
> > >>> only triggered in case pci device assignment is used.
> > >>
> > >> I've seen extreme slowness on physical platforms when we've mixed
> > >> up the MTRRs or page tables, causing code to be mapped uncached.
> > >>
> > >> Lzma uncompress of ROM could be pretty slow as well, if the ROM is
> > >> being read uncached.  Lzma probably reads the data a byte at a
> > >> time, which is the worst case for uncached accesses.  Since this is
> > >> a VM, it's not actually uncached at the hardware level, but I don't
> > >> know how QEMU/KVM handles uncached guest mappings.... It may be
> doing a VMEXIT for every byte.
> > >>
> > >> Anyway, I suggest double-checking your page tables and MTRRs.
> > >
> > > It happens very early at boot, before MTRRs are setup, running on
> > > the initial page tables created by the OVMF reset vector.  The
> > > initial page tables have just 'accessed', 'dirty', 'read/write' and
> > > 'present' bits set for the 0-4G identity mapping.
> > >
> > > It seems to have something to do with EPT.  It does not happen on
> > > AMD processors.  It also does not happen when disabling EPT support
> > > in kvm on the host machine.
> > >
> > > looked at kvm kernel traces, I don't see excessive vmexits.
> >
> > This discussion evokes vague memories in me. I'll dump them here, but
> > I have no idea if they will be useful. (They probably won't.)
> >
> > - edk2 commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in
> > initial page tables", 2013-09-24)
> >
> > - Linux (host) commit 879ae1880449 ("KVM: x86: obey
> > KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04)
> 
> I actually waded through the source code in both places ;)
> 
> Turned out kvm propagates guest MTRR settings to EPT memory types, but
> only in case kvm_arch_has_noncoherent_dma() is true, which why this
> triggers only with a mdev device assigned.
> 
> MTRR disabled gets translated to UNCACHABLE, this is where the slowdown
> comes from.  Test patch below fixes it for me.
> 
> take care,
>   Gerd
> 
> ----------------------------- cut here ---------------------------- commit
> eb9f40ffd8afad03ac1fb6ac0e2a9af12ae78152
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Tue Jan 23 15:33:51 2024 +0100
> 
>     OvmfPkg/Sec: early mtrr setup
> 
>     Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> index f042517bb64a..14f39236e44d 100644
> --- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> +++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
> @@ -1081,12 +1081,14 @@ PlatformQemuInitializeRam (
>    if (IsMtrrSupported () && (PlatformInfoHob->HostBridgeDevId !=
> CLOUDHV_DEVICE_ID)) {
>      MtrrGetAllMtrrs (&MtrrSettings);
> 
> +#if 0
>      //
>      // MTRRs disabled, fixed MTRRs disabled, default type is uncached
>      //
>      ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
>      ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
>      ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
> +#endif
> 
>      //
>      // flip default type to writeback
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index
> 9bd1b9c95227..2820be1bab7c 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -30,6 +30,7 @@
>  #include <Ppi/MpInitLibDep.h>
>  #include <Library/TdxHelperLib.h>
>  #include <Library/CcProbeLib.h>
> +#include <Register/Intel/ArchitecturalMsr.h>
>  #include "AmdSev.h"
> 
>  #define SEC_IDT_ENTRY_COUNT  34
> @@ -956,6 +957,19 @@ SecCoreStartupWithStack (
>    InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
>    DisableApicTimerInterrupt ();
> 
> +  //
> +  // Early MTRR setup (enable + set sefault)  //  {
> +    MSR_IA32_MTRR_DEF_TYPE_REGISTER  DefType;
> +
> +    DefType.Uint64    = 0;
> +    DefType.Bits.Type = 6; /* write back */
> +    DefType.Bits.E    = 1; /* enable */
> +    AsmWriteMsr64 (MSR_IA32_MTRR_DEF_TYPE, DefType.Uint64);
> +    DEBUG ((DEBUG_ERROR, "%a:%d early mtrr: %lx\n", __func__, __LINE__,
> + DefType.Uint64));  }
> +
>    //
>    // Initialize Debug Agent to support source level debug in SEC/PEI phases
> before memory ready.
>    //



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114249): https://edk2.groups.io/g/devel/message/114249
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Laszlo Ersek]

On 1/23/24 17:11, Gerd Hoffmann wrote:
>   Hi,
> 
>>>>> Well, it's OVMF in a virtual machine.  No boot guard involved.
>>>>> So we could probably go for a OVMF-specific patch here.
>>>>>
>>>>> But I'd prefer to figure what exactly is happening here before going
>>>>> down that route.  An extreme slowdown just because we flip that bit
>>>>> doesn't make sense to me.
>>>>>
>>>>>> Why is boot time increasing?
>>>>>
>>>>> Not clear.  It seems to be the lzma uncompress of the firmware volume
>>>>> in rom / pflash which is very slow.  Also it is apparently only
>>>>> triggered in case pci device assignment is used.
>>>>
>>>> I've seen extreme slowness on physical platforms when we've mixed up the
>>>> MTRRs or page tables, causing code to be mapped uncached.
>>>>
>>>> Lzma uncompress of ROM could be pretty slow as well, if the ROM is being
>>>> read uncached.  Lzma probably reads the data a byte at a time, which is the
>>>> worst case for uncached accesses.  Since this is a VM, it's not actually
>>>> uncached at the hardware level, but I don't know how QEMU/KVM handles
>>>> uncached guest mappings.... It may be doing a VMEXIT for every byte.
>>>>
>>>> Anyway, I suggest double-checking your page tables and MTRRs.
>>>
>>> It happens very early at boot, before MTRRs are setup, running on the
>>> initial page tables created by the OVMF reset vector.  The initial page
>>> tables have just 'accessed', 'dirty', 'read/write' and 'present' bits
>>> set for the 0-4G identity mapping.
>>>
>>> It seems to have something to do with EPT.  It does not happen on AMD
>>> processors.  It also does not happen when disabling EPT support in kvm
>>> on the host machine.
>>>
>>> looked at kvm kernel traces, I don't see excessive vmexits.
>>
>> This discussion evokes vague memories in me. I'll dump them here, but I
>> have no idea if they will be useful. (They probably won't.)
>>
>> - edk2 commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in
>> initial page tables", 2013-09-24)
>>
>> - Linux (host) commit 879ae1880449 ("KVM: x86: obey
>> KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04)
> 
> I actually waded through the source code in both places ;)
> 
> Turned out kvm propagates guest MTRR settings to EPT memory types,
> but only in case kvm_arch_has_noncoherent_dma() is true, which why
> this triggers only with a mdev device assigned.

... I should not have stopped myself. :)

(I'm aware that this is on edk2-devel, but a public reference to
virt-staff cannot hurt.)

So, yesterday I read your status on virt-staff, and I found an entry in
it that resembled this upstream thread pretty closely. However, your
status was the *only* mention of "mdev" specifically, and so I wasn't
sure if *mdev* meant the same thing as the more generic upstream
expression "pci device assignment" (see it above in the context).

Furthermore, I saw kvm_arch_has_noncoherent_dma() in my linux commit
879ae1880449, which superficially resembled device assignment, but... I
dismissed it. In the end, I only managed (and even that, only
reluctantly) the above pointers... Thanks for tracking it down!

But then, next question: why has this problem *not* been reported
repeatedly? There's a whole bunch of users (gamers) that run Windows
guests with device (GPU) assignment. I'm sure they'd absolutely complain
about very slow OVMF boot (like they actually have, in the past, about
similar LZMA slowdowns due to improper caching setup).

Something must be special about Min's assigned device.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114285): https://edk2.groups.io/g/devel/message/114285
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Gerd Hoffmann]

  Hi,

> So, yesterday I read your status on virt-staff, and I found an entry in
> it that resembled this upstream thread pretty closely. However, your
> status was the *only* mention of "mdev" specifically, and so I wasn't
> sure if *mdev* meant the same thing as the more generic upstream
> expression "pci device assignment" (see it above in the context).
> 
> Furthermore, I saw kvm_arch_has_noncoherent_dma() in my linux commit
> 879ae1880449, which superficially resembled device assignment, but... I
> dismissed it. In the end, I only managed (and even that, only
> reluctantly) the above pointers... Thanks for tracking it down!
> 
> But then, next question: why has this problem *not* been reported
> repeatedly? There's a whole bunch of users (gamers) that run Windows
> guests with device (GPU) assignment. I'm sure they'd absolutely complain
> about very slow OVMF boot (like they actually have, in the past, about
> similar LZMA slowdowns due to improper caching setup).

static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
{
[ ... ]
         * 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.
[ ... ]

> Something must be special about Min's assigned device.

Yep.  I think the magic word is "snoop control".  When pci-assigning a
*real* pci device VT-d (aka iommu) handles cache control that way.  When
assigning a mdev device this is not the case.

mdev is a virtual pci device emulated by the kernel.  This can be purely
virtual (see samples/vfio-mdev/mtty.c in the linux kernel, which can be
used to reproduce this).  More typical is hardware-assisted device
partitioning, used for some intel and nvidia gpus.  Roughly comparable
with SR/IOV, but not implemented completely in hardware, the kernel has
some device-specific support code instead.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114292): https://edk2.groups.io/g/devel/message/114292
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Laszlo Ersek]

On 1/24/24 14:26, Gerd Hoffmann wrote:
>   Hi,
> 
>> So, yesterday I read your status on virt-staff, and I found an entry in
>> it that resembled this upstream thread pretty closely. However, your
>> status was the *only* mention of "mdev" specifically, and so I wasn't
>> sure if *mdev* meant the same thing as the more generic upstream
>> expression "pci device assignment" (see it above in the context).
>>
>> Furthermore, I saw kvm_arch_has_noncoherent_dma() in my linux commit
>> 879ae1880449, which superficially resembled device assignment, but... I
>> dismissed it. In the end, I only managed (and even that, only
>> reluctantly) the above pointers... Thanks for tracking it down!
>>
>> But then, next question: why has this problem *not* been reported
>> repeatedly? There's a whole bunch of users (gamers) that run Windows
>> guests with device (GPU) assignment. I'm sure they'd absolutely complain
>> about very slow OVMF boot (like they actually have, in the past, about
>> similar LZMA slowdowns due to improper caching setup).
> 
> static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
> {
> [ ... ]
>          * 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.
> [ ... ]
> 
>> Something must be special about Min's assigned device.
> 
> Yep.  I think the magic word is "snoop control".  When pci-assigning a
> *real* pci device VT-d (aka iommu) handles cache control that way.  When
> assigning a mdev device this is not the case.
> 
> mdev is a virtual pci device emulated by the kernel.  This can be purely
> virtual (see samples/vfio-mdev/mtty.c in the linux kernel, which can be
> used to reproduce this).  More typical is hardware-assisted device
> partitioning, used for some intel and nvidia gpus.  Roughly comparable
> with SR/IOV, but not implemented completely in hardware, the kernel has
> some device-specific support code instead.

Very interesting, thanks! ... But, given that mdev is emulated in the
kernel: isn't that *all the more reason* for treating the guest memory
as writeback-cacheable? With a physical assigned device, the IOMMU has
to implement this "snoop control" with extra gymnastics. With an mdev (a
device emulated in the host kernel), there is just one "coherency
domain" -- we'd have to do extra gymnastics for *breaking* cache
coherence (or put differently, for simulating noncoherent DMA). It seems
to me that with only mdevs assigned, DMA should be assumed coherent (=
"snoop control" should be assumed).

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114305): https://edk2.groups.io/g/devel/message/114305
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V1 1/1] UefiCpuPkg/ResetVector: Cache Disable should not be set by default in CR0

[Gerd Hoffmann]

  Hi,

> > static u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
> > {
> > [ ... ]
> >          * 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.
> > [ ... ]
> > 
> >> Something must be special about Min's assigned device.
> > 
> > Yep.  I think the magic word is "snoop control".  When pci-assigning a
> > *real* pci device VT-d (aka iommu) handles cache control that way.  When
> > assigning a mdev device this is not the case.
> > 
> > mdev is a virtual pci device emulated by the kernel.  This can be purely
> > virtual (see samples/vfio-mdev/mtty.c in the linux kernel, which can be
> > used to reproduce this).  More typical is hardware-assisted device
> > partitioning, used for some intel and nvidia gpus.  Roughly comparable
> > with SR/IOV, but not implemented completely in hardware, the kernel has
> > some device-specific support code instead.
> 
> Very interesting, thanks! ... But, given that mdev is emulated in the
> kernel: isn't that *all the more reason* for treating the guest memory
> as writeback-cacheable?

For a 100% emulated device this would make sense indeed.  When making
some GPU resources available to VMs (including giving the GPU DMA access
to guest memory) not so much.  The later is the case with the
intel/nvidia gpu mdev devices.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114333): https://edk2.groups.io/g/devel/message/114333
Mute This Topic: https://groups.io/mt/100367559/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-