qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Liviu Ionescu
Subject: Re: [Qemu-devel] [RFC] QOM design - add instance data to Object
Date: Sat, 13 Jun 2015 14:41:34 +0300

> On 13 Jun 2015, at 12:29, Peter Crosthwaite <address@hidden> wrote:
> 
>> 
>> STM32F103RB* mcu = new STM32F103RB(&stm32f103rb_capabilities);
>> 
> 
> Is the capabilities genuinely variable from instance to instance? 

good point.

my example was not very accurate, the capabilities are indeed specific to a 
certain class, like this:

class STM32F103RB : public STM32 {
public:
        STM32F103RB() : STM32(&stm32f103rb_capabilities) {
                ...
        }
}

and constructing a new mcu is:

STM32F103RB* mcu = new STM32F103RB();

> This
> naming suggest a 1:1 between capabilities instances and MCU classes
> which would mean he capabilities can just be rolled into the classes
> themselves.

that's correct. but in practical terms it does not help much.

>> static void stm32f103rb_mcu_instance_init_callback(Object *obj)
> 
> No need for _callback.

this is a personal preference issue, I like to explicitly mark functions called 
by the framework to easily differentiate them from regular functions. 

>>    CortexMState *cm_state = CORTEXM_MCU_STATE(obj);
>>    cm_state->capabilities = (CortexMCapabilities *) 
>> &stm32f103rb_capabilities;
> 
> Following on from before, can you put the capabilities in the class
> instead? Then it is immediately accessible from the CortexMState
> instance_init.

yes, this can be done, but it doesn't help much.
 
> 
>> }
>> 
>> 
>> 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.
>> 
> 
> What in your construction process requires access to the capabilities
> information? Can you give concrete examples?

here is the actual capabilities structure:

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 } } };


the cortexm-mcu constructor needs to know a lot of things to construct the core 
object and some related devices, and the stm32-mcu constructor needs to know 
another set of details, to construct specific devices.


>> 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().
>> 
> 
> There is a way :). Check object_class_get_parent. qdev.c uses it to
> implement device properties on multiple levels without overriding. You
> still need to explicit call to superclass in each child class but that
> is normal of OO syntaxes.

thank you for the hint, I'll try this, but in this case why isn't this 
mechanism used for realize()? all examples I could find used a local copy  like 
parent_realize().

> So should kernel_filename instead be handled on the machine level
> rather then going into the MCU class?

the image is currently loaded in the mcu class, but indeed, it might not be 
needed that deep.

I'll try to move it to the machine level.

> cpu_model should be a class
> property, I don't see why instances of a same MCU class need varying
> cpu_models.

the point is to allow the command line cpu_model to overwrite the board mcu 
definition.

in other words, even if the board refers to a mcu that is marked as cortex-m4, 
the command line may change it to a cortex-m3.

> Even if you want to do this, one way is to use a property
> setter-callback on the string which does the cpu init late. property
> setters can let you invent new passes to the initialisation if needed
> but it is not pretty.

I considered this, but it looked even more complicated.

>> to avoid breaking compatibility, the solution that I suggest is to add 
>> separate initialisation functions
>> - add a new "void* instance_data;" member to Object,
> 
> I don't think it should persist in the Object struct like this. If is
> analagous to constructor fn arguments, then it is up the specific
> class instance_init functions to take the state into their subclass
> data.
> 
> It doesn't solve the problem of the subclass being able to specify
> superclass construction arguments either.

yes, I also noticed this (during the jogging session I just completed!).

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

the use case would look like:

  DeviceState *mcu = cortexm_mcu_alloc(machine, TYPE_STM32F103RB);
  {
    STM32F103R8_CLASS(mcu)->construct();

    qdev_prop_set_uint32(mcu, "some-property", "some value");
    ...
  }
  qdev_realize(mcu);

the stm32f103rb_construct_callback() will call:
 
  stm32_construct_callback(&stm32f103rb_capabilities);
  ...

the stm32_construct_callback(STM32Capabilities *p) will call something like:

  cortexm_construct_callback((CortexMCapabilities*)&(p->cortexm));
  ...

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.


regards,

Liviu

*) qdev_realize() is the same as qdev_init_nofail(), but with a more intuitive 
name.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]