[Qemu-devel] [PATCH v1 0/5] Expose the secure property to the machine

[Qemu-devel] [PATCH v1 0/5] Expose the secure property to the machine

[Alistair Francis]

The EL2 and EL3 work is working well now and interanlly we now have
tests that expect to start in EL3 and transition through EL2 to EL1. To
make this easy to run let's expose the secure property to the machine
and then use that to enable EL2.

This series also does some machine/name tidying up and makes the first
move to deprecating the EP108 machine, which was just an early access
development board.

Alistair Francis (5):
  xlnx-ep108: Rename to ZCU102
  xlnx-zcu102: Manually create the machines
  xlnx-zcu102: Add a machine level secure property
  xlnx-zynqmp: Allow the secure prop to enable EL2
  xlnx-zcu102: Mark the EP108 machine as deprecated

 hw/arm/Makefile.objs                   |   2 +-
 hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} | 131 ++++++++++++++++++++++++++++-----
 hw/arm/xlnx-zynqmp.c                   |   2 +-
 3 files changed, 114 insertions(+), 21 deletions(-)
 rename hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} (51%)

-- 
2.11.0

[Qemu-devel] [PATCH v1 2/5] xlnx-zcu102: Manually create the machines

[Alistair Francis]

In preperation for future work let's manually create the Xilnx machines.
This will allow us to set properties for the machines in the future.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zcu102.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 133a6a31a8..71261313b5 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -26,15 +26,25 @@
 #include "qemu/log.h"
 
 typedef struct XlnxZCU102 {
+
+    MachineState parent_obj;
+
     XlnxZynqMPState soc;
     MemoryRegion ddr_ram;
 } XlnxZCU102;
 
+#define TYPE_ZCU102_MACHINE   MACHINE_TYPE_NAME("xlnx-zcu102")
+#define ZCU102_MACHINE(obj) \
+    OBJECT_CHECK(XlnxZCU102, (obj), TYPE_ZCU102_MACHINE)
+
+#define TYPE_EP108_MACHINE   MACHINE_TYPE_NAME("xlnx-ep108")
+#define EP108_MACHINE(obj) \
+    OBJECT_CHECK(XlnxZCU102, (obj), TYPE_EP108_MACHINE)
+
 static struct arm_boot_info xlnx_zcu102_binfo;
 
-static void xlnx_zcu102_init(MachineState *machine)
+static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine)
 {
-    XlnxZCU102 *s = g_new0(XlnxZCU102, 1);
     int i;
     uint64_t ram_size = machine->ram_size;
 
@@ -116,22 +126,73 @@ static void xlnx_zcu102_init(MachineState *machine)
     arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo);
 }
 
-static void xlnx_ep108_machine_init(MachineClass *mc)
+static void xlnx_ep108_init(MachineState *machine)
 {
+    XlnxZCU102 *s = EP108_MACHINE(machine);
+
+    xlnx_zynqmp_init(s, machine);
+}
+
+static void xlnx_ep108_machine_instance_init(Object *obj)
+{
+}
+
+static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
     mc->desc = "Xilinx ZynqMP EP108 board";
-    mc->init = xlnx_zcu102_init;
+    mc->init = xlnx_ep108_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
 }
 
-DEFINE_MACHINE("xlnx-ep108", xlnx_ep108_machine_init)
+static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
+    .name       = MACHINE_TYPE_NAME("xlnx-ep108"),
+    .parent     = TYPE_MACHINE,
+    .class_init = xlnx_ep108_machine_class_init,
+    .instance_init = xlnx_ep108_machine_instance_init,
+    .instance_size = sizeof(XlnxZCU102),
+};
+
+static void xlnx_ep108_machine_init_register_types(void)
+{
+    type_register_static(&xlnx_ep108_machine_init_typeinfo);
+}
+
+static void xlnx_zcu102_init(MachineState *machine)
+{
+    XlnxZCU102 *s = ZCU102_MACHINE(machine);
+
+    xlnx_zynqmp_init(s, machine);
+}
 
