[Qemu-devel] [PATCH 00/13] target-arm: Add CPU security extension enablement

[Qemu-devel] [PATCH 00/13] target-arm: Add CPU security extension enablement

[Greg Bellows]

This patchset adds functionality for enabling the ARM CPU security extensions.
At this time, the only machines supported are Versatile Express and the QEMU
ARM virtual machines both with Cortex A9 & A15.

The patchset establishes the default security state along with adding
overriding controls of the state.  Booting with the "-kernel" QEMU command line
option will start by default in non-secure state with EL3 support disabled.
Booting with the "-bios" QEMU command line option will default to
secure state with EL3 features enabled.  An added "secure" machine property
may be set to either 'on' or 'off' to override this default behavior.  For
example, the below command line syntax would enable security extensions...

    aarch64-softmmu/qemu-system-aarch64
        -machine type=vexpress-a15,secure=on -kernel ...

In order to add the machine specific 'secure' property, the vexpress machine
object creation functionality needed to be updated.  The existing QEMU machine
mechanism was replaced with proper type, class, and instance usage.

Fabian Aggeler (1):
  target-arm: add cpu feature EL3 to CPUs with Security Extensions

Greg Bellows (12):
  target-arm: Add vexpress class and machine types
  target-arm: Add vexpress a9 & a15 machine objects
  target-arm: Switch to common vexpress machine init
  target-arm: Add secure qemu machine option
  target-arm: Add vexpress machine secure property
  target-arm: Change vexpress daughterboard init arg
  target-arm: Add virt class and machine types
  target-arm: Add virt machine secure property
  target-arm: Add feature unset function
  target-arm: Add ARMCPU secure property
  target-arm: Set CPU secure prop during VE init
  target-arm: Set CPU secure prop during virt init

 hw/arm/vexpress.c    | 150 +++++++++++++++++++++++++++++++++++++++++----------
 hw/arm/virt.c        |  87 +++++++++++++++++++++++++++---
 target-arm/cpu-qom.h |   2 +
 target-arm/cpu.c     |  33 ++++++++++++
 vl.c                 |   4 ++
 5 files changed, 242 insertions(+), 34 deletions(-)

--
1.8.3.2

[Qemu-devel] [PATCH 01/13] target-arm: Add vexpress class and machine types

[Greg Bellows]

Adds base Vexpress class and machine objects and infrastructure.  This is in
preparation for switching to the full QEMU object model.  The base vexpress
infrastructure is intended to handle common vexpress details.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 7cbd13f..8bec04a 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -157,6 +157,23 @@ static hwaddr motherboard_aseries_map[] = {
 
 typedef struct VEDBoardInfo VEDBoardInfo;
 
+typedef struct {
+    MachineClass parent;
+    VEDBoardInfo *daughterboard;
+} VexpressMachineClass;
+
+typedef struct {
+    MachineState parent;
+} VexpressMachineState;
+
+#define TYPE_VEXPRESS_MACHINE   "vexpress"
+#define VEXPRESS_MACHINE(obj) \
+    OBJECT_CHECK(VexpressMachineState, (obj), TYPE_VEXPRESS_MACHINE)
+#define VEXPRESS_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(VexpressMachineClass, obj, TYPE_VEXPRESS_MACHINE)
+#define VEXPRESS_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(VexpressMachineClass, klass, TYPE_VEXPRESS_MACHINE)
+
 typedef void DBoardInitFn(const VEDBoardInfo *daughterboard,
                           ram_addr_t ram_size,
                           const char *cpu_model,
@@ -681,6 +698,13 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
     arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
 }
 
+static void vexpress_init(MachineState *machine)
+{
+    VexpressMachineClass *vmc = VEXPRESS_MACHINE_GET_CLASS(machine);
+
+    vexpress_common_init(vmc->daughterboard, machine);
+}
+
 static void vexpress_a9_init(MachineState *machine)
 {
     vexpress_common_init(&a9_daughterboard, machine);
@@ -691,6 +715,25 @@ static void vexpress_a15_init(MachineState *machine)
     vexpress_common_init(&a15_daughterboard, machine);
 }
 
+static void vexpress_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = TYPE_VEXPRESS_MACHINE;
+    mc->desc = "ARM Versatile Express";
+    mc->init = vexpress_init;
+    mc->block_default_type = IF_SCSI;
+    mc->max_cpus = 4;
+}
+
+static const TypeInfo vexpress_info = {
+    .name = TYPE_VEXPRESS_MACHINE,
+    .parent = TYPE_MACHINE,
+    .instance_size = sizeof(VexpressMachineState),
+    .class_size = sizeof(VexpressMachineClass),
+    .class_init = vexpress_class_init,
+};
+
 static QEMUMachine vexpress_a9_machine = {
     .name = "vexpress-a9",
     .desc = "ARM Versatile Express for Cortex-A9",
@@ -709,6 +752,7 @@ static QEMUMachine vexpress_a15_machine = {
 
 static void vexpress_machine_init(void)
 {
+    type_register_static(&vexpress_info);
     qemu_register_machine(&vexpress_a9_machine);
     qemu_register_machine(&vexpress_a15_machine);
 }
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 01/13] target-arm: Add vexpress class and machine types

[Peter Maydell]

On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> Adds base Vexpress class and machine objects and infrastructure.  This is in
> preparation for switching to the full QEMU object model.  The base vexpress
> infrastructure is intended to handle common vexpress details.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

This patch results in an incorrect extra line appearing in
the "-M help output":
vexpress             ARM Versatile Express
and if you try to use it we dump core:

e104462:trusty:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M vexpress
Segmentation fault (core dumped)

We need to either figure out a way for indicating that
a subclass of MACHINE is actually an abstract subclass,
or just have the a15 and a9 directly inherit from MACHINE.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 01/13] target-arm: Add vexpress class and machine types

