[Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

Allowing arbitary file names on command line is setting us up for
failure: future guests will look for a specific QEMU-specified name and
will get confused finding a user file there.

We do warn but people are conditioned to ignore warnings by now,
so at best that will help users debug problem, not avoid it.

Disable this by default, so distros don't get to deal with it,
but leave an option for developers to configure this in,
with scary warnings so people only do it if they know
what they are doing.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

changes from v1:
    add configure option

 configure | 7 +++++++
 vl.c      | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/configure b/configure
index 2b32876..e5a692f 100755
--- a/configure
+++ b/configure
@@ -256,6 +256,7 @@ sysconfdir="\${prefix}/etc"
 local_statedir="\${prefix}/var"
 confsuffix="/qemu"
 slirp="yes"
+unsafe_fw_cfg="no"
 oss_lib=""
 bsd="no"
 linux="no"
@@ -875,6 +876,8 @@ for opt do
   ;;
   --enable-vnc-png) vnc_png="yes"
   ;;
+  --enable-unsafe-fw-cfg) unsafe_fw_cfg="yes"
+  ;;
   --disable-slirp) slirp="no"
   ;;
   --disable-uuid) uuid="no"
@@ -1286,6 +1289,7 @@ Advanced options (experts only):
                            Available backends: $($python $source_path/scripts/tracetool.py --list-backends)
   --with-trace-file=NAME   Full PATH,NAME of file to store traces
                            Default:trace-<pid>
+  --enable-unsafe-fw-cfg   Enable loading fw cfg entries at unsafe paths (broken! don't use in production!)
   --disable-slirp          disable SLIRP userspace network connectivity
   --enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)
   --oss-lib                path to OSS library
@@ -4911,6 +4915,9 @@ fi
 if test "$profiler" = "yes" ; then
   echo "CONFIG_PROFILER=y" >> $config_host_mak
 fi
+if test "$unsafe_fw_cfg" = "yes" ; then
+  echo "CONFIG_UNSAFE_FW_CFG=y" >> $config_host_mak
+fi
 if test "$slirp" = "yes" ; then
   echo "CONFIG_SLIRP=y" >> $config_host_mak
   echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak
diff --git a/vl.c b/vl.c
index 7a28982..5adf217 100644
--- a/vl.c
+++ b/vl.c
@@ -2321,6 +2321,9 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     if (strncmp(name, "opt/", 4) != 0) {
         error_report("warning: externally provided fw_cfg item names "
                      "should be prefixed with \"opt/\"");
+#ifndef CONFIG_UNSAFE_FW_CFG
+        return -1;
+#endif
     }
     if (nonempty_str(str)) {
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> Allowing arbitary file names on command line is setting us up for
> failure: future guests will look for a specific QEMU-specified name and
> will get confused finding a user file there.
>
> We do warn but people are conditioned to ignore warnings by now,
> so at best that will help users debug problem, not avoid it.
>
> Disable this by default, so distros don't get to deal with it,
> but leave an option for developers to configure this in,
> with scary warnings so people only do it if they know
> what they are doing.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

I'm having a hard time to see the point.  There are *countless* ways
users can mess up their guests from the command line or the monitor.
Yet we get so hung up on this rather obscure one to attempt to catch
some (but far from all) misuses that may trigger it, and then, because
that interferes with debugging, add a *configure* option to suppress it
again.

Misuse of -fw_cfg isn't even a molehill, but adding a configure option
*is* making a mountain out of it.

If you know what "fw cfg" is (few users do), and know how to put the
user-specified fw cfg files feature to use (even fewer), you're damn
well expected to know to stick to /opt or else.  We already print a
reminder.  That's as far as I'd want us to go.

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > Allowing arbitary file names on command line is setting us up for
> > failure: future guests will look for a specific QEMU-specified name and
> > will get confused finding a user file there.
> >
> > We do warn but people are conditioned to ignore warnings by now,
> > so at best that will help users debug problem, not avoid it.
> >
> > Disable this by default, so distros don't get to deal with it,
> > but leave an option for developers to configure this in,
> > with scary warnings so people only do it if they know
> > what they are doing.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I'm having a hard time to see the point.

Frankly, I am having a hard time to see the point of exposing fw cfg to
users at all.  It was designed as an internal interface between QEMU PC
hardware and firmware.  As a PC maintainer, I do not like it that users
get to poke at PC internals.

So it is yet another way to pass binaries to Linux guests.  Don't we
have enough of these?  But Gerd likes it for some reason, and merged it.
OK.

But please find a way to make sure it does not conflict with its current
usage in PC.  Asking that all files have an "opt/" prefix is one way
but only if it is enforced.

This is polluting global namespace.


> There are *countless* ways
> users can mess up their guests from the command line or the monitor.
> Yet we get so hung up on this rather obscure one to attempt to catch
> some (but far from all) misuses that may trigger it, and then, because
> that interferes with debugging, add a *configure* option to suppress it
> again.

In fact, how does it interfere with debugging? debugging what? by whom?
I think we should figure that out instead of carrying more opaque binaries
around.

E.g. if people want to replace built-in ACPI tables, say so,
and we'll look at sane ways to do this that do not
involve poking at acpi internals.


> Misuse of -fw_cfg isn't even a molehill, but adding a configure option
> *is* making a mountain out of it.
> 

Yea. That configure hack is an attempt at a compromise.

But maybe the real fix is to rip fw cfg tweaking from
command line out completely, at least for 2.6.

> If you know what "fw cfg" is (few users do), and know how to put the
> user-specified fw cfg files feature to use (even fewer), you're damn
> well expected to know to stick to /opt or else.  We already print a
> reminder.  That's as far as I'd want us to go.

What happens is that inevitable someone finds a bug, and then
finds a work around, and blog posts spring up advertising
the fix, and you can never go back again.

Or a tool uses this, no one notices for years,
and after an update someone gets to debug it
until finally the warning is noticed.
Or more likely it all ends up in the launchpad/bugzilla,
with yet another useful tool broken for no good reason.


-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Gabriel L. Somlo]

On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote:
> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> writes:
> > 
> > > Allowing arbitary file names on command line is setting us up for
> > > failure: future guests will look for a specific QEMU-specified name and
> > > will get confused finding a user file there.
> > >
> > > We do warn but people are conditioned to ignore warnings by now,
> > > so at best that will help users debug problem, not avoid it.
> > >
> > > Disable this by default, so distros don't get to deal with it,
> > > but leave an option for developers to configure this in,
> > > with scary warnings so people only do it if they know
> > > what they are doing.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > I'm having a hard time to see the point.
> 
> Frankly, I am having a hard time to see the point of exposing fw cfg to
> users at all.  It was designed as an internal interface between QEMU PC
> hardware and firmware.  As a PC maintainer, I do not like it that users
> get to poke at PC internals.
> 
> So it is yet another way to pass binaries to Linux guests.  Don't we
> have enough of these?  But Gerd likes it for some reason, and merged it.
> OK.

As the author of the feature, I feel I should jump back in and clarify:

It's a way to pass arbitrary blobs to any type of guest, with two
important properties: 1. asynchronous, and 2. out-of-band. When I
started looking, all existing methods involved either having the host
start polling for the guest to bring up qga and be ready to accept an
out-of-band connection (i.e., *not* asynchronous), or have the guest
mount some special cdrom or floppy image prepared by the host (i.e.,
*not* out of band).

fw_cfg is both asynchronous and out-of-band, so it appeared to be the
perfect choice.

> But please find a way to make sure it does not conflict with its current
> usage in PC.  Asking that all files have an "opt/" prefix is one way
> but only if it is enforced.

Enforcing the "opt/" prefix was clearly on the table when I submitted
the feature (and totally acceptable for my own needs). At the time, however,
most of the advice I received on the list was to leave it as a warning
only (i.e., "mechanism, not policy"), especially since other respondents
expressed interest in passing in non-"/opt" blobs for easier development
and debugging of alternative firmware (such as OVMF, iirc).

Having a mis-use of this feature become "institutionalized" over time was
seen as a low/negligible risk at the time. Do we have any new reasons
to worry about it ?