-static void xlnx_zcu102_machine_init(MachineClass *mc)
+static void xlnx_zcu102_machine_instance_init(Object *obj)
 {
+}
+
+static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
     mc->desc = "Xilinx ZynqMP ZCU102 board";
     mc->init = xlnx_zcu102_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
 }
 
-DEFINE_MACHINE("xlnx-zcu102", xlnx_zcu102_machine_init)
+static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
+    .name       = MACHINE_TYPE_NAME("xlnx-zcu102"),
+    .parent     = TYPE_MACHINE,
+    .class_init = xlnx_zcu102_machine_class_init,
+    .instance_init = xlnx_zcu102_machine_instance_init,
+    .instance_size = sizeof(XlnxZCU102),
+};
+
+static void xlnx_zcu102_machine_init_register_types(void)
+{
+    type_register_static(&xlnx_zcu102_machine_init_typeinfo);
+}
+
+type_init(xlnx_zcu102_machine_init_register_types)
+type_init(xlnx_ep108_machine_init_register_types)
-- 
2.11.0

Re: [Qemu-devel] [PATCH v1 2/5] xlnx-zcu102: Manually create the machines

[Edgar E. Iglesias]

On Thu, Aug 17, 2017 at 11:52:04AM -0700, Alistair Francis wrote:
> In preperation for future work let's manually create the Xilnx machines.
> This will allow us to set properties for the machines in the future.
> 
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> 
>  hw/arm/xlnx-zcu102.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 133a6a31a8..71261313b5 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -26,15 +26,25 @@
>  #include "qemu/log.h"
>  
>  typedef struct XlnxZCU102 {
> +

Looks like a stray newline here.

> +    MachineState parent_obj;
> +
>      XlnxZynqMPState soc;
>      MemoryRegion ddr_ram;
>  } XlnxZCU102;
>  
> +#define TYPE_ZCU102_MACHINE   MACHINE_TYPE_NAME("xlnx-zcu102")
> +#define ZCU102_MACHINE(obj) \
> +    OBJECT_CHECK(XlnxZCU102, (obj), TYPE_ZCU102_MACHINE)
> +
> +#define TYPE_EP108_MACHINE   MACHINE_TYPE_NAME("xlnx-ep108")
> +#define EP108_MACHINE(obj) \
> +    OBJECT_CHECK(XlnxZCU102, (obj), TYPE_EP108_MACHINE)
> +
>  static struct arm_boot_info xlnx_zcu102_binfo;
>  
> -static void xlnx_zcu102_init(MachineState *machine)
> +static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine)
>  {
> -    XlnxZCU102 *s = g_new0(XlnxZCU102, 1);
>      int i;
>      uint64_t ram_size = machine->ram_size;
>  
> @@ -116,22 +126,73 @@ static void xlnx_zcu102_init(MachineState *machine)
>      arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo);
>  }
>  
> -static void xlnx_ep108_machine_init(MachineClass *mc)
> +static void xlnx_ep108_init(MachineState *machine)
>  {
> +    XlnxZCU102 *s = EP108_MACHINE(machine);
> +
> +    xlnx_zynqmp_init(s, machine);
> +}
> +
> +static void xlnx_ep108_machine_instance_init(Object *obj)
> +{
> +}
> +
> +static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
>      mc->desc = "Xilinx ZynqMP EP108 board";
> -    mc->init = xlnx_zcu102_init;
> +    mc->init = xlnx_ep108_init;
>      mc->block_default_type = IF_IDE;
>      mc->units_per_default_bus = 1;
>  }
>  
> -DEFINE_MACHINE("xlnx-ep108", xlnx_ep108_machine_init)
> +static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
> +    .name       = MACHINE_TYPE_NAME("xlnx-ep108"),
> +    .parent     = TYPE_MACHINE,
> +    .class_init = xlnx_ep108_machine_class_init,
> +    .instance_init = xlnx_ep108_machine_instance_init,
> +    .instance_size = sizeof(XlnxZCU102),
> +};
> +
> +static void xlnx_ep108_machine_init_register_types(void)
> +{
> +    type_register_static(&xlnx_ep108_machine_init_typeinfo);
> +}
> +
> +static void xlnx_zcu102_init(MachineState *machine)
> +{
> +    XlnxZCU102 *s = ZCU102_MACHINE(machine);
> +
> +    xlnx_zynqmp_init(s, machine);
> +}
>  
> -static void xlnx_zcu102_machine_init(MachineClass *mc)
> +static void xlnx_zcu102_machine_instance_init(Object *obj)
>  {
> +}
> +
> +static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
>      mc->desc = "Xilinx ZynqMP ZCU102 board";
>      mc->init = xlnx_zcu102_init;
>      mc->block_default_type = IF_IDE;
>      mc->units_per_default_bus = 1;
>  }
>  
> -DEFINE_MACHINE("xlnx-zcu102", xlnx_zcu102_machine_init)
> +static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
> +    .name       = MACHINE_TYPE_NAME("xlnx-zcu102"),
> +    .parent     = TYPE_MACHINE,
> +    .class_init = xlnx_zcu102_machine_class_init,
> +    .instance_init = xlnx_zcu102_machine_instance_init,
> +    .instance_size = sizeof(XlnxZCU102),
> +};
> +
> +static void xlnx_zcu102_machine_init_register_types(void)
> +{
> +    type_register_static(&xlnx_zcu102_machine_init_typeinfo);
> +}
> +
> +type_init(xlnx_zcu102_machine_init_register_types)
> +type_init(xlnx_ep108_machine_init_register_types)
> -- 
> 2.11.0
> 

