[pve-devel] [PATCH edk2-firmware v2] update to edk2-stable202308

[pve-devel] [PATCH edk2-firmware v2] update to edk2-stable202308

[Stefan Hanreich]

* Removed 0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
  since it has been included upstream since commit c1e85376 [1].

* Updated the patch
  Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl so it applies
  again.

* had to increase FD_SIZE to 4M for the x64 build as otherwise the
  package wouldn't build due to the size of the resulting image

[1] https://github.com/tianocore/edk2/commit/c1e853769046b322690ad336fdb98966757e7414

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

Changes from v1 -> v2:
* fixed metadata in ArmVirtPkg patch

Wasn't able to build this package without setting FD_SIZE to 4M, not
sure if this is due to my failure or if the image just got bigger with
the new changes.

Due to CRLF shenanigans I have made this commit available via my repos
as well - you can find it under staff/s.hanreich/pve-edk2-firmware on
the branch 2023-08.

Still not sure if the line endings are 100% correct since I have
warnings about EOL and trailing spaces, but I think they are because
of gits CRLF handling.

 debian/changelog                              |  6 +++
 ...latformInitLib-limit-phys-bits-to-46.patch | 43 -------------------
 ...g-make-EFI_LOADER_DATA-non-executabl.patch |  6 +--
 debian/patches/series                         |  1 -
 debian/rules                                  |  3 +-
 edk2                                          |  2 +-
 6 files changed, 11 insertions(+), 50 deletions(-)
 delete mode 100644 debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch

diff --git a/debian/changelog b/debian/changelog
index 4d0ac21..dd61c6f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+pve-edk2-firmware (3.20230826-1) bookworm; urgency=medium
+
+  * update sources to upstream edk2-stable202308 tag
+
+ -- Proxmox Support Team <support@proxmox.com>  Tue, 29 Aug 2023 13:58:15 +0200
+
 pve-edk2-firmware (3.20230228-4) bookworm; urgency=medium

   * limiting the phys-bits to 46 instead of 47 to work around older guest