Thanks much,
--Gabriel

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/16/16 19:15, Gabriel L. Somlo wrote:
> On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote:
>> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>
>>>> Allowing arbitary file names on command line is setting us up for
>>>> failure: future guests will look for a specific QEMU-specified name and
>>>> will get confused finding a user file there.
>>>>
>>>> We do warn but people are conditioned to ignore warnings by now,
>>>> so at best that will help users debug problem, not avoid it.
>>>>
>>>> Disable this by default, so distros don't get to deal with it,
>>>> but leave an option for developers to configure this in,
>>>> with scary warnings so people only do it if they know
>>>> what they are doing.
>>>>
>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> I'm having a hard time to see the point.
>>
>> Frankly, I am having a hard time to see the point of exposing fw cfg to
>> users at all.  It was designed as an internal interface between QEMU PC
>> hardware and firmware.  As a PC maintainer, I do not like it that users
>> get to poke at PC internals.
>>
>> So it is yet another way to pass binaries to Linux guests.  Don't we
>> have enough of these?  But Gerd likes it for some reason, and merged it.
>> OK.
> 
> As the author of the feature, I feel I should jump back in and clarify:
> 
> It's a way to pass arbitrary blobs to any type of guest, with two
> important properties: 1. asynchronous, and 2. out-of-band. When I
> started looking, all existing methods involved either having the host
> start polling for the guest to bring up qga and be ready to accept an
> out-of-band connection (i.e., *not* asynchronous), or have the guest
> mount some special cdrom or floppy image prepared by the host (i.e.,
> *not* out of band).
> 
> fw_cfg is both asynchronous and out-of-band, so it appeared to be the
> perfect choice.
> 
>> But please find a way to make sure it does not conflict with its current
>> usage in PC.  Asking that all files have an "opt/" prefix is one way
>> but only if it is enforced.
> 
> Enforcing the "opt/" prefix was clearly on the table when I submitted
> the feature (and totally acceptable for my own needs). At the time, however,
> most of the advice I received on the list was to leave it as a warning
> only (i.e., "mechanism, not policy"), especially since other respondents
> expressed interest in passing in non-"/opt" blobs for easier development
> and debugging of alternative firmware (such as OVMF, iirc).
> 
> Having a mis-use of this feature become "institutionalized" over time was
> seen as a low/negligible risk at the time. Do we have any new reasons
> to worry about it ?

OVMF uses this feature for a few flags. They are all called
"opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
shouldn't be surprising since I seem to have reviewed every patch for
that file):

> NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
> when using the "-fw_cfg" command line option, to avoid conflicting with
> item names used internally by QEMU. For instance:
>
> -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>
> Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
> "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().

It says "should", not "must". I liked (and like) the "mechanism, not
policy" thing. Letting developers pass in whatever they want, for
development / debugging / testing purposes, is a plus to me.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
> On 03/16/16 19:15, Gabriel L. Somlo wrote:
> > On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote:
> >> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
> >>> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >>>
> >>>> Allowing arbitary file names on command line is setting us up for
> >>>> failure: future guests will look for a specific QEMU-specified name and
> >>>> will get confused finding a user file there.
> >>>>
> >>>> We do warn but people are conditioned to ignore warnings by now,
> >>>> so at best that will help users debug problem, not avoid it.
> >>>>
> >>>> Disable this by default, so distros don't get to deal with it,
> >>>> but leave an option for developers to configure this in,
> >>>> with scary warnings so people only do it if they know
> >>>> what they are doing.
> >>>>
> >>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>
> >>> I'm having a hard time to see the point.
> >>
> >> Frankly, I am having a hard time to see the point of exposing fw cfg to
> >> users at all.  It was designed as an internal interface between QEMU PC
> >> hardware and firmware.  As a PC maintainer, I do not like it that users
> >> get to poke at PC internals.
> >>
> >> So it is yet another way to pass binaries to Linux guests.  Don't we
> >> have enough of these?  But Gerd likes it for some reason, and merged it.
> >> OK.
> > 
> > As the author of the feature, I feel I should jump back in and clarify:
> > 
> > It's a way to pass arbitrary blobs to any type of guest, with two
> > important properties: 1. asynchronous, and 2. out-of-band. When I
> > started looking, all existing methods involved either having the host
> > start polling for the guest to bring up qga and be ready to accept an
> > out-of-band connection (i.e., *not* asynchronous), or have the guest
> > mount some special cdrom or floppy image prepared by the host (i.e.,
> > *not* out of band).
> > 
> > fw_cfg is both asynchronous and out-of-band, so it appeared to be the
> > perfect choice.
> > 
> >> But please find a way to make sure it does not conflict with its current
> >> usage in PC.  Asking that all files have an "opt/" prefix is one way
> >> but only if it is enforced.
> > 
> > Enforcing the "opt/" prefix was clearly on the table when I submitted
> > the feature (and totally acceptable for my own needs). At the time, however,
> > most of the advice I received on the list was to leave it as a warning
> > only (i.e., "mechanism, not policy"), especially since other respondents
> > expressed interest in passing in non-"/opt" blobs for easier development
> > and debugging of alternative firmware (such as OVMF, iirc).
> > 
> > Having a mis-use of this feature become "institutionalized" over time was
> > seen as a low/negligible risk at the time. Do we have any new reasons
> > to worry about it ?
> 
> OVMF uses this feature for a few flags. They are all called
> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
> shouldn't be surprising since I seem to have reviewed every patch for
> that file):
> 
> > NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
> > when using the "-fw_cfg" command line option, to avoid conflicting with
> > item names used internally by QEMU. For instance:
> >
> > -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> >
> > Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
> > "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().
> 
> It says "should", not "must".

should means "might be ok". So change it to must then?

> I liked (and like) the "mechanism, not
> policy" thing. Letting developers pass in whatever they want, for
> development / debugging / testing purposes, is a plus to me.
> 
> Thanks
> Laszlo

Could you flesh out the development / debugging / testing and
how is inserting files in PC namespace useful for that?

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/16/16 19:43, Michael S. Tsirkin wrote:
> On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
>> On 03/16/16 19:15, Gabriel L. Somlo wrote:
>>> On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote:
>>>> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
>>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>>>
>>>>>> Allowing arbitary file names on command line is setting us up for
>>>>>> failure: future guests will look for a specific QEMU-specified name and
>>>>>> will get confused finding a user file there.
>>>>>>
>>>>>> We do warn but people are conditioned to ignore warnings by now,
>>>>>> so at best that will help users debug problem, not avoid it.
>>>>>>
>>>>>> Disable this by default, so distros don't get to deal with it,
>>>>>> but leave an option for developers to configure this in,
>>>>>> with scary warnings so people only do it if they know
>>>>>> what they are doing.
>>>>>>
>>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>
>>>>> I'm having a hard time to see the point.
>>>>
>>>> Frankly, I am having a hard time to see the point of exposing fw cfg to
>>>> users at all.  It was designed as an internal interface between QEMU PC
>>>> hardware and firmware.  As a PC maintainer, I do not like it that users
>>>> get to poke at PC internals.
>>>>
>>>> So it is yet another way to pass binaries to Linux guests.  Don't we
>>>> have enough of these?  But Gerd likes it for some reason, and merged it.
>>>> OK.
>>>
>>> As the author of the feature, I feel I should jump back in and clarify:
>>>
>>> It's a way to pass arbitrary blobs to any type of guest, with two
>>> important properties: 1. asynchronous, and 2. out-of-band. When I
>>> started looking, all existing methods involved either having the host
>>> start polling for the guest to bring up qga and be ready to accept an
>>> out-of-band connection (i.e., *not* asynchronous), or have the guest
>>> mount some special cdrom or floppy image prepared by the host (i.e.,
>>> *not* out of band).
>>>
>>> fw_cfg is both asynchronous and out-of-band, so it appeared to be the
>>> perfect choice.
>>>
>>>> But please find a way to make sure it does not conflict with its current
>>>> usage in PC.  Asking that all files have an "opt/" prefix is one way
>>>> but only if it is enforced.
>>>
>>> Enforcing the "opt/" prefix was clearly on the table when I submitted
>>> the feature (and totally acceptable for my own needs). At the time, however,
>>> most of the advice I received on the list was to leave it as a warning
>>> only (i.e., "mechanism, not policy"), especially since other respondents
>>> expressed interest in passing in non-"/opt" blobs for easier development
>>> and debugging of alternative firmware (such as OVMF, iirc).
>>>
>>> Having a mis-use of this feature become "institutionalized" over time was
>>> seen as a low/negligible risk at the time. Do we have any new reasons
>>> to worry about it ?
>>
>> OVMF uses this feature for a few flags. They are all called
>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
>> shouldn't be surprising since I seem to have reviewed every patch for
>> that file):
>>
>>> NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
>>> when using the "-fw_cfg" command line option, to avoid conflicting with
>>> item names used internally by QEMU. For instance:
>>>
>>> -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>>>
>>> Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
>>> "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().
>>
>> It says "should", not "must".
> 
> should means "might be ok".

