Bug 2132951

Ok, lets have a look at the fedora package (rhel provides a subset of those).

 - /usr/share/qemu/firmware/40-edk2-ovmf-sb.json
   OVMF_CODE.secboot.fd + OVMF_VARS.secboot.fd

 - /usr/share/qemu/firmware/50-edk2-ovmf-amdsev.json
   OVMF.amdsev.fd (stateless)

 - /usr/share/qemu/firmware/50-edk2-ovmf-cc.json
   OVMF_CODE.cc.fd + OVMF_VARS.fd

 - /usr/share/qemu/firmware/50-edk2-ovmf-inteltdx.json
   OVMF.inteltdx.fd (stateless)

 - /usr/share/qemu/firmware/50-edk2-ovmf.json
   OVMF_CODE.secboot.fd + OVMF_VARS.fd

 - /usr/share/qemu/firmware/60-edk2-ovmf-microvm.json
   MICROVM.fd (stateless)

 - /usr/share/qemu/firmware/60-edk2-ovmf-nosb.json
   OVMF_CODE.fd + OVMF_VARS.fd

Note that OVMF_CODE.cc.fd and OVMF_CODE.fd are identical.

Now lets try to sort the capabilities of those builds:

 - OVMF_CODE.secboot.fd
   - needs smm (so q35 only).
   - supports statefull secure boot.
   - default secure boot state depends on VARS file used.

 - OVMF_CODE.fd + OVMF_CODE.cc.fd
   - supports both q35 and pc.
   - supports amd-sev and I think amd-sev-es too.
   - supports intel tdx.

 - OVMF.amdsev.fd
   - supports all amd-sev* variants.
   - stripped-down config (direct kernel boot only).

 - OVMF.inteltdx.fd
   - supports intel tdx.
   - supports intel tdx measurements.
   - stripped-down config (no network stack, ...).

So, sorting amdsev + inteltdx variants to the end of the list so they are only
picked in case their capabilities are requested certainly makes sense.

Not so sure what to do best with the cc build.  Is it an option to just drop it?
Also we might need more capabilities to better describe the amdsev + inteltdx builds.