[Marcel Apfelbaum]

On Fri, 2014-12-05 at 15:16 +0000, Peter Maydell wrote:
> On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Adds base Vexpress class and machine objects and infrastructure.  This is in
> > preparation for switching to the full QEMU object model.  The base vexpress
> > infrastructure is intended to handle common vexpress details.
> >
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> 
> This patch results in an incorrect extra line appearing in
> the "-M help output":
> vexpress             ARM Versatile Express
> and if you try to use it we dump core:
> 
> e104462:trusty:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M vexpress
> Segmentation fault (core dumped)
> 
> We need to either figure out a way for indicating that
> a subclass of MACHINE is actually an abstract subclass,
> or just have the a15 and a9 directly inherit from MACHINE.

We can just add ".abstract = true:  to vexpress_info that should be
enough.

Thanks,
Marcel

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 01/13] target-arm: Add vexpress class and machine types

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

Thanks Marcel that appears to have done the trick.

On 5 December 2014 at 13:02, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
wrote:

> On Fri, 2014-12-05 at 15:16 +0000, Peter Maydell wrote:
> > On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org>
> wrote:
> > > Adds base Vexpress class and machine objects and infrastructure.  This
> is in
> > > preparation for switching to the full QEMU object model.  The base
> vexpress
> > > infrastructure is intended to handle common vexpress details.
> > >
> > > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> >
> > This patch results in an incorrect extra line appearing in
> > the "-M help output":
> > vexpress             ARM Versatile Express
> > and if you try to use it we dump core:
> >
> > e104462:trusty:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M vexpress
> > Segmentation fault (core dumped)
> >
> > We need to either figure out a way for indicating that
> > a subclass of MACHINE is actually an abstract subclass,
> > or just have the a15 and a9 directly inherit from MACHINE.
>
> We can just add ".abstract = true:  to vexpress_info that should be
> enough.
>
> Thanks,
> Marcel
>
> >
> > thanks
> > -- PMM
> >
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 1861 bytes --]

[Qemu-devel] [PATCH 02/13] target-arm: Add vexpress a9 & a15 machine objects

[Greg Bellows]

Add Vexpress machine objects for the the Cortex A9 & A15 variants.  The older
style QEMUMachine types were replaced with dedicated TypeInfo objects. The new
objects include dedicated class init functions that currently ustilze dedicated
machine init methods.  The previous qemu_register_machine calls were replaced
with the newer type_register_status calls.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 50 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 8bec04a..0433300 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -167,6 +167,8 @@ typedef struct {
 } VexpressMachineState;
 
 #define TYPE_VEXPRESS_MACHINE   "vexpress"
+#define TYPE_VEXPRESS_A9_MACHINE   "vexpress-a9"
+#define TYPE_VEXPRESS_A15_MACHINE   "vexpress-a15"
 #define VEXPRESS_MACHINE(obj) \
     OBJECT_CHECK(VexpressMachineState, (obj), TYPE_VEXPRESS_MACHINE)
 #define VEXPRESS_MACHINE_GET_CLASS(obj) \
@@ -726,6 +728,30 @@ static void vexpress_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 4;
 }
 
+static void vexpress_a9_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    VexpressMachineClass *vmc = VEXPRESS_MACHINE_CLASS(oc);
+
+    mc->name = TYPE_VEXPRESS_A9_MACHINE;
+    mc->desc = "ARM Versatile Express for Cortex-A9";
+    mc->init = vexpress_a9_init;
+
+    vmc->daughterboard = &a9_daughterboard;;
+}
+
+static void vexpress_a15_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    VexpressMachineClass *vmc = VEXPRESS_MACHINE_CLASS(oc);
+
+    mc->name = TYPE_VEXPRESS_A15_MACHINE;
+    mc->desc = "ARM Versatile Express for Cortex-A15";
+    mc->init = vexpress_a15_init;
+
+    vmc->daughterboard = &a15_daughterboard;
+}
+
 static const TypeInfo vexpress_info = {
     .name = TYPE_VEXPRESS_MACHINE,
     .parent = TYPE_MACHINE,
@@ -734,27 +760,23 @@ static const TypeInfo vexpress_info = {
     .class_init = vexpress_class_init,
 };
 
-static QEMUMachine vexpress_a9_machine = {
-    .name = "vexpress-a9",
-    .desc = "ARM Versatile Express for Cortex-A9",
-    .init = vexpress_a9_init,
-    .block_default_type = IF_SCSI,
-    .max_cpus = 4,
+static const TypeInfo vexpress_a9_info = {
+    .name = TYPE_VEXPRESS_A9_MACHINE,
+    .parent = TYPE_VEXPRESS_MACHINE,
+    .class_init = vexpress_a9_class_init,
 };
 
-static QEMUMachine vexpress_a15_machine = {
-    .name = "vexpress-a15",
-    .desc = "ARM Versatile Express for Cortex-A15",
-    .init = vexpress_a15_init,
-    .block_default_type = IF_SCSI,
-    .max_cpus = 4,
+static const TypeInfo vexpress_a15_info = {
+    .name = TYPE_VEXPRESS_A15_MACHINE,
+    .parent = TYPE_VEXPRESS_MACHINE,
+    .class_init = vexpress_a15_class_init,
 };
 
 static void vexpress_machine_init(void)
 {
     type_register_static(&vexpress_info);
-    qemu_register_machine(&vexpress_a9_machine);
-    qemu_register_machine(&vexpress_a15_machine);
+    type_register_static(&vexpress_a9_info);
+    type_register_static(&vexpress_a15_info);
 }
 
 machine_init(vexpress_machine_init);
-- 
1.8.3.2

[Qemu-devel] [PATCH 03/13] target-arm: Switch to common vexpress machine init

[Greg Bellows]

Switched the Vexpress machine initialization to use the common function with
the machine pointer to board info.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 0433300..4a38a82 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -532,9 +532,10 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
     return OBJECT_CHECK(pflash_t, (dev), "cfi.pflash01");
 }
 
-static void vexpress_common_init(VEDBoardInfo *daughterboard,
-                                 MachineState *machine)
+static void vexpress_common_init(MachineState *machine)
 {
+    VexpressMachineClass *vmc = VEXPRESS_MACHINE_GET_CLASS(machine);
+    VEDBoardInfo *daughterboard = vmc->daughterboard;;
     DeviceState *dev, *sysctl, *pl041;
     qemu_irq pic[64];
     uint32_t sys_id;
@@ -700,30 +701,13 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
     arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
 }
 
-static void vexpress_init(MachineState *machine)
-{
-    VexpressMachineClass *vmc = VEXPRESS_MACHINE_GET_CLASS(machine);
-
-    vexpress_common_init(vmc->daughterboard, machine);
-}
-
-static void vexpress_a9_init(MachineState *machine)
-{
-    vexpress_common_init(&a9_daughterboard, machine);
-}
-
-static void vexpress_a15_init(MachineState *machine)
-{
-    vexpress_common_init(&a15_daughterboard, machine);
-}
-
 static void vexpress_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
 
     mc->name = TYPE_VEXPRESS_MACHINE;
     mc->desc = "ARM Versatile Express";
-    mc->init = vexpress_init;
+    mc->init = vexpress_common_init;
     mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
 }
@@ -735,7 +719,6 @@ static void vexpress_a9_class_init(ObjectClass *oc, void *data)
 
     mc->name = TYPE_VEXPRESS_A9_MACHINE;
     mc->desc = "ARM Versatile Express for Cortex-A9";
-    mc->init = vexpress_a9_init;
 
     vmc->daughterboard = &a9_daughterboard;;
 }
