On 1/27/20 12:55 PM, David Hildenbrand wrote:
> 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.
>
Hmm okay. I'll take a look at how spapr does it. I think I've registered a
vmstate via register_savevm_live() in an earlier version, but had difficulties
figuring out where to store the data. I'll revisit this approach.
Thanks for the feedback!
--
Respectfully,
- Collin Walling