Yes, if there is a pressing reason. And even then, "you have been warned".

> So change it to must then?

I didn't suggest that.

(For consistency, your patch should be attempting to modify the code and
the docs together -- but this doesn't mean that I agree with the patch.)

>> I liked (and like) the "mechanism, not
>> policy" thing. Letting developers pass in whatever they want, for
>> development / debugging / testing purposes, is a plus to me.
>>
>> Thanks
>> Laszlo
> 
> Could you flesh out the development / debugging / testing and
> how is inserting files in PC namespace useful for that?

I don't know -- which is part of the argument. Lack of a ready example
doesn't imply (to me) that the possibility should be excluded.

As Paolo said, I believe we've been through this discussion. I have no
new arguments; the old ones are valid to me still. I won't try to
influence this discussion any further; I only chimed in because OVMF was
mentioned (and because I noticed that the docs would get out of sync
with the code).

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 08:15:07PM +0100, Laszlo Ersek wrote:
> On 03/16/16 19:43, Michael S. Tsirkin wrote:
> > On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
> >> On 03/16/16 19:15, Gabriel L. Somlo wrote:
> >>> On Wed, 16 Mar 2016 at 18:50:57 +0200, Michael S. Tsirkin wrote:
> >>>> On Wed, Mar 16, 2016 at 05:29:45PM +0100, Markus Armbruster wrote:
> >>>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >>>>>
> >>>>>> Allowing arbitary file names on command line is setting us up for
> >>>>>> failure: future guests will look for a specific QEMU-specified name and
> >>>>>> will get confused finding a user file there.
> >>>>>>
> >>>>>> We do warn but people are conditioned to ignore warnings by now,
> >>>>>> so at best that will help users debug problem, not avoid it.
> >>>>>>
> >>>>>> Disable this by default, so distros don't get to deal with it,
> >>>>>> but leave an option for developers to configure this in,
> >>>>>> with scary warnings so people only do it if they know
> >>>>>> what they are doing.
> >>>>>>
> >>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>>>
> >>>>> I'm having a hard time to see the point.
> >>>>
> >>>> Frankly, I am having a hard time to see the point of exposing fw cfg to
> >>>> users at all.  It was designed as an internal interface between QEMU PC
> >>>> hardware and firmware.  As a PC maintainer, I do not like it that users
> >>>> get to poke at PC internals.
> >>>>
> >>>> So it is yet another way to pass binaries to Linux guests.  Don't we
> >>>> have enough of these?  But Gerd likes it for some reason, and merged it.
> >>>> OK.
> >>>
> >>> As the author of the feature, I feel I should jump back in and clarify:
> >>>
> >>> It's a way to pass arbitrary blobs to any type of guest, with two
> >>> important properties: 1. asynchronous, and 2. out-of-band. When I
> >>> started looking, all existing methods involved either having the host
> >>> start polling for the guest to bring up qga and be ready to accept an
> >>> out-of-band connection (i.e., *not* asynchronous), or have the guest
> >>> mount some special cdrom or floppy image prepared by the host (i.e.,
> >>> *not* out of band).
> >>>
> >>> fw_cfg is both asynchronous and out-of-band, so it appeared to be the
> >>> perfect choice.
> >>>
> >>>> But please find a way to make sure it does not conflict with its current
> >>>> usage in PC.  Asking that all files have an "opt/" prefix is one way
> >>>> but only if it is enforced.
> >>>
> >>> Enforcing the "opt/" prefix was clearly on the table when I submitted
> >>> the feature (and totally acceptable for my own needs). At the time, however,
> >>> most of the advice I received on the list was to leave it as a warning
> >>> only (i.e., "mechanism, not policy"), especially since other respondents
> >>> expressed interest in passing in non-"/opt" blobs for easier development
> >>> and debugging of alternative firmware (such as OVMF, iirc).
> >>>
> >>> Having a mis-use of this feature become "institutionalized" over time was
> >>> seen as a low/negligible risk at the time. Do we have any new reasons
> >>> to worry about it ?
> >>
> >> OVMF uses this feature for a few flags. They are all called
> >> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
> >> shouldn't be surprising since I seem to have reviewed every patch for
> >> that file):
> >>
> >>> NOTE: Users *SHOULD* choose item names beginning with the prefix "opt/"
> >>> when using the "-fw_cfg" command line option, to avoid conflicting with
> >>> item names used internally by QEMU. For instance:
> >>>
> >>> -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> >>>
> >>> Similarly, QEMU developers *SHOULD NOT* use item names prefixed with
> >>> "opt/" when inserting items programmatically, e.g. via fw_cfg_add_file().
> >>
> >> It says "should", not "must".
> > 
> > should means "might be ok".
> 
> Yes, if there is a pressing reason. And even then, "you have been warned".
> 
> > So change it to must then?
> 
> I didn't suggest that.
> 
> (For consistency, your patch should be attempting to modify the code and
> the docs together -- but this doesn't mean that I agree with the patch.)
> 
> >> I liked (and like) the "mechanism, not
> >> policy" thing. Letting developers pass in whatever they want, for
> >> development / debugging / testing purposes, is a plus to me.
> >>
> >> Thanks
> >> Laszlo
> > 
> > Could you flesh out the development / debugging / testing and
> > how is inserting files in PC namespace useful for that?
> 
> I don't know -- which is part of the argument. Lack of a ready example
> doesn't imply (to me) that the possibility should be excluded.
> 
> As Paolo said, I believe we've been through this discussion. I have no
> new arguments; the old ones are valid to me still. I won't try to
> influence this discussion any further; I only chimed in because OVMF was
> mentioned (and because I noticed that the docs would get out of sync
> with the code).
> 
> Thanks
> Laszlo

By the way, these are developer docs.  User docs do not say anything:

       -fw_cfg [name=]name,file=file
           Add named fw_cfg entry from file. name determines the name of
	   the entry in the fw_cfg file directory exposed to the guest.

       -fw_cfg [name=]name,string=str
           Add named fw_cfg entry from string.


-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 08:15:07PM +0100, Laszlo Ersek wrote:
> >> I liked (and like) the "mechanism, not
> >> policy" thing. Letting developers pass in whatever they want, for
> >> development / debugging / testing purposes, is a plus to me.
> >>
> >> Thanks
> >> Laszlo
> > 
> > Could you flesh out the development / debugging / testing and
> > how is inserting files in PC namespace useful for that?
> 
> I don't know -- which is part of the argument. Lack of a ready example
> doesn't imply (to me) that the possibility should be excluded.

When it comes to an interface that has to be maintained indefinitely,
only allowing what you absolutely need seems like exactly the correct
design.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
> OVMF uses this feature for a few flags. They are all called
> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
> shouldn't be surprising since I seem to have reviewed every patch for
> that file):

Wait a second.  You are saying upsteam OVMF puts files there.
If users add their own flags that happen to be in opt/ovmf/ then
what happens? And how do they *know* they should avoid that?
There's no warning, guest just breaks in various ways.

This is exactly the kind of mess I was worried about. We have a global
namespace with no way to control what goes where.

opt/ was for end users not firmware.

The right thing to do would be for ovmf to reserve itself
a directory under root.

This really needs more thought.  For now I'd suggest we drop the whole
interface from 2.6 and come back to it after 2.6.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Gerd Hoffmann]

  Hi,

> > Having a mis-use of this feature become "institutionalized" over time was
> > seen as a low/negligible risk at the time. Do we have any new reasons
> > to worry about it ?
> 
> OVMF uses this feature for a few flags. They are all called
> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
> shouldn't be surprising since I seem to have reviewed every patch for
> that file):

seabios has a few flags too, in etc/.  Some of them are supported
directly by qemu (such as setting the boot menu delay).  Others are not,
and I'd like to be able to use -fw_cfg for them for testing/debugging
(any use cases beyond that should be supported by adding a less obscure
way to set them to qemu, similar to the boot delay).

Oh, and that'll most likely be more seabios testing than qemu testing,
so why require me build a special qemu version for that?