@@ -747,7 +730,6 @@ static void vexpress_a15_class_init(ObjectClass *oc, void *data)
 
     mc->name = TYPE_VEXPRESS_A15_MACHINE;
     mc->desc = "ARM Versatile Express for Cortex-A15";
-    mc->init = vexpress_a15_init;
 
     vmc->daughterboard = &a15_daughterboard;
 }
-- 
1.8.3.2

[Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Greg Bellows]

Added 'secure' qemu boolean option to qemu_machine_opts[].

Signed-off-by: Greg Bellows <greg.bellows@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",
         },
         { /* End of list */ }
     },
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Peter Maydell]

On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> Added 'secure' qemu boolean option to qemu_machine_opts[].
>
> Signed-off-by: Greg Bellows <greg.bellows@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?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

On 5 December 2014 at 09:18, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Added 'secure' qemu boolean option to qemu_machine_opts[].
> >
> > Signed-off-by: Greg Bellows <greg.bellows@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'*



> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 2511 bytes --]

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Peter Maydell]

On 5 December 2014 at 15:33, Greg Bellows <greg.bellows@linaro.org> wrote:
>
>
> On 5 December 2014 at 09:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
>> > Added 'secure' qemu boolean option to qemu_machine_opts[].
>> >
>> > Signed-off-by: Greg Bellows <greg.bellows@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?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Marcel Apfelbaum]

On Fri, 2014-12-05 at 15:39 +0000, Peter Maydell wrote:
> On 5 December 2014 at 15:33, Greg Bellows <greg.bellows@linaro.org> wrote:
> >
> >
> > On 5 December 2014 at 09:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>
> >> On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> >> > Added 'secure' qemu boolean option to qemu_machine_opts[].
> >> >
> >> > Signed-off-by: Greg Bellows <greg.bellows@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

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 3477 bytes --]

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 <marcel.a@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.bellows@linaro.org>
> wrote:
> > >
> > >
> > > On 5 December 2014 at 09:18, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> > >>
> > >> On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org>
> wrote:
> > >> > Added 'secure' qemu boolean option to qemu_machine_opts[].
> > >> >
> > >> > Signed-off-by: Greg Bellows <greg.bellows@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
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 5439 bytes --]

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Marcel Apfelbaum]

On Fri, 2014-12-05 at 14:40 -0600, Greg Bellows wrote:
> 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.

Yes, this is correct.

What we have now is a way to determine if an option belongs to a specific machine,
for example trying to use your "secure" option with a PC machine will fail
since PC machines do not have this property.

But you still need to define that option in the global qemu_machine_opts
in order to use it. This is of course not good enough and we will take care of it.

We have two options here:
1. You add the "secure" option to the machines opts and I'll remove it once
   I'll fix the above limitation.
2. You wait until I fix this and you'll not need it at all.

I am OK with it other way, but the decision is not only mine :)
I'll try to come up with something next week, but it will need reviews
and it may postpone your series. However I suppose the series is for 2.3,
so we have plenty of time to do it properly.

Thanks,
Marcel 

> 
> 
> If I missed anything please let me know.
> 
> 
> Regards,
> 
> 
> Greg
> 
> On 5 December 2014 at 13:40, Marcel Apfelbaum <marcel.a@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.bellows@linaro.org> wrote:
>         > >
>         > >
>         > > On 5 December 2014 at 09:18, Peter Maydell
>         <peter.maydell@linaro.org> wrote:
>         > >>
>         > >> On 3 December 2014 at 20:05, Greg Bellows
>         <greg.bellows@linaro.org> wrote:
>         > >> > Added 'secure' qemu boolean option to
>         qemu_machine_opts[].
>         > >> >
>         > >> > Signed-off-by: Greg Bellows <greg.bellows@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
>         
>         
>         
> 
> 

Re: [Qemu-devel] [PATCH 04/13] target-arm: Add secure qemu machine option

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 5849 bytes --]

Thanks for the clarification Marcel.

