Re: [Qemu-devel] [RFC] QOM design - add instance data to Object

[Liviu Ionescu]

any object oriented gurus around?

regards,

Liviu


> On 12 Jun 2015, at 14:33, Liviu Ionescu <address@hidden> wrote:
> 
> while implementing the cortex-m hierarchical objects I faced several problems 
> that required some hack, and I wanted to know your comments on them.
> 
> for convenience I'll explain the problem in C++ and then return to the issues 
> of the C implementation. (the C++ syntax may not be very strict).
> 
> one of my goals was to emulate a representative number of MCUs, from several 
> vendors. the model I used is hierarchical, grouping vendor common 
> characteristics. for example for ST I have:
> 
> class CortexM {};
> class STM32 : public CortexM {};
> class STM32F103RB : public STM32 {};
> 
> to facilitate configuration of the CortexM and STM objects, I defined some 
> 'capabilities' classes:
> 
> class CortexMCapabilities {};
> class STM32Capabilities : public CortexMCapabilities {};
> 
> in the CortexMCapabilities I store the Cortex-M family 
> (M0/M0+/M1/M3/M4/M4F/M7/M7F), flash/sram memory sizes, number of interrupts, 
> flags telling if MPU, ITP, or other ARM peripherals are present.
> 
> in the STM32Capabilities I store STM family (F1/F2/F4/F4/etc), number of 
> GPIOs, etc.
> 
> a capabilities object must be defined for each device, for example:
> 
> STM32Capabilities stm32f103rb_capabilities; ...
> 
> this pointer is passed to the constructors and as such would be available to 
> all objects:
> 
> class CortexM 
> {
> public:
>       CortexM(CortexMCapabilities* capabilities) {
>               fCapabilities = capabilities;
>               ...
>       }
> 
>       CortexMCapabilities* fCapabilities;
>       ...
> }
> 
> class STM32 : public CortexM {
> public:
>       STM32(STM32Capabilities* capabilities) : CortexM(capabilities) {
>               ...
>       }
> }
> 
> class STM32F103RB : public STM32 {
> public:
>       STM32F103RB(STM32Capabilities* capabilities) : STM32(capabilities) {
>               ...
>       }
> }
> 
> when constructing a new mcu:
> 
> STM32F103RB* mcu = new STM32F103RB(&stm32f103rb_capabilities);
> 
> the constructors are executed in the natural 'parent first' order:
> 
> CortexM(...)
> STM32(...)
> STM32F103RB(...)
> 
> and each object gets access to the capabilities during the construction, as 
> expected.
> 
> 
> now let's return to the qemu objects I used, where I have
> 
> - a 'cortex-mcu' object derived from 'sys-bus-device', which has as member a 
> pointer to the capabilities
> - a 'stm32-mcu' object, derived from 'cortex-mcu'
> - a 'STM32F103RB' object, derived from 'stm32-mcu' (the name matches the 
> CMSIS device name).
> 
> the natural place to define the capabilities is the more specfic instance_init
> 
> static void stm32f103rb_mcu_instance_init_callback(Object *obj)
> {
>    qemu_log_function_name();
> 
>    CortexMState *cm_state = CORTEXM_MCU_STATE(obj);
>    cm_state->capabilities = (CortexMCapabilities *) &stm32f103rb_capabilities;
> }
> 
> 
> constructing a mcu is done with:
> 
> dev = qdev_create(NULL, 'STM32F103RB');
> 
> which calls in order:
> 
> cortexm_mcu_instance_init_callback()
> stm32_mcu_instance_init_callback()
> stm32f103rb_mcu_instance_init_callback()
> 
> as you can see, the stm32f103rb call is executed last (as it should be). this 
> also means that during the execution of the cortexm_ and stm_ functions the 
> capabilities **are not** available.
> 
> 
> the workaround I used was to move the object construction from instance_init 
> to instance_post_init, which are executed after the instance_init, so when 
> they run the capabilities pointer is already set.
> 
> unfortunately, these calls are executed in reverse order of the constructors, 
> 'child first', which might create problems if references to parent objects 
> are required.
> 
> stm32f103rb_mcu_instance_post_init_callback()
> stm32_mcu_instance_post_init_callback()
> ...
> cortexm_mcu_instance_post_init_callback()
> ...
> 
> to solve the problem of references to parent objects, the third construction 
> step is performed during realize().
> 
> unfortunately realize() is a simple virtual call, and since pointers to 
> virtuals are stored in the class structure and not as separate vtbls, access 
> to parent realize() is not possible directly, but only by manually storing 
> the pointer in a separate parent_realize(0 and explicitly calling it at the 
> beginning of each realize().
> 
> 
> ---
> 
> another similar problem that required a hack was passing some of the machine 
> fields (kernel_filename & cpu_model) to the mcu constructor.
> 
> so, the actual mcu costructor has, in addition to the mcu name 
> ('STM32F103RB'), a pointer to the machine.
> 
> for not being able to pass it to the constructor, I had to store it in a 
> static variable and in the constructor refer to it, but this is definitely a 
> non-reentrant kludge:
> 
> static MachineState *global_machine;
> 
> DeviceState *cortexm_mcu_create(MachineState *machine, const char *mcu_type)
> {
>    /*
>     * Kludge, since it is not possible to pass data to object creation,
>     * we use a static variable.
>     */
>    global_machine = machine;
>    DeviceState *dev;
>    dev = qdev_create(NULL, mcu_type);
> 
>    return dev;
> }
> 
> static void cortexm_mcu_instance_init_callback(Object *obj)
> {
>    qemu_log_function_name();
> 
>    CortexMState *cm_state = CORTEXM_MCU_STATE(obj);
>    assert(global_machine != NULL);
> 
>    if (global_machine->kernel_filename) {
>        cm_state->kernel_filename = global_machine->kernel_filename;
>    }
> 
>    if (global_machine->cpu_model) {
>        cm_state->cpu_model = global_machine->cpu_model;
>    }
> }
> 
> ---
> 
> I think that both the above cases show that a mechanism to pass instance data 
> to the constructor is missing from the QOM design.
> 
> a bit curious is that the designer considered a similar mechanism to pass 
> class data when creating class objects, but this cannot be used for classes 
> that have multiple instances.
> 
> 
> to avoid breaking compatibility, the solution that I suggest is to add 
> separate initialisation functions
> - add a new "void* instance_data;" member to Object, 
> - add a new function to object_initialize_with_instance_data(), to pass this 
> pointer
> - add a new function qdev_create_with_instance_data() to pass this pointer
> 
> calling the existing constructor functions will create objects with this 
> pointer set to NULL, calling the new functions will create objects with this 
> pointer set to the desired value.
> 
> please note that this pointer must be set before any call to the 
> instance_init() callbacks.
> 
> access to the instance data would be easy, using something like 
> "OBJECT(dev)->instance_data", or, even better, a specialised getter 
> 
>       void* object_get_instance_data(Object*);
> 
> ---
> 
> as a conclusion, a very simple thing like passing initialisation data to a 
> C++ constructor requires a pretty sustained effort with the Qemu C 
> implementation. unfortunately, this is also true for some other QOM aspects, 
> like calling parent virtual methods.
> 
> a solution to simplify things is to add instance data to the object, and 
> functions to set/get it.
> 
> my initial guess is that this solution can be implemented without breaking 
> compatibility with existing code, by adding new functions; the suggested 
> names are obviously not mandatory, any other more inspired names may be used.
> 
> 
> regards,
> 
> Liviu
> 
> 
> p.s. I generally appreciate creativity and the desire to invent new solutions 
> for various problems; Stroustrup solution to the need of objects was to 
> create a new programming language; your solution to the need of objects, 
> instead of choosing a more appropriate language, was to insist on using C and 
> invent a very, very complicated infrastructure, that requires a lot of 
> explicit code to be manually written, it is relatively error prone, and it 
> still has several design flaws; please believe me, it is quite difficult to 
> outsmart Stroustrup...
> 
> 
> 
>