Re: [Qemu-devel] [RFC] QOM design - add instance data to Object (-> add constructors)

[Liviu Ionescu]

> On 13 Jun 2015, at 14:41, Liviu Ionescu <address@hidden> wrote:
> 
> ... so I invented another solution:
> 
> - do not define any of the instance_init() and instance_post_init() 
> - add custom construct(...) callbacks to all my classes
> - make each instance call parent construct(...) at the beginning, as for any 
> OO constructor, than perform the specific construction
> - move the code that now is split between instance_post_init() and realize() 
> into this new construct(...)
> - make the realize() functions just call realize() for all its children, 
> without performing any other operations
> 
> ... with this approach the alloc() step will be the equivalent of the C++ 
> new() memory allocation and the construct() will be the equivalent of the C++ 
> object construction.
> 
> I think this mechanism is generic enough to easily accommodate any number of 
> arguments for the constructors, if needed.
> 
> 
> I'll test it shortly and let you know how it behaves.

well, to me it seems quite fine now, I'm curious what is your opinion.

the board creation code is exactly the following:

------------------------------------------------------
/* ----- Olimex STM32-H103 ----- */

GenericGPIOLEDInfo h103_machine_green_led = {
    .desc = "STM32-H103 Green LED, GPIOC[12], active low",
    .port_index = STM32_GPIO_PORT_C,
    .port_bit = 12,
    .active_low = true,
    .on_message = "[Green LED On]\n",
    .off_message = "[Green LED Off]\n",
    .use_stderr = true };

static void stm32_h103_board_init_callback(MachineState *machine)
{
    cortexm_board_greeting(machine);
    DeviceState *mcu = qdev_alloc(NULL, TYPE_STM32F103RB);
    {
        STM32F103RB_GET_CLASS(mcu)->construct(OBJECT(mcu), machine);

        /* Set the board specific oscillator frequencies. */
        DeviceState *rcc = stm32_mcu_get_rcc_dev(mcu);
        qdev_prop_set_uint32(rcc, "hse-freq-hz", 8000000); /* 8.0 MHz */
        qdev_prop_set_uint32(rcc, "lse-freq-hz", 32768); /* 32 KHz */
    }
    qdev_realize(mcu);

    /* Board peripheral objects */
    DeviceState *led = qdev_alloc(NULL, TYPE_STM32_GPIO_LED);
    {
        STM32_GPIO_LED_GET_CLASS(led)->construct(OBJECT(led),
                &h103_machine_green_led, mcu);
    }
    qdev_realize(led);
}

static QEMUMachine stm32_h103_machine = {
    .name = "STM32-H103",
    .desc = "Olimex Header Board for STM32F103RBT6 (Experimental)",
    .init = stm32_h103_board_init_callback };

------------------------------------------------------

the steps to create new objects are:
- alloc
- construct
- optionally set props
- realize

there is a small discussion related to what can be configured with separate 
properties and what should be passed in the constructor. well, my 
recommendation is to try to pass as much as possible with properties. if 
something is needed during construction, make it a constructor argument.

for a complete example, I'll also show the other constructors used in this 
implementation:

------------------------------------------------------
/* ----- STM32F103RB ----- */
static STM32Capabilities stm32f103rb_capabilities = {
    .cortexm = {
        .device_name = TYPE_STM32F103RB,
        .cortexm_model = CORTEX_M3,
        .flash_size_kb = 128,
        .sram_size_kb = 20,
        .has_mpu = true,
        .has_itm = true,
        .num_irq = 60,
        .nvic_bits = 4 },
    .stm32 = {
        .family = STM32_FAMILY_F1,
        .hsi_freq_hz = 8000000,
        .lsi_freq_hz = 40000,
        .has_gpioa = true,
        .has_gpiob = true,
        .has_gpioc = true,
        .has_gpiod = true,
        .has_gpioe = true,
        .f1 = {
            .is_md = true } } };

static void stm32f103rb_mcu_construct_callback(Object *obj,
        MachineState *machine)
{
    qemu_log_function_name();

    STM32_MCU_GET_CLASS(obj)->construct(obj, &stm32f103rb_capabilities,
            machine);
}

------------------------------------------------------

static void stm32_mcu_construct_callback(Object *obj,
        STM32Capabilities* capabilities, MachineState *machine)
{
    qemu_log_function_name();

    CORTEXM_MCU_GET_CLASS(obj)->construct(obj, &(capabilities->cortexm),
            machine);

    assert(capabilities != NULL);

    /* ... construct the STM32 MCU based on capabilities  ... */
}

------------------------------------------------------

static void cortexm_mcu_construct_callback(Object *obj,
        CortexMCapabilities* capabilities, MachineState *machine)
{
    qemu_log_function_name();

    CortexMState *cm_state = CORTEXM_MCU_STATE(obj);

    if (machine->kernel_filename) {
        cm_state->kernel_filename = machine->kernel_filename;
    }

    if (machine->cpu_model) {
        cm_state->cpu_model = machine->cpu_model;
    }

    cm_state->capabilities = capabilities;

    /* ... construct the Cortex-M MCU based on capabilities  ... */
}

------------------------------------------------------

static void stm_gpio_led_construct_callback(Object *obj,
        GenericGPIOLEDInfo* info, DeviceState *mcu)
{
    qemu_log_function_name();

    GENERIC_GPIO_LED_GET_CLASS(obj)->construct(obj, info, mcu);
}

------------------------------------------------------

static void generic_gpio_led_construct_callback(Object *obj,
        GenericGPIOLEDInfo* info, DeviceState *mcu)
{
    qemu_log_function_name();

    GenericGPIOLEDState *state = GENERIC_GPIO_LED_STATE(obj);

    state->info = info;
    state->mcu = mcu;
}

------------------------------------------------------


as it can be seen, calling the parent constructor is relatively straight 
forward.


to me this mechanism seems ok, it provides the required control on object 
creation (order and data availability), it is relatively easy to use (within 
the QOM limits), and the code seems reasonably maintainable. 

if you have no better suggestions, I'll probably use this model for all my 
future objects.


regards,

Liviu


p.s. please note the introduction of the LED object, intended to greatly 
simplify the board definitions.