I'm not sure my stuff is quite ready to go in either, so why don't we both
move ahead and we can address it when we have a better idea of who might
make it in first.  Yes, 2.3 would be the target and we have plenty of time.

Greg

On 5 December 2014 at 16:44, Marcel Apfelbaum <marcel.a@redhat.com> wrote:

> On Fri, 2014-12-05 at 14:40 -0600, Greg Bellows wrote:
> > 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.
>
> Yes, this is correct.
>
> What we have now is a way to determine if an option belongs to a specific
> machine,
> for example trying to use your "secure" option with a PC machine will fail
> since PC machines do not have this property.
>
> But you still need to define that option in the global qemu_machine_opts
> in order to use it. This is of course not good enough and we will take
> care of it.
>
> We have two options here:
> 1. You add the "secure" option to the machines opts and I'll remove it once
>    I'll fix the above limitation.
> 2. You wait until I fix this and you'll not need it at all.
>
> I am OK with it other way, but the decision is not only mine :)
> I'll try to come up with something next week, but it will need reviews
> and it may postpone your series. However I suppose the series is for 2.3,
> so we have plenty of time to do it properly.
>
> Thanks,
> Marcel
>
> >
> >
> > If I missed anything please let me know.
> >
> >
> > Regards,
> >
> >
> > Greg
> >
> > On 5 December 2014 at 13:40, Marcel Apfelbaum <marcel.a@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.bellows@linaro.org> wrote:
> >         > >
> >         > >
> >         > > On 5 December 2014 at 09:18, Peter Maydell
> >         <peter.maydell@linaro.org> wrote:
> >         > >>
> >         > >> On 3 December 2014 at 20:05, Greg Bellows
> >         <greg.bellows@linaro.org> wrote:
> >         > >> > Added 'secure' qemu boolean option to
> >         qemu_machine_opts[].
> >         > >> >
> >         > >> > Signed-off-by: Greg Bellows <greg.bellows@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
> >
> >
> >
> >
> >
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 8439 bytes --]

[Qemu-devel] [PATCH 05/13] target-arm: Add vexpress machine secure property

[Greg Bellows]

Add "secure" Vexpress machine specific property to allow override of the
default secure state configuration.  By default, when using the QEMU
-kernel command line argument, Vexpress machines boot into NS/SVC.  When using
the QEMU -bios command line argument, Vexpress machines boot into S/SVC.

The secure state can be changed from the default specifying the secure
state as a machine property.  For example, the below command line would enable
secure state on a -linux boot:

    aarch64-softmmu/qemu-system-aarch64
        -machine type=vexpress-a15,secure=on
        -kernel ...

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 4a38a82..6dc5d3b 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -164,6 +164,7 @@ typedef struct {
 
 typedef struct {
     MachineState parent;
+    bool secure;
 } VexpressMachineState;
 
 #define TYPE_VEXPRESS_MACHINE   "vexpress"
@@ -701,6 +702,41 @@ static void vexpress_common_init(MachineState *machine)
     arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
 }
 
+static bool vexpress_get_secure(Object *obj, Error **errp)
+{
+    VexpressMachineState *vms = VEXPRESS_MACHINE(obj);
+
+    return vms->secure;
+}
+
+static void vexpress_set_secure(Object *obj, bool value, Error **errp)
+{
+    VexpressMachineState *vms = VEXPRESS_MACHINE(obj);
+
+    vms->secure = value;
+}
+
+static void vexpress_instance_init(Object *obj)
+{
+    VexpressMachineState *vms = VEXPRESS_MACHINE(obj);
+
+    /* All Vexpress machine instances have a secure property
+     * Determine whether to start in a secure state or non-secure state based
+     * on whether we are directly booting a kernel ("-kernel" option).  If we
+     * are, then we default to booting into non-secure state.  Otherwise, we
+     * default to the machine default which is secure EL1/SVC.
+     * This may be overridden by the "secure" machine property.
+     */
+    if (qemu_opt_get(qemu_get_machine_opts(), "kernel")) {
+        vms->secure = false;
+    } else {
+        vms->secure = true;
+    }
+
+    object_property_add_bool(obj, "secure", vexpress_get_secure,
+                             vexpress_set_secure, NULL);
+}
+
 static void vexpress_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -738,6 +774,7 @@ static const TypeInfo vexpress_info = {
     .name = TYPE_VEXPRESS_MACHINE,
     .parent = TYPE_MACHINE,
     .instance_size = sizeof(VexpressMachineState),
+    .instance_init = vexpress_instance_init,
     .class_size = sizeof(VexpressMachineClass),
     .class_init = vexpress_class_init,
 };
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 05/13] target-arm: Add vexpress machine secure property

[Peter Maydell]

On 3 December 2014 at 20:05, Greg Bellows <greg.bellows@linaro.org> wrote:
> Add "secure" Vexpress machine specific property to allow override of the
> default secure state configuration.  By default, when using the QEMU
> -kernel command line argument, Vexpress machines boot into NS/SVC.  When using
> the QEMU -bios command line argument, Vexpress machines boot into S/SVC.
>
> The secure state can be changed from the default specifying the secure
> state as a machine property.  For example, the below command line would enable
> secure state on a -linux boot:
>
>     aarch64-softmmu/qemu-system-aarch64
>         -machine type=vexpress-a15,secure=on
>         -kernel ...

> +static void vexpress_instance_init(Object *obj)
> +{
> +    VexpressMachineState *vms = VEXPRESS_MACHINE(obj);
> +
> +    /* All Vexpress machine instances have a secure property
> +     * Determine whether to start in a secure state or non-secure state based
> +     * on whether we are directly booting a kernel ("-kernel" option).  If we
> +     * are, then we default to booting into non-secure state.  Otherwise, we
> +     * default to the machine default which is secure EL1/SVC.
> +     * This may be overridden by the "secure" machine property.
> +     */
> +    if (qemu_opt_get(qemu_get_machine_opts(), "kernel")) {
> +        vms->secure = false;
> +    } else {
> +        vms->secure = true;
> +    }
> +
> +    object_property_add_bool(obj, "secure", vexpress_get_secure,
> +                             vexpress_set_secure, NULL);
> +}

