Thanks Marcel.
Just to make sure I understand, at this point do to limitations in the
existing functionality, there is nothing that can be done other than adding
the option to the global qemu_machine_opts list. Once you add a fix then
it will be possible to add it dynamically.
If I missed anything please let me know.
Regards,
Greg
On 5 December 2014 at 13:40, Marcel Apfelbaum <marce...@redhat.com> wrote:
> On Fri, 2014-12-05 at 15:39 +0000, Peter Maydell wrote:
> > On 5 December 2014 at 15:33, Greg Bellows <greg.bell...@linaro.org>
> wrote:
> > >
> > >
> > > On 5 December 2014 at 09:18, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
> > >>
> > >> On 3 December 2014 at 20:05, Greg Bellows <greg.bell...@linaro.org>
> wrote:
> > >> > Added 'secure' qemu boolean option to qemu_machine_opts[].
> > >> >
> > >> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> > >> > ---
> > >> > vl.c | 4 ++++
> > >> > 1 file changed, 4 insertions(+)
> > >> >
> > >> > diff --git a/vl.c b/vl.c
> > >> > index eb89d62..5d640f7 100644
> > >> > --- a/vl.c
> > >> > +++ b/vl.c
> > >> > @@ -387,6 +387,10 @@ static QemuOptsList qemu_machine_opts = {
> > >> > .name = "iommu",
> > >> > .type = QEMU_OPT_BOOL,
> > >> > .help = "Set on/off to enable/disable Intel IOMMU
> (VT-d)",
> > >> > + },{
> > >> > + .name = "secure",
> > >> > + .type = QEMU_OPT_BOOL,
> > >> > + .help = "Set on/off to enable/disable secure state",
> > >> > },
> > >>
> > >> If patch 5 adds 'secure' as a machine property for only those
> > >> boards where it makes sense, why do we need this global switch?
> > >>
> > >
> > > That is what I thought as well, but this is apparently needed as we
> get an
> > > invalid machine property otherwise. Below is the error, I'll look
> again,
> > > but it appeared all machine properties were defined here.
> > >
> > > qemu-system-aarch64: -machine type=vexpress-a15,secure=off: Invalid
> > > parameter 'secure'
> >
> > That would seem to defeat the point of the machine opts design,
> > so it looks a bit strange. Marcel: how is this supposed to work
> > for board-specific -machine options?
> Hi Peter,
>
> We have indeed properties per machine type and it works like this:
> 1. You add a QOM property in the specific machine init code.
> (Exactly as in [PATCH 05/13] target-arm: Add vexpress machine secure
> property)
>
> 2. In vl.c the following code should automatically fill in the per machine
> properties.
>
> machine_opts = qemu_get_machine_opts();
> if (qemu_opt_foreach(machine_opts, machine_set_property,
> current_machine,
> 1) < 0) {
> object_unref(OBJECT(current_machine));
> exit(1);
> }
>
> 3. machine_set_property should handle the "per machine" properties.
>
> That being said, we do have a problem in the way the machine_opts are
> built.
> In order for the property to be "visible", we need to add it to a global
> qemu_machine_opts list.
> The reason (I think) is the parsing code that checks it against a
> predefined list:
>
> The correct way to to this is to build the machine option list dynamically
> and not from the predefined global list and check them against the
> specific machine instance.
> Andreas, does it seems right to you?
>
> Thanks for bringing this to my attention!
> I'll fix this and submit a patch shortly.
>
> Thanks,
> Marcel
>
>
>
>
> >
> > thanks
> > -- PMM
>
>
>
>
