Adding IA32 to X64 pkg, because secure boot is not working on Focal

The attachment "lp2022312_focal.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

In focal, you need to pass -global ICH9-LPC.disable_s3=1.

See https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56

Hi Seyeong,

Thanks for looking at this and providing a patch.

The change to edk2/ovmf doesn't seem appropriate for a SRU (it's not adding, but switching; focal is out for ~3 years already; and a report about this only came up now).

Could you please review/address Dann's comment above (patch author)?

I'd think the change should be in the component driving the qemu command,
(in this case, nova, which builds the libvirt xml, per the pastebin),
to align with how it should be done on focal (disable s3).

Thanks!

Thanks for checking mfo.
yeah.
I thought fixing this pkg is more simple :D
The next step should be related to nova.
I'm going to analyze further.

Christian wrote in this bug here that the edk2 fix was preferable, and that setting ICH9-LPC.disable_s3=1 by default may break more things than it fixes:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1903681

It's also notable that bug was raised in 2020 on focal's release so it has been raised (and fixed) earlier than now 3 years local.

According to the above bug, the current version of the file OVMF_CODE.ms.fd is just plainly broken in all cases. So if this patch -only- effects OVMF_CODE.ms.fd and we can confirm that the file is not usable in any situation, the SRU may be more acceptable.

The question remains whether we can safely SRU this change.

My suggestion is that:
(1) We know focal-yoga is broken, but we need to determine whether focal-ussuri (the openstack version that ships with focal) works. If it does work, we could consider a backport of edk2 from jammy into the openstack cloud-archive for yoga. The SRU still needs to be understood thoroughly to be safe but we potentially only affect openstack users, rather than all Ubuntu users.

(2) If focal-ussuri is broken, we probably need to look at actually accepting this change but need some people with much more detailed knowledge to help understand if it will be OK and help understand, detail and test the risk of the change.

It seems libvirt does have code to sometimes set -global ICH9-LPC.disable_s3=1, but it's not 100% clear to me from a quick read why that might not be getting done here. Perhaps Seyeong you could check into that?

The code to do that seems quite a bit older than focal.

Example:
https://github.com/libvirt/libvirt/commit/d6d31e00ebc8c72b88f0248f1bfde7e2ff7607af#diff-b40c0e564d59de99367e1b623fb4e4cf83abe7e74f4f4e7e54358d487b552b9eR5074

> According to the above bug, the current version of the file OVMF_CODE.ms.fd is just plainly broken in all cases. So if this patch -only- effects OVMF_CODE.ms.fd and we can confirm that the file is not usable in any situation, the SRU may be more acceptable.

As DannF stated, it is usable if the user or the management system sets -global ICH9-LPC.disable_s3=1.
So I guess we do not get over the SRU barrier by saying "it is not working at all, so there can't be a regression.

I agree that finding out if focal-ussuri works or not is a valid data point.
But again, if you just use qemu and edk2 they will most likely work by users or management setting -global ICH9-LPC.disable_s3=1 (to be confirmed).

> it's not 100% clear to me from a quick read why that might not be getting done here

The domain needs to have something like:

<pm>
  <suspend-to-mem enabled='no'/>
</pm>

Then it should get the it set, which is depending on the rest of the guest config then either ICH9-LPC.disable_s3=1 or PIIX4_PM.disable_s3=1.

So the entry above is something that nova could set on secure boot for focal-yoga to get the case working without chaning EDK2 or the qemu defaults (as we consider those two more regression risky).

I cannot find any mention in any openstack repo of that suspend-to-mem property. Not sure if I'm missing something but perhaps OpenStack actually doesn't currently support this option?

I am surprised by that given how many people seem to have run into this issue. Has everyone else just fixed their edk2 build?

Secure Boot was definitely working at some point, maybe bionic? It's not clear to me what changed to break this in focal. Was it a change in edk2 itself or?

Is there any possibility it's safe to SRU this edk2 change?

I tested focal-ussuri, It doesn't work either. the same black screen there.

I think it is edk2 issue in focal. I also can't find any nova upstream work related to this(maybe i missed someting though). qemu, libvirt have related options.

I'm curious, does it work if you drop "acpi-s3" from the features in /usr/share/qemu/firmware/40-edk2-x86_64-secure-enrolled.json ? If I understand that setting correctly, it should be giving libvirt the hint it needs to figure out whether or not the firmware supports S3, and in focal that setting looks wrong since we're using 64-bit PEI which does not. Hopefully libvirt has the smarts to pass '-global ICH9-LPC.disable_s3=1' when "acpi-s3" is absent. If so, correcting the descriptor in an SRU seems like a path forward.

Note: We introduced the 4M images into Ubuntu w/ 64-bit PEI at some point after focal, but we switched them to 32-bit PEI before 22.04, so I believe advertising the "acpi-s3" feature is correct in jammy. When we backported the 4M images to focal earlier this year (bug 1885662), we backported the intermediate version that still used 64-bit PEI. I regret that we didn't think to also include the switch to 32-bit PEI at that time, it would have been the safest time to do it, and consistent with jammy. But now that we've released them w/ 64-bit PEI into an LTS, I don't know how we'd go about demonstrating that no one has grown reliant upon it.