What I had in mind when we were discussing this was that whether
you used -kernel or not wouldn't affect the configuration of the
CPU, but would affect what we started the CPU in (ie hw/arm/boot.c
code would put the CPU in some sensible initial state as required
by the kernel booting API). Sorry for the confusion.

-- PMM

[Qemu-devel] [PATCH 06/13] target-arm: Change vexpress daughterboard init arg

[Greg Bellows]

Change the Vexpress daughterboard initialization method to take a vexpress
machine state pointer instead of the daughterboard struct pointer.  The machine
state now contains the daughterboard pointer.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 6dc5d3b..4a345d8 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -177,7 +177,7 @@ typedef struct {
 #define VEXPRESS_MACHINE_CLASS(klass) \
     OBJECT_CLASS_CHECK(VexpressMachineClass, klass, TYPE_VEXPRESS_MACHINE)
 
-typedef void DBoardInitFn(const VEDBoardInfo *daughterboard,
+typedef void DBoardInitFn(const VexpressMachineState *machine,
                           ram_addr_t ram_size,
                           const char *cpu_model,
                           qemu_irq *pic);
@@ -252,7 +252,7 @@ static void init_cpus(const char *cpu_model, const char *privdev,
     }
 }
 
-static void a9_daughterboard_init(const VEDBoardInfo *daughterboard,
+static void a9_daughterboard_init(const VexpressMachineState *vms,
                                   ram_addr_t ram_size,
                                   const char *cpu_model,
                                   qemu_irq *pic)
@@ -342,7 +342,7 @@ static VEDBoardInfo a9_daughterboard = {
     .init = a9_daughterboard_init,
 };
 
-static void a15_daughterboard_init(const VEDBoardInfo *daughterboard,
+static void a15_daughterboard_init(const VexpressMachineState *vms,
                                    ram_addr_t ram_size,
                                    const char *cpu_model,
                                    qemu_irq *pic)
@@ -535,6 +535,7 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
 
 static void vexpress_common_init(MachineState *machine)
 {
+    VexpressMachineState *vms = VEXPRESS_MACHINE(machine);
     VexpressMachineClass *vmc = VEXPRESS_MACHINE_GET_CLASS(machine);
     VEDBoardInfo *daughterboard = vmc->daughterboard;;
     DeviceState *dev, *sysctl, *pl041;
@@ -551,8 +552,7 @@ static void vexpress_common_init(MachineState *machine)
     const hwaddr *map = daughterboard->motherboard_map;
     int i;
 
-    daughterboard->init(daughterboard, machine->ram_size, machine->cpu_model,
-                        pic);
+    daughterboard->init(vms, machine->ram_size, machine->cpu_model, pic);
 
     /*
      * If a bios file was provided, attempt to map it into memory
-- 
1.8.3.2

[Qemu-devel] [PATCH 07/13] target-arm: Add virt class and machine types

[Greg Bellows]

Switch virt qemu machine support to use the newer object type, class, and
instance model.  Added virt TypeInfo with static registration along with virt
specific class and machine structs.  Also added virt class initialization
method.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/virt.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 314e55b..b6bb914 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -86,6 +86,23 @@ typedef struct VirtBoardInfo {
     uint32_t clock_phandle;
 } VirtBoardInfo;
 
+typedef struct {
+    MachineClass parent;
+    VirtBoardInfo *daughterboard;
+} VirtMachineClass;
+
+typedef struct {
+    MachineState parent;
+} VirtMachineState;
+
+#define TYPE_VIRT_MACHINE   "virt"
+#define VIRT_MACHINE(obj) \
+    OBJECT_CHECK(VirtMachineState, (obj), TYPE_VIRT_MACHINE)
+#define VIRT_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(VirtMachineClass, obj, TYPE_VIRT_MACHINE)
+#define VIRT_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
+
 /* Addresses and sizes of our components.
  * 0..128MB is space for a flash device so we can run bootrom code such as UEFI.
  * 128MB..256MB is used for miscellaneous device I/O.
@@ -615,16 +632,27 @@ static void machvirt_init(MachineState *machine)
     arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
 }
 
-static QEMUMachine machvirt_a15_machine = {
-    .name = "virt",
-    .desc = "ARM Virtual Machine",
-    .init = machvirt_init,
-    .max_cpus = 8,
+static void virt_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = TYPE_VIRT_MACHINE;
+    mc->desc = "ARM Virtual Machine",
+    mc->init = machvirt_init;
+    mc->max_cpus = 8;
+}
+
+static const TypeInfo machvirt_info = {
+    .name = TYPE_VIRT_MACHINE,
+    .parent = TYPE_MACHINE,
+    .instance_size = sizeof(VirtMachineState),
+    .class_size = sizeof(VirtMachineClass),
+    .class_init = virt_class_init,
 };
 
 static void machvirt_machine_init(void)
 {
-    qemu_register_machine(&machvirt_a15_machine);
+    type_register_static(&machvirt_info);
 }
 
 machine_init(machvirt_machine_init);
-- 
1.8.3.2

[Qemu-devel] [PATCH 08/13] target-arm: Add virt machine secure property

[Greg Bellows]

Add "secure" virt machine specific property to allow override of the
default secure state configuration.  By default, when using the QEMU
-kernel command line argument, virt machines boot into NS/SVC.  When using
the QEMU -bios command line argument, virt machines boot into S/SVC.

The secure state can be changed from the default specifying the secure
state as a machine property.  For example, the below command line would
enable secure state on a -linux boot:

    aarch64-softmmu/qemu-system-aarch64
        -machine type=virt,secure=on
        -kernel ...

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/virt.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b6bb914..ba034e4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -93,6 +93,7 @@ typedef struct {
 
 typedef struct {
     MachineState parent;
+    bool secure;
 } VirtMachineState;
 
 #define TYPE_VIRT_MACHINE   "virt"
@@ -632,6 +633,40 @@ static void machvirt_init(MachineState *machine)
     arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
 }
 
+static bool virt_get_secure(Object *obj, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    return vms->secure;
+}
+
+static void virt_set_secure(Object *obj, bool value, Error **errp)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->secure = value;
+}
+
+static void virt_instance_init(Object *obj)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    /* Determine whether to start in a secure state or non-secure state based
+     * on whether we are directly booting a kernel ("-kernel" option).  If we
+     * are, then we default to booting into non-secure state.  Otherwise, we
+     * default to the machine default which is secure EL1/SVC.
+     * This may be overridden by the "secure" machine property.
+     */
+    if (qemu_opt_get(qemu_get_machine_opts(), "kernel")) {
+        vms->secure = false;
+    } else {
+        vms->secure = true;
+    }
+
+    object_property_add_bool(obj, "secure", virt_get_secure,
+                             virt_set_secure, NULL);
+}
+
 static void virt_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -646,6 +681,7 @@ static const TypeInfo machvirt_info = {
     .name = TYPE_VIRT_MACHINE,
     .parent = TYPE_MACHINE,
     .instance_size = sizeof(VirtMachineState),
+    .instance_init = virt_instance_init,
     .class_size = sizeof(VirtMachineClass),
     .class_init = virt_class_init,
 };
-- 
1.8.3.2

[Qemu-devel] [PATCH 09/13] target-arm: Add feature unset function

[Greg Bellows]

Add an unset_feature() function to compliment the set_feature() function.  This
will be used to disable functions after they have been enabled during
initialization.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d3db279..01afed2 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -327,6 +327,11 @@ static inline void set_feature(CPUARMState *env, int feature)
     env->features |= 1ULL << feature;
 }
 
+static inline void unset_feature(CPUARMState *env, int feature)
+{
+    env->features &= ~(1ULL << feature);
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 09/13] target-arm: Add feature unset function

[Peter Maydell]

On 3 December 2014 at 20:06, Greg Bellows <greg.bellows@linaro.org> wrote:
> Add an unset_feature() function to compliment the set_feature() function.  This
> will be used to disable functions after they have been enabled during
> initialization.
>
> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH 10/13] target-arm: Add ARMCPU secure property

[Greg Bellows]

Added a "secure" state property to the ARMCPU descriptor.  This property
indicates whether the ARMCPU is enabled for secure state or not.  By default it
is disabled at this time.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/cpu-qom.h |  2 ++
 target-arm/cpu.c     | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index dcfda7d..8dab91b 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -100,6 +100,8 @@ typedef struct ARMCPU {
     bool start_powered_off;
     /* CPU currently in PSCI powered-off state */
     bool powered_off;
+    /* CPU secure state enabled */
+    bool secure;
 
     /* PSCI conduit used to invoke PSCI methods
      * 0 - disabled, 1 - smc, 2 - hvc
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 01afed2..0e660f9 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -388,6 +388,9 @@ static Property arm_cpu_reset_hivecs_property =
 static Property arm_cpu_rvbar_property =
             DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0);
 
+static Property arm_cpu_secure_property =
+            DEFINE_PROP_BOOL("secure", ARMCPU, secure, false);
+
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -407,6 +410,14 @@ static void arm_cpu_post_init(Object *obj)
         qdev_property_add_static(DEVICE(obj), &arm_cpu_rvbar_property,
                                  &error_abort);
     }
+
+    if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
+        /* Add the secure state CPU property only if EL3 is allowed.  This will
+         * prevent "secure" from existing on non EL3 enabled machines.
+         */
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_secure_property,
+                                 &error_abort);
+    }
 }
 
 static void arm_cpu_finalizefn(Object *obj)