(In reply to Gerd Hoffmann from comment #1)
> Ok, lets have a look at the fedora package (rhel provides a subset of those).
> 
[...]
> 
>  - /usr/share/qemu/firmware/60-edk2-ovmf-microvm.json
>    MICROVM.fd (stateless)

Probably too late to change this now, because it would risk breaking
existing guests, but I have to say that I'm really not a fan of how
the microvm variant strayed from the established naming convention :(

>  - /usr/share/qemu/firmware/60-edk2-ovmf-nosb.json
>    OVMF_CODE.fd + OVMF_VARS.fd
> 
> Note that OVMF_CODE.cc.fd and OVMF_CODE.fd are identical.

Yup, literally the same file.

However:

  --- /usr/share/qemu/firmware/60-edk2-ovmf-nosb.json	2022-09-21 00:04:40.000000000 +0200
  +++ /usr/share/qemu/firmware/50-edk2-ovmf-cc.json	2022-09-21 00:35:28.000000000 +0200
  @@ -1,12 +1,12 @@
   {
  -    "description": "OVMF for x86_64, without SB, without SMM, with empty varstore",
  +    "description": "OVMF with SEV-ES support",
       "interface-types": [
           "uefi"
       ],
       "mapping": {
           "device": "flash",
           "executable": {
  -            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.fd",
  +            "filename": "/usr/share/edk2/ovmf/OVMF_CODE.cc.fd",
               "format": "raw"
           },
           "nvram-template": {
  @@ -18,14 +18,13 @@
           {
               "architecture": "x86_64",
               "machines": [
  -                "pc-i440fx-*",
                   "pc-q35-*"
               ]
           }
       ],
       "features": [
  -        "acpi-s3",
           "amd-sev",
  +        "amd-sev-es",
           "verbose-dynamic"
       ],
       "tags": [

Which doesn't seem right.

> Now lets try to sort the capabilities of those builds:
> 
[...]
> 
>  - OVMF_CODE.fd + OVMF_CODE.cc.fd
>    - supports both q35 and pc.

According to the JSON files, only the non-CC version supports i440fx.

>    - supports amd-sev and I think amd-sev-es too.

According to the JSON files, only the CC version supports amd-sev-es.

I actually noticed and forgot to mention this in the original report:
does the regular build really support SEV? Whereas the Secure Boot
build doesn't?

If you asked for a firmware that supports the amd-sev feature,
wouldn't you expect the SEV-specific build to be selected instead of
this one anyway?

>    - supports intel tdx.

According to the JSON files, neither version supports the intel-tdx
feature.

Overall, the data contained in these firmware descriptor files
doesn't quite match with my expectations. It's entirely possible that
my expectations are wrong O:-)

> So, sorting amdsev + inteltdx variants to the end of the list so they are
> only
> picked in case their capabilities are requested certainly makes sense.

Great!

> Not so sure what to do best with the cc build.  Is it an option to just drop
> it?

I wonder if it's just a leftover from the very early days of
Confidential Computing?

Also note how the existence of this build, which as you pointed out
is in fact the same as the non Secure Boot capable one, appears to go
directly against the obvious intention of *not* shipping EFI firmware
builds that don't support Secure Boot in RHEL.

> > Note that OVMF_CODE.cc.fd and OVMF_CODE.fd are identical.
> 
> Yup, literally the same file.
> 
> However:

>            {
>                "architecture": "x86_64",
>                "machines": [
>   -                "pc-i440fx-*",
>                    "pc-q35-*"
>                ]
>            }
>        ],
>        "features": [
>   -        "acpi-s3",
>            "amd-sev",
>   +        "amd-sev-es",
>            "verbose-dynamic"
>        ],

> Which doesn't seem right.

Yes, that indeed needs fixing too.

> I actually noticed and forgot to mention this in the original report:
> does the regular build really support SEV? Whereas the Secure Boot
> build doesn't?

We have stateful secure boot (stores state in flash, requires SMM mode
support to make sure only the firmware can update flash).

We have (soon) stateless secure boot (doesn't need flash, so no SMM
support needed).

SMM mode is incompatible with confidential computing.  The host handles
the world switch between SMM mode and non-SMM mode and needs access to
guest registers for that.  Which is not allowed with SEV-ES, SEV-SNP
and TDX (plain SEV might work, not fully sure).  Which is essentially
the reason why we got stateless secure boot.

> If you asked for a firmware that supports the amd-sev feature,
> wouldn't you expect the SEV-specific build to be selected instead of
> this one anyway?

Well.  Ideally the normal (non-SMM) build supports everything we need,
including all confidential computing use cases.  OVMF can detect both
SEV and TDX at runtime and adapt accordingly, so to a certain extend
that works already.  Some stuff requires the special amdsev and inteltdx
builds though:

 * amdsev + inteltdx are stripped down (features like network boot dropped).
 * amdsev has the grub boot loader compiled in.
 * inteltdx supports runtime measurements.

> >    - supports intel tdx.
> 
> According to the JSON files, neither version supports the intel-tdx
> feature.

Yep.  Needs fixing.  Wasn't top priority so far.  Getting your hands on
tdx capable hardware is still not easy as it is pre-release still.

> Overall, the data contained in these firmware descriptor files
> doesn't quite match with my expectations. It's entirely possible that
> my expectations are wrong O:-)

Firmware descriptors need some love indeed.  Related:
https://patchwork.ozlabs.org/project/qemu-devel/patch/20220930133220.1771336-1-kraxel@redhat.com/

> > Not so sure what to do best with the cc build.  Is it an option to just drop
> > it?
> 
> I wonder if it's just a leftover from the very early days of
> Confidential Computing?

I suspect it got added to rhel first (which doesn't ship OVMF_CODE.fd).
Then added to fedora too for compatibility.

> Also note how the existence of this build, which as you pointed out
> is in fact the same as the non Secure Boot capable one, appears to go
> directly against the obvious intention of *not* shipping EFI firmware
> builds that don't support Secure Boot in RHEL.

Indeed.  Also not shipping efi firmware which supports 'pc' machine types,
which might be the reason for the differences in the json files.

Cc'ing Laszlo, maybe he can add some actual background on the history of the 'cc' variant instead of /me just speculating ...

Also adding Dave for awareness and SEV expertise.

Hi Gerd, Andrea,

please refer to downstream edk2 commit 14f5d2513745 ("redhat: add OVMF binary that will support SEV-ES", 2021-05-12), on the "rhel-8.5.0-20200602" branch. That commit has many references (and those references seem to unfold to a forest of further references -- bugzillas, gitlab merge requests etc)

Regarding the priority order between the cc and other descriptors: at the time, RHEL shipped only one binary (containing SB+SMM), exposed by two descriptors (one with certificates enrolled, and another without). This particular change would reintroduce the very first "ancient" (RHEL7+) build (the one without SB/SMM), in preparation for the actual SEV-ES compatibility to appear in it later (see the other BZ references in the commit message). No conflict was supposed to exist between "edk2-ovmf.json" (OVMF with SB+SMM, empty varstore) and "edk2-ovmf-cc.json" (OVMF with SEV-ES support) due to the use of different feature flags:

    >      "features": [
    > -        "acpi-s3",
    >          "amd-sev",
    > -        "requires-smm",
    > -        "secure-boot",
    > +        "amd-sev-es",
    >          "verbose-dynamic"
    >      ],

because at the time, we expected libvirtd to filter out one or the other based on these, IIRC. (See references in the commit msg.)

I guess in RHEL9 the RHEL and Fedora descriptors got unified, and that's how we must have ended with a duplicate firmware binary in RHEL. Additionally, the actual libvirt work done since may have significantly diverged from the original ideas.

HTH
Laszlo

(In reply to Gerd Hoffmann from comment #3)
> > I actually noticed and forgot to mention this in the original report:
> > does the regular build really support SEV? Whereas the Secure Boot
> > build doesn't?
> 
> We have stateful secure boot (stores state in flash, requires SMM mode
> support to make sure only the firmware can update flash).
> 
> We have (soon) stateless secure boot (doesn't need flash, so no SMM
> support needed).
> 
> SMM mode is incompatible with confidential computing.  The host handles
> the world switch between SMM mode and non-SMM mode and needs access to
> guest registers for that.  Which is not allowed with SEV-ES, SEV-SNP
> and TDX (plain SEV might work, not fully sure).  Which is essentially
> the reason why we got stateless secure boot.
> 
> > If you asked for a firmware that supports the amd-sev feature,
> > wouldn't you expect the SEV-specific build to be selected instead of
> > this one anyway?
> 
> Well.  Ideally the normal (non-SMM) build supports everything we need,
> including all confidential computing use cases.  OVMF can detect both
> SEV and TDX at runtime and adapt accordingly, so to a certain extend
> that works already.  Some stuff requires the special amdsev and inteltdx
> builds though:
> 
>  * amdsev + inteltdx are stripped down (features like network boot dropped).
>  * amdsev has the grub boot loader compiled in.
>  * inteltdx supports runtime measurements.

So if you want to be able to use the full range of capabilities of
either SEV or TDX you need to pick the corresponding specific build,
right? Whereas the non-SMM build will work with both technologies,
but only give you access to a subset of the features of either.

Considering that there is no generic "confidential-computing"
capability, the user will always ask for a firmware that provides
either the "amd-sev" or "intel-tdx" capability.

Given such a request, I think it's reasonable to expect a build that
supports the full set of features of the specific Confidential
Computing implementation to be selected.

Basically, I don't see the point in shipping a build that tries (but
fails) to cover all bases.

Having separate builds for SEV and TDX is already necessary in order
to expose all features of each technology, and especially in the
context of Confidential Computing I would imagine that *not* having
support for an implementation that you're not actually using built
into the firmware image would be seen as a very good thing.

Is there a use case that I'm overlooking and where the "a subset of
SEV and also a subset of TDX" build is a better match?

(In reply to Andrea Bolognani from comment #7)
> (In reply to Gerd Hoffmann from comment #3)
> > > I actually noticed and forgot to mention this in the original report:
> > > does the regular build really support SEV? Whereas the Secure Boot
> > > build doesn't?
> > 
> > We have stateful secure boot (stores state in flash, requires SMM mode
> > support to make sure only the firmware can update flash).
> > 
> > We have (soon) stateless secure boot (doesn't need flash, so no SMM
> > support needed).
> > 
> > SMM mode is incompatible with confidential computing.  The host handles
> > the world switch between SMM mode and non-SMM mode and needs access to
> > guest registers for that.  Which is not allowed with SEV-ES, SEV-SNP
> > and TDX (plain SEV might work, not fully sure).  Which is essentially
> > the reason why we got stateless secure boot.
> > 
> > > If you asked for a firmware that supports the amd-sev feature,
> > > wouldn't you expect the SEV-specific build to be selected instead of
> > > this one anyway?
> > 
> > Well.  Ideally the normal (non-SMM) build supports everything we need,
> > including all confidential computing use cases.  OVMF can detect both
> > SEV and TDX at runtime and adapt accordingly, so to a certain extend
> > that works already.  Some stuff requires the special amdsev and inteltdx
> > builds though:
> > 
> >  * amdsev + inteltdx are stripped down (features like network boot dropped).
> >  * amdsev has the grub boot loader compiled in.

NB, our builds don't include the grub bits, as they were somewhat crazy.

> >  * inteltdx supports runtime measurements.
> 
> So if you want to be able to use the full range of capabilities of
> either SEV or TDX you need to pick the corresponding specific build,
> right? Whereas the non-SMM build will work with both technologies,
> but only give you access to a subset of the features of either.
> 
> Considering that there is no generic "confidential-computing"
> capability, the user will always ask for a firmware that provides
> either the "amd-sev" or "intel-tdx" capability.
> 
> Given such a request, I think it's reasonable to expect a build that
> supports the full set of features of the specific Confidential
> Computing implementation to be selected.
> 
> Basically, I don't see the point in shipping a build that tries (but
> fails) to cover all bases.

The generic builds are useless for attestation, but still get your
VM working with encrypted memory, and keep most traditional
virt features working. These are what are used today by most
deployed confidential VMs.

The amdsev/inteltdx  builds are required if the mgmt app wants to
support boot attestation of the confidential VMs. This requires
specific configurations by the mgmt app and sacrifices a number
of features. Mgmt apps need to know about these limitations and
take special actions / configurations for VMs uses this firmware
builds. These are what we anticipate mgmt apps increasingly
wanting to opt-in to in future

(In reply to Daniel Berrangé from comment #8)
> > So if you want to be able to use the full range of capabilities of
> > either SEV or TDX you need to pick the corresponding specific build,
> > right? Whereas the non-SMM build will work with both technologies,
> > but only give you access to a subset of the features of either.
> > 
> > Considering that there is no generic "confidential-computing"
> > capability, the user will always ask for a firmware that provides
> > either the "amd-sev" or "intel-tdx" capability.
> > 
> > Given such a request, I think it's reasonable to expect a build that
> > supports the full set of features of the specific Confidential
> > Computing implementation to be selected.
> > 
> > Basically, I don't see the point in shipping a build that tries (but
> > fails) to cover all bases.
> 
> The generic builds are useless for attestation, but still get your
> VM working with encrypted memory, and keep most traditional
> virt features working. These are what are used today by most
> deployed confidential VMs.
> 
> The amdsev/inteltdx  builds are required if the mgmt app wants to
> support boot attestation of the confidential VMs. This requires
> specific configurations by the mgmt app and sacrifices a number
> of features. Mgmt apps need to know about these limitations and
> take special actions / configurations for VMs uses this firmware
> builds. These are what we anticipate mgmt apps increasingly
> wanting to opt-in to in future

In this case, shouldn't we introduce a "boot-attestation" feature and
add it to the firmware descriptor for the builds that support it? As
things are currently, there is no way for the management app to
request one or the other, and which one you get is all down to
filename sorting.


Trickier situation: if you want to boot an unsigned guest OS, the
correct request would be

  secure-boot=enabled
  enrolled-keys=disabled

but it wouldn't really be unreasonable for a user to try

  secure-boot=disabled

instead. In fact, we document this very configuration in

  https://libvirt.org/kbase/secureboot.html

and even list it before the correct one, suggesting that it's the
preferred option of the two.

However, if you try that on both Fedora and RHEL, you're going to get
the generic CC build selected instead of the no-SecureBoot build on
the former or an error on the latter. I couldn't find a way to force
the no-SecureBoot build to be selected short of switching the machine
type to i440fx, which is very heavy-handed.

I'm not sure how we should deal with this. Throwing our hands in the
air and declaring it a user error feels unhelpful. For Fedora we
could (and should) move the edk2-ovmf-nosb.json file earlier in the
sort order, before any CC build, consistently with what's being
suggested for edk2-ovmf.json in this very bug report; for RHEL
though, the no-SecureBoot build simply doesn't exist, so you're
always going to end up with the generic CC build, which again doesn't
feel right even though it technically works just fine.

It almost feels like there should be a hidden
"traditional-virtualization" feature that libvirt by default looks
for, and that none of the builds that are intended for CC use
advertises. But then libvirt would have to know *not* to look for
that feature when "amd-sev" or "intel-tdx" are explicitly requested,
and hidden logic is always going to become a liability down the line
anyway.

I really feel like we should behave better in this scenario, but I
can't figure out how to get there. Any ideas? :)

(In reply to Laszlo Ersek from comment #6)
> Hi Gerd, Andrea,
> 
> please refer to downstream edk2 commit 14f5d2513745 ("redhat: add OVMF
> binary that will support SEV-ES", 2021-05-12), on the "rhel-8.5.0-20200602"
> branch. That commit has many references (and those references seem to unfold
> to a forest of further references -- bugzillas, gitlab merge requests etc)

Thanks for providing context!

> Regarding the priority order between the cc and other descriptors: at the
> time, RHEL shipped only one binary (containing SB+SMM), exposed by two
> descriptors (one with certificates enrolled, and another without). This
> particular change would reintroduce the very first "ancient" (RHEL7+) build
> (the one without SB/SMM), in preparation for the actual SEV-ES compatibility
> to appear in it later (see the other BZ references in the commit message).
> 
> No conflict was supposed to exist between "edk2-ovmf.json" (OVMF with
> SB+SMM, empty varstore) and "edk2-ovmf-cc.json" (OVMF with SEV-ES support)
> due to the use of different feature flags:
> 
>     >      "features": [
>     > -        "acpi-s3",
>     >          "amd-sev",
>     > -        "requires-smm",
>     > -        "secure-boot",
>     > +        "amd-sev-es",
>     >          "verbose-dynamic"
>     >      ],
> 
> because at the time, we expected libvirtd to filter out one or the other
> based on these, IIRC. (See references in the commit msg.)
> 
> I guess in RHEL9 the RHEL and Fedora descriptors got unified, and that's how
> we must have ended with a duplicate firmware binary in RHEL. Additionally,
> the actual libvirt work done since may have significantly diverged from the
> original ideas.

The filtering works according to expectation if you apply it
"positively": asking for the "amd-sev-es" feature to be present will
never result in a firmware that doesn't support that feature being
selected.

The problem only arises if you use filtering "negatively": when you
ask for a firmware that doesn't have the "secure-boot" feature, you
are mentally working backwards from the baseline (where the feature
is present) and expect to be provided a firmware that has the same
features as the default one *except for* the "secure-boot" one.

On Fedora, this would be the one described by edk2-ovmf-nosb.json; on
RHEL, you'd expect such a query to fail, because all edk2 builds are
supposed to come with Secure Boot support.

Instead, in both cases you get a firmware built for the Confidential
Computing use case without having explicitly asked for it.

We can improve the situation quite a lot in Fedora by just renaming a
bunch of files, but as far as RHEL is concerned I don't see a way to
arrange things so that they behave as expected.

(In reply to Andrea Bolognani from comment #9)
> (In reply to Daniel Berrangé from comment #8)

> > The amdsev/inteltdx  builds are required if the mgmt app wants to
> > support boot attestation of the confidential VMs. This requires
> > specific configurations by the mgmt app and sacrifices a number
> > of features. Mgmt apps need to know about these limitations and
> > take special actions / configurations for VMs uses this firmware
> > builds. These are what we anticipate mgmt apps increasingly
> > wanting to opt-in to in future
> 
> In this case, shouldn't we introduce a "boot-attestation" feature and
> add it to the firmware descriptor for the builds that support it? As
> things are currently, there is no way for the management app to
> request one or the other, and which one you get is all down to
> filename sorting.

There's no requirement for a new feature. The attestable versions of
the firmware are already distinguished by using type=stateless, as
the key factor that makes them attestable is the absence of an NVRAM
storage blob.

Since these firmwares are marked stateless, libvirt will never
select them by default. The guest XML has to have <loader stateless='yes'/>
in order for the atestable firmware to be selected, and when this is
present all the other stateful firmwares will be ignored.

Essentially 'stateless' is a top level  switch between two
completely separate lists of firmware.  We don't have to consider
the interaction between the two sets at all.

(In reply to Daniel Berrangé from comment #11)
> (In reply to Andrea Bolognani from comment #9)
> > In this case, shouldn't we introduce a "boot-attestation" feature and
> > add it to the firmware descriptor for the builds that support it? As
> > things are currently, there is no way for the management app to
> > request one or the other, and which one you get is all down to
> > filename sorting.

Makes sense to me (see also last line of comment #1 ;)

> There's no requirement for a new feature. The attestable versions of
> the firmware are already distinguished by using type=stateless, as
> the key factor that makes them attestable is the absence of an NVRAM
> storage blob.

That happens to be true today.  But stateless doesn't imply attestation
support.  Stateless secure boot support landed upstream in september,
and it's not limited to inteltdx builds.  Effectively it allows secure
boot support without flash and smm mode, and it can be used to have
secure boot support for microvm and i440fx (probably not relevant for
rhel, but for upstream we need consider fedora & others too ...).

(In reply to Gerd Hoffmann from comment #12)
> (In reply to Daniel Berrangé from comment #11)
> > (In reply to Andrea Bolognani from comment #9)
> > > In this case, shouldn't we introduce a "boot-attestation" feature and
> > > add it to the firmware descriptor for the builds that support it? As
> > > things are currently, there is no way for the management app to
> > > request one or the other, and which one you get is all down to
> > > filename sorting.
> 
> Makes sense to me (see also last line of comment #1 ;)
> 
> > There's no requirement for a new feature. The attestable versions of
> > the firmware are already distinguished by using type=stateless, as
> > the key factor that makes them attestable is the absence of an NVRAM
> > storage blob.
> 
> That happens to be true today.  But stateless doesn't imply attestation
> support.  Stateless secure boot support landed upstream in september,
> and it's not limited to inteltdx builds.  Effectively it allows secure
> boot support without flash and smm mode, and it can be used to have
> secure boot support for microvm and i440fx (probably not relevant for
> rhel, but for upstream we need consider fedora & others too ...).

The combination of type=stateless and features=amd-sev(-es) should still be sufficient though ?

> > That happens to be true today.  But stateless doesn't imply attestation
> > support.  Stateless secure boot support landed upstream in september,
> > and it's not limited to inteltdx builds.  Effectively it allows secure
> > boot support without flash and smm mode, and it can be used to have
> > secure boot support for microvm and i440fx (probably not relevant for
> > rhel, but for upstream we need consider fedora & others too ...).
> 
> The combination of type=stateless and features=amd-sev(-es) should still be
> sufficient though ?

Not guaranteed.

If we:
 (a) drop the cc variant, and
 (b) tag 60-edk2-ovmf-nosb.json correctly (add sev + tdx), and
 (c) add a stateless secure boot variant of that build with the next rebase

that will not hold true.

Also being explicit about attestation support will be less confusing I think.

(In reply to Gerd Hoffmann from comment #14)
> > > That happens to be true today.  But stateless doesn't imply attestation
> > > support.  Stateless secure boot support landed upstream in september,
> > > and it's not limited to inteltdx builds.  Effectively it allows secure
> > > boot support without flash and smm mode, and it can be used to have
> > > secure boot support for microvm and i440fx (probably not relevant for
> > > rhel, but for upstream we need consider fedora & others too ...).
> > 
> > The combination of type=stateless and features=amd-sev(-es) should still be
> > sufficient though ?
> 
> Not guaranteed.
> 
> If we:
>  (a) drop the cc variant, and
>  (b) tag 60-edk2-ovmf-nosb.json correctly (add sev + tdx), and
>  (c) add a stateless secure boot variant of that build with the next rebase
> 
> that will not hold true.

But (c) would be attestable since it is stateless and has 'sev' support IIUC. It is different from the 'amdsev.fd' build we have today, but that's related to the possible embedded grub, and support for measuring kernel hashes. I'd suggest the kernel hash measurement feature should be made to present in any build that has SEV support and is stateless. 

IOW, I think the primary distinguishing feature from the amdsev.fd build ought to be nothing more than the embeded grub support which we've disabled anyway.

> But (c) would be attestable since it is stateless and has 'sev' support
> IIUC. It is different from the 'amdsev.fd' build we have today, but that's
> related to the possible embedded grub, and support for measuring kernel
> hashes. I'd suggest the kernel hash measurement feature should be made to
> present in any build that has SEV support and is stateless. 

I think that is true for launch measurements.
TDX runtime measurement is supported only by the inteltdx build variant though.

> IOW, I think the primary distinguishing feature from the amdsev.fd build
> ought to be nothing more than the embeded grub support which we've disabled
> anyway.

Measurement aside the differences are:
 (a) the embedded boot loader (amdsev, not functional indeed in our builds).
 (b) more restricted boot policy (amdsev wouldn't try fetch a boot loader from disk).
 (c) no network stack (both amdsev and inteltdx).
 (d) maybe other drivers/modules dropped too.

(In reply to Gerd Hoffmann from comment #12)
> But stateless doesn't imply attestation
> support.  Stateless secure boot support landed upstream in september,
> and it's not limited to inteltdx builds.  Effectively it allows secure
> boot support without flash and smm mode, and it can be used to have
> secure boot support for microvm and i440fx (probably not relevant for
> rhel, but for upstream we need consider fedora & others too ...).

Stateless Secure Boot also seems to be the only feasible way, at
least in the short term, to bring Secure Boot support to Arm. So it
is absolutely relevant to RHEL.

(In reply to Gerd Hoffmann from comment #14)
> > The combination of type=stateless and features=amd-sev(-es) should still be
> > sufficient though ?
> 
> Not guaranteed.
> 
[...]
> 
> Also being explicit about attestation support will be less confusing I think.

I absolutely agree.

As a user, my goal is to select a firmware that has attestation
support. Even if that can be technically be achieved with a filter
like

  type=stateless
  amd-sev-es=enabled

that's not nearly as intuitive as

  amd-sev-es=enabled
  boot-attestation=enabled

The firmware selection situation is extremely complicated already,
even for people who are quite familiar with the low-level details. We
should strive to make things as simple as possible for users.

(In reply to Gerd Hoffmann from comment #16)
> > IOW, I think the primary distinguishing feature from the amdsev.fd build
> > ought to be nothing more than the embeded grub support which we've disabled
> > anyway.
> 
> Measurement aside the differences are:
>  (a) the embedded boot loader (amdsev, not functional indeed in our builds).
>  (b) more restricted boot policy (amdsev wouldn't try fetch a boot loader
> from disk).
>  (c) no network stack (both amdsev and inteltdx).
>  (d) maybe other drivers/modules dropped too.

Dan mentioned that most confidential VMs that exist today are using
the generic CC build. Does any of the above prevent such VMs from
switching over to the amdsev or inteltdx builds? If that's the case,
I guess we're pretty much forced to keep shipping it for the
foreseeable future :(

(In reply to Andrea Bolognani from comment #19)
> (In reply to Gerd Hoffmann from comment #16)
> > > IOW, I think the primary distinguishing feature from the amdsev.fd build
> > > ought to be nothing more than the embeded grub support which we've disabled
> > > anyway.
> > 
> > Measurement aside the differences are:
> >  (a) the embedded boot loader (amdsev, not functional indeed in our builds).
> >  (b) more restricted boot policy (amdsev wouldn't try fetch a boot loader
> > from disk).
> >  (c) no network stack (both amdsev and inteltdx).
> >  (d) maybe other drivers/modules dropped too.
> 
> Dan mentioned that most confidential VMs that exist today are using
> the generic CC build. Does any of the above prevent such VMs from
> switching over to the amdsev or inteltdx builds? If that's the case,
> I guess we're pretty much forced to keep shipping it for the
> foreseeable future :(

(b) makes is incompatible with all existing SEV VM usage, given that we didn't implement (a) in our builds.

(In reply to Andrea Bolognani from comment #18)
> (In reply to Gerd Hoffmann from comment #14)
> > > The combination of type=stateless and features=amd-sev(-es) should still be
> > > sufficient though ?
> > 
> > Not guaranteed.
> > 
> [...]
> > 
> > Also being explicit about attestation support will be less confusing I think.
> 
> I absolutely agree.
> 
> As a user, my goal is to select a firmware that has attestation
> support. Even if that can be technically be achieved with a filter
> like
> 
>   type=stateless
>   amd-sev-es=enabled
> 
> that's not nearly as intuitive as
> 
>   amd-sev-es=enabled
>   boot-attestation=enabled
> 
> The firmware selection situation is extremely complicated already,
> even for people who are quite familiar with the low-level details. We
> should strive to make things as simple as possible for users.

This is looking at things from the wrong POV, users should NOT be configuring things in terms of firmware features in general. 

In a confidential VM world, the user has a desired hardware configuration, and they need to be able to validate that the hardware environment they received, matches their expectations. 

They do not want to say give me a firmware with features  X, Y, and Z. They need to directly express the desired virtual machine features A, B & C. Libvirt needs to select a firmware which is then compatible with features A, B & C, and to do this, libvirt will look for firmware options advertizing X, Y, and Z.

Whether or not a firmware is attestible is determined by whether it is supporting the set of hardware features the users has requested. The set of hardware features are chosen to match what the user will input for calculating thue launch measurement

So in the case of wanting to attest the SEV-ES boot measurement, as a user I want to have a virtual machine with no NVRAM, SEV-ES encryption, with kernel hashing. IOW, they'll say

  <os type='efi'>
    <loader type=stateless>
  </os>

  <launchSecurity type='sev' kernelHashes='yes'>
     <policy>0x7</policy>
  </launchSecurity>

Libvirt needs to look for the firmware that can support all these requests.

Currently we assume that  a firmware satsifying  'amd-sev' / 'amd-sev-es'  and  'stateless' is sufficient.

What this misses, is that some amdsev.fd builds can't support bootloader launch. We could introduce new features for 'bootloade-launch' and 'direct-kernel-launch', but hard to do in a back compatible manner.

(In reply to Andrea Bolognani from comment #10)

> The problem only arises if you use filtering "negatively": when you
> ask for a firmware that doesn't have the "secure-boot" feature, you
> are mentally working backwards from the baseline (where the feature
> is present) and expect to be provided a firmware that has the same
> features as the default one *except for* the "secure-boot" one.

Indeed there's a difference between
- filtering for a feature's absence and 
- ignoring a feature.

Now, regarding comment#0 in isolation, I think the problem laid out
there is solvable. Personally I'm not against

- either reordering the files,

- or requiring the user to explicitly request:

  <os firmware='efi'>
    <firmware>
      <feature enabled='yes' name='secure-boot'/>
      <feature enabled='no' name='enrolled-keys'/>
    </firmware>
  </os>

However (IIUC the discussion) a *global* ordering may not exist that
allowed a user to find *any* particular Confidential Computing use case
with a *simple* ("intuitive") query.

So to me that suggests we should just live with the queries being
complicated (non-intuitive) in general -- it's reflective of how
non-overlapping the firmware features themselves are. And the quirky
queries could be hidden in higher levels of the stack (see Dan's
comment#21).

As long as we keep exposing finely granular firmware feature knobs to
the user (= as long as we make public APIs flexible), we'll always end
up with complexity like this, due to the valid feature combinations
being very peculiar.

While being at it: what about 32-bit builds (IA32, ARM)?
Fedora only obviously.  Both i386 and armv7 variants of
fedora are gone now (or will be with f37), and EFI never
really took off on these platforms anyway ...

https://copr.fedorainfracloud.org/coprs/kraxel/edk2.testbuilds/

renumbered json files using the following scheme:
      30-* secure boot supported and enabled
      40-* secure boot supported but disabled
      50-* secure boot not supported
      60-* stateless cc builds

comments?

It's in rawhide now and will probably land in f37 soon.  Last chance to veto ;)
Plan to do the same in rhel-9 after the 2022-11 rebase is done.

(In reply to Gerd Hoffmann from comment #29)
> It's in rawhide now and will probably land in f37 soon.  Last chance to veto ;)
> Plan to do the same in rhel-9 after the 2022-11 rebase is done.

Sorry, I sort of lost track of the discussion. I promise to catch up
and give your work a look by the end of the week.

Still combing through the discussion and the changes, but I got
curious about this snippet

  -Source10: edk2-aarch64-verbose.json
  -Source11: edk2-aarch64.json
  -Source12: edk2-ovmf-sb.json
  -Source13: edk2-ovmf.json
  -Source14: edk2-ovmf-amdsev.json
  -Source15: edk2-ovmf-inteltdx.json
  -
  -# Fedora specific sources
  -Source50: softfloat-%{softfloat_version}.tar.xz
  -Source55: 40-edk2-ovmf-ia32-sb-enrolled.json
  -Source56: 50-edk2-ovmf-ia32-sb.json
  -Source57: 60-edk2-ovmf-ia32.json
  -Source58: edk2-ovmf-nosb.json
  -Source59: 70-edk2-arm-verbose.json
  -Source60: edk2-microvm.json
  +# json description files
  +Source10: json-aa64/50-edk2-aarch64.json
  +Source11: json-aa64/51-edk2-aarch64-verbose.json
  +
  +Source20: json-arm/50-edk2-arm-verbose.json
  +
  +Source30: json-ia32/30-edk2-ovmf-ia32-sb-enrolled.json
  +Source31: json-ia32/40-edk2-ovmf-ia32-sb.json
  +Source32: json-ia32/50-edk2-ovmf-ia32.json
  +
  +Source40: json-x64/30-edk2-ovmf-x64-sb.json
  +Source41: json-x64/40-edk2-ovmf-x64.json
  +Source42: json-x64/50-edk2-ovmf-x64-microvm.json
  +Source43: json-x64/50-edk2-ovmf-x64-nosb.json
  +Source44: json-x64/60-edk2-ovmf-x64-amdsev.json
  +Source45: json-x64/60-edk2-ovmf-x64-inteltdx.json

from

  https://src.fedoraproject.org/rpms/edk2/c/8e960a72f3112cc2b66676a6d825cec6010dbc95?branch=rawhide

When I look at the repository, the json-*/ directories don't exist,
and the *.json files all live in the top-level directory.

Can you please explain how this can work? Thanks!

>   +# json description files
>   +Source10: json-aa64/50-edk2-aarch64.json

> When I look at the repository, the json-*/ directories don't exist,
> and the *.json files all live in the top-level directory.
> 
> Can you please explain how this can work? Thanks!

rpmbuild uses $(basename $file) for Source*:, so the directory is ignored.

Yes, I've tried to move the json files into per-arch subdirectories.
Didn't work due to rpmbuild ignoring the directory.  This is just
a leftover from that, I'll drop that on the next update.

(In reply to Gerd Hoffmann from comment #32)
> rpmbuild uses $(basename $file) for Source*:, so the directory is ignored.

That's a very "interesting" approach, thank you for explaining it :)


I've looked at the changes and they mostly seem good.


I'm not sure about dropping the -cc descriptor though. Based on what
Dan said (Comment #8, Comment #20) most existing CC VMs will ask for
a firmware that implements the amd-sev (-es?) feature and is
stateful.

Doesn't dropping the -cc descriptor mean these VMs will be unable to
get a firmware matching their requirements? Or will the -nosb
descriptor pick up the slack now that it has had the amd-sev-es
feature added to it?


Speaking of the -nosb descriptor, we used to only ship that in Fedora
but now we also ship it in RHEL AFAICT. Is that okay? Doesn't it
somehow undermine the message that we don't recommend people disable
Secure Boot?

Then again, there's basically no difference from a practical point of
view between a build that contains Secure Boot support but has it
disabled because of an empty varstore, and one that doesn't contain
support in the first place...


One more thing. For consistency and to better communicate what the
various files are about, I suggest the following renames:

  30-edk2-ovmf-x64-sb.json -> 30-edk2-ovmf-x64-sb-enrolled.json
  40-edk2-ovmf-x64.json    -> 40-edk2-ovmf-x64-sb.json
  50-edk2-ovmf-ia32.json   -> 50-edk2-ovmf-ia32-nosb.json

> I'm not sure about dropping the -cc descriptor though. Based on what
> Dan said (Comment #8, Comment #20) most existing CC VMs will ask for
> a firmware that implements the amd-sev (-es?) feature and is
> stateful.
> 
> Doesn't dropping the -cc descriptor mean these VMs will be unable to
> get a firmware matching their requirements? Or will the -nosb
> descriptor pick up the slack now that it has had the amd-sev-es
> feature added to it?

50-edk2-ovmf-x64-nosb.json has:

    "features": [
        "acpi-s3",
        "amd-sev",
        "amd-sev-es",
        "verbose-dynamic"
    ],


> Speaking of the -nosb descriptor, we used to only ship that in Fedora
> but now we also ship it in RHEL AFAICT. Is that okay? Doesn't it
> somehow undermine the message that we don't recommend people disable
> Secure Boot?

Well, we do ship .cc in 8.x and 9.1 which does not support secure boot
and it'll happily boot non-cc VMs too.  So the only thing which changes
is the name of the json file ...

> One more thing. For consistency and to better communicate what the
> various files are about, I suggest the following renames:
> 
>   30-edk2-ovmf-x64-sb.json -> 30-edk2-ovmf-x64-sb-enrolled.json
>   40-edk2-ovmf-x64.json    -> 40-edk2-ovmf-x64-sb.json
>   50-edk2-ovmf-ia32.json   -> 50-edk2-ovmf-ia32-nosb.json

That makes sense indeed.

(In reply to Gerd Hoffmann from comment #34)
> > Speaking of the -nosb descriptor, we used to only ship that in Fedora
> > but now we also ship it in RHEL AFAICT. Is that okay? Doesn't it
> > somehow undermine the message that we don't recommend people disable
> > Secure Boot?
> 
> Well, we do ship .cc in 8.x and 9.1 which does not support secure boot
> and it'll happily boot non-cc VMs too.  So the only thing which changes
> is the name of the json file ...

Yeah, you're right that it was already possible to configure a VM so
that Secure Boot support would be disabled, and users are going to be
looking at the libvirt XML rather than the name of the JSON firmware
descriptor, so from that point of view there's literally no change.


One difference between the original -cc descriptor and the -nosb
descriptor remains: the former only declared support for q35, while
the latter will be a match for i440fx as well.

Going back to what you wrote in Comment #3:

> > Also note how the existence of this build, which as you pointed out
> > is in fact the same as the non Secure Boot capable one, appears to go
> > directly against the obvious intention of *not* shipping EFI firmware
> > builds that don't support Secure Boot in RHEL.
> 
> Indeed.  Also not shipping efi firmware which supports 'pc' machine types,
> which might be the reason for the differences in the json files.

Should we have two versions of the -nosb descriptor? One for Fedora,
which declares i440fx support, and one for RHEL, which claims to only
support q35?

> > Indeed.  Also not shipping efi firmware which supports 'pc' machine types,
> > which might be the reason for the differences in the json files.
> 
> Should we have two versions of the -nosb descriptor? One for Fedora,
> which declares i440fx support, and one for RHEL, which claims to only
> support q35?

My plan is to drop the line for 'pc' machine type when porting this over
to the rhel-9 package (unless someone raises objections).

(In reply to Gerd Hoffmann from comment #36)
> > Should we have two versions of the -nosb descriptor? One for Fedora,
> > which declares i440fx support, and one for RHEL, which claims to only
> > support q35?
> 
> My plan is to drop the line for 'pc' machine type when porting this over
> to the rhel-9 package (unless someone raises objections).

I'm unclear on how close the RHEL packaging is to the Fedora one, but
seeing how you've added an edk2-build.rhel-9 file and there are a
bunch of '%if %{defined rhel}' checks in the spec, I sort of assumed
that you'd import it as-is.

If that's roughly the case, it might make sense to have two variants
of the -nosb descriptor in the Fedora package, to remove the risk of
possibly forgetting to drop i440fx support during a future import
into RHEL.

Ultimately, do whatever you think will work better :)

https://kojihub.stream.rdu2.redhat.com/koji/taskinfo?taskID=1685612
https://gitlab.com/kraxel/centos-edk2/-/commits/rebase.kraxel/

Set 'Verified:Tested,SanityOnly' as gating/tier1 test pass.

Install edk2-ovmf-20221207gitfff6d81270b5-1.el9.noarch and edk2-aarch64-20221207gitfff6d81270b5-1.el9.noarch, the following firmware descriptor files are shipped.

# ll /usr/share/qemu/firmware/
total 28
-rw-r--r-- 1 root root 770 Dec 19 12:33 30-edk2-ovmf-x64-sb-enrolled.json
-rw-r--r-- 1 root root 722 Dec 19 12:33 40-edk2-ovmf-x64-sb.json
-rw-r--r-- 1 root root 643 Dec 19 12:32 50-edk2-aarch64.json
-rw-r--r-- 1 root root 692 Dec 19 12:33 50-edk2-ovmf-x64-nosb.json
-rw-r--r-- 1 root root 674 Dec 19 12:32 51-edk2-aarch64-verbose.json
-rw-r--r-- 1 root root 588 Dec 19 12:33 60-edk2-ovmf-x64-amdsev.json
-rw-r--r-- 1 root root 544 Dec 19 12:33 60-edk2-ovmf-x64-inteltdx.json

Install edk2-ovmf-20221207gitfff6d81270b5-1.el9.noarch, the following files are shipped.

# ls /usr/share/edk2/ovmf/ -l
total 19872
-rw-r--r-- 1 root root   18816 Dec 19 12:32 EnrollDefaultKeys.efi
-rw-r--r-- 1 root root 4194304 Dec 19 12:33 OVMF.amdsev.fd
lrwxrwxrwx 1 root root      12 Dec 19 12:33 OVMF_CODE.cc.fd -> OVMF_CODE.fd
-rw-r--r-- 1 root root 3653632 Dec 19 12:31 OVMF_CODE.fd
-rw-r--r-- 1 root root 3653632 Dec 19 12:32 OVMF_CODE.secboot.fd
-rw-r--r-- 1 root root 4194304 Dec 19 12:33 OVMF.inteltdx.fd
-rw-r--r-- 1 root root  540672 Dec 19 12:31 OVMF_VARS.fd
-rw-r--r-- 1 root root  540672 Dec 19 12:33 OVMF_VARS.secboot.fd
-rw-r--r-- 1 root root  950912 Dec 19 12:31 Shell.efi
-rw-r--r-- 1 root root 2596864 Dec 19 12:33 UefiShell.iso

Hi Zixi,

According to Comment 41 and Comment 45, can you help check your confidential computing features?

Test with edk2-ovmf-20221207gitfff6d81270b5-1.el9.noarch, boot sev guest with libvirt and boot uefi, no issue found

Version:
edk2-ovmf-20221207gitfff6d81270b5-1.el9.noarch
qemu-kvm-7.2.0-1.el9.x86_64
kernel-5.14.0-212.el9.x86_64
libvirt-8.10.0-2.el9.x86_64

Steps:
1. Install and boot a sev-es guest with just uefi set:
# virt-install  --name rhel9_vm --mac *--location * --cpu host-model --memory 4096 --memtune hard_limit=4718592 --vcpus=4,sockets=1,cores=4,threads=1 --boot uefi--disk path=/home/rhel9_vm.qcow2,size=20,cache=none,bus=scsi,format=qcow2 --controller type=scsi,model=virtio-scsi --os-variant rhel9.0 --os-type=linux --graphics vnc,listen=0.0.0.0 --video virtio --controller type=virtio-serial --rng type=/dev/random --launchSecurity sev --debug --network bridge:br3,model=virtio --extra-args "ksdevice=eth0 inst.ks=* serial console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0" --noreboot

  <os>
    <type arch='x86_64' machine='pc-q35-rhel9.*'>hvm</type>
    <loader readonly='yes' type='pflash'>/usr/share/edk2/ovmf/OVMF_CODE.fd</loader>
    <nvram template='/usr/share/edk2/ovmf/OVMF_VARS.fd'>/var/lib/libvirt/qemu/nvram/rhel9_vm_VARS.fd</nvram>
    <boot dev='hd'/>
  </os>

Result:
Boot sev guest with -boot uefi, libvirt assign correct /usr/share/edk2/ovmf/OVMF_CODE.fd to VM, VM also enabled sev.

From libvirt perspective, this bug can also be verified.

Test Version:
libvirt-8.10.0-2.el9.x86_64
qemu-kvm-7.2.0-3.el9.x86_64
edk2-ovmf-20221207gitfff6d81270b5-2.el9.noarch

Test Steps:
S1: Start guest with efi firmware by default.
# virsh edit lmn
......
  <os firmware='efi'>
    <type arch='x86_64' machine='pc-q35-rhel9.2.0'>hvm</type>
    <boot dev='hd'/>
  </os>
......
# virsh start lmn
Domain 'lmn' started
# virsh dumpxml lmn | xmllint --xpath //os -
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <loader readonly="yes" secure="yes" type="pflash">/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</loader>
    <nvram template="/usr/share/edk2/ovmf/OVMF_VARS.secboot.fd">/var/lib/libvirt/qemu/nvram/lmn_VARS.fd</nvram>
    <boot dev="hd"/>
  </os>

S2: Start guest with enrolled-keys disabled.
# virsh edit lmn
......
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="no" name="enrolled-keys"/>
    </firmware>
    <boot dev="hd"/>
  </os>
......
# virsh start lmn
Domain 'lmn' started
# virsh dumpxml lmn | xmllint --xpath //os -<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="no" name="enrolled-keys"/>
    </firmware>
    <loader readonly="yes" secure="yes" type="pflash">/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</loader>
    <nvram template="/usr/share/edk2/ovmf/OVMF_VARS.fd">/var/lib/libvirt/qemu/nvram/lmn_VARS.fd</nvram>
    <boot dev="hd"/>
  </os>

S3: Start guest with enrolled-keys enabled.
# virsh edit lmn
......
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="yes" name="enrolled-keys"/>
      <feature enabled="yes" name="secure-boot"/>
    </firmware>
    <boot dev="hd"/>
  </os>
......
# virsh start lmn
Domain 'lmn' started
# virsh dumpxml lmn | xmllint --xpath //os -
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="yes" name="enrolled-keys"/>
      <feature enabled="yes" name="secure-boot"/>
    </firmware>
    <loader readonly="yes" secure="yes" type="pflash">/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</loader>
    <nvram template="/usr/share/edk2/ovmf/OVMF_VARS.secboot.fd">/var/lib/libvirt/qemu/nvram/lmn_VARS.fd</nvram>
    <boot dev="hd"/>
  </os>

S4: Start guest with secure-boot disabled.
# virsh edit lmn
......
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="no" name="enrolled-keys"/>
      <feature enabled="no" name="secure-boot"/>
    </firmware>
    <boot dev="hd"/>
  </os>
......
# virsh start lmn
Domain 'lmn' started
# virsh dumpxml lmn | xmllint --xpath //os -
<os>
    <type arch="x86_64" machine="pc-q35-rhel9.2.0">hvm</type>
    <firmware>
      <feature enabled="no" name="enrolled-keys"/>
      <feature enabled="no" name="secure-boot"/>
    </firmware>
    <loader readonly="yes" type="pflash">/usr/share/edk2/ovmf/OVMF_CODE.fd</loader>
    <nvram template="/usr/share/edk2/ovmf/OVMF_VARS.fd">/var/lib/libvirt/qemu/nvram/lmn_VARS.fd</nvram>
    <boot dev="hd"/>
  </os>

Thank you Meina and Zixi. According to Comment 41, Comment 45, Comment 47 and Comment 49, set status to VERIFIED.

/usr/share/qemu/firmware/30-edk2-ovmf-x64-sb-enrolled.json shows supported features amd-sev, but boot a sev guest with sb-enrolled will result in kernel panic. Gerd, could you help to check if it is a bug?

Version: 
edk2-ovmf-20221207gitfff6d81270b5-4.el9.noarch


Steps:
/usr/libexec/qemu-kvm \
-blockdev {"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"} \
-blockdev {"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"} \
-blockdev {"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/rhel86_sev_noes_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"} \
-blockdev {"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"} \
-machine pc-q35-rhel9.2.0,usb=off,smm=on,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \
...
-object {"qom-type":"sev-guest","id":"lsec0","cbitpos":51,"reduced-phys-bits":1,"policy":3} \
...

Actual result:
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
[    1.013909] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-252.el9.x86_64 #1
[    1.014735] Hardware name: Red Hat KVM/RHEL, BIOS edk2-20221207gitfff6d81270b5-4.el9 unknown
[    1.015635] Call Trace:
[    1.016198]  <TASK>
[    1.016741]  dump_stack_lvl+0x34/0x48
[    1.017367]  panic+0xf4/0x2c6
[    1.017949]  mount_block_root+0x28c/0x29f
[    1.018581]  prepare_namespace+0x13b/0x16a
[    1.019209]  kernel_init_freeable+0x17d/0x1a2
[    1.019850]  ? rest_init+0xd0/0xd0
[    1.020425]  kernel_init+0x16/0x130
[    1.020996]  ret_from_fork+0x22/0x30
[    1.021566]  </TASK>
[    1.022171] Kernel Offset: 0x2de00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[    1.023180] ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) ]---

> Actual result:
> Kernel panic - not syncing: VFS: Unable to mount root fs on
> unknown-block(0,0)

Doesn't look firmware-related on a quick glance.  Kernel booted up just fine, but then failed to find the root filesystem.

Does it work with OVMF_CODE.fd?
If so, can you attach firmware and kernel logs for both working and non-working cases?

(In reply to Gerd Hoffmann from comment #52)
> > Actual result:
> > Kernel panic - not syncing: VFS: Unable to mount root fs on
> > unknown-block(0,0)
> 
> Doesn't look firmware-related on a quick glance.  Kernel booted up just
> fine, but then failed to find the root filesystem.
> 
> Does it work with OVMF_CODE.fd?
It only reproduces with /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd and /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd when install guest with sev config

> If so, can you attach firmware and kernel logs for both working and
> non-working cases?
Sure.
Failed case: /usr/share/edk2/ovmf/OVMF_CODE.secboot.fd and /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd smm=on
console log: secureboot_console.log
firmware log: secureboot_ovmf.log

Working case:/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd and /usr/share/edk2/ovmf/OVMF_VARS.fd smm=on
console log: sevguest_console.log
firmware log: ovmf.log

--- good.log    2023-02-10 10:10:12.873794699 +0100
+++ bad.log     2023-02-10 10:10:28.201525337 +0100
@@ -1,8 +1,9 @@
 3h3h3h3h3hEFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
 EFI stub: Measured initrd data into PCR 9
+EFI stub: UEFI Secure Boot is enabled.
 Linux version 5.14.0-244.el9.x86_64 (mockbuild.eng.bos.redhat.com) (gcc (GCC) 11.3.1 20221121 (Red Hat 11.3.1-4), GNU ld version 2.35.2-35.el9) #1 SMP PREEMPT_DYNAMIC Wed Jan 25 15:35:31 EST 2023
 The list of certified hardware and cloud instances for Red Hat Enterprise Linux 9 can be viewed at the Red Hat Ecosystem Catalog, https://catalog.redhat.com.
-Command line: ksdevice=eth0 inst.ks=http://beaker.engineering.redhat.com/kickstart/11302736 serial console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0 swiotlb=262144 inst.repo=http://download.eng.pek2.redhat.com/rhel-9/composes/RHEL-9/RHEL-9.2.0-20230127.12/compose/BaseOS/x86_64/os/ initrd=initrd
+Command line: ksdevice=eth0 inst.ks=http://beaker.engineering.redhat.com/kickstart/11302736 serial console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0 swiotlb=262144 inst.repo=http://download.eng.pek2.redhat.com/rhel-9/composes/RHEL-9/RHEL-9.2.0-20230127.12/compose/BaseOS/x86_64/os/
 x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
 x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
 x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'

Note that the second kernel command line lacks the initrd=initrd line.
Looks like a bug indeed.  Probably unrelated to SEV.

Can you try whenever this reproduces with RHEL-9.1 guest?

(In reply to Gerd Hoffmann from comment #58)
> --- good.log    2023-02-10 10:10:12.873794699 +0100
> +++ bad.log     2023-02-10 10:10:28.201525337 +0100
> @@ -1,8 +1,9 @@
>  3h3h3h3h3hEFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device
> path
>  EFI stub: Measured initrd data into PCR 9
> +EFI stub: UEFI Secure Boot is enabled.
>  Linux version 5.14.0-244.el9.x86_64
> (mockbuild.eng.bos.redhat.com) (gcc (GCC) 11.3.1 20221121
> (Red Hat 11.3.1-4), GNU ld version 2.35.2-35.el9) #1 SMP PREEMPT_DYNAMIC Wed
> Jan 25 15:35:31 EST 2023
>  The list of certified hardware and cloud instances for Red Hat Enterprise
> Linux 9 can be viewed at the Red Hat Ecosystem Catalog,
> https://catalog.redhat.com.
> -Command line: ksdevice=eth0
> inst.ks=http://beaker.engineering.redhat.com/kickstart/11302736 serial
> console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0 swiotlb=262144
> inst.repo=http://download.eng.pek2.redhat.com/rhel-9/composes/RHEL-9/RHEL-9.
> 2.0-20230127.12/compose/BaseOS/x86_64/os/ initrd=initrd
> +Command line: ksdevice=eth0
> inst.ks=http://beaker.engineering.redhat.com/kickstart/11302736 serial
> console=tty0 console=ttyS0,115200 net.ifnames=0 biosdevname=0 swiotlb=262144
> inst.repo=http://download.eng.pek2.redhat.com/rhel-9/composes/RHEL-9/RHEL-9.
> 2.0-20230127.12/compose/BaseOS/x86_64/os/
>  x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
>  x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
>  x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
> 
> Note that the second kernel command line lacks the initrd=initrd line.
> Looks like a bug indeed.  Probably unrelated to SEV.
> 
> Can you try whenever this reproduces with RHEL-9.1 guest?
Thanks Gerd, I already tried with RHEL9.1 compose both host and guest, it can reproduce this issue. I will create a new bug to track.

Reported https://bugzilla.redhat.com/show_bug.cgi?id=2169247 to track sev guest with enrolled secure boot issue.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: edk2 security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2023:2165