On Tue, 25 Jun 2019 11:17:09 -0400
Collin Walling <wall...@linux.ibm.com> wrote:
> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
> be intercepted by SIE and handled via KVM. Let's introduce some
> functions to communicate between QEMU and KVM via ioctls. These
> will be used to get/set the diag318 information.
>
> The availability of this instruction is determined by byte 134, bit 0
> of the Read Info block. This coincidentally expands into the space used
> for CPU entries, which means VMs running with the diag318 capability
> will have a reduced maximum CPU count. Let's reduce the maximum CPU
> count from 248 to 247.
>
> In order to simplify the migration and system reset requirements of
> the diag318 data, let's introduce it as a device class and include
> a VMStateDescription.
>
> Diag318 is set to 0 during modified clear and load normal resets.
>
> Signed-off-by: Collin Walling <wall...@linux.ibm.com>
> ---
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/diag318.c | 80
> +++++++++++++++++++++++++++++++++++++++++
> hw/s390x/diag318.h | 38 ++++++++++++++++++++
> hw/s390x/s390-virtio-ccw.c | 17 +++++++++
> hw/s390x/sclp.c | 3 ++
> include/hw/s390x/sclp.h | 2 ++
> target/s390x/cpu.h | 8 ++++-
> target/s390x/cpu_features.c | 3 ++
> target/s390x/cpu_features.h | 1 +
> target/s390x/cpu_features_def.h | 3 ++
> target/s390x/gen-features.c | 1 +
> target/s390x/kvm-stub.c | 10 ++++++
> target/s390x/kvm.c | 29 +++++++++++++++
> target/s390x/kvm_s390x.h | 2 ++
> 14 files changed, 197 insertions(+), 1 deletion(-)
> create mode 100644 hw/s390x/diag318.c
> create mode 100644 hw/s390x/diag318.h
(...)
> diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
> new file mode 100644
> index 0000000..0eb80fe
> --- /dev/null
> +++ b/hw/s390x/diag318.c
> @@ -0,0 +1,80 @@
> +/*
> + * DIAGNOSE 0x318 functions for reset and migration
> + *
> + * Copyright IBM, Corp. 2019
> + *
> + * Authors:
> + * Collin Walling <wall...@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> your
> + * option) any later version. See the COPYING file in the top-level
> directory.
> + */
> +
> +#include "hw/s390x/diag318.h"
> +#include "qapi/error.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/kvm.h"
> +
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> + DIAG318State *d = opaque;
> +
> + kvm_s390_set_diag318_info(d->info);
Shouldn't we at least moan if something goes wrong here?
> + return 0;
> +}
> +
> +static int diag318_pre_save(void *opaque)
> +{
> + DIAG318State *d = opaque;
> +
> + kvm_s390_get_diag318_info(&d->info);
> + return 0;
> +}
> +
> +static bool diag318_needed(void *opaque)
> +{
> + return s390_has_feat(S390_FEAT_DIAG318);
What happens if we emulate diag 318 in tcg and set this feature bit? We
probably don't want to call kvm_ functions in that case.
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> + .name = "vmstate_diag318",
> + .post_load = diag318_post_load,
> + .pre_save = diag318_pre_save,
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = diag318_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT64(info, DIAG318State),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void s390_diag318_reset(DeviceState *dev)
> +{
> + kvm_s390_set_diag318_info(0);
Same here; we probably need a s390_set_diag318_info() function that
either calls the kvm_ function or resets some internal state.
> +}
> +
> +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: Set automatically during IPL */
"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);
> +}
> +
> +type_init(s390_diag318_register_types)
(..)
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 87b2039..54230c7 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -38,6 +38,7 @@
> #include "cpu_models.h"
> #include "hw/nmi.h"
> #include "hw/s390x/tod.h"
> +#include "hw/s390x/diag318.h"
>
> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> {
> @@ -94,6 +95,7 @@ static const char *const reset_dev_types[] = {
> "s390-sclp-event-facility",
> "s390-flic",
> "diag288",
> + TYPE_S390_DIAG318,
This looks a bit odd, although it is not wrong :)
> };
>
> static void subsystem_reset(void)
> @@ -258,6 +260,17 @@ static void s390_create_sclpconsole(const char *type,
> Chardev *chardev)
> qdev_init_nofail(dev);
> }
>
> +static void s390_init_diag318(void)
> +{
> + Object *new = object_new(TYPE_S390_DIAG318);
> + DeviceState *dev = DEVICE(new);
> +
> + object_property_add_child(qdev_get_machine(), TYPE_S390_DIAG318,
> + new, NULL);
> + object_unref(new);
> + qdev_init_nofail(dev);
> +}
> +
> static void ccw_init(MachineState *machine)
> {
> int ret;
> @@ -315,6 +328,9 @@ static void ccw_init(MachineState *machine)
>
> /* init the TOD clock */
> s390_init_tod();
> +
> + /* init object used for migrating diag318 info */
> + s390_init_diag318();
> }
>
> static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> @@ -583,6 +599,7 @@ static void machine_set_loadparm(Object *obj, const char
> *val, Error **errp)
> ms->loadparm[i] = ' '; /* pad right with spaces */
> }
> }
> +
unrelated whitespace change :)
> static inline void s390_machine_initfn(Object *obj)
> {
> object_property_add_bool(obj, "aes-key-wrap",
(...)
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index f64f581..77a1df5 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -127,6 +127,9 @@ static const S390FeatDef s390_features[] = {
> FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF
> interpretation facility"),
> FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE:
> Interlock-and-broadcast-suppression facility"),
>
> + /* SCLP SCCB Byte 134 */
> + FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "Control program
> name and version codes"),
> +
> FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format
> 2 (Virtual SIE)"),
> FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key
> facility"),
> FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER
> enhancement facility"),
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index da695a8..954544e 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
> S390_FEAT_TYPE_STFL,
> S390_FEAT_TYPE_SCLP_CONF_CHAR,
> S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> + S390_FEAT_TYPE_SCLP_BYTE_134,
> S390_FEAT_TYPE_SCLP_CPU,
> S390_FEAT_TYPE_MISC,
> S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 292b17b..4f2c23e 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -115,6 +115,9 @@ typedef enum {
> S390_FEAT_SIE_PFMFI,
> S390_FEAT_SIE_IBS,
>
> + /* Sclp Byte 134 */
> + S390_FEAT_DIAG318,
> +
> /* Sclp Cpu */
> S390_FEAT_SIE_F2,
> S390_FEAT_SIE_SKEY,
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index dc320a0..cdd1875 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -521,6 +521,7 @@ static uint16_t full_GEN12_GA1[] = {
> S390_FEAT_AP_QUERY_CONFIG_INFO,
> S390_FEAT_AP_FACILITIES_TEST,
> S390_FEAT_AP,
> + S390_FEAT_DIAG318,
> };
>
> static uint16_t full_GEN12_GA2[] = {
The feature bit stuff probably needs some (easy) adaption on top of my
s390-next branch.