@@ -476,6 +487,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
             cpu->reset_sctlr |= (1 << 13);
     }
 
+    if (arm_feature(env, ARM_FEATURE_V6) && !cpu->secure) {
+        /* The security extension and ID_PFR1 only apply to ARMv6 and up. IF
+         * this is the case and secure state has not been enabled then we
+         * disable the security extension feature.
+         */
+        unset_feature(env, ARM_FEATURE_EL3);
+
+        /* Disable the security extension feature bits in the processor feature
+         * register as well.  This is id_pfr1[7:4].
+         */
+        cpu->id_pfr1 &= ~0xf0;
+    }
+
     register_cp_regs_for_features(cpu);
     arm_cpu_register_gdb_regs_for_features(cpu);
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 10/13] target-arm: Add ARMCPU secure property

[Peter Maydell]

On 3 December 2014 at 20:06, Greg Bellows <greg.bellows@linaro.org> wrote:
> Added a "secure" state property to the ARMCPU descriptor.  This property
> indicates whether the ARMCPU is enabled for secure state or not.  By default it
> is disabled at this time.

Shouldn't this feature be "has_el3" ? It's the configurable
version of the ARM_FEATURE_EL3.

> Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> ---
>  target-arm/cpu-qom.h |  2 ++
>  target-arm/cpu.c     | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index dcfda7d..8dab91b 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -100,6 +100,8 @@ typedef struct ARMCPU {
>      bool start_powered_off;
>      /* CPU currently in PSCI powered-off state */
>      bool powered_off;
> +    /* CPU secure state enabled */
> +    bool secure;
>
>      /* PSCI conduit used to invoke PSCI methods
>       * 0 - disabled, 1 - smc, 2 - hvc
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 01afed2..0e660f9 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -388,6 +388,9 @@ static Property arm_cpu_reset_hivecs_property =
>  static Property arm_cpu_rvbar_property =
>              DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0);
>
> +static Property arm_cpu_secure_property =
> +            DEFINE_PROP_BOOL("secure", ARMCPU, secure, false);
> +
>  static void arm_cpu_post_init(Object *obj)
>  {
>      ARMCPU *cpu = ARM_CPU(obj);
> @@ -407,6 +410,14 @@ static void arm_cpu_post_init(Object *obj)
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_rvbar_property,
>                                   &error_abort);
>      }
> +
> +    if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
> +        /* Add the secure state CPU property only if EL3 is allowed.  This will
> +         * prevent "secure" from existing on non EL3 enabled machines.

"on CPUs which cannot support EL3".

> +         */
> +        qdev_property_add_static(DEVICE(obj), &arm_cpu_secure_property,
> +                                 &error_abort);
> +    }
>  }
>
>  static void arm_cpu_finalizefn(Object *obj)
> @@ -476,6 +487,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>              cpu->reset_sctlr |= (1 << 13);
>      }
>
> +    if (arm_feature(env, ARM_FEATURE_V6) && !cpu->secure) {
> +        /* The security extension and ID_PFR1 only apply to ARMv6 and up. IF
> +         * this is the case and secure state has not been enabled then we
> +         * disable the security extension feature.
> +         */
> +        unset_feature(env, ARM_FEATURE_EL3);

You don't need to conditionalize this on V6 being set;
just do
       if (!cpu->secure) {
           unset_feature(...);
       }

to reflect the property setting into the feature bits. The
property won't exist in the first place on pre-v6 cores.

> +
> +        /* Disable the security extension feature bits in the processor feature
> +         * register as well.  This is id_pfr1[7:4].
> +         */
> +        cpu->id_pfr1 &= ~0xf0;

This is a bit of a tricky area, because we don't in general
mess with our ID register values to match the features we do or
don't happen to implement, so this would be an odd special case.
Can we get away without touching PFR1 here?

> +    }
> +
>      register_cp_regs_for_features(cpu);
>      arm_cpu_register_gdb_regs_for_features(cpu);