[Qemu-devel] [PATCH v1 4/5] xlnx-zynqmp: Allow the secure prop to enable EL2

[Alistair Francis]

If the user sets the secure property to true we want to enalbe both EL2
and EL3.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zynqmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 9eceadbdc8..d6ca5dcd4e 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -260,7 +260,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         object_property_set_bool(OBJECT(&s->apu_cpu[i]),
                                  s->secure, "has_el3", NULL);
         object_property_set_bool(OBJECT(&s->apu_cpu[i]),
-                                 false, "has_el2", NULL);
+                                 s->secure, "has_el2", NULL);
         object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
                                 "reset-cbar", &error_abort);
         object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
-- 
2.11.0

[Qemu-devel] [PATCH v1 5/5] xlnx-zcu102: Mark the EP108 machine as deprecated

[Alistair Francis]

The EP108 is the same as the ZCU102, mark it as deprecated as we don't
need two machines.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zcu102.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 8a53221d0a..54207ba095 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -163,7 +163,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "Xilinx ZynqMP EP108 board";
+    mc->desc = "Xilinx ZynqMP EP108 board (Deprecated, please use xlnx-zcu102)";
     mc->init = xlnx_ep108_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
-- 
2.11.0

Re: [Qemu-devel] [PATCH v1 0/5] Expose the secure property to the machine

[Edgar E. Iglesias]

On Thu, Aug 17, 2017 at 11:51:59AM -0700, Alistair Francis wrote:
> The EL2 and EL3 work is working well now and interanlly we now have
> tests that expect to start in EL3 and transition through EL2 to EL1. To
> make this easy to run let's expose the secure property to the machine
> and then use that to enable EL2.
> 
> This series also does some machine/name tidying up and makes the first
> move to deprecating the EP108 machine, which was just an early access
> development board.

Hi Alistair,

Reconsidering this, I tend to agree that we're probably better off with
EL2/no-GICv2-virt compared to the possible confusiong of having EL2
without GICv2-virt..

But I wonder if we should have similar options as the virt machine?
I.e, a virtualization option to enable EL2.

Cheers,
Edgar


