On Fri, Aug 21, 2020 at 01:53:52PM +0300, Roman Bolshakov wrote: > On Thu, Aug 20, 2020 at 05:55:29PM -0400, Eduardo Habkost wrote: > > While trying to convert TypeInfo declarations to the new > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases > > where instance_size or class_size is not set, despite having type > > checker macros that use a specific type. > > > > The ones with "WARNING" are abstract types (maybe not serious if > > subclasses set the appropriate sizes). The ones with "ERROR" > > don't seem to be abstract types. > > > > > ERROR: target/i386/hvf/hvf.c:908:1: instance_size should be set to > > sizeof(HVFState)? > > Hi Eduardo, > > How do you get the error?
My script looks for corresponding type checking macros, and check if instance_size is set to sizeof(T) with the right type from the type checking macro. The code is here: https://github.com/ehabkost/qemu-hacks/blob/920b2c521ad2a29fa663256854e24ed2059ba9cd/scripts/codeconverter/codeconverter/qom_type_info.py#L136 > > Given your changes, instance size should really be sizeof(HVFState). > The changes I've made shouldn't make any difference (if there's an issue, it is there before or after my series). > BTW, the object definition for hvf seems different from KVM (and perhaps > wrong?), e.g. HVFState is allocated within init_machine handler and then > assigned to a global variable: Interesting. It looks like hvf_state is _not_ the actual QOM object instance. The actual TYPE_HVF_ACCEL instance is created by do_configure_accelerator(). That would explain why the lack of instance_init never caused any problems. Luckily, no code ever used the HVF_STATE macro. If HVF_STATE(hvf_state) got called, it would crash because of uninitialized object instance data. If HVF_STATE(machine->accel) got called, it would return an invalid HVFState pointer (not hvf_state). I believe the simplest short term solution here is to just delete the HVF_STATE macro and HVFState::parent field. We can worry about actually moving hvf_state to the machine->accel QOM object later. > > static int hvf_accel_init(MachineState *ms) > { > int x; > hv_return_t ret; > HVFState *s; > > ret = hv_vm_create(HV_VM_DEFAULT); > assert_hvf_ok(ret); > > s = g_new0(HVFState, 1); > > s->num_slots = 32; > for (x = 0; x < s->num_slots; ++x) { > s->slots[x].size = 0; > s->slots[x].slot_id = x; > } > > hvf_state = s; > cpu_interrupt_handler = hvf_handle_interrupt; > memory_listener_register(&hvf_memory_listener, &address_space_memory); > return 0; > } > > static void hvf_accel_class_init(ObjectClass *oc, void *data) > { > AccelClass *ac = ACCEL_CLASS(oc); > ac->name = "HVF"; > ac->init_machine = hvf_accel_init; > ac->allowed = &hvf_allowed; > } > > static const TypeInfo hvf_accel_type = { > .name = TYPE_HVF_ACCEL, > .parent = TYPE_ACCEL, > .class_init = hvf_accel_class_init, > }; > > Thanks, > Roman > -- Eduardo