thanks
-- PMM

Re: [Qemu-devel] [PATCH 10/13] target-arm: Add ARMCPU secure property

[Greg Bellows]

[-- Attachment #1: Type: text/plain, Size: 4237 bytes --]

On 5 December 2014 at 09:26, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 3 December 2014 at 20:06, Greg Bellows <greg.bellows@linaro.org> wrote:
> > Added a "secure" state property to the ARMCPU descriptor.  This property
> > indicates whether the ARMCPU is enabled for secure state or not.  By
> default it
> > is disabled at this time.
>
> Shouldn't this feature be "has_el3" ? It's the configurable
> version of the ARM_FEATURE_EL3.
>

Sure, that makes sense, I can rename it.


>
> > Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
> > ---
> >  target-arm/cpu-qom.h |  2 ++
> >  target-arm/cpu.c     | 24 ++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> > index dcfda7d..8dab91b 100644
> > --- a/target-arm/cpu-qom.h
> > +++ b/target-arm/cpu-qom.h
> > @@ -100,6 +100,8 @@ typedef struct ARMCPU {
> >      bool start_powered_off;
> >      /* CPU currently in PSCI powered-off state */
> >      bool powered_off;
> > +    /* CPU secure state enabled */
> > +    bool secure;
> >
> >      /* PSCI conduit used to invoke PSCI methods
> >       * 0 - disabled, 1 - smc, 2 - hvc
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index 01afed2..0e660f9 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -388,6 +388,9 @@ static Property arm_cpu_reset_hivecs_property =
> >  static Property arm_cpu_rvbar_property =
> >              DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0);
> >
> > +static Property arm_cpu_secure_property =
> > +            DEFINE_PROP_BOOL("secure", ARMCPU, secure, false);
> > +
> >  static void arm_cpu_post_init(Object *obj)
> >  {
> >      ARMCPU *cpu = ARM_CPU(obj);
> > @@ -407,6 +410,14 @@ static void arm_cpu_post_init(Object *obj)
> >          qdev_property_add_static(DEVICE(obj), &arm_cpu_rvbar_property,
> >                                   &error_abort);
> >      }
> > +
> > +    if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
> > +        /* Add the secure state CPU property only if EL3 is allowed.
> This will
> > +         * prevent "secure" from existing on non EL3 enabled machines.
>
> "on CPUs which cannot support EL3".
>
>
That's better, fixed in next version.


> > +         */
> > +        qdev_property_add_static(DEVICE(obj), &arm_cpu_secure_property,
> > +                                 &error_abort);
> > +    }
> >  }
> >
> >  static void arm_cpu_finalizefn(Object *obj)
> > @@ -476,6 +487,19 @@ static void arm_cpu_realizefn(DeviceState *dev,
> Error **errp)
> >              cpu->reset_sctlr |= (1 << 13);
> >      }
> >
> > +    if (arm_feature(env, ARM_FEATURE_V6) && !cpu->secure) {
> > +        /* The security extension and ID_PFR1 only apply to ARMv6 and
> up. IF
> > +         * this is the case and secure state has not been enabled then
> we
> > +         * disable the security extension feature.
> > +         */
> > +        unset_feature(env, ARM_FEATURE_EL3);
>
> You don't need to conditionalize this on V6 being set;
> just do
>        if (!cpu->secure) {
>            unset_feature(...);
>        }
>
> to reflect the property setting into the feature bits. The
> property won't exist in the first place on pre-v6 cores.
>
>
Originally, I had just what you suggested, but I didn't want updates to
id_pfr1 on pre v6 as I was not sure they had the equivalent bits.  I
removed the check as it is likely benign.


> > +
> > +        /* Disable the security extension feature bits in the processor
> feature
> > +         * register as well.  This is id_pfr1[7:4].
> > +         */
> > +        cpu->id_pfr1 &= ~0xf0;
>
> This is a bit of a tricky area, because we don't in general
> mess with our ID register values to match the features we do or
> don't happen to implement, so this would be an odd special case.
> Can we get away without touching PFR1 here?
>

It depends if you want SW to be able to correctly use the register.  For
instance, my TZ test checks the ID_PFR1 to see if it should be performing
security extension operations.  I'm not sure of another way for this check
to be performed.


>
> > +    }
> > +
> >      register_cp_regs_for_features(cpu);
> >      arm_cpu_register_gdb_regs_for_features(cpu);
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 6223 bytes --]

