On Tue, Aug 05, 2025 at 04:28:18PM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 05, 2025 at 08:54:02AM -0500, Andrea Bolognani wrote:
> > On Thu, Jul 31, 2025 at 07:33:21PM +0100, Daniel P. Berrangé via Devel
> > wrote:
> > > +++ b/src/qemu/qemu_firmware.c
> > > @@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> > > bool requiresSMM = false;
> > > bool supportsSecureBoot = false;
> > > bool hasEnrolledKeys = false;
> > > + bool cvm = false;
> >
> > Maybe isConfidential instead, to follow the existing convention and
> > be a little more descriptive?
> >
> > > @@ -1566,7 +1569,8 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> > > }
> > > }
> > >
> > > - if ((supportsSecureBoot != requiresSMM) ||
> > > + if ((!cvm &&
> > > + (supportsSecureBoot != requiresSMM)) ||
> > > (hasEnrolledKeys && !supportsSecureBoot)) {
> > > VIR_WARN("Firmware description '%s' has invalid set of features:
> > > "
> > > "%s = %d, %s = %d, %s = %d",
> >
> > This could use a short comment explaining why firmware intended for
> > CVM doesn't need SSM for Secure Boot.
> >
> > Regardless of whether you want to act on any of the above
> > suggestions, the change makes sense so
>
> I made both those changes and pushed.
Looks great, thank you!
--
Andrea Bolognani / Red Hat / Virtualization