On the ovmf flags:  What kind of flags are there?  Anything a normal
user might want to set?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/16/16 21:31, Michael S. Tsirkin wrote:
> On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
>> OVMF uses this feature for a few flags. They are all called
>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
>> shouldn't be surprising since I seem to have reviewed every patch for
>> that file):
> 
> Wait a second.  You are saying upsteam OVMF puts files there.

Sorry, I wasn't clear enough. OVMF consumes files that are put there by
the user.

> If users add their own flags that happen to be in opt/ovmf/ then
> what happens? And how do they *know* they should avoid that?
> There's no warning, guest just breaks in various ways.
> 
> This is exactly the kind of mess I was worried about. We have a global
> namespace with no way to control what goes where.
> 
> opt/ was for end users not firmware.
> 
> The right thing to do would be for ovmf to reserve itself
> a directory under root.
> 
> This really needs more thought.  For now I'd suggest we drop the whole
> interface from 2.6 and come back to it after 2.6.

I very strongly disagree. -fw_cfg was invented exactly for the purpose
that guest code (mostly, but not exclusively, guest firmware code) can
take settings from the user without QEMU's knowledge. We were conscious
of the namespace question, which is why the opt/ prefix was strongly
recommended for such knobs. Trying to control it all from QEMU (beyond
setting aside the opt/ subdirectory, regardless if it's worded "should"
or "must"), introducing a central registry for prefixes, would defeat
the entire purpose.

Subdividing the namespace under opt/ was purposely left open. Then I
went with opt/ovmf/ for files that OVMF should take from users because
that's the obvious choice.

If we want to be extremely paranoid about the namespace, we can modify
the recommendation to say "use opt/UUID/...", where UUID is generated
with "uuidgen". That's just an example I know from UEFI; it is how the
entire namespace question is side-stepped without a central registry.
Nonetheless, OVMF is the second of two guest firmwares in total, so
"opt/ovmf/" is the obvious choice.

Removing the interface would break existing code in OVMF that (a)
consumes opt/ovmf/... files and (b) doesn't disturb anything at all
otherwise.

Plus, the -fw_cfg option was released in 2.4 (added around commit
81b2b81062).

Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 09:49, Laszlo Ersek wrote:
> On 03/16/16 21:31, Michael S. Tsirkin wrote:
>> On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
>>> OVMF uses this feature for a few flags. They are all called
>>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
>>> shouldn't be surprising since I seem to have reviewed every patch for
>>> that file):
>>
>> Wait a second.  You are saying upsteam OVMF puts files there.
> 
> Sorry, I wasn't clear enough. OVMF consumes files that are put there by
> the user.

I think what Michael is saying is that OVMF now has to worry about users
calling their own files "opt/ovmf/foo" and causing a conflict.

I actually agree with his worry, but probably not with how to resolve
it.  For me, the way to resolve it would be:

1) files should actually be named etc/ovmf/foo.  OVMF could optionally
accept both the old and the new names for a while, you would decide
whether this is useful.

2) in turn, because of (1) even the warning on opt/ should be removed.

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/17/16 09:42, Gerd Hoffmann wrote:
>   Hi,
> 
>>> Having a mis-use of this feature become "institutionalized" over time was
>>> seen as a low/negligible risk at the time. Do we have any new reasons
>>> to worry about it ?
>>
>> OVMF uses this feature for a few flags. They are all called
>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
>> shouldn't be surprising since I seem to have reviewed every patch for
>> that file):
> 
> seabios has a few flags too, in etc/.  Some of them are supported
> directly by qemu (such as setting the boot menu delay).  Others are not,
> and I'd like to be able to use -fw_cfg for them for testing/debugging
> (any use cases beyond that should be supported by adding a less obscure
> way to set them to qemu, similar to the boot delay).
> 
> Oh, and that'll most likely be more seabios testing than qemu testing,
> so why require me build a special qemu version for that?

+1

> On the ovmf flags:  What kind of flags are there?  Anything a normal
> user might want to set?

Occasionally, yes.

- "opt/ovmf/PcdPropertiesTableEnable" controls whether the "properties
table" UEFI feature is enabled. Some guest OSes support it, some others
are broken by it. It defaults to false.

- "opt/ovmf/PcdSetNxForStack" controls whether the stack is made
non-executable at the start of the DXE phase. This is meant as a
security feature in edk2 generally, it is not specific to OVMF; only the
way it is exposed to users is specific to OVMF. It used to default to
TRUE, but then people reported that a non-executable stack was breaking
grub2-efi binaries that had been shipped on Debian installer ISOs
sometime during the middle ages. So I was forced to change the default
to FALSE (and I hate that compromise to this day).

- "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which the
64-bit MMIO BARs are allocated. It defaults to 32GB (and as you
suggested, a natural alignment is also ensured, both for the default
32GB, and for the user's choice, if any). The "X-" prefix signals that
this is an experimental knob without any promise of stability. There are
two imporant use cases for this knob at the moment: (a) if a user has a
bunch of MMIO-hungry assignable devices, allow him/her to experiment
with this flag, and report his/her data to us, so we can come up with
smart heuristics, (b) allow a user to disable the high allocation of
64-bit MMIO BARs entirely, by setting the value to 0.

The edk2 BAR allocation code prefers to allocate 64-bit BARs actually
high -- unlike SeaBIOS --, and this has already exposed problems. For
example, such devices don't work -- at the moment -- behind PXBs, if you
use OVMF. Marcel is going to enhance PXB to support this (thank you!),
but even until then (and for yet unknown similar issues), this flag
allows a user to force the edk2 allocator to keep those 64-bit BARs
under 4GB, at least if there is enough room for them there.)

In downstream, we have two more (same purpose but separately for ARM and
x86):
- opt/aavmf/PcdResizeXterm
- opt/ovmf/PcdResizeXterm

(These settings are downstream only because the underlying feature that
they control was rejected by upstream.)

The feature they control is the following: assume you are at the UEFI
shell prompt, and you have at least two console devices (e.g., one
serial console you are looking at in xterm, and one video display you
are looking at with spice), with the text output being mirrored to both.
The MODE command allows you to select various text resolutions that are
supported by all of your console devices simultaneously. Let's say you
change from the default 80x25 to 100x31. This will correctly resize your
video display, and also change the textual resolution used on the serial
console, but it will *not* resize your xterm. Which means the serial
console output will be garbled when you scroll, when you use commands
that print wide tables, and so on. If the user sets the above knob to Y,
then the xterm will also be resized (via control sequences). Sometimes
this feature is invaluable, sometimes it's very annoying. It defaults to N.

- Another flag I might expose later is "PcdHiiOsRuntimeSupport". This
would control whether various bits of UEFI configuration that are
normally accessible through TUI forms in the UEFI setup are also exposed
as a large binary blob to the runtime OS. As I understand it, this
feature is meant to allow OS-level vendor applications to mess with
various firmware settings. For now OVMF inherits the central default Y.
It is somewhat costly, so I think I'll flip the default for OVMF to N,
but expose the knob under "opt/ovmf/PcdHiiOsRuntimeSupport" to support
people who want to experiment with this.

-*-

The X-... knob might go away at any time, the others are likely to stay.

And, obviously, exposing this kind of knob with dedicated QEMU options
is out of question.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/17/16 09:42, Gerd Hoffmann wrote:
>   Hi,
> 
>>> Having a mis-use of this feature become "institutionalized" over time was
>>> seen as a low/negligible risk at the time. Do we have any new reasons
>>> to worry about it ?
>>
>> OVMF uses this feature for a few flags. They are all called
>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
>> shouldn't be surprising since I seem to have reviewed every patch for
>> that file):
> 
> seabios has a few flags too, in etc/.  Some of them are supported
> directly by qemu (such as setting the boot menu delay).

I forgot to confirm: clearly, when some flag already exists on which we
can foist, kicking and screaming, a somewhat similar interpretation in
OVMF, we do that without inventing a new flag. The boot menu delay is
like this. Namely, if you set "-boot menu=on,splash-time=5000", you will
get a progress bar for five seconds before auto-booting the otherwise
relevant boot option.

The QEMU manual documents the -boot option as guest firmware-specific,
and OVMF operates with that in mind -- support what it can, as closely
it can.

Thanks
Laszlo