[Qemu-devel] [PATCH 11/13] target-arm: Set CPU secure prop during VE init

[Greg Bellows]

Adds setting of the CPU secure state property based on the vexpress machine
secure state property during initialization.  This enablesi/disables secure
state during start-up.  Changes include adding an additional secure state
boolean during vexpress CPU initialization.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/vexpress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 4a345d8..392293f 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -196,7 +196,7 @@ struct VEDBoardInfo {
 };
 
 static void init_cpus(const char *cpu_model, const char *privdev,
-                      hwaddr periphbase, qemu_irq *pic)
+                      hwaddr periphbase, qemu_irq *pic, bool secure)
 {
     ObjectClass *cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
     DeviceState *dev;
@@ -213,6 +213,15 @@ static void init_cpus(const char *cpu_model, const char *privdev,
         Object *cpuobj = object_new(object_class_get_name(cpu_oc));
         Error *err = NULL;
 
+        if (secure) {
+            object_property_set_bool(cpuobj, true, "secure", &err);
+            if (err) {
+                error_report("'secure' machine property not supported "
+                             "with %s cpu", cpu_model);
+                exit(1);
+            }
+        }
+
         if (object_property_find(cpuobj, "reset-cbar", NULL)) {
             object_property_set_int(cpuobj, periphbase,
                                     "reset-cbar", &error_abort);
@@ -288,7 +297,7 @@ static void a9_daughterboard_init(const VexpressMachineState *vms,
     memory_region_add_subregion(sysmem, 0x60000000, ram);
 
     /* 0x1e000000 A9MPCore (SCU) private memory region */
-    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic);
+    init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic, vms->secure);
 
     /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */
 
@@ -374,7 +383,7 @@ static void a15_daughterboard_init(const VexpressMachineState *vms,
     memory_region_add_subregion(sysmem, 0x80000000, ram);
 
     /* 0x2c000000 A15MPCore private memory region (GIC) */
-    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic);
+    init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic, vms->secure);
 
     /* A15 daughterboard peripherals: */
 
-- 
1.8.3.2

[Qemu-devel] [PATCH 12/13] target-arm: Set CPU secure prop during virt init

[Greg Bellows]

Add setting of the CPU secure property based on the virt machine secure
property during initialization.  This enables/disables secure state during
start-up.

Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 hw/arm/virt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ba034e4..2f075e2 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -547,6 +547,7 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 
 static void machvirt_init(MachineState *machine)
 {
+    VirtMachineState *vms = VIRT_MACHINE(machine);
     qemu_irq pic[NUM_IRQS];
     MemoryRegion *sysmem = get_system_memory();
     int n;
@@ -584,6 +585,16 @@ static void machvirt_init(MachineState *machine)
         }
         cpuobj = object_new(object_class_get_name(oc));
 
+        if (vms->secure) {
+            Error *err = NULL;
+            object_property_set_bool(cpuobj, true, "secure", &err);
+            if (err) {
+                error_report("'secure' machine property not supported "
+                             "with %s cpu", cpu_model);
+                exit(1);
+            }
+        }
+
         object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC, "psci-conduit",
                                 NULL);
 
-- 
1.8.3.2

[Qemu-devel] [PATCH 13/13] target-arm: add cpu feature EL3 to CPUs with Security Extensions

[Greg Bellows]

From: Fabian Aggeler <aggelerf@ethz.ch>

Set ARM_FEATURE_EL3 feature for CPUs that implement Security Extensions.

Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
Signed-off-by: Greg Bellows <greg.bellows@linaro.org>
---
 target-arm/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 0e660f9..9e8d40c 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -669,6 +669,7 @@ static void arm1176_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_DIRTY_REG);
     set_feature(&cpu->env, ARM_FEATURE_CACHE_BLOCK_OPS);
+    set_feature(&cpu->env, ARM_FEATURE_EL3);
     cpu->midr = 0x410fb767;
     cpu->reset_fpsid = 0x410120b5;
     cpu->mvfr0 = 0x11111111;
@@ -757,6 +758,7 @@ static void cortex_a8_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    set_feature(&cpu->env, ARM_FEATURE_EL3);
     cpu->midr = 0x410fc080;
     cpu->reset_fpsid = 0x410330c0;
     cpu->mvfr0 = 0x11110222;
@@ -824,6 +826,7 @@ static void cortex_a9_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_VFP_FP16);
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
+    set_feature(&cpu->env, ARM_FEATURE_EL3);
     /* Note that A9 supports the MP extensions even for
      * A9UP and single-core A9MP (which are both different
      * and valid configurations; we don't model A9UP).
@@ -891,6 +894,7 @@ static void cortex_a15_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
     set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
     set_feature(&cpu->env, ARM_FEATURE_LPAE);
+    set_feature(&cpu->env, ARM_FEATURE_EL3);
     cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
     cpu->midr = 0x412fc0f1;
     cpu->reset_fpsid = 0x410430f0;
-- 
1.8.3.2