Re: [Qemu-devel] [qemu RFC] qapi: add "firmware.json"

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Apr 10, 2018 at 11:20:33AM +0100, Daniel P. Berrangé wrote:
> On Sat, Apr 07, 2018 at 02:01:17AM +0200, Laszlo Ersek wrote:
> > Add a schema that describes the properties of virtual machine firmware.
> > 
> > Each firmware executable installed on a host system should come with a
> > JSON file that conforms to this schema, and informs the management
> > applications about the firmware's properties.
> > 
> > In addition, a configuration directory with symlinks to the JSON files
> > should exist, with the symlinks carefully named to reflect a priority
> > order. Management applications can then search this directory in priority
> > order for the first firmware executable that satisfies their search
> > criteria. The found JSON file provides the management layer with domain
> > configuration bits that are required to run the firmware binary.
> > 
> 
> > diff --git a/qapi/firmware.json b/qapi/firmware.json
> > new file mode 100644
> > index 000000000000..f267240f44dd
> > --- /dev/null
> > +++ b/qapi/firmware.json
> 
> [snip]
> 
> > +{ 'struct' : 'SystemFirmware',
> > +  'data'   : { 'executable'                 : 'FirmwareFile',
> > +               'type'                       : 'SystemFirmwareType',
> > +               'targets'                    : [ 'str' ],
> > +               'sysfw-map'                  : 'FirmwareMapping',
> > +               '*nvram-slots'               : [ 'NVRAMSlot' ],
> > +               '*supports-uefi-secure-boot' : 'bool',
> > +               '*supports-amd-sev'          : 'bool',
> > +               '*supports-acpi-s3'          : 'bool',
> > +               '*supports-acpi-s4'          : 'bool' } }
> 
> Elsewhere in the thread I mentioned that I think we should try to use a
> union approach to isolate which information is relevant to "flash" loader
> format and which is relevant to "memory" and "kernel". To try to illustrate
> what I mean by that I've knocked up an alternative structure. I also
> incorporated the points about features & target/machine types.  I've left
> out the read/write/etc fields, but they could be put back in at the
> relevant position
> 
> 
> { 'enum' : 'SystemFirmwareType',
>   'data' : [ 'bios', 'slof', 'uboot', 'uefi' ] }
> 
> { 'enum' : 'SystemFirmwareDevice',
>   'data' : [ 'memory', 'kernel', 'flash' ] }
> 
> { 'enum' : 'SystemFirmwareArchitecture',
>   'data':  ['x86_64', 'i386', ..etc.. ] }
>   
> { 'enum' : 'SystemFirmwareFeature',
>   'data': ['acpi-s3', 'acpi-s5', 'secure-boot', 'amd-sev' ]}
> 
> 
> ## Struct(s) for device==memory
> 
> { 'struct': 'SystemFirmwareBinaryMemory',
>   'data': { 'pathname': 'str' } }
> 
> 
> ## Struct(s) for device==kernel
> 
> { 'struct': 'SystemFirmwareBinaryKernel',
>   'data': { 'pathname': 'str' } }
> 
> 
> ## Struct(s) for device==flash
> 
> { 'struct': 'SystemFirmwareBinaryFlashFile',
>   'data':  { 'filename': 'str',
>              'format': 'BlockdevDriver' } }
> 
> { 'struct': 'SystemFirmwareBinaryFlashCode',
>   'base': 'SystemFirmwareBinaryFlashFile' }
> 
> { 'struct': 'SystemFirmwareBinaryFlashVars',
>   'base': 'SystemFirmwareBinaryFlashFile',
>   'data': { 'secure-boot-key-enroll': 'bool' } }
> 
> { 'struct': 'SystemFirmwareBinaryFlash',
>   'data': { 'code': 'SystemFirmwareBinaryFlashCode',
>             'vars': ['SystemFirmwareBinaryFlashVars' ] } }
> 
> 
> ## Discriminated struct for different loading approaches
> 
> { 'union': 'SystemFirmwareBinary',
>   'base': { 'device': 'SystemFirmwareDevice' },
>   'discriminator': 'device',
>   'data': { 'memory': 'SystemFirmwareBinaryMemory',
>             'kernel': 'SystemFirmwareBinaryKernel',
>             'flash': 'SystemFirmwareBinaryFlash' } }
> 
> 
> 
> { 'struct' : 'SystemFirmwareTarget',
>   'data': { 'architecture': 'SystemFirmwareArchitecture',
>             'machines': [ 'str' ] } }
> 
> 
> { 'struct' : 'SystemFirmware',
>   'data'   : {
>       'description'  : 'str',
>       'type'         : 'SystemFirmwareType',
>       'binary'       : 'SystemFirmwareBinary',
>       'targets'      : [ 'SystemFirmwareTarget' ],
>       'features'     : ['SystemFirmwareFeature'] } } 
> 
> 
> 
> # Examples:
> #
> # {
> #    'description': 'SeaBIOS 256k',
> #    'type': 'bios',
> #    'binary': {
> #        'type': 'memory',
> #        'filename': '/path/to/seabios/rom-256k',
> #    }
> #    'targets':  {
> #        'x86_64': [ "pc", "q35"],
> #        'i386': [ "pc", "q35"],
> #    }
> #    'features': ['acpi-s3', 'acpi-s5'],
> # }
> # {
> #    'description': 'SeaBIOS 128k',
> #    'type': 'bios',
> #    'binary': {
> #        'type': 'memory',
> #        'filename': '/path/to/seabios/rom-128k',
> #    }
> #    'targets':  {
> #        'x86_64': [ "isapc"],
> #        'i386': [ "isapc"],
> #    }
> #    'features': [],
> # }
> # {
> #    'description': 'OVMF',
> #    'type': 'uefi'
> #    'binary': {
> #        'type': 'flash',
> #        'code': {
> #          'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd',
> #          'format': 'raw',
> #        },
> #        'vars': [
> #           {
> #              'filename': '/usr/share/OVMF/OVMF_VARS.fd',
> #              'format': 'raw',
> #              'secure=boot-key-enroll': false,
> #           },
> #           {
> #              'filename': '/usr/share/OVMF/OVMF_VARS.secboot.fd',
> #              'format': 'raw',
> #              'secure=boot-key-enroll': true,
> #           }