> Others are not,
> and I'd like to be able to use -fw_cfg for them for testing/debugging
> (any use cases beyond that should be supported by adding a less obscure
> way to set them to qemu, similar to the boot delay).
> 
> Oh, and that'll most likely be more seabios testing than qemu testing,
> so why require me build a special qemu version for that?
> 
> On the ovmf flags:  What kind of flags are there?  Anything a normal
> user might want to set?
> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Markus Armbruster]

Top level reply, because this isn't in reply to any specific message in
the thread, more like in reply to all of them.

FW CFG's primary user is QEMU, which uses it to expose configuration
information (in the widest sense) to Firmware.  Thus the name FW CFG.

FW CFG can also be used by others for their own purposes.  QEMU is
merely acting as transport then.

I think there are actually three separate questions that should not be
mixed up.

1. Is it a good idea to offer FW CFG as a transport to others?

   I have no opinion on this myself, but I trust Gerd's and Laszlo's
   judgement.  Their answer seems to be a clear yes.

2. Is it a good idea to let users mess with QEMU's use of FW CFG?

   I think this is a special case of a more general question: should we
   try to protect the user from himself?

   We should definitely try not to trap the user.  Obvious usage should
   be as safe as we can make it.  Risky usage should be marked in the
   docs and/or warn on use.

   However, we should not try to stop our users from doing stupid
   things, as that would also stop them from doing clever things.

3. How should the FW CFG name space be shared among its users?

   Bad things can happen if others use the namespace in ways that
   conflict with QEMU's use, or conflict with another "other".

   This isn't an issue specific to FW CFG.  For instance, upstream QEMU
   and the various downstream QEMUs all use the QMP command name space,
   and bad things can happen if they conflict.  The difference is they'd
   conflict at compile time.  Conflicts are easier to detect, but just
   as hard to resolve.

   QMP's solution is to reserve part of the name space for "others"
   (downstreams), and subdivide the reserved part further via RFQDN:
   owning a DNS domain name makes you own that RFQDN subdivision.

   For FW CFG, we did only the first half, namely reserving part of the
   name space for others: /opt/.  We neglected to spell out rules for
   its safe sharing, i.e. the second part.

   I don't think it's too late to fix that: amend the specification to
   stipulate that owning a DNS domain name makes you own /opt/RFQDN/.
   Throw in known existing uses like /opt/ovmf/ as special cases.

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Gerd Hoffmann]

  Hi,

> Occasionally, yes.
> 
> - "opt/ovmf/PcdPropertiesTableEnable" controls whether the "properties

> - "opt/ovmf/PcdSetNxForStack" controls whether the stack is made

> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which the

> In downstream, we have two more (same purpose but separately for ARM and
> x86):
> - opt/aavmf/PcdResizeXterm
> - opt/ovmf/PcdResizeXterm
> 
> - Another flag I might expose later is "PcdHiiOsRuntimeSupport". This

I think we should probably move those out of the opt/ namespace space,
as this is reserved for user-defined files.

I think either "etc/efi/" or "efi/" is useful (so we don't have
different names on ovmf and aavmf).  Possibly add "pcd/".  Maybe even
support setting *any* pcd via "efi/pcd/$name" (just an idea, not sure
whenever that is useful and is possible without being too invasive).

> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which 

Hmm.  Leave as-is for now I'd say.  If we decide to keep it we should
probably rename it and place it below etc/, next to reserved-memory-end.
Export the size in bytes, like reserved-memory-end does.  Add an option
to qemu to set it, so you can specify the size as "32G" using the
standard qemu size parser.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 10:40:24AM +0100, Paolo Bonzini wrote:
> 
> 
> On 17/03/2016 09:49, Laszlo Ersek wrote:
> > On 03/16/16 21:31, Michael S. Tsirkin wrote:
> >> On Wed, Mar 16, 2016 at 07:35:09PM +0100, Laszlo Ersek wrote:
> >>> OVMF uses this feature for a few flags. They are all called
> >>> "opt/ovmf/...". I followed the advice in "docs/specs/fw_cfg.txt" (which
> >>> shouldn't be surprising since I seem to have reviewed every patch for
> >>> that file):
> >>
> >> Wait a second.  You are saying upsteam OVMF puts files there.
> > 
> > Sorry, I wasn't clear enough. OVMF consumes files that are put there by
> > the user.
> 
> I think what Michael is saying is that OVMF now has to worry about users
> calling their own files "opt/ovmf/foo" and causing a conflict.
> 
> I actually agree with his worry, but probably not with how to resolve
> it.  For me, the way to resolve it would be:
> 
> 1) files should actually be named etc/ovmf/foo.  OVMF could optionally
> accept both the old and the new names for a while, you would decide
> whether this is useful.

IOW if etc/ovmf exists, then ignore opt/ovmf? OK.

> 2) in turn, because of (1) even the warning on opt/ should be removed.
> 
> Paolo

If we do 2) then users might put files in etc/ovmf/foo.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 12:32, Michael S. Tsirkin wrote:
> > 1) files should actually be named etc/ovmf/foo.  OVMF could optionally
> > accept both the old and the new names for a while, you would decide
> > whether this is useful.
> 
> IOW if etc/ovmf exists, then ignore opt/ovmf? OK.
> 
> > 2) in turn, because of (1) even the warning on opt/ should be removed.
> 
> If we do 2) then users might put files in etc/ovmf/foo.

Right, that's exactly the point.  These are user-configurable knobs for
the firmware (user-configurable meaning they can't be expected to
recompile QEMU).

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 11:09:24AM +0100, Markus Armbruster wrote:
> Top level reply, because this isn't in reply to any specific message in
> the thread, more like in reply to all of them.
> 
> FW CFG's primary user is QEMU, which uses it to expose configuration
> information (in the widest sense) to Firmware.  Thus the name FW CFG.
> 
> FW CFG can also be used by others for their own purposes.  QEMU is
> merely acting as transport then.
> 
> I think there are actually three separate questions that should not be
> mixed up.
> 
> 1. Is it a good idea to offer FW CFG as a transport to others?
> 
>    I have no opinion on this myself, but I trust Gerd's and Laszlo's
>    judgement.  Their answer seems to be a clear yes.
> 
> 2. Is it a good idea to let users mess with QEMU's use of FW CFG?
> 
>    I think this is a special case of a more general question: should we
>    try to protect the user from himself?
> 
>    We should definitely try not to trap the user.  Obvious usage should
>    be as safe as we can make it.  Risky usage should be marked in the
>    docs and/or warn on use.
> 
>    However, we should not try to stop our users from doing stupid
>    things, as that would also stop them from doing clever things.
>
> 3. How should the FW CFG name space be shared among its users?
> 
>    Bad things can happen if others use the namespace in ways that
>    conflict with QEMU's use, or conflict with another "other".
> 
>    This isn't an issue specific to FW CFG.  For instance, upstream QEMU
>    and the various downstream QEMUs all use the QMP command name space,
>    and bad things can happen if they conflict.  The difference is they'd
>    conflict at compile time.  Conflicts are easier to detect, but just
>    as hard to resolve.
> 
>    QMP's solution is to reserve part of the name space for "others"
>    (downstreams), and subdivide the reserved part further via RFQDN:
>    owning a DNS domain name makes you own that RFQDN subdivision.
> 
>    For FW CFG, we did only the first half, namely reserving part of the
>    name space for others: /opt/.  We neglected to spell out rules for
>    its safe sharing, i.e. the second part.
> 
>    I don't think it's too late to fix that: amend the specification to
>    stipulate that owning a DNS domain name makes you own /opt/RFQDN/.
>    Throw in known existing uses like /opt/ovmf/ as special cases.

As usual, very well put.

This makes complete sense to me. I would rate the current interface
rather as attempting to trap users.  The only way to figure it out is to
read the source, and the source includes ovmf and maybe other firmware.
Here is a proposal trying to address the above, including a way to do
stupid things:

QEMU command line:
	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
	   with QEMU hardware

	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
		org.qemu/unsupported/ and leaves just XXX,
		for people who want to break^?^?^?^?^?debug QEMU hardware

	C. -fw-cfg opt/FOO accepts any path, for backwards compatibility

	D. any other use fails

QEMU Documentation
	Document the above in the man page


OVMF:
	Can use the compatible opt/ovmf/ for now.

	Long term: take one of two paths:

	A. Move part of values passed to OVMF into QEMU codebase

	If it's commonly needed for guests to boot, we should provide
	sane defaults and a friendly interface, with
	range checking etc.

	or

	B. Gradually transition OVMF to look up paths in usr/org.uefi/: if
	nothing is found there, look up in opt/ovmf/ for backwards
	compatibility.

	This makes sense if the feature is esoteric and very rarely
	used.


