* [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
@ 2013-12-03 6:59 ` Peter Crosthwaite
2013-12-03 13:19 ` Andreas Färber
2013-12-03 13:26 ` Andreas Färber
2013-12-03 7:00 ` [Qemu-devel] [PATCH arm-devs v3 2/9] target-arm/helper.c: Allow cp15.c15 dummy override Peter Crosthwaite
` (7 subsequent siblings)
8 siblings, 2 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 6:59 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
Currently the uintXX property adders make a read only property. This
is not useful for devices that want to create board (or container)
configurable dynamic device properties. Fix by trivially adding property
setters to object_property_add_uintXX.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v2:
msg typo: "trivially"
qom/object.c | 44 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..07b454b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1353,6 +1353,15 @@ static void property_get_uint8_ptr(Object *obj, Visitor *v,
visit_type_uint8(v, &value, name, errp);
}
+static void property_set_uint8_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ uint8_t value;
+ visit_type_uint8(v, &value, name, errp);
+ *(uint8_t *)opaque = value;
+}
+
static void property_get_uint16_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1361,6 +1370,15 @@ static void property_get_uint16_ptr(Object *obj, Visitor *v,
visit_type_uint16(v, &value, name, errp);
}
+static void property_set_uint16_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ uint16_t value;
+ visit_type_uint16(v, &value, name, errp);
+ *(uint16_t *)opaque = value;
+}
+
static void property_get_uint32_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1369,6 +1387,15 @@ static void property_get_uint32_ptr(Object *obj, Visitor *v,
visit_type_uint32(v, &value, name, errp);
}
+static void property_set_uint32_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ uint32_t value;
+ visit_type_uint32(v, &value, name, errp);
+ *(uint32_t *)opaque = value;
+}
+
static void property_get_uint64_ptr(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1377,32 +1404,41 @@ static void property_get_uint64_ptr(Object *obj, Visitor *v,
visit_type_uint64(v, &value, name, errp);
}
+static void property_set_uint64_ptr(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ uint64_t value;
+ visit_type_uint64(v, &value, name, errp);
+ *(uint64_t *)opaque = value;
+}
+
void object_property_add_uint8_ptr(Object *obj, const char *name,
const uint8_t *v, Error **errp)
{
object_property_add(obj, name, "uint8", property_get_uint8_ptr,
- NULL, NULL, (void *)v, errp);
+ property_set_uint8_ptr, NULL, (void *)v, errp);
}
void object_property_add_uint16_ptr(Object *obj, const char *name,
const uint16_t *v, Error **errp)
{
object_property_add(obj, name, "uint16", property_get_uint16_ptr,
- NULL, NULL, (void *)v, errp);
+ property_set_uint16_ptr, NULL, (void *)v, errp);
}
void object_property_add_uint32_ptr(Object *obj, const char *name,
const uint32_t *v, Error **errp)
{
object_property_add(obj, name, "uint32", property_get_uint32_ptr,
- NULL, NULL, (void *)v, errp);
+ property_set_uint32_ptr, NULL, (void *)v, errp);
}
void object_property_add_uint64_ptr(Object *obj, const char *name,
const uint64_t *v, Error **errp)
{
object_property_add(obj, name, "uint64", property_get_uint64_ptr,
- NULL, NULL, (void *)v, errp);
+ property_set_uint64_ptr, NULL, (void *)v, errp);
}
static void object_instance_init(Object *obj)
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-03 6:59 ` [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable Peter Crosthwaite
@ 2013-12-03 13:19 ` Andreas Färber
2013-12-06 14:49 ` Peter Maydell
2013-12-15 10:23 ` Michael S. Tsirkin
2013-12-03 13:26 ` Andreas Färber
1 sibling, 2 replies; 30+ messages in thread
From: Andreas Färber @ 2013-12-03 13:19 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel, peter.maydell
Cc: mark.langsdorf, Michael S. Tsirkin
Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
> Currently the uintXX property adders make a read only property. This
> is not useful for devices that want to create board (or container)
> configurable dynamic device properties. Fix by trivially adding property
> setters to object_property_add_uintXX.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> changed since v2:
> msg typo: "trivially"
Not sure if I've asked already, but these functions were added by mst
(so let's CC him) for accessing read-only constants in ACPI code. Your
change seems to make them writable - can anything go wrong when the
setters are used via QMP? I fear we may need two separate sets of
functions, one read-only, one read-write.
Andreas
>
> qom/object.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index fc19cf6..07b454b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1353,6 +1353,15 @@ static void property_get_uint8_ptr(Object *obj, Visitor *v,
> visit_type_uint8(v, &value, name, errp);
> }
>
> +static void property_set_uint8_ptr(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + uint8_t value;
> + visit_type_uint8(v, &value, name, errp);
> + *(uint8_t *)opaque = value;
> +}
> +
> static void property_get_uint16_ptr(Object *obj, Visitor *v,
> void *opaque, const char *name,
> Error **errp)
> @@ -1361,6 +1370,15 @@ static void property_get_uint16_ptr(Object *obj, Visitor *v,
> visit_type_uint16(v, &value, name, errp);
> }
>
> +static void property_set_uint16_ptr(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + uint16_t value;
> + visit_type_uint16(v, &value, name, errp);
> + *(uint16_t *)opaque = value;
> +}
> +
> static void property_get_uint32_ptr(Object *obj, Visitor *v,
> void *opaque, const char *name,
> Error **errp)
> @@ -1369,6 +1387,15 @@ static void property_get_uint32_ptr(Object *obj, Visitor *v,
> visit_type_uint32(v, &value, name, errp);
> }
>
> +static void property_set_uint32_ptr(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + uint32_t value;
> + visit_type_uint32(v, &value, name, errp);
> + *(uint32_t *)opaque = value;
> +}
> +
> static void property_get_uint64_ptr(Object *obj, Visitor *v,
> void *opaque, const char *name,
> Error **errp)
> @@ -1377,32 +1404,41 @@ static void property_get_uint64_ptr(Object *obj, Visitor *v,
> visit_type_uint64(v, &value, name, errp);
> }
>
> +static void property_set_uint64_ptr(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + uint64_t value;
> + visit_type_uint64(v, &value, name, errp);
> + *(uint64_t *)opaque = value;
> +}
> +
> void object_property_add_uint8_ptr(Object *obj, const char *name,
> const uint8_t *v, Error **errp)
> {
> object_property_add(obj, name, "uint8", property_get_uint8_ptr,
> - NULL, NULL, (void *)v, errp);
> + property_set_uint8_ptr, NULL, (void *)v, errp);
> }
>
> void object_property_add_uint16_ptr(Object *obj, const char *name,
> const uint16_t *v, Error **errp)
> {
> object_property_add(obj, name, "uint16", property_get_uint16_ptr,
> - NULL, NULL, (void *)v, errp);
> + property_set_uint16_ptr, NULL, (void *)v, errp);
> }
>
> void object_property_add_uint32_ptr(Object *obj, const char *name,
> const uint32_t *v, Error **errp)
> {
> object_property_add(obj, name, "uint32", property_get_uint32_ptr,
> - NULL, NULL, (void *)v, errp);
> + property_set_uint32_ptr, NULL, (void *)v, errp);
> }
>
> void object_property_add_uint64_ptr(Object *obj, const char *name,
> const uint64_t *v, Error **errp)
> {
> object_property_add(obj, name, "uint64", property_get_uint64_ptr,
> - NULL, NULL, (void *)v, errp);
> + property_set_uint64_ptr, NULL, (void *)v, errp);
> }
>
> static void object_instance_init(Object *obj)
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-03 13:19 ` Andreas Färber
@ 2013-12-06 14:49 ` Peter Maydell
2013-12-15 5:59 ` Peter Crosthwaite
2013-12-15 10:23 ` Michael S. Tsirkin
1 sibling, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2013-12-06 14:49 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Crosthwaite, QEMU Developers, Mark Langsdorf,
Michael S. Tsirkin
On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>> Currently the uintXX property adders make a read only property. This
>> is not useful for devices that want to create board (or container)
>> configurable dynamic device properties. Fix by trivially adding property
>> setters to object_property_add_uintXX.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> changed since v2:
>> msg typo: "trivially"
>
> Not sure if I've asked already, but these functions were added by mst
> (so let's CC him) for accessing read-only constants in ACPI code. Your
> change seems to make them writable - can anything go wrong when the
> setters are used via QMP? I fear we may need two separate sets of
> functions, one read-only, one read-write.
We don't want a generically writable property for CBAR either, though:
we want the standard qdev property semantics of "writable until
realize, readonly thereafter".
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-06 14:49 ` Peter Maydell
@ 2013-12-15 5:59 ` Peter Crosthwaite
2013-12-15 17:56 ` Andreas Färber
0 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-15 5:59 UTC (permalink / raw)
To: Peter Maydell, Antony Pavlov
Cc: Michael S. Tsirkin, Andreas Färber, Mark Langsdorf,
QEMU Developers
Ping!
I'm trying to figure out what way I want to go here.
On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>>> Currently the uintXX property adders make a read only property. This
>>> is not useful for devices that want to create board (or container)
>>> configurable dynamic device properties. Fix by trivially adding property
>>> setters to object_property_add_uintXX.
>>>
>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> ---
>>> changed since v2:
>>> msg typo: "trivially"
>>
>> Not sure if I've asked already, but these functions were added by mst
>> (so let's CC him) for accessing read-only constants in ACPI code. Your
>> change seems to make them writable - can anything go wrong when the
>> setters are used via QMP?
Maybe. But that should be an ACPI problem. It seems that the semantics
of these qom/object.c APIs has been set by the lead example. Maybe
just an extra arg for RD/WR flags would do the trick however?
I fear we may need two separate sets of
>> functions, one read-only, one read-write.
>
> We don't want a generically writable property for CBAR either, though:
> we want the standard qdev property semantics of "writable until
> realize, readonly thereafter".
>
Well, with a bit of replumbing I spose we could make qdev property
adder framework accessible to post_init to have access to
setter/getter fns that implement these semantics.
Any thoughts?
Regards,
Peter
> -- PMM
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-15 5:59 ` Peter Crosthwaite
@ 2013-12-15 17:56 ` Andreas Färber
2013-12-15 18:07 ` Michael S. Tsirkin
2013-12-16 2:37 ` Peter Crosthwaite
0 siblings, 2 replies; 30+ messages in thread
From: Andreas Färber @ 2013-12-15 17:56 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Michael S. Tsirkin, Antony Pavlov, Mark Langsdorf,
QEMU Developers
Am 15.12.2013 06:59, schrieb Peter Crosthwaite:
> Ping!
>
> I'm trying to figure out what way I want to go here.
>
> On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>>>> Currently the uintXX property adders make a read only property. This
>>>> is not useful for devices that want to create board (or container)
>>>> configurable dynamic device properties. Fix by trivially adding property
>>>> setters to object_property_add_uintXX.
>>>>
>>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>> ---
>>>> changed since v2:
>>>> msg typo: "trivially"
>>>
>>> Not sure if I've asked already, but these functions were added by mst
>>> (so let's CC him) for accessing read-only constants in ACPI code. Your
>>> change seems to make them writable - can anything go wrong when the
>>> setters are used via QMP?
>
> Maybe. But that should be an ACPI problem.
No, it means that if you change it you need to touch ACPI code as well -
or to design your change in a way that avoids exactly that, e.g. by
adding a new API reusing the existing getters rather than changing the
semantics of the existing API used by ACPI.
> It seems that the semantics
> of these qom/object.c APIs has been set by the lead example. Maybe
> just an extra arg for RD/WR flags would do the trick however?
If you can get the extra arg passed through as opaque then sure, that
would be an option, passing false for all existing users.
>>> I fear we may need two separate sets of
>>> functions, one read-only, one read-write.
>>
>> We don't want a generically writable property for CBAR either, though:
>> we want the standard qdev property semantics of "writable until
>> realize, readonly thereafter".
>>
>
> Well, with a bit of replumbing I spose we could make qdev property
> adder framework accessible to post_init to have access to
> setter/getter fns that implement these semantics.
Sorry, I don't get how that is related to post_init? All that's needed
is a check of DeviceState::realized in your setter and to error_setg()
out if true.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-15 17:56 ` Andreas Färber
@ 2013-12-15 18:07 ` Michael S. Tsirkin
2013-12-16 2:39 ` Peter Crosthwaite
2013-12-16 2:37 ` Peter Crosthwaite
1 sibling, 1 reply; 30+ messages in thread
From: Michael S. Tsirkin @ 2013-12-15 18:07 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, Peter Crosthwaite, Antony Pavlov, Mark Langsdorf,
QEMU Developers
On Sun, Dec 15, 2013 at 06:56:48PM +0100, Andreas Färber wrote:
> Am 15.12.2013 06:59, schrieb Peter Crosthwaite:
> > Ping!
> >
> > I'm trying to figure out what way I want to go here.
> >
> > On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> >> On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
> >>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
> >>>> Currently the uintXX property adders make a read only property. This
> >>>> is not useful for devices that want to create board (or container)
> >>>> configurable dynamic device properties. Fix by trivially adding property
> >>>> setters to object_property_add_uintXX.
> >>>>
> >>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>>> ---
> >>>> changed since v2:
> >>>> msg typo: "trivially"
> >>>
> >>> Not sure if I've asked already, but these functions were added by mst
> >>> (so let's CC him) for accessing read-only constants in ACPI code. Your
> >>> change seems to make them writable - can anything go wrong when the
> >>> setters are used via QMP?
> >
> > Maybe. But that should be an ACPI problem.
>
> No, it means that if you change it you need to touch ACPI code as well -
> or to design your change in a way that avoids exactly that, e.g. by
> adding a new API reusing the existing getters rather than changing the
> semantics of the existing API used by ACPI.
>
> > It seems that the semantics
> > of these qom/object.c APIs has been set by the lead example. Maybe
> > just an extra arg for RD/WR flags would do the trick however?
>
> If you can get the extra arg passed through as opaque then sure, that
> would be an option, passing false for all existing users.
>
> >>> I fear we may need two separate sets of
> >>> functions, one read-only, one read-write.
> >>
> >> We don't want a generically writable property for CBAR either, though:
> >> we want the standard qdev property semantics of "writable until
> >> realize, readonly thereafter".
> >>
> >
> > Well, with a bit of replumbing I spose we could make qdev property
> > adder framework accessible to post_init to have access to
> > setter/getter fns that implement these semantics.
>
> Sorry, I don't get how that is related to post_init? All that's needed
> is a check of DeviceState::realized in your setter and to error_setg()
> out if true.
>
> Regards,
> Andreas
I think ability to add read only properties is reasonable though.
ACPI wants these since we calculate the value ourselves.
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-15 18:07 ` Michael S. Tsirkin
@ 2013-12-16 2:39 ` Peter Crosthwaite
0 siblings, 0 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-16 2:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: QEMU Developers, Peter Maydell, Andreas Färber,
Mark Langsdorf, Antony Pavlov
On Mon, Dec 16, 2013 at 4:07 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Dec 15, 2013 at 06:56:48PM +0100, Andreas Färber wrote:
>> Am 15.12.2013 06:59, schrieb Peter Crosthwaite:
>> > Ping!
>> >
>> > I'm trying to figure out what way I want to go here.
>> >
>> > On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> >> On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
>> >>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>> >>>> Currently the uintXX property adders make a read only property. This
>> >>>> is not useful for devices that want to create board (or container)
>> >>>> configurable dynamic device properties. Fix by trivially adding property
>> >>>> setters to object_property_add_uintXX.
>> >>>>
>> >>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> >>>> ---
>> >>>> changed since v2:
>> >>>> msg typo: "trivially"
>> >>>
>> >>> Not sure if I've asked already, but these functions were added by mst
>> >>> (so let's CC him) for accessing read-only constants in ACPI code. Your
>> >>> change seems to make them writable - can anything go wrong when the
>> >>> setters are used via QMP?
>> >
>> > Maybe. But that should be an ACPI problem.
>>
>> No, it means that if you change it you need to touch ACPI code as well -
>> or to design your change in a way that avoids exactly that, e.g. by
>> adding a new API reusing the existing getters rather than changing the
>> semantics of the existing API used by ACPI.
>>
>> > It seems that the semantics
>> > of these qom/object.c APIs has been set by the lead example. Maybe
>> > just an extra arg for RD/WR flags would do the trick however?
>>
>> If you can get the extra arg passed through as opaque then sure, that
>> would be an option, passing false for all existing users.
>>
>> >>> I fear we may need two separate sets of
>> >>> functions, one read-only, one read-write.
>> >>
>> >> We don't want a generically writable property for CBAR either, though:
>> >> we want the standard qdev property semantics of "writable until
>> >> realize, readonly thereafter".
>> >>
>> >
>> > Well, with a bit of replumbing I spose we could make qdev property
>> > adder framework accessible to post_init to have access to
>> > setter/getter fns that implement these semantics.
>>
>> Sorry, I don't get how that is related to post_init? All that's needed
>> is a check of DeviceState::realized in your setter and to error_setg()
>> out if true.
>>
>> Regards,
>> Andreas
>
> I think ability to add read only properties is reasonable though.
> ACPI wants these since we calculate the value ourselves.
>
So i'm starting to wonder whether specific semantic rules such as this
as the responsibility of the QOM core or the layers above. Is this
just a more restrictive variant of qdev properties and should be
implemented on that layer as qdev extension?
Regards,
Peter
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-15 17:56 ` Andreas Färber
2013-12-15 18:07 ` Michael S. Tsirkin
@ 2013-12-16 2:37 ` Peter Crosthwaite
1 sibling, 0 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-16 2:37 UTC (permalink / raw)
To: Andreas Färber
Cc: QEMU Developers, Peter Maydell, Antony Pavlov, Mark Langsdorf,
Michael S. Tsirkin
On Mon, Dec 16, 2013 at 3:56 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 15.12.2013 06:59, schrieb Peter Crosthwaite:
>> Ping!
>>
>> I'm trying to figure out what way I want to go here.
>>
>> On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 3 December 2013 13:19, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>>>>> Currently the uintXX property adders make a read only property. This
>>>>> is not useful for devices that want to create board (or container)
>>>>> configurable dynamic device properties. Fix by trivially adding property
>>>>> setters to object_property_add_uintXX.
>>>>>
>>>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>>>> ---
>>>>> changed since v2:
>>>>> msg typo: "trivially"
>>>>
>>>> Not sure if I've asked already, but these functions were added by mst
>>>> (so let's CC him) for accessing read-only constants in ACPI code. Your
>>>> change seems to make them writable - can anything go wrong when the
>>>> setters are used via QMP?
>>
>> Maybe. But that should be an ACPI problem.
>
> No, it means that if you change it you need to touch ACPI code as well -
> or to design your change in a way that avoids exactly that, e.g. by
> adding a new API reusing the existing getters rather than changing the
> semantics of the existing API used by ACPI.
>
>> It seems that the semantics
>> of these qom/object.c APIs has been set by the lead example. Maybe
>> just an extra arg for RD/WR flags would do the trick however?
>
> If you can get the extra arg passed through as opaque then sure, that
> would be an option, passing false for all existing users.
>
>>>> I fear we may need two separate sets of
>>>> functions, one read-only, one read-write.
>>>
>>> We don't want a generically writable property for CBAR either, though:
>>> we want the standard qdev property semantics of "writable until
>>> realize, readonly thereafter".
>>>
>>
>> Well, with a bit of replumbing I spose we could make qdev property
>> adder framework accessible to post_init to have access to
>> setter/getter fns that implement these semantics.
>
> Sorry, I don't get how that is related to post_init? All that's needed
> is a check of DeviceState::realized in your setter and to error_setg()
> out if true.
>
So the whole point of this patch and the next is to avoid having to
write setters and getters for every property. Its going to be super
verbose, is every vendor configurable CPU arm property needs the qdev
setter and getter boilerplate. And the vast majority of these
properties are going to be family or device conditional in some way so
the dynamic property approach is looking like the rule rather than the
exception.
So I guess this problem is why qdev was invented. Since last night,
i've figured out that whole series can actually be done quite cleanly
using qdevs setters and getters. Please see v5 for illustration. Ive
dropped these two patches, so this series is now pure ARM (and
hopefully makes life easier for PMM).
Regards,
Peter
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-03 13:19 ` Andreas Färber
2013-12-06 14:49 ` Peter Maydell
@ 2013-12-15 10:23 ` Michael S. Tsirkin
1 sibling, 0 replies; 30+ messages in thread
From: Michael S. Tsirkin @ 2013-12-15 10:23 UTC (permalink / raw)
To: Andreas Färber
Cc: peter.maydell, Peter Crosthwaite, qemu-devel, mark.langsdorf
On Tue, Dec 03, 2013 at 02:19:34PM +0100, Andreas Färber wrote:
> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
> > Currently the uintXX property adders make a read only property. This
> > is not useful for devices that want to create board (or container)
> > configurable dynamic device properties. Fix by trivially adding property
> > setters to object_property_add_uintXX.
> >
> > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > ---
> > changed since v2:
> > msg typo: "trivially"
>
> Not sure if I've asked already, but these functions were added by mst
> (so let's CC him) for accessing read-only constants in ACPI code.
Yes - for ACPI we used these for properties which users shouldn't
be able to change.
> Your
> change seems to make them writable - can anything go wrong when the
> setters are used via QMP?
Guest won't boot.
> I fear we may need two separate sets of
> functions, one read-only, one read-write.
>
> Andreas
I think so, yes.
> >
> > qom/object.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/qom/object.c b/qom/object.c
> > index fc19cf6..07b454b 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -1353,6 +1353,15 @@ static void property_get_uint8_ptr(Object *obj, Visitor *v,
> > visit_type_uint8(v, &value, name, errp);
> > }
> >
> > +static void property_set_uint8_ptr(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + uint8_t value;
> > + visit_type_uint8(v, &value, name, errp);
> > + *(uint8_t *)opaque = value;
> > +}
> > +
> > static void property_get_uint16_ptr(Object *obj, Visitor *v,
> > void *opaque, const char *name,
> > Error **errp)
> > @@ -1361,6 +1370,15 @@ static void property_get_uint16_ptr(Object *obj, Visitor *v,
> > visit_type_uint16(v, &value, name, errp);
> > }
> >
> > +static void property_set_uint16_ptr(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + uint16_t value;
> > + visit_type_uint16(v, &value, name, errp);
> > + *(uint16_t *)opaque = value;
> > +}
> > +
> > static void property_get_uint32_ptr(Object *obj, Visitor *v,
> > void *opaque, const char *name,
> > Error **errp)
> > @@ -1369,6 +1387,15 @@ static void property_get_uint32_ptr(Object *obj, Visitor *v,
> > visit_type_uint32(v, &value, name, errp);
> > }
> >
> > +static void property_set_uint32_ptr(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + uint32_t value;
> > + visit_type_uint32(v, &value, name, errp);
> > + *(uint32_t *)opaque = value;
> > +}
> > +
> > static void property_get_uint64_ptr(Object *obj, Visitor *v,
> > void *opaque, const char *name,
> > Error **errp)
> > @@ -1377,32 +1404,41 @@ static void property_get_uint64_ptr(Object *obj, Visitor *v,
> > visit_type_uint64(v, &value, name, errp);
> > }
> >
> > +static void property_set_uint64_ptr(Object *obj, Visitor *v,
> > + void *opaque, const char *name,
> > + Error **errp)
> > +{
> > + uint64_t value;
> > + visit_type_uint64(v, &value, name, errp);
> > + *(uint64_t *)opaque = value;
> > +}
> > +
> > void object_property_add_uint8_ptr(Object *obj, const char *name,
> > const uint8_t *v, Error **errp)
> > {
> > object_property_add(obj, name, "uint8", property_get_uint8_ptr,
> > - NULL, NULL, (void *)v, errp);
> > + property_set_uint8_ptr, NULL, (void *)v, errp);
> > }
> >
> > void object_property_add_uint16_ptr(Object *obj, const char *name,
> > const uint16_t *v, Error **errp)
> > {
> > object_property_add(obj, name, "uint16", property_get_uint16_ptr,
> > - NULL, NULL, (void *)v, errp);
> > + property_set_uint16_ptr, NULL, (void *)v, errp);
> > }
> >
> > void object_property_add_uint32_ptr(Object *obj, const char *name,
> > const uint32_t *v, Error **errp)
> > {
> > object_property_add(obj, name, "uint32", property_get_uint32_ptr,
> > - NULL, NULL, (void *)v, errp);
> > + property_set_uint32_ptr, NULL, (void *)v, errp);
> > }
> >
> > void object_property_add_uint64_ptr(Object *obj, const char *name,
> > const uint64_t *v, Error **errp)
> > {
> > object_property_add(obj, name, "uint64", property_get_uint64_ptr,
> > - NULL, NULL, (void *)v, errp);
> > + property_set_uint64_ptr, NULL, (void *)v, errp);
> > }
> >
> > static void object_instance_init(Object *obj)
> >
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable
2013-12-03 6:59 ` [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable Peter Crosthwaite
2013-12-03 13:19 ` Andreas Färber
@ 2013-12-03 13:26 ` Andreas Färber
1 sibling, 0 replies; 30+ messages in thread
From: Andreas Färber @ 2013-12-03 13:26 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, mark.langsdorf
Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
> Currently the uintXX property adders make a read only property. This
> is not useful for devices that want to create board (or container)
> configurable dynamic device properties. Fix by trivially adding property
> setters to object_property_add_uintXX.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> changed since v2:
> msg typo: "trivially"
>
> qom/object.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 40 insertions(+), 4 deletions(-)
BTW "qom: " is totally sufficient for this. :)
/-F
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH arm-devs v3 2/9] target-arm/helper.c: Allow cp15.c15 dummy override
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
2013-12-03 6:59 ` [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable Peter Crosthwaite
@ 2013-12-03 7:00 ` Peter Crosthwaite
2013-12-06 14:36 ` Peter Maydell
2013-12-03 7:00 ` [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR Peter Crosthwaite
` (6 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:00 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
The cp15.c15 space is implementation defined. Currently there is a
dummy placeholder register RAZing it. Allow overriding of this RAZ
so implementations of specific registers can take precedence.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
target-arm/helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3445813..587ff49 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1338,7 +1338,8 @@ static const ARMCPRegInfo dummy_c15_cp_reginfo[] = {
*/
{ .name = "C15_IMPDEF", .cp = 15, .crn = 15,
.crm = CP_ANY, .opc1 = CP_ANY, .opc2 = CP_ANY,
- .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_NO_MIGRATE,
+ .access = PL1_RW,
+ .type = ARM_CP_CONST | ARM_CP_NO_MIGRATE | ARM_CP_OVERRIDE,
.resetvalue = 0 },
REGINFO_SENTINEL
};
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
2013-12-03 6:59 ` [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable Peter Crosthwaite
2013-12-03 7:00 ` [Qemu-devel] [PATCH arm-devs v3 2/9] target-arm/helper.c: Allow cp15.c15 dummy override Peter Crosthwaite
@ 2013-12-03 7:00 ` Peter Crosthwaite
2013-12-06 14:12 ` Peter Maydell
2013-12-03 7:01 ` [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property Peter Crosthwaite
` (5 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:00 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
Some processors (notably A9 within Highbank) define and use the
CP15 configuration base address (CBAR). This is vendor specific
so its best implemented as a CPU property (otherwise we would need
vendor specific child classes for every ARM implementation).
This patch prepares support for converting CBAR reset value to
a CPU property by moving the CP registration out of the CPU
init fn, as registration will need to happen at realize time
to pick up any property updates. The easiest way to do this
is via definition of a new ARM_FEATURE to flag the existence
of the register.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v2:
msg typo: existence
Enable CBAR for a15 as well
target-arm/cpu.c | 12 +++---------
target-arm/cpu.h | 1 +
target-arm/helper.c | 9 +++++++++
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d40f2a7..90413ee 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -590,6 +590,7 @@ static void cortex_a9_initfn(Object *obj)
* and valid configurations; we don't model A9UP).
*/
set_feature(&cpu->env, ARM_FEATURE_V7MP);
+ set_feature(&cpu->env, ARM_FEATURE_CBAR);
cpu->midr = 0x410fc090;
cpu->reset_fpsid = 0x41033090;
cpu->mvfr0 = 0x11110222;
@@ -612,15 +613,7 @@ static void cortex_a9_initfn(Object *obj)
cpu->clidr = (1 << 27) | (1 << 24) | 3;
cpu->ccsidr[0] = 0xe00fe015; /* 16k L1 dcache. */
cpu->ccsidr[1] = 0x200fe015; /* 16k L1 icache. */
- {
- ARMCPRegInfo cbar = {
- .name = "CBAR", .cp = 15, .crn = 15, .crm = 0, .opc1 = 4,
- .opc2 = 0, .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
- .fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
- };
- define_one_arm_cp_reg(cpu, &cbar);
- define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
- }
+ define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
}
#ifndef CONFIG_USER_ONLY
@@ -657,6 +650,7 @@ static void cortex_a15_initfn(Object *obj)
set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+ set_feature(&cpu->env, ARM_FEATURE_CBAR);
set_feature(&cpu->env, ARM_FEATURE_LPAE);
cpu->midr = 0x412fc0f1;
cpu->reset_fpsid = 0x410430f0;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 9f110f1..859750a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -461,6 +461,7 @@ enum arm_features {
ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
+ ARM_FEATURE_CBAR, /* has cp15 CBAR */
ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
ARM_FEATURE_V8,
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 587ff49..59b59c7 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1745,6 +1745,15 @@ void register_cp_regs_for_features(ARMCPU *cpu)
define_one_arm_cp_reg(cpu, &auxcr);
}
+ if (arm_feature(env, ARM_FEATURE_CBAR)) {
+ ARMCPRegInfo cbar = {
+ .name = "CBAR", .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
+ .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
+ .fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
+ };
+ define_one_arm_cp_reg(cpu, &cbar);
+ }
+
/* Generic registers whose values depend on the implementation */
{
ARMCPRegInfo sctlr = {
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR
2013-12-03 7:00 ` [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR Peter Crosthwaite
@ 2013-12-06 14:12 ` Peter Maydell
2013-12-11 0:57 ` Peter Crosthwaite
0 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2013-12-06 14:12 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On 3 December 2013 07:00, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Some processors (notably A9 within Highbank) define and use the
> CP15 configuration base address (CBAR). This is vendor specific
> so its best implemented as a CPU property (otherwise we would need
> vendor specific child classes for every ARM implementation).
>
> This patch prepares support for converting CBAR reset value to
> a CPU property by moving the CP registration out of the CPU
> init fn, as registration will need to happen at realize time
> to pick up any property updates. The easiest way to do this
> is via definition of a new ARM_FEATURE to flag the existence
> of the register.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> changed since v2:
> msg typo: existence
> Enable CBAR for a15 as well
>
> target-arm/cpu.c | 12 +++---------
> target-arm/cpu.h | 1 +
> target-arm/helper.c | 9 +++++++++
> 3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d40f2a7..90413ee 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -590,6 +590,7 @@ static void cortex_a9_initfn(Object *obj)
> * and valid configurations; we don't model A9UP).
> */
> set_feature(&cpu->env, ARM_FEATURE_V7MP);
> + set_feature(&cpu->env, ARM_FEATURE_CBAR);
> cpu->midr = 0x410fc090;
> cpu->reset_fpsid = 0x41033090;
> cpu->mvfr0 = 0x11110222;
> @@ -612,15 +613,7 @@ static void cortex_a9_initfn(Object *obj)
> cpu->clidr = (1 << 27) | (1 << 24) | 3;
> cpu->ccsidr[0] = 0xe00fe015; /* 16k L1 dcache. */
> cpu->ccsidr[1] = 0x200fe015; /* 16k L1 icache. */
> - {
> - ARMCPRegInfo cbar = {
> - .name = "CBAR", .cp = 15, .crn = 15, .crm = 0, .opc1 = 4,
> - .opc2 = 0, .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
> - .fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
> - };
> - define_one_arm_cp_reg(cpu, &cbar);
> - define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
> - }
> + define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
> }
>
> #ifndef CONFIG_USER_ONLY
> @@ -657,6 +650,7 @@ static void cortex_a15_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
> set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> + set_feature(&cpu->env, ARM_FEATURE_CBAR);
> set_feature(&cpu->env, ARM_FEATURE_LPAE);
> cpu->midr = 0x412fc0f1;
> cpu->reset_fpsid = 0x410430f0;
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9f110f1..859750a 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -461,6 +461,7 @@ enum arm_features {
> ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
> ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
> ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
> + ARM_FEATURE_CBAR, /* has cp15 CBAR */
> ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
> ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
> ARM_FEATURE_V8,
env->features gets migrated, so reordering feature bits breaks
cross-version migration; better just to add at the end of the list.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR
2013-12-06 14:12 ` Peter Maydell
@ 2013-12-11 0:57 ` Peter Crosthwaite
0 siblings, 0 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-11 0:57 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On Sat, Dec 7, 2013 at 12:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 December 2013 07:00, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Some processors (notably A9 within Highbank) define and use the
>> CP15 configuration base address (CBAR). This is vendor specific
>> so its best implemented as a CPU property (otherwise we would need
>> vendor specific child classes for every ARM implementation).
>>
>> This patch prepares support for converting CBAR reset value to
>> a CPU property by moving the CP registration out of the CPU
>> init fn, as registration will need to happen at realize time
>> to pick up any property updates. The easiest way to do this
>> is via definition of a new ARM_FEATURE to flag the existence
>> of the register.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> changed since v2:
>> msg typo: existence
>> Enable CBAR for a15 as well
>>
>> target-arm/cpu.c | 12 +++---------
>> target-arm/cpu.h | 1 +
>> target-arm/helper.c | 9 +++++++++
>> 3 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index d40f2a7..90413ee 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -590,6 +590,7 @@ static void cortex_a9_initfn(Object *obj)
>> * and valid configurations; we don't model A9UP).
>> */
>> set_feature(&cpu->env, ARM_FEATURE_V7MP);
>> + set_feature(&cpu->env, ARM_FEATURE_CBAR);
>> cpu->midr = 0x410fc090;
>> cpu->reset_fpsid = 0x41033090;
>> cpu->mvfr0 = 0x11110222;
>> @@ -612,15 +613,7 @@ static void cortex_a9_initfn(Object *obj)
>> cpu->clidr = (1 << 27) | (1 << 24) | 3;
>> cpu->ccsidr[0] = 0xe00fe015; /* 16k L1 dcache. */
>> cpu->ccsidr[1] = 0x200fe015; /* 16k L1 icache. */
>> - {
>> - ARMCPRegInfo cbar = {
>> - .name = "CBAR", .cp = 15, .crn = 15, .crm = 0, .opc1 = 4,
>> - .opc2 = 0, .access = PL1_R|PL3_W, .resetvalue = cpu->reset_cbar,
>> - .fieldoffset = offsetof(CPUARMState, cp15.c15_config_base_address)
>> - };
>> - define_one_arm_cp_reg(cpu, &cbar);
>> - define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>> - }
>> + define_arm_cp_regs(cpu, cortexa9_cp_reginfo);
>> }
>>
>> #ifndef CONFIG_USER_ONLY
>> @@ -657,6 +650,7 @@ static void cortex_a15_initfn(Object *obj)
>> set_feature(&cpu->env, ARM_FEATURE_ARM_DIV);
>> set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
>> set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
>> + set_feature(&cpu->env, ARM_FEATURE_CBAR);
>> set_feature(&cpu->env, ARM_FEATURE_LPAE);
>> cpu->midr = 0x412fc0f1;
>> cpu->reset_fpsid = 0x410430f0;
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 9f110f1..859750a 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -461,6 +461,7 @@ enum arm_features {
>> ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
>> ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
>> ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
>> + ARM_FEATURE_CBAR, /* has cp15 CBAR */
>> ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
>> ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
>> ARM_FEATURE_V8,
>
> env->features gets migrated, so reordering feature bits breaks
> cross-version migration; better just to add at the end of the list.
>
Done:
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 318642e..947a1e7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -463,11 +463,11 @@ enum arm_features {
ARM_FEATURE_CACHE_DIRTY_REG, /* 1136/1176 cache dirty status register */
ARM_FEATURE_CACHE_BLOCK_OPS, /* v6 optional cache block operations */
ARM_FEATURE_MPIDR, /* has cp15 MPIDR */
- ARM_FEATURE_CBAR, /* has cp15 CBAR */
ARM_FEATURE_PXN, /* has Privileged Execute Never bit */
ARM_FEATURE_LPAE, /* has Large Physical Address Extension */
ARM_FEATURE_V8,
ARM_FEATURE_AARCH64, /* supports 64 bit mode */
+ ARM_FEATURE_CBAR, /* has cp15 CBAR */
};
Regards,
Peter
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
>
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (2 preceding siblings ...)
2013-12-03 7:00 ` [Qemu-devel] [PATCH arm-devs v3 3/9] target-arm: Define and use ARM_FEATURE_CBAR Peter Crosthwaite
@ 2013-12-03 7:01 ` Peter Crosthwaite
2013-12-06 14:41 ` Peter Maydell
2013-12-03 7:01 ` [Qemu-devel] [PATCH arm-devs v3 5/9] arm/highbank: Use object_new() rather than cpu_arm_init() Peter Crosthwaite
` (4 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:01 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
The reset Value of the CP15 CBAR is a vendor (machine) configurable
property. If ARM_FEATURE_CBAR is set, add it as a property at
post_init time.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Change since v1:
Re-implement as dynamic property
target-arm/cpu.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 90413ee..cbe108b 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -20,6 +20,7 @@
#include "cpu.h"
#include "qemu-common.h"
+#include "qapi/qmp/qerror.h"
#if !defined(CONFIG_USER_ONLY)
#include "hw/loader.h"
#endif
@@ -223,6 +224,18 @@ static void arm_cpu_initfn(Object *obj)
}
}
+static void arm_cpu_post_init(Object *obj)
+{
+ ARMCPU *cpu = ARM_CPU(obj);
+ Error *err = NULL;
+
+ if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
+ object_property_add_uint32_ptr(obj, "reset-cbar", &cpu->reset_cbar,
+ &err);
+ assert_no_error(err);
+ }
+}
+
static void arm_cpu_finalizefn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
@@ -934,6 +947,7 @@ static const TypeInfo arm_cpu_type_info = {
.parent = TYPE_CPU,
.instance_size = sizeof(ARMCPU),
.instance_init = arm_cpu_initfn,
+ .instance_post_init = arm_cpu_post_init,
.instance_finalize = arm_cpu_finalizefn,
.abstract = true,
.class_size = sizeof(ARMCPUClass),
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property
2013-12-03 7:01 ` [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property Peter Crosthwaite
@ 2013-12-06 14:41 ` Peter Maydell
2013-12-11 1:03 ` Peter Crosthwaite
2013-12-16 1:32 ` Peter Crosthwaite
0 siblings, 2 replies; 30+ messages in thread
From: Peter Maydell @ 2013-12-06 14:41 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On 3 December 2013 07:01, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> The reset Value of the CP15 CBAR is a vendor (machine) configurable
no need to capitalize "value" here.
> property. If ARM_FEATURE_CBAR is set, add it as a property at
> post_init time.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
PS: this patch has conflicts (largely textual, easy enough to resolve)
with the patches in my v7-mach-virt series. If you do a respin of this
before I get round to the next target-arm pullreq (likely to be early next
week) it might be simplest to base that respin on top of the target-arm.next
branch (in git://git.linaro.org/people/pmaydell/qemu-arm.git ; nb the
server is down for maintanance this Sunday). That would save me
the effort of fixing up the conflicts ;-)
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property
2013-12-06 14:41 ` Peter Maydell
@ 2013-12-11 1:03 ` Peter Crosthwaite
2013-12-16 1:32 ` Peter Crosthwaite
1 sibling, 0 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-11 1:03 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On Sat, Dec 7, 2013 at 12:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 December 2013 07:01, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> The reset Value of the CP15 CBAR is a vendor (machine) configurable
>
> no need to capitalize "value" here.
>
Fixed,
Regards,
Peter
>> property. If ARM_FEATURE_CBAR is set, add it as a property at
>> post_init time.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> PS: this patch has conflicts (largely textual, easy enough to resolve)
> with the patches in my v7-mach-virt series. If you do a respin of this
> before I get round to the next target-arm pullreq (likely to be early next
> week) it might be simplest to base that respin on top of the target-arm.next
> branch (in git://git.linaro.org/people/pmaydell/qemu-arm.git ; nb the
> server is down for maintanance this Sunday). That would save me
> the effort of fixing up the conflicts ;-)
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property
2013-12-06 14:41 ` Peter Maydell
2013-12-11 1:03 ` Peter Crosthwaite
@ 2013-12-16 1:32 ` Peter Crosthwaite
1 sibling, 0 replies; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-16 1:32 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On Sat, Dec 7, 2013 at 12:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 December 2013 07:01, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> The reset Value of the CP15 CBAR is a vendor (machine) configurable
>
> no need to capitalize "value" here.
>
>> property. If ARM_FEATURE_CBAR is set, add it as a property at
>> post_init time.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
I am trying a different implementation that removes the need for the
QOM core patches and implements the desired Qdev write-until-realize
semantic so I have removed your RB in V5 due to fundamental
differences.
Regards,
Peter
> PS: this patch has conflicts (largely textual, easy enough to resolve)
> with the patches in my v7-mach-virt series. If you do a respin of this
> before I get round to the next target-arm pullreq (likely to be early next
> week) it might be simplest to base that respin on top of the target-arm.next
> branch (in git://git.linaro.org/people/pmaydell/qemu-arm.git ; nb the
> server is down for maintanance this Sunday). That would save me
> the effort of fixing up the conflicts ;-)
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH arm-devs v3 5/9] arm/highbank: Use object_new() rather than cpu_arm_init()
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (3 preceding siblings ...)
2013-12-03 7:01 ` [Qemu-devel] [PATCH arm-devs v3 4/9] target-arm/cpu: Convert reset CBAR to a property Peter Crosthwaite
@ 2013-12-03 7:01 ` Peter Crosthwaite
2013-12-06 14:42 ` Peter Maydell
2013-12-03 7:02 ` [Qemu-devel] [PATCH arm-devs v3 6/9] arm/highbank: Fix CBAR initialisation Peter Crosthwaite
` (3 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:01 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
To allow the machine model to set device properties before CPU
realization.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
use error_report rather than fprintf(stderr
hw/arm/highbank.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index fe98ef1..1d19d8f 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -26,6 +26,7 @@
#include "hw/boards.h"
#include "sysemu/blockdev.h"
#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
#define SMP_BOOT_ADDR 0x100
#define SMP_BOOT_REG 0x40
@@ -229,10 +230,15 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine)
}
for (n = 0; n < smp_cpus; n++) {
+ ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
ARMCPU *cpu;
- cpu = cpu_arm_init(cpu_model);
- if (cpu == NULL) {
- fprintf(stderr, "Unable to find CPU definition\n");
+ Error *err = NULL;
+
+ cpu = ARM_CPU(object_new(object_class_get_name(oc)));
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err) {
+ error_report("%s", error_get_pretty(err));
exit(1);
}
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH arm-devs v3 6/9] arm/highbank: Fix CBAR initialisation
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (4 preceding siblings ...)
2013-12-03 7:01 ` [Qemu-devel] [PATCH arm-devs v3 5/9] arm/highbank: Use object_new() rather than cpu_arm_init() Peter Crosthwaite
@ 2013-12-03 7:02 ` Peter Crosthwaite
2013-12-06 14:43 ` Peter Maydell
2013-12-03 7:02 ` [Qemu-devel] [PATCH arm-devs v3 7/9] arm/xilinx_zynq: Use object_new() rather than cpu_arm_init() Peter Crosthwaite
` (2 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:02 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
Fix the CBAR initialisation by using the newly defined static property.
CBAR is now set before realization, so the intended value is now
actually used.
So I have kind of tested this. I booted an ARM kernel on Highbank with
the stock Highbank DTB. It doesn't boot (and I will be doing something
wrong), but before this patch I got this:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at /workspaces/pcrost/public/linux2.git/arch/arm/mm/ioremap.c:301 __arm_ioremap_pfn_caller+0x180/0x198()
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.13.0-rc1-next-20131126-dirty #2
[<c0015164>] (unwind_backtrace) from [<c00118c0>] (show_stack+0x10/0x14)
[<c00118c0>] (show_stack) from [<c02bd5fc>] (dump_stack+0x78/0x90)
[<c02bd5fc>] (dump_stack) from [<c001f110>] (warn_slowpath_common+0x68/0x84)
[<c001f110>] (warn_slowpath_common) from [<c001f1f4>] (warn_slowpath_null+0x1c/0x24)
[<c001f1f4>] (warn_slowpath_null) from [<c0017c6c>] (__arm_ioremap_pfn_caller+0x180/0x198)
[<c0017c6c>] (__arm_ioremap_pfn_caller) from [<c0017cd8>] (__arm_ioremap_caller+0x54/0x5c)
[<c0017cd8>] (__arm_ioremap_caller) from [<c0017d10>] (__arm_ioremap+0x18/0x1c)
[<c0017d10>] (__arm_ioremap) from [<c03913c0>] (highbank_init_irq+0x34/0x8c)
[<c03913c0>] (highbank_init_irq) from [<c038c228>] (init_IRQ+0x28/0x2c)
[<c038c228>] (init_IRQ) from [<c03899ec>] (start_kernel+0x234/0x398)
[<c03899ec>] (start_kernel) from [<00008074>] (0x8074)
---[ end trace 3406ff24bd97382f ]---
Which disappears with this patch.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v2:
Fix msg typos
changed since v1:
use error report rather than fprintf(stderr
hw/arm/highbank.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 1d19d8f..cb32325 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -236,14 +236,16 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine)
cpu = ARM_CPU(object_new(object_class_get_name(oc)));
+ object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar", &err);
+ if (err) {
+ error_report("%s", error_get_pretty(err));
+ exit(1);
+ }
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
if (err) {
error_report("%s", error_get_pretty(err));
exit(1);
}
-
- /* This will become a QOM property eventually */
- cpu->reset_cbar = GIC_BASE_ADDR;
cpu_irq[n] = qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ);
}
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH arm-devs v3 6/9] arm/highbank: Fix CBAR initialisation
2013-12-03 7:02 ` [Qemu-devel] [PATCH arm-devs v3 6/9] arm/highbank: Fix CBAR initialisation Peter Crosthwaite
@ 2013-12-06 14:43 ` Peter Maydell
0 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2013-12-06 14:43 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: QEMU Developers, Mark Langsdorf, Andreas Färber
On 3 December 2013 07:02, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> Fix the CBAR initialisation by using the newly defined static property.
> CBAR is now set before realization, so the intended value is now
> actually used.
>
> So I have kind of tested this. I booted an ARM kernel on Highbank with
> the stock Highbank DTB. It doesn't boot (and I will be doing something
> wrong), but before this patch I got this:
Highbank insists on you specifying 4GB RAM to QEMU, IIRC; maybe it's that?
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at /workspaces/pcrost/public/linux2.git/arch/arm/mm/ioremap.c:301 __arm_ioremap_pfn_caller+0x180/0x198()
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 3.13.0-rc1-next-20131126-dirty #2
> [<c0015164>] (unwind_backtrace) from [<c00118c0>] (show_stack+0x10/0x14)
> [<c00118c0>] (show_stack) from [<c02bd5fc>] (dump_stack+0x78/0x90)
> [<c02bd5fc>] (dump_stack) from [<c001f110>] (warn_slowpath_common+0x68/0x84)
> [<c001f110>] (warn_slowpath_common) from [<c001f1f4>] (warn_slowpath_null+0x1c/0x24)
> [<c001f1f4>] (warn_slowpath_null) from [<c0017c6c>] (__arm_ioremap_pfn_caller+0x180/0x198)
> [<c0017c6c>] (__arm_ioremap_pfn_caller) from [<c0017cd8>] (__arm_ioremap_caller+0x54/0x5c)
> [<c0017cd8>] (__arm_ioremap_caller) from [<c0017d10>] (__arm_ioremap+0x18/0x1c)
> [<c0017d10>] (__arm_ioremap) from [<c03913c0>] (highbank_init_irq+0x34/0x8c)
> [<c03913c0>] (highbank_init_irq) from [<c038c228>] (init_IRQ+0x28/0x2c)
> [<c038c228>] (init_IRQ) from [<c03899ec>] (start_kernel+0x234/0x398)
> [<c03899ec>] (start_kernel) from [<00008074>] (0x8074)
> ---[ end trace 3406ff24bd97382f ]---
>
> Which disappears with this patch.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH arm-devs v3 7/9] arm/xilinx_zynq: Use object_new() rather than cpu_arm_init()
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (5 preceding siblings ...)
2013-12-03 7:02 ` [Qemu-devel] [PATCH arm-devs v3 6/9] arm/highbank: Fix CBAR initialisation Peter Crosthwaite
@ 2013-12-03 7:02 ` Peter Crosthwaite
2013-12-06 14:43 ` Peter Maydell
2013-12-03 7:03 ` [Qemu-devel] [PATCH arm-devs v3 8/9] arm/xilinx_zynq: Implement CBAR initialisation Peter Crosthwaite
2013-12-03 7:04 ` [Qemu-devel] [PATCH arm-devs v3 9/9] arm/highbank.c: Fix MPCore periphbase name Peter Crosthwaite
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:02 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
To allow the machine model to set device properties before CPU
realization.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
use error report rather than fprintf(stderr
hw/arm/xilinx_zynq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 46924a0..1c954a3 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -25,6 +25,7 @@
#include "sysemu/blockdev.h"
#include "hw/loader.h"
#include "hw/ssi.h"
+#include "qemu/error-report.h"
#define NUM_SPI_FLASHES 4
#define NUM_QSPI_FLASHES 2
@@ -102,6 +103,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
const char *kernel_filename = args->kernel_filename;
const char *kernel_cmdline = args->kernel_cmdline;
const char *initrd_filename = args->initrd_filename;
+ ObjectClass *cpu_oc;
ARMCPU *cpu;
MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
@@ -110,15 +112,19 @@ static void zynq_init(QEMUMachineInitArgs *args)
SysBusDevice *busdev;
qemu_irq pic[64];
NICInfo *nd;
+ Error *err = NULL;
int n;
if (!cpu_model) {
cpu_model = "cortex-a9";
}
+ cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
- cpu = cpu_arm_init(cpu_model);
- if (!cpu) {
- fprintf(stderr, "Unable to find CPU definition\n");
+ cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
+
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+ if (err) {
+ error_report("%s", error_get_pretty(err));
exit(1);
}
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH arm-devs v3 8/9] arm/xilinx_zynq: Implement CBAR initialisation
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (6 preceding siblings ...)
2013-12-03 7:02 ` [Qemu-devel] [PATCH arm-devs v3 7/9] arm/xilinx_zynq: Use object_new() rather than cpu_arm_init() Peter Crosthwaite
@ 2013-12-03 7:03 ` Peter Crosthwaite
2013-12-06 14:44 ` Peter Maydell
2013-12-03 7:04 ` [Qemu-devel] [PATCH arm-devs v3 9/9] arm/highbank.c: Fix MPCore periphbase name Peter Crosthwaite
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:03 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
Fix the CBAR initialisation by using the newly defined static property.
Zynq will now correctly init the CBAR to the SCU base address.
Needed to boot Linux on the xilinx_zynq machine model.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
use error report rather than fprintf(stderr
rename SCU_BASE_ADDR to MPCORE_PERIPHBASE
hw/arm/xilinx_zynq.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1c954a3..17251c7 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -36,6 +36,8 @@
#define IRQ_OFFSET 32 /* pic interrupts start from index 32 */
+#define MPCORE_PERIPHBASE 0xF8F00000
+
static const int dma_irqs[8] = {
46, 47, 48, 49, 72, 73, 74, 75
};
@@ -122,6 +124,11 @@ static void zynq_init(QEMUMachineInitArgs *args)
cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
+ object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
+ if (err) {
+ error_report("%s", error_get_pretty(err));
+ exit(1);
+ }
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
if (err) {
error_report("%s", error_get_pretty(err));
@@ -160,7 +167,7 @@ static void zynq_init(QEMUMachineInitArgs *args)
qdev_prop_set_uint32(dev, "num-cpu", 1);
qdev_init_nofail(dev);
busdev = SYS_BUS_DEVICE(dev);
- sysbus_mmio_map(busdev, 0, 0xF8F00000);
+ sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
sysbus_connect_irq(busdev, 0,
qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH arm-devs v3 9/9] arm/highbank.c: Fix MPCore periphbase name
2013-12-03 6:58 [Qemu-devel] [PATCH arm-devs v3 0/9] Fix Support for ARM A9 CBAR Peter Crosthwaite
` (7 preceding siblings ...)
2013-12-03 7:03 ` [Qemu-devel] [PATCH arm-devs v3 8/9] arm/xilinx_zynq: Implement CBAR initialisation Peter Crosthwaite
@ 2013-12-03 7:04 ` Peter Crosthwaite
2013-12-06 14:47 ` Peter Maydell
8 siblings, 1 reply; 30+ messages in thread
From: Peter Crosthwaite @ 2013-12-03 7:04 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: peter.crosthwaite, afaerber, mark.langsdorf
GIC_BASE_ADDR is not the base address of the GIC. Its clear from the
code that this is the base address of the MPCore. Rename to
MPCORE_PERIPHBASE accordingly.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v2: Fixed broken comment (PMM review)
hw/arm/highbank.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index cb32325..c75b425 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -28,11 +28,11 @@
#include "exec/address-spaces.h"
#include "qemu/error-report.h"
-#define SMP_BOOT_ADDR 0x100
-#define SMP_BOOT_REG 0x40
-#define GIC_BASE_ADDR 0xfff10000
+#define SMP_BOOT_ADDR 0x100
+#define SMP_BOOT_REG 0x40
+#define MPCORE_PERIPHBASE 0xfff10000
-#define NIRQ_GIC 160
+#define NIRQ_GIC 160
/* Board init. */
@@ -55,7 +55,7 @@ static void hb_write_secondary(ARMCPU *cpu, const struct arm_boot_info *info)
0xe1110001, /* tst r1, r1 */
0x0afffffb, /* beq <wfi> */
0xe12fff11, /* bx r1 */
- GIC_BASE_ADDR /* privbase: gic address. */
+ MPCORE_PERIPHBASE /* privbase: MPCore peripheral base address. */
};
for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
smpboot[n] = tswap32(smpboot[n]);
@@ -236,7 +236,8 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine)
cpu = ARM_CPU(object_new(object_class_get_name(oc)));
- object_property_set_int(OBJECT(cpu), GIC_BASE_ADDR, "reset-cbar", &err);
+ object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar",
+ &err);
if (err) {
error_report("%s", error_get_pretty(err));
exit(1);
@@ -287,7 +288,7 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine)
qdev_prop_set_uint32(dev, "num-irq", NIRQ_GIC);
qdev_init_nofail(dev);
busdev = SYS_BUS_DEVICE(dev);
- sysbus_mmio_map(busdev, 0, GIC_BASE_ADDR);
+ sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
for (n = 0; n < smp_cpus; n++) {
sysbus_connect_irq(busdev, n, cpu_irq[n]);
}
--
1.8.4.4
^ permalink raw reply related [flat|nested] 30+ messages in thread