Comment 19 for bug 2004618

Revision history for this message
Mustafa Kemal Gilor (mustafakemalgilor) wrote (last edit ):

Hello @trya,

Yes, you are correct, thanks for pointing that out! I believe I somehow mixed the test results while I was fiddling with the code and the test cases. I've re-checked and confirmed that the `r` does not have any effect on this specific pattern. Given that it contains no escaped characters, it makes sense.

After @trya pointed that out, I went back to the drawing board. I've re-read the @dannf's original diagnosis and extended mine. The pattern set covers for ".*OVMF.*", so what we're seeing here is not related to the lack of a pattern in virt-install's internal pattern set as stated. virt-install processes the offered firmware descriptors in order (i.e. 40-50-60), and stops whenever a firmware descriptor is found suitable. The name "OVMF_CODE_4M.ms.fd" is offered first and matches the pattern set, so it's picked. virt-install is behaving as expected here.

As for the original code before the patch, the first offered firmware name `OVMF_CODE.ms.fd` should've matched the ".*OVMF.*" pattern as well, but why it doesn't get picked as FW? Because even though the OVMF_CODE.ms.fd is offered first, the implementation iterates over *patterns* and matches them against the firmware list, not the other way around. Therefore, it picks the first firmware that matches the first pattern, which is "OVMF_CODE.fd". It seems that the order of patterns matters here. The order of FW descriptors matters when they're going to match the same pattern.

As for the updated code, the _4M suffix caused "OVMF_CODE\.fd" pattern match to fail, so now the subsequent patterns are also evaluated, one of them being ".*OVMF.*", and not to your surprise, it matches the first offered firmware name. No capability-based search is made.

The question here is if we *want* firmware descriptor consumers to pick up non-"secure boot enforced" FW first, shouldn't we offer it first (i.e. 40-edk2-x86_64.json)? If we want to keep the original behavior, I think it is the most solid way to go.

To conclude, my opinion on this is to reflect the FW order we want to firmware descriptor files. We can fix Focal by offering non-secure boot firmware first, and I believe it would be better to offer non-ms specific firmware first in Jammy and above:

-- my proposal --

Focal:

40-edk2-x86_64.json - OVMF_CODE_4M.fd
50-edk2-x86_64-secure.json - OVMF_CODE_4M.secboot.fd
60-edk2-x86_64-secure-enrolled.json - OVMF_CODE_4M.ms.fd

Jammy and above:

40-edk2-x86_64-secure.json - OVMF_CODE_4M.secboot.fd
50-edk2-x86_64.json - OVMF_CODE_4M.fd
60-edk2-x86_64-secure-enrolled.json - OVMF_CODE_4M.ms.fd

@dannf any opinions?

------------------------------

Test steps for reference:

# /usr/share/OVMF/OVMF_CODE_4M.ms.fd supports: "pc-q35-*"
# /usr/share/OVMF/OVMF_CODE_4M.secboot.fd "pc-q35-*"
# /usr/share/OVMF/OVMF_CODE_4M.fd supports : "pc-i440fx-*", "pc-q35-*"

lxc launch ubuntu:focal test-ovmf-virtinst
lxc shell test-ovmf-virtinst
apt update

#### Focal new ovmf ####
apt install ovmf=ovmf=0~20191122.bd85bf54-2ubuntu3.4 virtinst
apt install qemu-kvm libvirt-daemon-system

# Just --boot uefi
virt-install --boot uefi --print-xml --memory 1 --disk none | grep "/usr/share/OVMF"
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE_4M.fd</loader>
# uses pc-i440fx-focal machine type

# --boot uefi & --os-variant ubuntu20.04
virt-install --boot uefi --print-xml --memory 1 --disk none --os-variant ubuntu20.04 | grep "/usr/share/OVMF"
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE_4M.ms.fd</loader>
# uses q35 machine type

# --boot uefi & --machine q35
virt-install --boot uefi --print-xml --memory 1 --disk none --machine q35 | grep "/usr/share/OVMF"
# uses q35 machine type
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE_4M.ms.fd</loader>

#### Focal old ovmf ####
apt purge ovmf
apt install ovmf=0~20191122.bd85bf54-2ubuntu3.3
systemctl restart libvirtd

# Just --boot uefi
virt-install --boot uefi --print-xml --memory 1 --disk none | grep "/usr/share/OVMF"
# uses pc-i440fx-focal machine type
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE.fd</loader>

# --boot uefi & --os-variant ubuntu20.04
virt-install --boot uefi --print-xml --memory 1 --disk none --os-variant ubuntu20.04 | grep "/usr/share/OVMF"
# uses q35 machine type
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE.fd</loader>

# --boot uefi & --machine q35 ubuntu20.04
virt-install --boot uefi --print-xml --memory 1 --disk none --machine q35 | grep "/usr/share/OVMF"
# uses q35 machine type
# prints <loader readonly="yes" type="pflash">/usr/share/OVMF/OVMF_CODE.fd</loader>