-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 02:12:56PM +0100, Paolo Bonzini wrote:
> 
> 
> On 17/03/2016 12:32, Michael S. Tsirkin wrote:
> > > 1) files should actually be named etc/ovmf/foo.  OVMF could optionally
> > > accept both the old and the new names for a while, you would decide
> > > whether this is useful.
> > 
> > IOW if etc/ovmf exists, then ignore opt/ovmf? OK.
> > 
> > > 2) in turn, because of (1) even the warning on opt/ should be removed.
> > 
> > If we do 2) then users might put files in etc/ovmf/foo.
> 
> Right, that's exactly the point.  These are user-configurable knobs for
> the firmware (user-configurable meaning they can't be expected to
> recompile QEMU).
> 
> Paolo

Right.

See Markus' answer and my proposal that attempts to be a superset
of this.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 10:43:08AM +0100, Laszlo Ersek wrote:
> And, obviously, exposing this kind of knob with dedicated QEMU options
> is out of question.

We are moving away from dedicated options anyway.

But these could easily be machine properties, with
the benefit that there is actual validation of values,
some help and documentation for users, nice shortcuts
like G/M/K for numbers, introspection, unit tests and more.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/17/16 11:22, Gerd Hoffmann wrote:
>   Hi,
> 
>> Occasionally, yes.
>>
>> - "opt/ovmf/PcdPropertiesTableEnable" controls whether the "properties
> 
>> - "opt/ovmf/PcdSetNxForStack" controls whether the stack is made
> 
>> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which the
> 
>> In downstream, we have two more (same purpose but separately for ARM and
>> x86):
>> - opt/aavmf/PcdResizeXterm
>> - opt/ovmf/PcdResizeXterm
>>
>> - Another flag I might expose later is "PcdHiiOsRuntimeSupport". This
> 
> I think we should probably move those out of the opt/ namespace space,
> as this is reserved for user-defined files.
> 
> I think either "etc/efi/"

This is more or less what Paolo suggests.

It confuses me.

I have now re-read the last section of "docs/specs/fw_cfg.txt" to make
sure my current mental image of the original idea has not diverged from
the actual original idea. I don't think so.

So, "user defined files" exist so that users can control "stuff" with
them, without QEMU's knowledge. OVMF is "stuff". Just because it's
firmware, it remains "stuff". Whatever Gabriel wants to control with
such fw_cfg files in the guest, is also "stuff". I don't see any
difference between a user controlling an ad-hoc application of his with
"opt/blah1", and controlling OVMF behavior under "opt/ovmf/xizzy". If
the user tries to control his own app by choosing the name
"opt/ovmf/xizzy", that's *his* fault. "opt/ovmf/" is not a prefix
someone comes up with by chance.

We meant just two partitions of the namespace. "opt/" and non-"opt/".
The latter belongs to QEMU, the former belongs to everything else, and
the subdivision of everything else doesn't belong into QEMU. OVMF is
part of everything else.

If we're paranoid about it, we can make a recommendation that each user
pick "opt/UUID/..." for himself, running "uuidgen" and then sticking
with the output for his own stuff. That will guarantee that no conflicts
will be seen. OVMF could be adapted to that as well. (The only snag with
uuidgen is that it would leave only 56 - 4 - 36 - 1 == 15 characters for
the actual knob name.)

Choosing "etc/" for such files of OVMF that are passed in with the
-fw_cfg switch would be the textbook example of violating the original
idea, because etc/ is heavily populated by QEMU itself.

> or "efi/" is useful (so we don't have
> different names on ovmf and aavmf).

I dislike efi/. It suggests that it controls some features that all UEFI
implementations offer.

> Possibly add "pcd/".  Maybe even
> support setting *any* pcd via "efi/pcd/$name" (just an idea, not sure
> whenever that is useful and is possible without being too invasive).

It's all of: too invasive, not always useful, and not flexible enough.
Knobs should be exposed (with their different value domains) on a
case-by-case basis.

>> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which 
> 
> Hmm.  Leave as-is for now I'd say.

Will do.

>  If we decide to keep it we should
> probably rename it and place it below etc/, next to reserved-memory-end.
> Export the size in bytes, like reserved-memory-end does.  Add an option
> to qemu to set it, so you can specify the size as "32G" using the
> standard qemu size parser.

I certainly don't disagree with this, but if it goes under etc/, and --
hence necessarily -- also gets its own command line option, then SeaBIOS
should probably adhere to it as well (or the QEMU manual should state
that this option is also firmware specific).

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

I frankly think it's overengineered, but it's already much better and if
it helps converging to a compromise why not.

Alternatives to your proposals follow:

On 17/03/2016 14:13, Michael S. Tsirkin wrote:
> 
> QEMU command line:
> 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
> 	   with QEMU hardware

Alternative: no need to prepend usr/, I think.

> 	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
> 		org.qemu/unsupported/ and leaves just XXX,
> 		for people who want to break^?^?^?^?^?debug QEMU hardware

Alternative: fail on:

