>>> +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?
[...]
>
> How about we introduce a union in the ReadInfo struct. Something like:
>
> union {
> uint8_t byte_134;
> struct CPUEntry entries[0];
> } x;
Or drop the "entries" pointer completely and introduce
static int cpu_entries_offset(void)
{
/*
* When we have to indicate features in byte 134, we have to move
* the start of the cpu entries.
*/
if (s390_has_feat(S390_FEAT_DIAG318)) {
return 144;
}
return 128;
}
struct CPUEntry *cpu_entries(ReadInfo *ri)
{
return (struct CPUEntry *)((void *)ri + cpu_entries_offset());
}
unsigned int cpu_entries)count(ReadInfo *ri)
{
return (SCCB_SIZE - cpu_entries_offset()) / sizeof(CPUEntry);
}
etc. (might take some tweaking to make it compile) and a comment for the
struct. Not sure what's better. Having two struct CPUEntry entries[0] is
also confusing.
Thanks!
--
Thanks,
David / dhildenb