> 
> Alistair Francis (5):
>   xlnx-ep108: Rename to ZCU102
>   xlnx-zcu102: Manually create the machines
>   xlnx-zcu102: Add a machine level secure property
>   xlnx-zynqmp: Allow the secure prop to enable EL2
>   xlnx-zcu102: Mark the EP108 machine as deprecated
> 
>  hw/arm/Makefile.objs                   |   2 +-
>  hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} | 131 ++++++++++++++++++++++++++++-----
>  hw/arm/xlnx-zynqmp.c                   |   2 +-
>  3 files changed, 114 insertions(+), 21 deletions(-)
>  rename hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} (51%)
> 
> -- 
> 2.11.0
> 

Re: [Qemu-devel] [PATCH v1 0/5] Expose the secure property to the machine

[Alistair Francis]

On Tue, Aug 22, 2017 at 10:24 AM, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> On Thu, Aug 17, 2017 at 11:51:59AM -0700, Alistair Francis wrote:
>> The EL2 and EL3 work is working well now and interanlly we now have
>> tests that expect to start in EL3 and transition through EL2 to EL1. To
>> make this easy to run let's expose the secure property to the machine
>> and then use that to enable EL2.
>>
>> This series also does some machine/name tidying up and makes the first
>> move to deprecating the EP108 machine, which was just an early access
>> development board.
>
> Hi Alistair,
>
> Reconsidering this, I tend to agree that we're probably better off with
> EL2/no-GICv2-virt compared to the possible confusiong of having EL2
> without GICv2-virt..
>
> But I wonder if we should have similar options as the virt machine?
> I.e, a virtualization option to enable EL2.

That's fine with me.

Just to clarify do you think we should keep the secure option for EL3
and add a virt option for EL2?

Thanks,
Alistair

>
> Cheers,
> Edgar
>
>
>>
>> Alistair Francis (5):
>>   xlnx-ep108: Rename to ZCU102
>>   xlnx-zcu102: Manually create the machines
>>   xlnx-zcu102: Add a machine level secure property
>>   xlnx-zynqmp: Allow the secure prop to enable EL2
>>   xlnx-zcu102: Mark the EP108 machine as deprecated
>>
>>  hw/arm/Makefile.objs                   |   2 +-
>>  hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} | 131 ++++++++++++++++++++++++++++-----
>>  hw/arm/xlnx-zynqmp.c                   |   2 +-
>>  3 files changed, 114 insertions(+), 21 deletions(-)
>>  rename hw/arm/{xlnx-ep108.c => xlnx-zcu102.c} (51%)
>>
>> --
>> 2.11.0
>>

Re: [Qemu-devel] [PATCH v1 0/5] Expose the secure property to the machine

[Peter Maydell]

On 22 August 2017 at 18:57, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Tue, Aug 22, 2017 at 10:24 AM, Edgar E. Iglesias
> <edgar.iglesias@xilinx.com> wrote:
>> On Thu, Aug 17, 2017 at 11:51:59AM -0700, Alistair Francis wrote:
>>> The EL2 and EL3 work is working well now and interanlly we now have
>>> tests that expect to start in EL3 and transition through EL2 to EL1. To
>>> make this easy to run let's expose the secure property to the machine
>>> and then use that to enable EL2.
>>>
>>> This series also does some machine/name tidying up and makes the first
>>> move to deprecating the EP108 machine, which was just an early access
>>> development board.
>>
>> Hi Alistair,
>>
>> Reconsidering this, I tend to agree that we're probably better off with
>> EL2/no-GICv2-virt compared to the possible confusiong of having EL2
>> without GICv2-virt..
>>
>> But I wonder if we should have similar options as the virt machine?
>> I.e, a virtualization option to enable EL2.
>
> That's fine with me.
>
> Just to clarify do you think we should keep the secure option for EL3
> and add a virt option for EL2?

I think unless you have a really strong reason to be different
(eg back-compat with existing guests) then following what
the 'virt' board does for machine options is probably going
to be least confusing. NB that that does include letting you
specify some combos that don't work, like gicv2 + virt...

(We should really get round to implementing virt support in
the GICv2, though.)

thanks
-- PMM