- a blacklist of etc/* files including etc/system-states,
etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
etc/boot-menu-wait

- on all org.qemu/* files

- iff etc/boot-menu-wait is blacklisted, fail on
org.seabios/boot-menu-wait too.

Everything else is passed through.  No hacks required.

> 	C. -fw-cfg opt/FOO accepts any path, for backwards compatibility

Implicit in my proposed alternative to A.

> 	D. any other use fails

Replaced by my alternative to B.  RFQDN is just a best practice, and it
is not enforced except as proposed in B.  For the same reason, no
changes are required in the Linux driver.

> OVMF:
> 	Can use the compatible opt/ovmf/ for now. [snip]
> 	Long term: Gradually transition OVMF to look up paths in usr/org.uefi/:
> 	if nothing is found there, look up in opt/ovmf/ for backwards
> 	compatibility.

Agreed except it would be org.tianocore.edk2.ovmf/ rather than usr/org.uefi.

Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
stuff usable from both Coreboot and QEMU, e.g.
org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
QEMU).

Files that could be moved from etc/ to org.qemu/ correspond to the ones
that are blacklisted in (B), e.g. etc/system-states ->
org.qemu/system-states.

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 02:28:38PM +0100, Laszlo Ersek wrote:
> On 03/17/16 11:22, Gerd Hoffmann wrote:
> >   Hi,
> > 
> >> Occasionally, yes.
> >>
> >> - "opt/ovmf/PcdPropertiesTableEnable" controls whether the "properties
> > 
> >> - "opt/ovmf/PcdSetNxForStack" controls whether the stack is made
> > 
> >> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which the
> > 
> >> In downstream, we have two more (same purpose but separately for ARM and
> >> x86):
> >> - opt/aavmf/PcdResizeXterm
> >> - opt/ovmf/PcdResizeXterm
> >>
> >> - Another flag I might expose later is "PcdHiiOsRuntimeSupport". This
> > 
> > I think we should probably move those out of the opt/ namespace space,
> > as this is reserved for user-defined files.
> > 
> > I think either "etc/efi/"
> 
> This is more or less what Paolo suggests.
> 
> It confuses me.
> 
> I have now re-read the last section of "docs/specs/fw_cfg.txt" to make
> sure my current mental image of the original idea has not diverged from
> the actual original idea. I don't think so.
> 
> So, "user defined files" exist so that users can control "stuff" with
> them, without QEMU's knowledge. OVMF is "stuff". Just because it's
> firmware, it remains "stuff". Whatever Gabriel wants to control with
> such fw_cfg files in the guest, is also "stuff". I don't see any
> difference between a user controlling an ad-hoc application of his with
> "opt/blah1", and controlling OVMF behavior under "opt/ovmf/xizzy". If
> the user tries to control his own app by choosing the name
> "opt/ovmf/xizzy", that's *his* fault. "opt/ovmf/" is not a prefix
> someone comes up with by chance.
> 
> We meant just two partitions of the namespace. "opt/" and non-"opt/".
> The latter belongs to QEMU, the former belongs to everything else, and
> the subdivision of everything else doesn't belong into QEMU. OVMF is
> part of everything else.

This is where we made a design mistake.  There are 3 kinds of users
adding entries: QEMU, QEMU firmware developers and QEMU users.

And QEMU users could be further subdivided.


> If we're paranoid about it, we can make a recommendation that each user
> pick "opt/UUID/..." for himself, running "uuidgen" and then sticking
> with the output for his own stuff. That will guarantee that no conflicts
> will be seen. OVMF could be adapted to that as well. (The only snag with
> uuidgen is that it would leave only 56 - 4 - 36 - 1 == 15 characters for
> the actual knob name.)
> 
> Choosing "etc/" for such files of OVMF that are passed in with the
> -fw_cfg switch would be the textbook example of violating the original
> idea, because etc/ is heavily populated by QEMU itself.
> 
> > or "efi/" is useful (so we don't have
> > different names on ovmf and aavmf).
> 
> I dislike efi/. It suggests that it controls some features that all UEFI
> implementations offer.
> 
> > Possibly add "pcd/".  Maybe even
> > support setting *any* pcd via "efi/pcd/$name" (just an idea, not sure
> > whenever that is useful and is possible without being too invasive).
> 
> It's all of: too invasive, not always useful, and not flexible enough.
> Knobs should be exposed (with their different value domains) on a
> case-by-case basis.
> 
> >> - "opt/ovmf/X-PciMmio64Mb" controls the size of the range from which 
> > 
> > Hmm.  Leave as-is for now I'd say.
> 
> Will do.
> 
> >  If we decide to keep it we should
> > probably rename it and place it below etc/, next to reserved-memory-end.
> > Export the size in bytes, like reserved-memory-end does.  Add an option
> > to qemu to set it, so you can specify the size as "32G" using the
> > standard qemu size parser.
> 
> I certainly don't disagree with this, but if it goes under etc/, and --
> hence necessarily -- also gets its own command line option, then SeaBIOS
> should probably adhere to it as well (or the QEMU manual should state
> that this option is also firmware specific).
> 
> Thanks
> Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 14:35, Michael S. Tsirkin wrote:
> > We meant just two partitions of the namespace. "opt/" and non-"opt/".
> > The latter belongs to QEMU, the former belongs to everything else, and
> > the subdivision of everything else doesn't belong into QEMU. OVMF is
> > part of everything else.
>
> This is where we made a design mistake.  There are 3 kinds of users
> adding entries: QEMU, QEMU firmware developers and QEMU users.

I can definitely agree with this.

Paolo

> And QEMU users could be further subdivided.

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Laszlo Ersek]

On 03/17/16 14:30, Paolo Bonzini wrote:

> On 17/03/2016 14:13, Michael S. Tsirkin wrote:

>> OVMF:
>> 	Can use the compatible opt/ovmf/ for now. [snip]
>> 	Long term: Gradually transition OVMF to look up paths in usr/org.uefi/:
>> 	if nothing is found there, look up in opt/ovmf/ for backwards
>> 	compatibility.
> 
> Agreed except it would be org.tianocore.edk2.ovmf/ rather than usr/org.uefi.
> 
> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
> stuff usable from both Coreboot and QEMU, e.g.
> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
> QEMU).

I think it's feasible (in the long term) to make OVMF look for
"standard" fw_cfg files under "org.qemu/", and for the OVMF-specific
knobs under "org.tianocore.edk2.ovmf/". (The longest knob name OVMF uses
at the moment is 24 chars; it would fit.)

It's just that I don't see how a user is any more likely to randomly
pick "opt/ovmf/" for his own ad-hoc purposes than to pick
"org.tianocore.edk2.ovmf/".

Anyway, I agree that RFQDNs leave a few characters for the actual knob
names, and they are mostly unique.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 02:30:34PM +0100, Paolo Bonzini wrote:
> I frankly think it's overengineered, but it's already much better and if
> it helps converging to a compromise why not.

Thanks, I'll think of your suggestions over the weekend. We might be able
to simplify things a bit.

> Alternatives to your proposals follow:

I wonder what are the advantages of the alternative though?
It is certainly more code, the rules for users are more
complex to figure out and need more effort for user to follow.


> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
> > 
> > QEMU command line:
> > 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
> > 	   with QEMU hardware
> 
> Alternative: no need to prepend usr/, I think.

I personally dislike telling user "do X". I don't see a reason not to be
friendly and do X. The rare case where users do not want X can be
easily enabled.

> > 	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
> > 		org.qemu/unsupported/ and leaves just XXX,
> > 		for people who want to break^?^?^?^?^?debug QEMU hardware
> 
> Alternative: fail on:
> 
> - a blacklist of etc/* files including etc/system-states,
> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
> etc/boot-menu-wait

We can not predict the future. Future firmware will look for
files under etc/mst. Users using this firmware with
current QEMU will get a nasty surprise where it previously
worked.

Besides, it is way easier to maintain and understand a simple rule than
a blacklist.

> - on all org.qemu/* files
> 
> - iff etc/boot-menu-wait is blacklisted, fail on
> org.seabios/boot-menu-wait too.
> 
> Everything else is passed through.  No hacks required.
>
> > 	C. -fw-cfg opt/FOO accepts any path, for backwards compatibility
> 
> Implicit in my proposed alternative to A.
> 
> > 	D. any other use fails
> 
> Replaced by my alternative to B.  RFQDN is just a best practice, and it
> is not enforced except as proposed in B.  For the same reason, no
> changes are required in the Linux driver.
>
> > OVMF:
> > 	Can use the compatible opt/ovmf/ for now. [snip]
> > 	Long term: Gradually transition OVMF to look up paths in usr/org.uefi/:
> > 	if nothing is found there, look up in opt/ovmf/ for backwards
> > 	compatibility.
> 
> Agreed except it would be org.tianocore.edk2.ovmf/ rather than usr/org.uefi.
>
> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
> stuff usable from both Coreboot and QEMU, e.g.
> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
> QEMU).
> 
> Files that could be moved from etc/ to org.qemu/ correspond to the ones
> that are blacklisted in (B), e.g. etc/system-states ->
> org.qemu/system-states.
> 
> Paolo

I am not sure about moving things into usr/org.qemu.
These are system files, not user-provided ones.
But we can argue about future plans down the road.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 14:49, Michael S. Tsirkin wrote:
>> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
>>>
>>> QEMU command line:
>>> 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
>>> 	   with QEMU hardware
>>
>> Alternative: no need to prepend usr/, I think.
> 
> I personally dislike telling user "do X". I don't see a reason not to be
> friendly and do X. The rare case where users do not want X can be
> easily enabled.

I wouldn't include usr/ at all in the paths.  The RFQDN recommendation
is enough to avoid clashes with etc/ and opt/.

>>> 	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
>>> 		org.qemu/unsupported/ and leaves just XXX,
>>> 		for people who want to break^?^?^?^?^?debug QEMU hardware
>>
>> Alternative: fail on:
>>
>> - a blacklist of etc/* files including etc/system-states,
>> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
>> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
>> etc/boot-menu-wait
> 
> We can not predict the future. Future firmware will look for
> files under etc/mst. Users using this firmware with
> current QEMU will get a nasty surprise where it previously
> worked.
> 
> Besides, it is way easier to maintain and understand a simple rule than
> a blacklist.

The reason for the blacklist is that these are files owned by QEMU but
traditionally under etc/.  The error can be simply "fw_cfg file %s is
provided by QEMU".  If a file is added in the future that is owned by
QEMU, it will be under org.qemu/* so the blacklist will not grow.

>> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
>> stuff usable from both Coreboot and QEMU, e.g.
>> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
>> QEMU).
>>
>> Files that could be moved from etc/ to org.qemu/ correspond to the ones
>> that are blacklisted in (B), e.g. etc/system-states ->
>> org.qemu/system-states.
> 
> I am not sure about moving things into usr/org.qemu.
> These are system files, not user-provided ones.
> But we can argue about future plans down the road.

Does it make more sense if it's just org.qemu, not usr/org.qemu?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 02:55:52PM +0100, Paolo Bonzini wrote:
> 
> 
> On 17/03/2016 14:49, Michael S. Tsirkin wrote:
> >> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
> >>>
> >>> QEMU command line:
> >>> 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
> >>> 	   with QEMU hardware
> >>
> >> Alternative: no need to prepend usr/, I think.
> > 
> > I personally dislike telling user "do X". I don't see a reason not to be
> > friendly and do X. The rare case where users do not want X can be
> > easily enabled.
> 
> I wouldn't include usr/ at all in the paths.  The RFQDN recommendation
> is enough to avoid clashes with etc/ and opt/.

Yes but then we need a blacklist. And usr/ is not visible to
users so I do not see a problem with it.


> >>> 	B. -fw-cfg org.qemu/unsupported/XXX as a hack, removes
> >>> 		org.qemu/unsupported/ and leaves just XXX,
> >>> 		for people who want to break^?^?^?^?^?debug QEMU hardware
> >>
> >> Alternative: fail on:
> >>
> >> - a blacklist of etc/* files including etc/system-states,
> >> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
> >> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
> >> etc/boot-menu-wait
> > 
> > We can not predict the future. Future firmware will look for
> > files under etc/mst. Users using this firmware with
> > current QEMU will get a nasty surprise where it previously
> > worked.
> > 
> > Besides, it is way easier to maintain and understand a simple rule than
> > a blacklist.
> 
> The reason for the blacklist is that these are files owned by QEMU but
> traditionally under etc/.  The error can be simply "fw_cfg file %s is
> provided by QEMU".  If a file is added in the future that is owned by
> QEMU, it will be under org.qemu/* so the blacklist will not grow.


Yes, but a new prefix seems like a cleaner way.

> >> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
> >> stuff usable from both Coreboot and QEMU, e.g.
> >> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
> >> QEMU).
> >>
> >> Files that could be moved from etc/ to org.qemu/ correspond to the ones
> >> that are blacklisted in (B), e.g. etc/system-states ->
> >> org.qemu/system-states.
> > 
> > I am not sure about moving things into usr/org.qemu.
> > These are system files, not user-provided ones.
> > But we can argue about future plans down the road.
> 
> Does it make more sense if it's just org.qemu, not usr/org.qemu?
> 
> Thanks,
> 
> Paolo

I am not sure, let's discuss after 2.6.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 15:17, Michael S. Tsirkin wrote:
> On Thu, Mar 17, 2016 at 02:55:52PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 17/03/2016 14:49, Michael S. Tsirkin wrote:
>>>> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
>>>>>
>>>>> QEMU command line:
>>>>> 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
>>>>> 	   with QEMU hardware
>>>>
>>>> Alternative: no need to prepend usr/, I think.
>>>
>>> I personally dislike telling user "do X". I don't see a reason not to be
>>> friendly and do X. The rare case where users do not want X can be
>>> easily enabled.
>>
>> I wouldn't include usr/ at all in the paths.  The RFQDN recommendation
>> is enough to avoid clashes with etc/ and opt/.
> 
> Yes but then we need a blacklist.

Can't the blacklist be as simple as "org.qemu/*", plus some handling of
legacy "etc/*"?

We'd need special handling of "etc/*" anyway because SeaBIOS is using it
(Gerd's usecase) and you certainly don't want to use
org.qemu/unsupported/etc/XYZ hacks for that.

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 03:50:08PM +0100, Paolo Bonzini wrote:
> 
> 
> On 17/03/2016 15:17, Michael S. Tsirkin wrote:
> > On Thu, Mar 17, 2016 at 02:55:52PM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 17/03/2016 14:49, Michael S. Tsirkin wrote:
> >>>> On 17/03/2016 14:13, Michael S. Tsirkin wrote:
> >>>>>
> >>>>> QEMU command line:
> >>>>> 	A. -fw-cfg RFQDN/PATH prepends usr/. So users will not get conflicts
> >>>>> 	   with QEMU hardware
> >>>>
> >>>> Alternative: no need to prepend usr/, I think.
> >>>
> >>> I personally dislike telling user "do X". I don't see a reason not to be
> >>> friendly and do X. The rare case where users do not want X can be
> >>> easily enabled.
> >>
> >> I wouldn't include usr/ at all in the paths.  The RFQDN recommendation
> >> is enough to avoid clashes with etc/ and opt/.
> > 
> > Yes but then we need a blacklist.
> 
> Can't the blacklist be as simple as "org.qemu/*", plus some handling of
> legacy "etc/*"?
> 
> We'd need special handling of "etc/*" anyway because SeaBIOS is using it
> (Gerd's usecase) and you certainly don't want to use
> org.qemu/unsupported/etc/XYZ hacks for that.
> 
> Paolo

I would prefer a white-list, but I have an idea: look for "."
in the name. If not there, it's not an RFQDN.

-- 
MST

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Gerd Hoffmann]

  Hi,

> So, "user defined files" exist so that users can control "stuff" with
> them, without QEMU's knowledge. OVMF is "stuff". Just because it's
> firmware, it remains "stuff". Whatever Gabriel wants to control with
> such fw_cfg files in the guest, is also "stuff".

Well, non-firmware stuff was my interpretation, i.e. user-specific stuff
in both guest and host.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Gerd Hoffmann]

  Hi,

> Alternative: fail on:
> 
> - a blacklist of etc/* files including etc/system-states,
> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
> etc/boot-menu-wait

I think that fails already because qemu throws an error on duplicate
firmware entries.

> Likewise SeaBIOS would switch from etc/ to an org.seabios/ prefix (for
> stuff usable from both Coreboot and QEMU, e.g.
> org.seabios/bootsplash.bmp) or org.qemu/ (for stuff that is specific to
> QEMU).

Do we really want shuffle around stuff like this?  It means we have to
support both paths in both firmware and qemu for a quite a while for
compatibility reasons.

(moving around opt/ovmf/ has this issue too, even though to a lesser
 extend as there isn't stuff as critical as acpi tables or boot order).

Also note that most stuff in etc/ is used by all firmwares, so moving
this to org.seabios looks pointless to me, especially as things might
change over time (i.e. ovmf starting to use cfg options it used to
ignore first, which is the case for etc/boot-menu-wait IIRC, so it
started as seabios only but isn't any more).

Also note that the fw_cfg layout is modeled a bit after cbfs (coreboot
filesystem) which is provided by coreboot and used by seabios.  That'll
cause trouble too if we try to redefine the paths.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Paolo Bonzini]

On 17/03/2016 18:17, Gerd Hoffmann wrote:
>   Hi,
> 
>> Alternative: fail on:
>>
>> - a blacklist of etc/* files including etc/system-states,
>> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
>> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
>> etc/boot-menu-wait
> 
> I think that fails already because qemu throws an error on duplicate
> firmware entries.

The problem is that some files are only added conditionally (e.g.
etc/boot-menu-wait).

> Do we really want shuffle around stuff like this?  It means we have to
> support both paths in both firmware and qemu for a quite a while for
> compatibility reasons.
> 
> Also note that most stuff in etc/ is used by all firmwares, so moving
> this to org.seabios looks pointless to me, especially as things might
> change over time (i.e. ovmf starting to use cfg options it used to
> ignore first, which is the case for etc/boot-menu-wait IIRC, so it
> started as seabios only but isn't any more).

Stuff that is used by all firmwares could move to org.coreboot/ or
org.qemu/, but keeping etc/ as a special case is certainly fine by me.

Paolo

Re: [Qemu-devel] [PATCH v2] vl.c: disallow command line fw cfg without opt/

[Michael S. Tsirkin]

On Thu, Mar 17, 2016 at 08:35:25PM +0100, Paolo Bonzini wrote:
> 
> 
> On 17/03/2016 18:17, Gerd Hoffmann wrote:
> >   Hi,
> > 
> >> Alternative: fail on:
> >>
> >> - a blacklist of etc/* files including etc/system-states,
> >> etc/smbios/smbios-tables, etc/smbios/smbios-anchor,
> >> etc/reserved-memory-end, etc/pvpanic-port, etc/e820, and possibly
> >> etc/boot-menu-wait
> > 
> > I think that fails already because qemu throws an error on duplicate
> > firmware entries.
> 
> The problem is that some files are only added conditionally (e.g.
> etc/boot-menu-wait).

Another problem is that future firmware can run on an older QEMU.
Any blacklist-only strategy fails because of that, we need
a schema.

> > Do we really want shuffle around stuff like this?  It means we have to
> > support both paths in both firmware and qemu for a quite a while for
> > compatibility reasons.
> > 
> > Also note that most stuff in etc/ is used by all firmwares, so moving
> > this to org.seabios looks pointless to me, especially as things might
> > change over time (i.e. ovmf starting to use cfg options it used to
> > ignore first, which is the case for etc/boot-menu-wait IIRC, so it
> > started as seabios only but isn't any more).
> 
> Stuff that is used by all firmwares could move to org.coreboot/ or
> org.qemu/, but keeping etc/ as a special case is certainly fine by me.
> 
> Paolo