On 27.01.20 18:29, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 18:09:11 +0100
> David Hildenbrand <da...@redhat.com> wrote:
>
>>>>> +static void s390_diag318_reset(DeviceState *dev)
>>>>> +{
>>>>> + if (kvm_enabled())
>>>>> + kvm_s390_set_diag318_info(0);
>>>>> +}
>>>>> +
>>>>> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
>>>>> +{
>>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>>> +
>>>>> + dc->reset = s390_diag318_reset;
>>>>> + dc->vmsd = &vmstate_diag318;
>>>>> + dc->hotpluggable = false;
>>>>> + /* Reason: Created automatically during machine instantiation */
>>>>> + dc->user_creatable = false;
>>>>> +}
>>>>> +
>>>>> +static const TypeInfo s390_diag318_info = {
>>>>> + .class_init = s390_diag318_class_init,
>>>>> + .parent = TYPE_DEVICE,
>>>>> + .name = TYPE_S390_DIAG318,
>>>>> + .instance_size = sizeof(DIAG318State),
>>>>> +};
>>>>> +
>>>>> +static void s390_diag318_register_types(void)
>>>>> +{
>>>>> + type_register_static(&s390_diag318_info);
>>>>> +}
>>>>
>>>> Do we really need a new device? Can't we simply glue that extended state
>>>> to the machine state?
>>>>
>>>> -> target/s390x/machine.c
>>>>
>>>
>>> Those VM States relate to the CPU state... does it make sense to store the
>>> diag318 info in a CPU state? (It doesn't seem necessary to store / migrate
>>> this info for each CPU).
>>
>> I'm sorry, I was looking at the wrong file ...
>>
>>>
>>> Should we store this in the S390CcwMachineState? Or perhaps create a generic
>>> S390MachineState for information that needs to be stored once and migrated
>>> once?
>>
>> ... I actually thought we have something like this already. Personally,
>> I think that would make sense. At least spapr seems to have something
>> like this already (hw/ppc/spapr.c:spapr_machine_init().
>>
>> @Conny?
>
> What are you referring to? I only see the one with the FIXME in front
> of it...
That's the one I mean. The fixme states something about qdev ... but
AFAIK that's only applicable if TYPE_DEVICE is involved. So maybe right
now there is no other way than registering the vmstate directly.
--
Thanks,
David / dhildenb