It seems this SRU has some pending questions that need to be addressed, and is marked as 'Incomplete'. I'm unsubscribing ~se-sponsors for now, please feel free to re-subscribe when this is ready for sponsorship!

updated

I just tested pure focal machine without openstack.

and put below lines for new vm

  <os>
    <type arch="x86_64" machine="pc-q35-4.2">hvm</type>
    <loader readonly="yes" secure="yes" type="pflash">/usr/share/OVMF/OVMF_CODE_4M.ms.fd</loader>
    <nvram>/var/lib/libvirt/qemu/nvram/ubuntu22.04_VARS.fd</nvram>
    <boot dev="hd"/>
    <smbios mode="sysinfo"/>
  </os>
  <features>
    <acpi/>
    <apic/>
    <vmport state="off"/>
    <smm state="on"/>
  </features>

it is booted well.

@dannf

I deployed test env and removed acpi-s3 from /usr/share/qemu/firmware/40-edk2-x86_64-secure-enrolled.json

then rebooted instance. but the same behaviour. is this right step to test it?

Thanks.

for #16, I just confirmed that there are below options with virt-manager created instance.

-global ICH9-LPC.disable_s3=1
-global ICH9-LPC.disable_s4=1

but nova created instance doesn't have it

in libvirt xml
  <pm>
    <suspend-to-mem enabled="no"/>
    <suspend-to-disk enabled="no"/>
  </pm>

so maybe belows are the way we can do

1. extra feature adding below for uca(for focal) only
- <pm><suspend-to-mem enabled="no"/><suspend-to-disk enabled="no"/></pm>
- no upstream related work so I'm not sure it can be done only for focal-uca
2. patch edk2 for focal
- not possible?
3. include patched edk2 pkg in uca
- not sure but possible solution..?

I appreciate if there are any other ideas

Apologies i forgot to add the uca target to reflect the fact that the request is to backport to Focal Yoga UCA (so that we have parity with Jammy Yoga i.e. jammy-updates)

Has secure boot ever worked on focal? If not, then what is being asked for is a new feature.

I'm not comfortable backporting edk2 to the focal-yoga cloud archive. The delta between the focal and jammy version is massive. I don't have an exact LOC count but a quick diff command shows the following:

diff -Naur edk2-0~20191122.bd85bf54/ edk2-2022.02/|wc -l
1954492

Secure Boot does work in focal, but apparently never has through OpenStack. That's a bug or a new feature request, depending on how you look at it.

*If* we want to fix that/enable that feature, it sounds like we're back to Seyeoung's options #1 and #2 from comment #20. Option #2 - switching the 4M images to 32-bit PEI - is starting to sound like the best option. Patching Nova to do something atypical for focal feels like it would carry more risk.

We've had 4M images in focal for about 8 months (2023-01-24). The regression risk with this change would be that someone has created a VM in focal during that time that somehow needs 64-bit PEI. I'm not aware of any reason a VM would require 64-bit PEI, and I'm also unaware of any user-perceivable changes the switch would make. I haven't see any user reports about problems with that since we introduced that change in Debian/Ubuntu. If users are already booting Secure Boot VMs w/ S3 disabled to use them, they can continue doing that[*]. The fact that the firmware would now support booting them w/ S3 enabled wouldn't be a regression. And it would correct an internal inconsistency in OVMF - where it tells libvirt that the 4M images support s3 when they currently do not.

If you do move forward with this SRU Seyeoung, I'd suggest using the same or similar debian/changelog entry from when we introduced the change in 2020.11-1, as it provides more context.

[*] Of course, we need to verify that - it should be a verification test

@dannf

Thanks for the info.

I already mentioned below link in debian/changelog
https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56

But you think I need to copy the string to changelog? or the other link?

I tested my patch at the time I was making the patch, and at least secure booting was fine.

but I'm going to test one more test this week.

Thanks.

I updated description as with simple test result.

On Mon, Sep 4, 2023 at 1:25 AM Seyeong Kim <email address hidden> wrote:
>
> @dannf
>
> Thanks for the info.
>
> I already mentioned below link in debian/changelog
> https://salsa.debian.org/qemu-team/edk2/-/commit/f839ec864df811395863fdd383842ffbcba5cd56
>
> But you think I need to copy the string to changelog? or the other link?

I prefer changelog entries to be self-contained and not require the
reader to go out to the Internet for details. Internet links break,
and users do not always have Internet access. But I'm just expressing
my preference - I'm not on the SRU team.

  -dann

Thanks for your feedback @dann.

@mfo
I think we(SRU) should patch edk2 pkg for focal with my patch(or modified) if there is no critical issue.

As dann and Corey mentioned, option 1 and 3 may not acceptible for now.

Could you please consider this again?