[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] docs: Discourage users from using fwcfg



On 09/07/20 15:48, Michal Privoznik wrote:
> Even though this was brought up in upstream discussion [1] it
> missed my patches: users should prefer <oemStrings/> over fwcfg.
> The reason is that fwcfg is considered somewhat internal to QEMU
> and it has limited number of slots and neither of these applies
> to <oemStrings/>.
>
> While I'm at it, I'm fixing the example too (because it contains
> incorrect element name) and clarifying sysfs/ exposure.
>
> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html
>
> Reported-by: Laszlo Ersek <lersek redhat com>
> Signed-off-by: Michal Privoznik <mprivozn redhat com>
> ---
>  docs/formatdomain.rst | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 1979dfb8d3..821ffe8d60 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of:
>     Some hypervisors provide unified way to tweak how firmware configures itself,
>     or may contain tables to be installed for the guest OS, for instance boot
>     order, ACPI, SMBIOS, etc. It even allows users to define their own config
> -   blobs. In case of QEMU, these then appear under domain's sysfs, under
> +   blobs. In case of QEMU, these then appear under domain's sysfs (if the guest
> +   kernel has FW_CFG_SYSFS config option enabled), under
>     ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the
>     <smbios/> mode under <os/>. :since:`Since 6.5.0`
>
> +   **Please note that because of limited number of data slots use of fwcfg is
> +   strongly discouraged and <oemStrings/> should be used instead**.

please replace:

  strongly discouraged

with:

  strongly discouraged for configuring any guest-side component other
  than the firmware

(

Consider for example the following feature:

  https://bugzilla.tianocore.org/show_bug.cgi?id=2681

Namely, the following QEMU switches:

  -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn]
  -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn]

alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be
stable. They do not need dedicated QEMU or libvirtd enablement. They
influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine
(even ideal!) for tweaking them, through the domain XML. What's not fine
is configuring any random guest payload via <sysinfo type='fwcfg'>.

The point is that people who parse new fw_cfg files in edk2 such as
"opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in
QEMU. They *can* bump the "x-file-slots" property in QEMU, for new
machine types, they just need to be aware of the property.

)

> +
>     ::
>
> -        <smbios type='fwcfg'>
> +        <sysinfo type='fwcfg'>
>            <entry name='opt/com.example/name'>example value</entry>

I suggest (according to the above):

- name: opt/org.tianocore/IPv4PXESupport
- value: n

> -          <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/>
> -        </smbios>
> +          <entry name='opt/com.example/config' file='/tmp/provision.ign'/>

We have a functional -- working, stable -- example for name+file as
well:

- name: etc/edk2/https/cacerts
- file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin

(This is documented in "OvmfPkg/README" in edk2, but it's not really
relevant here.)

> +        </sysinfo>
>
> -   The ``smbios`` element can have multiple ``entry`` child elements. Each
> +   The ``sysinfo`` element can have multiple ``entry`` child elements. Each
>     element then has mandatory ``name`` attribute, which defines the name of the
>     blob and must begin with ``"opt/"`` and to avoid clashing with other names is
>     advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is a reverse
>

It's hard to express this cleanly.

- The opt/RFQDN notation is a mitigation for users that are hell-bent on
  using fw-cfg files of their own purposes (not heeding our advice about
  not using fw-cfg for such purposes at all). So the idea is, "if you
  ignore our request, then (a) be prepared to run out of slots, and (b)
  *at least* use a name pattern (opt/RFQDN) that minimizes conflicts
  with other, similar-minded users / projects"

- For "officially supported" knobs that the firmware looks at, it's fine
  to use any names -- they avoid conflicts with the above "rogue" files.
  Examples:

  - opt/ovmf/          -- reserved for historical reasons

  - opt/org.tianocore/ -- should never conflict due to RFQDN

  - etc/edk2/https/... -- should never conflict due to being outside of
                          opt/

So I guess the short rule is, "Feel free to refer to any fw_cfg file
name that your firmware officially supports. When defining other fw_cfg
file names (i.e., for your own purposes), then prepare for breakage in
the long-term, and then at least use the opt/RFQDN/ name pattern".

Thank you,
Laszlo


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]