| Message ID | 1463698459-31312-2-git-send-email-edgar.iglesias@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19 May 2016 at 23:54, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Add a secure prop to en/disable ARM Security Extensions. > This is particulary useful for KVM runs. "particularly" > Default to disabled to match the behavior of KVM. This is a change in behaviour, though, right? Is that OK? > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > hw/arm/xlnx-zynqmp.c | 3 +++ > include/hw/arm/xlnx-zynqmp.h | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > index 4d504da..965a250 100644 > --- a/hw/arm/xlnx-zynqmp.c > +++ b/hw/arm/xlnx-zynqmp.c > @@ -238,6 +238,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > } > g_free(name); > > + object_property_set_bool(OBJECT(&s->apu_cpu[i]), > + s->secure, "has_el3", 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", > @@ -370,6 +372,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > > static Property xlnx_zynqmp_props[] = { > DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu), > + DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false), > DEFINE_PROP_END_OF_LIST() > }; > > diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h > index 2332596..38d4c8c 100644 > --- a/include/hw/arm/xlnx-zynqmp.h > +++ b/include/hw/arm/xlnx-zynqmp.h > @@ -84,6 +84,9 @@ typedef struct XlnxZynqMPState { > > char *boot_cpu; > ARMCPU *boot_cpu_ptr; > + > + /* Has the ARM Security extensions? */ > + bool secure; > } XlnxZynqMPState; thanks -- PMM
On Tue, May 24, 2016 at 05:30:54PM +0100, Peter Maydell wrote: > On 19 May 2016 at 23:54, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > Add a secure prop to en/disable ARM Security Extensions. > > This is particulary useful for KVM runs. > > "particularly" > > > Default to disabled to match the behavior of KVM. > > This is a change in behaviour, though, right? Is that OK? IMO it's OK. But I don't have a very strong opinion. We didn't have EL3 in this machine to start with, it came in when we enabled it on the a53. We don't have a big user-base that really depends on either default I'd say. Best regards, Edgar > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > --- > > hw/arm/xlnx-zynqmp.c | 3 +++ > > include/hw/arm/xlnx-zynqmp.h | 3 +++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > > index 4d504da..965a250 100644 > > --- a/hw/arm/xlnx-zynqmp.c > > +++ b/hw/arm/xlnx-zynqmp.c > > @@ -238,6 +238,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > > } > > g_free(name); > > > > + object_property_set_bool(OBJECT(&s->apu_cpu[i]), > > + s->secure, "has_el3", 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", > > @@ -370,6 +372,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > > > > static Property xlnx_zynqmp_props[] = { > > DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu), > > + DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false), > > DEFINE_PROP_END_OF_LIST() > > }; > > > > diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h > > index 2332596..38d4c8c 100644 > > --- a/include/hw/arm/xlnx-zynqmp.h > > +++ b/include/hw/arm/xlnx-zynqmp.h > > @@ -84,6 +84,9 @@ typedef struct XlnxZynqMPState { > > > > char *boot_cpu; > > ARMCPU *boot_cpu_ptr; > > + > > + /* Has the ARM Security extensions? */ > > + bool secure; > > } XlnxZynqMPState; > > thanks > -- PMM
On 24 May 2016 at 17:57, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote: > On Tue, May 24, 2016 at 05:30:54PM +0100, Peter Maydell wrote: >> On 19 May 2016 at 23:54, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: >> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> >> > >> > Add a secure prop to en/disable ARM Security Extensions. >> > This is particulary useful for KVM runs. >> >> "particularly" >> >> > Default to disabled to match the behavior of KVM. >> >> This is a change in behaviour, though, right? Is that OK? > > IMO it's OK. But I don't have a very strong opinion. We > didn't have EL3 in this machine to start with, it came in when > we enabled it on the a53. We don't have a big user-base that > really depends on either default I'd say. I'll defer to your preference, but we should mention in the commit message that we changed the behaviour (and perhaps also that it was accidental that EL3 got enabled). thanks -- PMM
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index 4d504da..965a250 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -238,6 +238,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) } g_free(name); + object_property_set_bool(OBJECT(&s->apu_cpu[i]), + s->secure, "has_el3", 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", @@ -370,6 +372,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) static Property xlnx_zynqmp_props[] = { DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu), + DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false), DEFINE_PROP_END_OF_LIST() }; diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h index 2332596..38d4c8c 100644 --- a/include/hw/arm/xlnx-zynqmp.h +++ b/include/hw/arm/xlnx-zynqmp.h @@ -84,6 +84,9 @@ typedef struct XlnxZynqMPState { char *boot_cpu; ARMCPU *boot_cpu_ptr; + + /* Has the ARM Security extensions? */ + bool secure; } XlnxZynqMPState; #define XLNX_ZYNQMP_H