Re: [Qemu-devel] [Qemu-arm] [PATCH 06/16] hw/misc/iotkit: Wire up the system control element

[Philippe Mathieu-Daudé]

On 08/18/2018 06:55 AM, Peter Maydell wrote:
> On 18 August 2018 at 01:00, Philippe Mathieu-Daudé <address@hidden> wrote:
>> Hi Peter,
>>
>> On 08/09/2018 10:01 AM, Peter Maydell wrote:
>>> Wire up the system control element's register banks.
>>>
>>> This is the last of the previously completely unimplemented
>>> components in the IoTKit.
>>>
>>> Signed-off-by: Peter Maydell <address@hidden>
>>> ---
>>>  include/hw/arm/iotkit.h |  4 +++-
>>>  hw/arm/iotkit.c         | 19 +++++++++++--------
>>>  2 files changed, 14 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/hw/arm/iotkit.h b/include/hw/arm/iotkit.h
>>> index 0f5c5101708..1ffa31d521b 100644
>>> --- a/include/hw/arm/iotkit.h
>>> +++ b/include/hw/arm/iotkit.h
>>> @@ -58,7 +58,7 @@
>>>  #include "hw/timer/cmsdk-apb-timer.h"
>>>  #include "hw/timer/cmsdk-apb-dualtimer.h"
>>>  #include "hw/watchdog/cmsdk-apb-watchdog.h"
>>> -#include "hw/misc/unimp.h"
>>> +#include "hw/misc/iotkit-sysctl.h"
>>>  #include "hw/or-irq.h"
>>>  #include "hw/core/split-irq.h"
>>>
>>> @@ -97,6 +97,8 @@ typedef struct IoTKit {
>>>      CMSDKAPBWatchdog nswatchdog;
>>>      CMSDKAPBWatchdog swatchdog;
>>>
>>> +    IoTKitSysCtl sysctl;
>>> +
>>>      MemoryRegion container;
>>>      MemoryRegion alias1;
>>>      MemoryRegion alias2;
>>> diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
>>> index cb0ec456f39..5d59ed5489f 100644
>>> --- a/hw/arm/iotkit.c
>>> +++ b/hw/arm/iotkit.c
>>> @@ -16,7 +16,6 @@
>>>  #include "hw/sysbus.h"
>>>  #include "hw/registerfields.h"
>>>  #include "hw/arm/iotkit.h"
>>> -#include "hw/misc/unimp.h"
>>>  #include "hw/arm/arm.h"
>>>
>>>  /* Clock frequency in HZ of the 32KHz "slow clock" */
>>> @@ -151,6 +150,8 @@ static void iotkit_init(Object *obj)
>>>                            sizeof(s->nswatchdog), TYPE_CMSDK_APB_WATCHDOG);
>>>      sysbus_init_child_obj(obj, "swatchdog", &s->swatchdog,
>>>                            sizeof(s->swatchdog), TYPE_CMSDK_APB_WATCHDOG);
>>> +    sysbus_init_child_obj(obj, "iotkit-sysctl", &s->sysctl, 
>>> sizeof(s->sysctl),
>>> +                          TYPE_IOTKIT_SYSCTL);
>>>      object_initialize_child(obj, "nmi-orgate", &s->nmi_orgate,
>>>                              sizeof(s->nmi_orgate), TYPE_OR_IRQ,
>>>                              &error_abort, NULL);
>>> @@ -516,13 +517,15 @@ static void iotkit_realize(DeviceState *dev, Error 
>>> **errp)
>>>                            qdev_get_gpio_in_named(dev_apb_ppc1,
>>>                                                   "cfg_sec_resp", 0));
>>>
>>> -    /* Using create_unimplemented_device() maps the stub into the
>>> -     * system address space rather than into our container, but the
>>> -     * overall effect to the guest is the same.
>>> -     */
>>> -    create_unimplemented_device("SYSINFO", 0x40020000, 0x1000);
>>> -
>>> -    create_unimplemented_device("SYSCONTROL", 0x50021000, 0x1000);
>>> +    object_property_set_bool(OBJECT(&s->sysctl), true, "realized", &err);
>>> +    if (err) {
>>> +        error_propagate(errp, err);
>>> +        return;
>>> +    }
>>> +    /* System information registers */
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysctl), 0, 0x40020000);
>>
>> What about the Secure access?
>>
>>        sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysctl), 0, 0x50020000);
> 
> That should be handled by the general alias set up earlier
>     make_alias(s, &s->alias3, "alias 3", 0x50000000, 0x10000000, 0x40000000);
> which maps all of the 0x5... space to 0x4... if there's nothing
> higher priority mapped there.

Oh I missed that! Perfect then :)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>