It occurs to me that we are actually over-thinking things, by making it
possible to list a choice of vars files per firmware. We could remove this
special case by just having separate tpo level firmware entries and a main
feature flag to say if it is enrolled or not - see below example

> #        ],
> #    },
> #    'targets':  {
> #        'x86_64': [ "q35"],
> #    }
> #    'features': ['acpi-s3', 'acpi-s5', 'secure-boot'],
> # }
> #


{
   'description': 'OVMF secboot',
   'type': 'uefi'
   'binary': {
       'type': 'flash',
       'code': {
         'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd',
         'format': 'raw',
       },
       'vars': {
         'filename': '/usr/share/OVMF/OVMF_VARS.fd',
         'format': 'raw',
       },
   },
   'targets':  {
       'x86_64': [ "q35"],
   }
   'features': ['acpi-s3', 'acpi-s5', 'secure-boot'],
}

{
   'description': 'OVMF secboot enrolled',
   'type': 'uefi'
   'binary': {
       'type': 'flash',
       'code': {
         'filename': '/usr/share/OVMF/OVMF_CODE.secboot.fd',
         'format': 'raw',
       },
       'vars': {
         'filename': '/usr/share/OVMF/OVMF_VARS.secboot.fd',
         'format': 'raw',
       }
   },
   'targets':  {
       'x86_64': [ "q35"],
   }
   'features': ['acpi-s3', 'acpi-s5', 'secure-boot', "secure-boot-enrolled-keys"],
}

Avoiding recording the notion of secureboot enrollment against the VARs
files, means that you have more flexibility. One could just have a single
file containing both CODE+VARS, which is enrolled instead of separating
them.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|