diff --git a/debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch b/debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
deleted file mode 100644
index 1708f40..0000000
--- a/debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
+++ /dev/null
@@ -1,43 +0,0 @@
-From 89a12f2a42b989e7925b4a71e503209971eaa271 Mon Sep 17 00:00:00 2001
-From: Gerd Hoffmann <kraxel@redhat.com>
-Date: Thu, 1 Jun 2023 09:57:31 +0200
-Subject: [PATCH] OvmfPkg/PlatformInitLib: limit phys-bits to 46.
-
-Older linux kernels have problems with phys-bits larger than 46,
-ubuntu 18.04 (kernel 4.15) has been reported to be affected.
-
-Reduce phys-bits limit from 47 to 46.
-
-Reported-by: Fiona Ebner <f.ebner@proxmox.com>
-Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
----
- OvmfPkg/Library/PlatformInitLib/MemDetect.c | 9 ++++++---
- 1 file changed, 6 insertions(+), 3 deletions(-)
-
-diff --git a/OvmfPkg/Library/PlatformInitLib/MemDetect.c b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
-index 38cece9173..4d0522ce22 100644
---- a/OvmfPkg/Library/PlatformInitLib/MemDetect.c
-+++ b/OvmfPkg/Library/PlatformInitLib/MemDetect.c
-@@ -657,16 +657,19 @@ PlatformAddressWidthFromCpuid (
-     ));
- 
-   if (Valid) {
--    if (PhysBits > 47) {
-+    if (PhysBits > 46) {
-       /*
-        * Avoid 5-level paging altogether for now, which limits
-        * PhysBits to 48.  Also avoid using address bit 48, due to sign
-        * extension we can't identity-map these addresses (and lots of
-        * places in edk2 assume we have everything identity-mapped).
-        * So the actual limit is 47.
-+       *
-+       * Also some older linux kernels apparently have problems handling
-+       * phys-bits > 46 correctly, so use that as limit.
-        */
--      DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 47 (avoid 5-level paging)\n", __func__));
--      PhysBits = 47;
-+      DEBUG ((DEBUG_INFO, "%a: limit PhysBits to 46 (avoid 5-level paging)\n", __func__));
-+      PhysBits = 46;
-     }
- 
-     if (!Page1GSupport && (PhysBits > 40)) {
diff --git a/debian/patches/Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch b/debian/patches/Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch
index 7e1417a..d389cfd 100644
--- a/debian/patches/Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch
+++ b/debian/patches/Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch
@@ -9,12 +9,12 @@ Last-Update: 2023-03-09

 --- a/ArmVirtPkg/ArmVirt.dsc.inc
 +++ b/ArmVirtPkg/ArmVirt.dsc.inc
-@@ -361,7 +361,7 @@
+@@ -365,7 +365,7 @@
    # reserved ones, with the exception of LoaderData regions, of which OS loaders
    # (i.e., GRUB) may assume that its contents are executable.
    #
 -  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD5
 +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
  
- [Components.common]
-   #
+   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|TRUE
+ 
diff --git a/debian/patches/series b/debian/patches/series
index a9ee2be..f1ec614 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -2,4 +2,3 @@ no-stack-protector-all-archs.diff
 brotlicompress-disable.diff
 x64-baseline-abi.patch
 Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch
-0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
diff --git a/debian/rules b/debian/rules
index eb0ee89..934fe50 100755
--- a/debian/rules
+++ b/debian/rules
@@ -29,7 +29,6 @@ COMMON_FLAGS += -DPVSCSI_ENABLE=TRUE
 OVMF_COMMON_FLAGS = $(COMMON_FLAGS)
 OVMF_2M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_2MB
 OVMF_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB
-OVMF_2M_SMM_FLAGS = $(OVMF_2M_FLAGS) -DSMM_REQUIRE=TRUE
 OVMF_4M_SMM_FLAGS = $(OVMF_4M_FLAGS) -DSMM_REQUIRE=TRUE
 OVMF32_4M_FLAGS = $(OVMF_COMMON_FLAGS) -DFD_SIZE_4MB
 OVMF32_4M_SMM_FLAGS =  $(OVMF32_4M_FLAGS) -DSMM_REQUIRE=TRUE
@@ -132,7 +131,7 @@ $(OVMF_BINARIES) $(OVMF_IMAGES): debian/setup-build-stamp
 			-t $(EDK2_TOOLCHAIN) \
 			-p OvmfPkg/OvmfPkgX64.dsc \
 			$(PCD_OPTIONS) \
-			$(OVMF_2M_SMM_FLAGS) -b $(BUILD_TYPE)
+			$(OVMF_4M_SMM_FLAGS) -b $(BUILD_TYPE)
 	cp $(OVMF_BUILD_DIR)/FV/OVMF_CODE.fd \
 		$(OVMF_INSTALL_DIR)/OVMF_CODE.secboot.fd
 	rm -rf Build/OvmfX64
diff --git a/edk2 b/edk2
index f80f052..819cfc6 160000
--- a/edk2
+++ b/edk2
@@ -1 +1 @@
-Subproject commit f80f052277c88a67c55e107b550f504eeea947d3
+Subproject commit 819cfc6b42a68790a23509e4fcc58ceb70e1965e
--
2.39.2

Re: [pve-devel] [PATCH edk2-firmware v2] update to edk2-stable202308

[Thomas Lamprecht]

Am 30/08/2023 um 17:50 schrieb Stefan Hanreich:
> * Removed 0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
>   since it has been included upstream since commit c1e85376 [1].
> 
> * Updated the patch
>   Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl so it applies
>   again.
> 
> * had to increase FD_SIZE to 4M for the x64 build as otherwise the
>   package wouldn't build due to the size of the resulting image

I mean new versions are great and what not, but is there a reason why you
send this update now? Maybe fix some issue our users face? If so, it'd be
great if you could record that here.

> 
> [1] https://github.com/tianocore/edk2/commit/c1e853769046b322690ad336fdb98966757e7414
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Changes from v1 -> v2:
> * fixed metadata in ArmVirtPkg patch
> 
> Wasn't able to build this package without setting FD_SIZE to 4M, not
> sure if this is due to my failure or if the image just got bigger with
> the new changes.

isn't this breaking live-migration and snapshot rollback compat though
for old machines using that?

For new ones we only use 4 MB images anyway, so if, I'd rather move a
existing build for 2MB images in some pve-edk2-firmware-legacy package
for backward compat and then drop building those variants here completely.

Would be great if you could check qemu-server history for when the 2MB
SMM ones where used, if at all, could help deciding things.

> 
> Due to CRLF shenanigans I have made this commit available via my repos
> as well - you can find it under staff/s.hanreich/pve-edk2-firmware on
> the branch 2023-08.
> 
> Still not sure if the line endings are 100% correct since I have
> warnings about EOL and trailing spaces, but I think they are because
> of gits CRLF handling.
> 
>  debian/changelog                              |  6 +++
>  ...latformInitLib-limit-phys-bits-to-46.patch | 43 -------------------
>  ...g-make-EFI_LOADER_DATA-non-executabl.patch |  6 +--
>  debian/patches/series                         |  1 -
>  debian/rules                                  |  3 +-
>  edk2                                          |  2 +-
>  6 files changed, 11 insertions(+), 50 deletions(-)
>  delete mode 100644 debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
> 
> diff --git a/debian/changelog b/debian/changelog
> index 4d0ac21..dd61c6f 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +pve-edk2-firmware (3.20230826-1) bookworm; urgency=medium
> +
> +  * update sources to upstream edk2-stable202308 tag
> +
> + -- Proxmox Support Team <support@proxmox.com>  Tue, 29 Aug 2023 13:58:15 +0200
> +

bumps should be separate commits and restricted to plain version and
changelog metadata, like here it'd only touch d/changelog.

Also, it's most of the time best to let the release team handle the
bumps.

Re: [pve-devel] [PATCH edk2-firmware v2] update to edk2-stable202308

[Stefan Hanreich]

On 8/31/23 14:39, Thomas Lamprecht wrote:
> Am 30/08/2023 um 17:50 schrieb Stefan Hanreich:
>> * Removed 0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
>>    since it has been included upstream since commit c1e85376 [1].
>>
>> * Updated the patch
>>    Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl so it applies
>>    again.
>>
>> * had to increase FD_SIZE to 4M for the x64 build as otherwise the
>>    package wouldn't build due to the size of the resulting image
> 
> I mean new versions are great and what not, but is there a reason why you
> send this update now? Maybe fix some issue our users face? If so, it'd be
> great if you could record that here.
>

I've been trying to reproduce an issue one of our users is facing and 
wanted to check whether newer firmware versions trigger the issue as 
well. Currently, this issue seems fixed with the newer firmware although 
it's hard to say since there is no definite reproducer.

Since I had to build the newer version for that I figured why not put it 
on the mailinglist as well after a short talk with Fiona.

> isn't this breaking live-migration and snapshot rollback compat though
> for old machines using that?
> 
> For new ones we only use 4 MB images anyway, so if, I'd rather move a
> existing build for 2MB images in some pve-edk2-firmware-legacy package
> for backward compat and then drop building those variants here completely.
> 
> Would be great if you could check qemu-server history for when the 2MB
> SMM ones where used, if at all, could help deciding things.
> 

I will check and perform some tests regarding that and report back.

> bumps should be separate commits and restricted to plain version and
> changelog metadata, like here it'd only touch d/changelog.
> 
> Also, it's most of the time best to let the release team handle the
> bumps.

duly noted.

Re: [pve-devel] [PATCH edk2-firmware v2] update to edk2-stable202308

[Fiona Ebner]

Am 31.08.23 um 14:47 schrieb Stefan Hanreich:
> On 8/31/23 14:39, Thomas Lamprecht wrote:
>> Am 30/08/2023 um 17:50 schrieb Stefan Hanreich:
>>> * Removed 0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
>>>    since it has been included upstream since commit c1e85376 [1].
>>>
>>> * Updated the patch
>>>    Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl so it applies
>>>    again.
>>>
>>> * had to increase FD_SIZE to 4M for the x64 build as otherwise the
>>>    package wouldn't build due to the size of the resulting image
>>
>> I mean new versions are great and what not, but is there a reason why you
>> send this update now? Maybe fix some issue our users face? If so, it'd be
>> great if you could record that here.
>>
> 
> I've been trying to reproduce an issue one of our users is facing and
> wanted to check whether newer firmware versions trigger the issue as
> well. Currently, this issue seems fixed with the newer firmware although
> it's hard to say since there is no definite reproducer.

To be a bit more precise, it's an issue were a RHEL VM with 4.x kernel
gets stuck after guest reboot, as far as we can tell right after OVMF
initialization. Mira and Stefan were able to reproduce the issue
semi-regularly with a bunch of VMs rebooting periodically. And Stefan
said it didn't yet trigger with the updated edk2 package for a few days
now. We can't be sure, as it is a race after all, but it does sound
promising.