* [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-06-04 11:38 ` Cornelia Huck
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 2/7] s390x/tcg: drop tod_basetime David Hildenbrand
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
Let's treat this like a separate device. TCG will have to store the
actual state/time later on.
Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
hw/s390x/Makefile.objs | 3 +
hw/s390x/s390-virtio-ccw.c | 57 +-----------------
hw/s390x/tod-kvm.c | 64 ++++++++++++++++++++
hw/s390x/tod-qemu.c | 47 +++++++++++++++
hw/s390x/tod.c | 120 +++++++++++++++++++++++++++++++++++++
include/hw/s390x/tod.h | 46 ++++++++++++++
target/s390x/cpu.c | 32 ----------
target/s390x/cpu.h | 2 -
target/s390x/kvm_s390x.h | 2 +
9 files changed, 285 insertions(+), 88 deletions(-)
create mode 100644 hw/s390x/tod-kvm.c
create mode 100644 hw/s390x/tod-qemu.c
create mode 100644 hw/s390x/tod.c
create mode 100644 include/hw/s390x/tod.h
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index dc704b57d6..93282f7c59 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -14,6 +14,9 @@ obj-$(CONFIG_PCI) += s390-pci-bus.o s390-pci-inst.o
obj-$(call lnot,$(CONFIG_PCI)) += s390-pci-stub.o
obj-y += s390-skeys.o
obj-y += s390-stattrib.o
+obj-y += tod.o
+obj-$(CONFIG_KVM) += tod-kvm.o
+obj-$(CONFIG_TCG) += tod-qemu.o
obj-$(CONFIG_KVM) += s390-skeys-kvm.o
obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
obj-y += s390-ccw.o
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e548d341a0..390f021107 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -35,6 +35,7 @@
#include "migration/register.h"
#include "cpu_models.h"
#include "hw/nmi.h"
+#include "hw/s390x/tod.h"
S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
{
@@ -187,58 +188,6 @@ static void s390_memory_init(ram_addr_t mem_size)
s390_stattrib_init();
}
-#define S390_TOD_CLOCK_VALUE_MISSING 0x00
-#define S390_TOD_CLOCK_VALUE_PRESENT 0x01
-
-static void gtod_save(QEMUFile *f, void *opaque)
-{
- uint64_t tod_low;
- uint8_t tod_high;
- int r;
-
- r = s390_get_clock(&tod_high, &tod_low);
- if (r) {
- warn_report("Unable to get guest clock for migration: %s",
- strerror(-r));
- error_printf("Guest clock will not be migrated "
- "which could cause the guest to hang.");
- qemu_put_byte(f, S390_TOD_CLOCK_VALUE_MISSING);
- return;
- }
-
- qemu_put_byte(f, S390_TOD_CLOCK_VALUE_PRESENT);
- qemu_put_byte(f, tod_high);
- qemu_put_be64(f, tod_low);
-}
-
-static int gtod_load(QEMUFile *f, void *opaque, int version_id)
-{
- uint64_t tod_low;
- uint8_t tod_high;
- int r;
-
- if (qemu_get_byte(f) == S390_TOD_CLOCK_VALUE_MISSING) {
- warn_report("Guest clock was not migrated. This could "
- "cause the guest to hang.");
- return 0;
- }
-
- tod_high = qemu_get_byte(f);
- tod_low = qemu_get_be64(f);
-
- r = s390_set_clock(&tod_high, &tod_low);
- if (r) {
- error_report("Unable to set KVM guest TOD clock: %s", strerror(-r));
- }
-
- return r;
-}
-
-static SaveVMHandlers savevm_gtod = {
- .save_state = gtod_save,
- .load_state = gtod_load,
-};
-
static void s390_init_ipl_dev(const char *kernel_filename,
const char *kernel_cmdline,
const char *initrd_filename, const char *firmware,
@@ -363,8 +312,8 @@ static void ccw_init(MachineState *machine)
s390_create_sclpconsole("sclplmconsole", serial_hd(1));
}
- /* Register savevm handler for guest TOD clock */
- register_savevm_live(NULL, "todclock", 0, 1, &savevm_gtod, NULL);
+ /* init the TOD clock */
+ s390_init_tod();
}
static void s390_cpu_plug(HotplugHandler *hotplug_dev,
diff --git a/hw/s390x/tod-kvm.c b/hw/s390x/tod-kvm.c
new file mode 100644
index 0000000000..3fe43fc114
--- /dev/null
+++ b/hw/s390x/tod-kvm.c
@@ -0,0 +1,64 @@
+/*
+ * TOD (Time Of Day) clock - KVM implementation
+ *
+ * Copyright 2018 Red Hat, Inc.
+ * Author(s): David Hildenbrand <david@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/s390x/tod.h"
+#include "kvm_s390x.h"
+
+static void kvm_s390_tod_get(S390TODState *td, S390TOD *tod, Error **errp)
+{
+ int r;
+
+ r = kvm_s390_get_clock_ext(&tod->high, &tod->low);
+ if (r == -ENXIO) {
+ r = kvm_s390_get_clock(&tod->high, &tod->low);
+ }
+ if (r) {
+ error_setg(errp, "Unable to get KVM guest TOD clock: %s",
+ strerror(-r));
+ }
+}
+
+static void kvm_s390_tod_set(S390TODState *td, S390TOD *tod, Error **errp)
+{
+ int r;
+
+ r = kvm_s390_set_clock_ext(&tod->high, &tod->low);
+ if (r == -ENXIO) {
+ r = kvm_s390_set_clock(&tod->high, &tod->low);
+ }
+ if (r) {
+ error_setg(errp, "Unable to set KVM guest TOD clock: %s",
+ strerror(-r));
+ }
+}
+
+static void kvm_s390_tod_class_init(ObjectClass *oc, void *data)
+{
+ S390TODClass *tdc = S390_TOD_CLASS(oc);
+
+ tdc->get = kvm_s390_tod_get;
+ tdc->set = kvm_s390_tod_set;
+}
+
+static TypeInfo kvm_s390_tod_info = {
+ .name = TYPE_KVM_S390_TOD,
+ .parent = TYPE_S390_TOD,
+ .instance_size = sizeof(S390TODState),
+ .class_init = kvm_s390_tod_class_init,
+ .class_size = sizeof(S390TODClass),
+};
+
+static void register_types(void)
+{
+ type_register_static(&kvm_s390_tod_info);
+}
+type_init(register_types);
diff --git a/hw/s390x/tod-qemu.c b/hw/s390x/tod-qemu.c
new file mode 100644
index 0000000000..7997ba2b1a
--- /dev/null
+++ b/hw/s390x/tod-qemu.c
@@ -0,0 +1,47 @@
+/*
+ * TOD (Time Of Day) clock - QEMU implementation
+ *
+ * Copyright 2018 Red Hat, Inc.
+ * Author(s): David Hildenbrand <david@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/s390x/tod.h"
+
+static void qemu_s390_tod_get(S390TODState *td, S390TOD *tod, Error **errp)
+{
+ /* FIXME */
+ tod->high = 0;
+ tod->low = 0;
+}
+
+static void qemu_s390_tod_set(S390TODState *td, S390TOD *tod, Error **errp)
+{
+ /* FIXME */
+}
+
+static void qemu_s390_tod_class_init(ObjectClass *oc, void *data)
+{
+ S390TODClass *tdc = S390_TOD_CLASS(oc);
+
+ tdc->get = qemu_s390_tod_get;
+ tdc->set = qemu_s390_tod_set;
+}
+
+static TypeInfo qemu_s390_tod_info = {
+ .name = TYPE_QEMU_S390_TOD,
+ .parent = TYPE_S390_TOD,
+ .instance_size = sizeof(S390TODState),
+ .class_init = qemu_s390_tod_class_init,
+ .class_size = sizeof(S390TODClass),
+};
+
+static void register_types(void)
+{
+ type_register_static(&qemu_s390_tod_info);
+}
+type_init(register_types);
diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
new file mode 100644
index 0000000000..c1b3438452
--- /dev/null
+++ b/hw/s390x/tod.c
@@ -0,0 +1,120 @@
+/*
+ * TOD (Time Of Day) clock
+ *
+ * Copyright 2018 Red Hat, Inc.
+ * Author(s): David Hildenbrand <david@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/s390x/tod.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "sysemu/kvm.h"
+#include "migration/register.h"
+
+void s390_init_tod(void)
+{
+ Object *obj;
+
+ if (kvm_enabled()) {
+ obj = object_new(TYPE_KVM_S390_TOD);
+ } else {
+ obj = object_new(TYPE_QEMU_S390_TOD);
+ }
+ object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj, NULL);
+ object_unref(obj);
+
+ qdev_init_nofail(DEVICE(obj));
+}
+
+#define S390_TOD_CLOCK_VALUE_MISSING 0x00
+#define S390_TOD_CLOCK_VALUE_PRESENT 0x01
+
+static void s390_tod_save(QEMUFile *f, void *opaque)
+{
+ S390TODState *td = opaque;
+ S390TODClass *tdc = S390_TOD_GET_CLASS(td);
+ Error *err = NULL;
+ S390TOD tod;
+
+ tdc->get(td, &tod, &err);
+ if (err) {
+ warn_report_err(err);
+ error_printf("Guest clock will not be migrated "
+ "which could cause the guest to hang.");
+ qemu_put_byte(f, S390_TOD_CLOCK_VALUE_MISSING);
+ return;
+ }
+
+ qemu_put_byte(f, S390_TOD_CLOCK_VALUE_PRESENT);
+ qemu_put_byte(f, tod.high);
+ qemu_put_be64(f, tod.low);
+}
+
+static int s390_tod_load(QEMUFile *f, void *opaque, int version_id)
+{
+ S390TODState *td = opaque;
+ S390TODClass *tdc = S390_TOD_GET_CLASS(td);
+ Error *err = NULL;
+ S390TOD tod;
+
+ if (qemu_get_byte(f) == S390_TOD_CLOCK_VALUE_MISSING) {
+ warn_report("Guest clock was not migrated. This could "
+ "cause the guest to hang.");
+ return 0;
+ }
+
+ tod.high = qemu_get_byte(f);
+ tod.low = qemu_get_be64(f);
+
+ tdc->set(td, &tod, &err);
+ if (err) {
+ error_report_err(err);
+ return -1;
+ }
+ return 0;
+}
+
+static SaveVMHandlers savevm_tod = {
+ .save_state = s390_tod_save,
+ .load_state = s390_tod_load,
+};
+
+static void s390_tod_realize(DeviceState *dev, Error **errp)
+{
+ S390TODState *td = S390_TOD(dev);
+
+ /* Legacy migration interface */
+ register_savevm_live(NULL, "todclock", 0, 1, &savevm_tod, td);
+}
+
+static void s390_tod_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->desc = "TOD (Time Of Day) Clock";
+ dc->realize = s390_tod_realize;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+
+ /* We only have one TOD clock in the system attached to the machine */
+ dc->user_creatable = false;
+ dc->hotpluggable = false;
+}
+
+static TypeInfo s390_tod_info = {
+ .name = TYPE_S390_TOD,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(S390TODState),
+ .class_init = s390_tod_class_init,
+ .class_size = sizeof(S390TODClass),
+ .abstract = true,
+};
+
+static void register_types(void)
+{
+ type_register_static(&s390_tod_info);
+}
+type_init(register_types);
diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
new file mode 100644
index 0000000000..43ed71600f
--- /dev/null
+++ b/include/hw/s390x/tod.h
@@ -0,0 +1,46 @@
+/*
+ * TOD (Time Of Day) clock
+ *
+ * Copyright 2018 Red Hat, Inc.
+ * Author(s): David Hildenbrand <david@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_S390_TOD_H
+#define HW_S390_TOD_H
+
+#include "hw/qdev.h"
+
+typedef struct S390TOD {
+ uint8_t high;
+ uint64_t low;
+} S390TOD;
+
+#define TYPE_S390_TOD "s390-tod"
+#define S390_TOD(obj) OBJECT_CHECK(S390TODState, (obj), TYPE_S390_TOD)
+#define S390_TOD_CLASS(oc) OBJECT_CLASS_CHECK(S390TODClass, (oc), \
+ TYPE_S390_TOD)
+#define S390_TOD_GET_CLASS(obj) OBJECT_GET_CLASS(S390TODClass, (obj), \
+ TYPE_S390_TOD)
+#define TYPE_KVM_S390_TOD "kvm-" TYPE_S390_TOD
+#define TYPE_QEMU_S390_TOD "qemu-" TYPE_S390_TOD
+
+typedef struct S390TODState {
+ /* private */
+ DeviceState parent_obj;
+} S390TODState;
+
+typedef struct S390TODClass {
+ /* private */
+ DeviceClass parent_class;
+
+ /* public */
+ void (*get)(S390TODState *td, S390TOD *tod, Error **errp);
+ void (*set)(S390TODState *td, S390TOD *tod, Error **errp);
+} S390TODClass;
+
+void s390_init_tod(void);
+
+#endif
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index c2b775f4eb..167d089722 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -391,38 +391,6 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
return s390_count_running_cpus();
}
-int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
- int r = 0;
-
- if (kvm_enabled()) {
- r = kvm_s390_get_clock_ext(tod_high, tod_low);
- if (r == -ENXIO) {
- return kvm_s390_get_clock(tod_high, tod_low);
- }
- } else {
- /* Fixme TCG */
- *tod_high = 0;
- *tod_low = 0;
- }
-
- return r;
-}
-
-int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
- int r = 0;
-
- if (kvm_enabled()) {
- r = kvm_s390_set_clock_ext(tod_high, tod_low);
- if (r == -ENXIO) {
- return kvm_s390_set_clock(tod_high, tod_low);
- }
- }
- /* Fixme TCG */
- return r;
-}
-
int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
{
if (kvm_enabled()) {
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 6629a533f3..ac51c17fb4 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -714,8 +714,6 @@ static inline void s390_do_cpu_load_normal(CPUState *cs, run_on_cpu_data arg)
/* cpu.c */
-int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
-int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
void s390_crypto_reset(void);
bool s390_get_squash_mcss(void);
int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index c383bf4ee9..8567aa49f7 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -10,6 +10,8 @@
#ifndef KVM_S390X_H
#define KVM_S390X_H
+#include "cpu-qom.h"
+
struct kvm_s390_irq;
void kvm_s390_floating_interrupt_legacy(struct kvm_s390_irq *irq);
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device David Hildenbrand
@ 2018-06-04 11:38 ` Cornelia Huck
2018-06-04 12:00 ` David Hildenbrand
0 siblings, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-06-04 11:38 UTC (permalink / raw)
To: David Hildenbrand
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On Fri, 25 May 2018 13:37:02 +0200
David Hildenbrand <david@redhat.com> wrote:
> Let's treat this like a separate device. TCG will have to store the
> actual state/time later on.
>
> Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
Can't you include it in tod-kvm.c instead? Why was it not needed before?
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> hw/s390x/Makefile.objs | 3 +
> hw/s390x/s390-virtio-ccw.c | 57 +-----------------
> hw/s390x/tod-kvm.c | 64 ++++++++++++++++++++
> hw/s390x/tod-qemu.c | 47 +++++++++++++++
> hw/s390x/tod.c | 120 +++++++++++++++++++++++++++++++++++++
> include/hw/s390x/tod.h | 46 ++++++++++++++
> target/s390x/cpu.c | 32 ----------
> target/s390x/cpu.h | 2 -
> target/s390x/kvm_s390x.h | 2 +
> 9 files changed, 285 insertions(+), 88 deletions(-)
> create mode 100644 hw/s390x/tod-kvm.c
> create mode 100644 hw/s390x/tod-qemu.c
> create mode 100644 hw/s390x/tod.c
> create mode 100644 include/hw/s390x/tod.h
Otherwise, looks good.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-06-04 11:38 ` Cornelia Huck
@ 2018-06-04 12:00 ` David Hildenbrand
2018-06-04 12:14 ` Cornelia Huck
0 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-06-04 12:00 UTC (permalink / raw)
To: Cornelia Huck
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On 04.06.2018 13:38, Cornelia Huck wrote:
> On Fri, 25 May 2018 13:37:02 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> Let's treat this like a separate device. TCG will have to store the
>> actual state/time later on.
>>
>> Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
>
> Can't you include it in tod-kvm.c instead? Why was it not needed before?
Guess I can, kvm_s390x.h looks to me like the right place, as S390CPU is
used in there.
All other users include "cpu.h" already, so it isn't needed.
>
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> hw/s390x/Makefile.objs | 3 +
>> hw/s390x/s390-virtio-ccw.c | 57 +-----------------
>> hw/s390x/tod-kvm.c | 64 ++++++++++++++++++++
>> hw/s390x/tod-qemu.c | 47 +++++++++++++++
>> hw/s390x/tod.c | 120 +++++++++++++++++++++++++++++++++++++
>> include/hw/s390x/tod.h | 46 ++++++++++++++
>> target/s390x/cpu.c | 32 ----------
>> target/s390x/cpu.h | 2 -
>> target/s390x/kvm_s390x.h | 2 +
>> 9 files changed, 285 insertions(+), 88 deletions(-)
>> create mode 100644 hw/s390x/tod-kvm.c
>> create mode 100644 hw/s390x/tod-qemu.c
>> create mode 100644 hw/s390x/tod.c
>> create mode 100644 include/hw/s390x/tod.h
>
> Otherwise, looks good.
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-06-04 12:00 ` David Hildenbrand
@ 2018-06-04 12:14 ` Cornelia Huck
2018-06-07 10:49 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
0 siblings, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-06-04 12:14 UTC (permalink / raw)
To: David Hildenbrand
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On Mon, 4 Jun 2018 14:00:43 +0200
David Hildenbrand <david@redhat.com> wrote:
> On 04.06.2018 13:38, Cornelia Huck wrote:
> > On Fri, 25 May 2018 13:37:02 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >
> >> Let's treat this like a separate device. TCG will have to store the
> >> actual state/time later on.
> >>
> >> Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
> >
> > Can't you include it in tod-kvm.c instead? Why was it not needed before?
>
> Guess I can, kvm_s390x.h looks to me like the right place, as S390CPU is
> used in there.
>
> All other users include "cpu.h" already, so it isn't needed.
Hm, and for kvm_s390_irq, this file is using a forward declaration,
which would likely be more awkward for this structure.
Header dependencies are always fun :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-06-04 12:14 ` Cornelia Huck
@ 2018-06-07 10:49 ` David Hildenbrand
2018-06-07 11:10 ` Cornelia Huck
0 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-06-07 10:49 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J . Herne, Thomas Huth, Alexander Graf, qemu-devel,
Christian Borntraeger, qemu-s390x, Collin L . Walling,
Richard Henderson
On 04.06.2018 14:14, Cornelia Huck wrote:
> On Mon, 4 Jun 2018 14:00:43 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> On 04.06.2018 13:38, Cornelia Huck wrote:
>>> On Fri, 25 May 2018 13:37:02 +0200
>>> David Hildenbrand <david@redhat.com> wrote:
>>>
>>>> Let's treat this like a separate device. TCG will have to store the
>>>> actual state/time later on.
>>>>
>>>> Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
>>>
>>> Can't you include it in tod-kvm.c instead? Why was it not needed before?
>>
>> Guess I can, kvm_s390x.h looks to me like the right place, as S390CPU is
>> used in there.
>>
>> All other users include "cpu.h" already, so it isn't needed.
>
> Hm, and for kvm_s390_irq, this file is using a forward declaration,
> which would likely be more awkward for this structure.
I remember that we wanted to use cpu-qom.h for this typedef fun, that's
why I decided to do it this way.
>
> Header dependencies are always fun :)
>
So what do you suggest? Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device
2018-06-07 10:49 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
@ 2018-06-07 11:10 ` Cornelia Huck
0 siblings, 0 replies; 20+ messages in thread
From: Cornelia Huck @ 2018-06-07 11:10 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason J . Herne, Thomas Huth, Alexander Graf, qemu-devel,
Christian Borntraeger, qemu-s390x, Collin L . Walling,
Richard Henderson
On Thu, 7 Jun 2018 12:49:23 +0200
David Hildenbrand <david@redhat.com> wrote:
> On 04.06.2018 14:14, Cornelia Huck wrote:
> > On Mon, 4 Jun 2018 14:00:43 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >
> >> On 04.06.2018 13:38, Cornelia Huck wrote:
> >>> On Fri, 25 May 2018 13:37:02 +0200
> >>> David Hildenbrand <david@redhat.com> wrote:
> >>>
> >>>> Let's treat this like a separate device. TCG will have to store the
> >>>> actual state/time later on.
> >>>>
> >>>> Include cpu-qom.h in kvm_s390x.h (due to S390CPU) to compile tod-kvm.c.
> >>>
> >>> Can't you include it in tod-kvm.c instead? Why was it not needed before?
> >>
> >> Guess I can, kvm_s390x.h looks to me like the right place, as S390CPU is
> >> used in there.
> >>
> >> All other users include "cpu.h" already, so it isn't needed.
> >
> > Hm, and for kvm_s390_irq, this file is using a forward declaration,
> > which would likely be more awkward for this structure.
>
> I remember that we wanted to use cpu-qom.h for this typedef fun, that's
> why I decided to do it this way.
>
> >
> > Header dependencies are always fun :)
> >
>
> So what do you suggest? Thanks!
>
Let's go with cpu-qom.h for this, then.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v1 2/7] s390x/tcg: drop tod_basetime
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD David Hildenbrand
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
Never set to anything but 0.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu.c | 1 -
target/s390x/cpu.h | 1 -
target/s390x/misc_helper.c | 4 ++--
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 167d089722..557074165a 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -290,7 +290,6 @@ static void s390_cpu_initfn(Object *obj)
qemu_get_timedate(&tm, 0);
env->tod_offset = TOD_UNIX_EPOCH +
(time2tod(mktimegm(&tm)) * 1000000000ULL);
- env->tod_basetime = 0;
env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index ac51c17fb4..4abfe88a3d 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -131,7 +131,6 @@ struct CPUS390XState {
#endif
uint64_t tod_offset;
- uint64_t tod_basetime;
QEMUTimer *tod_timer;
QEMUTimer *cpu_timer;
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 1f834f35ef..044ffc64c4 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -142,7 +142,7 @@ uint64_t HELPER(stck)(CPUS390XState *env)
uint64_t time;
time = env->tod_offset +
- time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - env->tod_basetime);
+ time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
return time;
}
@@ -162,7 +162,7 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
/* nanoseconds */
time = tod2time(time);
- timer_mod(env->tod_timer, env->tod_basetime + time);
+ timer_mod(env->tod_timer, time);
}
/* Set Tod Programmable Field */
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 1/7] s390x/tod: factor out TOD into separate device David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 2/7] s390x/tcg: drop tod_basetime David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-06-04 11:57 ` Cornelia Huck
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 4/7] s390x/tcg: SET CLOCK COMPARATOR can clear CKC interrupts David Hildenbrand
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
Right now, each CPU has its own TOD. Especially, the TOD will differ
based on creation time of a CPU - e.g. when hotplugging a CPU the times
will differ quite a lot, resulting in stall warnings in the guest.
Let's use a single TOD by implementing our new TOD device. Prepare it
for TOD-clock epoch extension.
Most importantly, whenever we set the TOD, we have to update the CKC
timer.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
hw/s390x/tod-qemu.c | 45 ++++++++++++++++++++++++++++++++++----
hw/s390x/tod.c | 11 ++++++++++
include/hw/s390x/tod.h | 19 ++++++++++++++++
target/s390x/cpu.c | 8 +------
target/s390x/cpu.h | 6 ++++-
target/s390x/internal.h | 15 -------------
target/s390x/misc_helper.c | 32 ++++++++++++++++++++++-----
7 files changed, 103 insertions(+), 33 deletions(-)
diff --git a/hw/s390x/tod-qemu.c b/hw/s390x/tod-qemu.c
index 7997ba2b1a..41388f4c7d 100644
--- a/hw/s390x/tod-qemu.c
+++ b/hw/s390x/tod-qemu.c
@@ -11,17 +11,40 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/s390x/tod.h"
+#include "qemu/timer.h"
+#include "qemu/cutils.h"
+#include "cpu.h"
static void qemu_s390_tod_get(S390TODState *td, S390TOD *tod, Error **errp)
{
- /* FIXME */
- tod->high = 0;
- tod->low = 0;
+ *tod = td->base;
+
+ tod->low += time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ if (tod->low < td->base.low) {
+ tod->high++;
+ }
}
static void qemu_s390_tod_set(S390TODState *td, S390TOD *tod, Error **errp)
{
- /* FIXME */
+ CPUState *cpu;
+
+ td->base = *tod;
+
+ td->base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ if (tod->low < td->base.low) {
+ td->base.high--;
+ }
+
+ /*
+ * The TOD has been changed and we have to recalculate the CKC values
+ * for all CPUs. We do this asynchronously, as "SET CLOCK should be
+ * issued only while all other activity on all CPUs .. has been
+ * suspended".
+ */
+ CPU_FOREACH(cpu) {
+ async_run_on_cpu(cpu, tcg_s390_tod_updated, RUN_ON_CPU_NULL);
+ }
}
static void qemu_s390_tod_class_init(ObjectClass *oc, void *data)
@@ -32,10 +55,24 @@ static void qemu_s390_tod_class_init(ObjectClass *oc, void *data)
tdc->set = qemu_s390_tod_set;
}
+static void qemu_s390_tod_init(Object *obj)
+{
+ S390TODState *td = S390_TOD(obj);
+ struct tm tm;
+
+ qemu_get_timedate(&tm, 0);
+ td->base.high = 0;
+ td->base.low = TOD_UNIX_EPOCH + (time2tod(mktimegm(&tm)) * 1000000000ULL);
+ if (td->base.low < TOD_UNIX_EPOCH) {
+ td->base.high += 1;
+ }
+}
+
static TypeInfo qemu_s390_tod_info = {
.name = TYPE_QEMU_S390_TOD,
.parent = TYPE_S390_TOD,
.instance_size = sizeof(S390TODState),
+ .instance_init = qemu_s390_tod_init,
.class_init = qemu_s390_tod_class_init,
.class_size = sizeof(S390TODClass),
};
diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
index c1b3438452..7495d8c057 100644
--- a/hw/s390x/tod.c
+++ b/hw/s390x/tod.c
@@ -30,6 +30,17 @@ void s390_init_tod(void)
qdev_init_nofail(DEVICE(obj));
}
+S390TODState *s390_get_tod(void)
+{
+ static S390TODState *ts;
+
+ if (!ts) {
+ ts = S390_TOD(object_resolve_path_type("", TYPE_S390_TOD, NULL));
+ }
+
+ return ts;
+}
+
#define S390_TOD_CLOCK_VALUE_MISSING 0x00
#define S390_TOD_CLOCK_VALUE_PRESENT 0x01
diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
index 43ed71600f..5491245b86 100644
--- a/include/hw/s390x/tod.h
+++ b/include/hw/s390x/tod.h
@@ -30,6 +30,9 @@ typedef struct S390TOD {
typedef struct S390TODState {
/* private */
DeviceState parent_obj;
+
+ /* unused by KVM implementation */
+ S390TOD base;
} S390TODState;
typedef struct S390TODClass {
@@ -41,6 +44,22 @@ typedef struct S390TODClass {
void (*set)(S390TODState *td, S390TOD *tod, Error **errp);
} S390TODClass;
+/* The value of the TOD clock for 1.1.1970. */
+#define TOD_UNIX_EPOCH 0x7d91048bca000000ULL
+
+/* Converts ns to s390's clock format */
+static inline uint64_t time2tod(uint64_t ns)
+{
+ return (ns << 9) / 125;
+}
+
+/* Converts s390's clock format to ns */
+static inline uint64_t tod2time(uint64_t t)
+{
+ return (t * 125) >> 9;
+}
+
void s390_init_tod(void);
+S390TODState *s390_get_tod(void);
#endif
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 557074165a..e4d7e82a23 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -30,7 +30,6 @@
#include "kvm_s390x.h"
#include "sysemu/kvm.h"
#include "qemu-common.h"
-#include "qemu/cutils.h"
#include "qemu/timer.h"
#include "qemu/error-report.h"
#include "trace.h"
@@ -44,6 +43,7 @@
#include "hw/hw.h"
#include "sysemu/arch_init.h"
#include "sysemu/sysemu.h"
+#include "hw/s390x/s390-virtio-ccw.h"
#endif
#include "fpu/softfloat.h"
@@ -276,9 +276,6 @@ static void s390_cpu_initfn(Object *obj)
CPUState *cs = CPU(obj);
S390CPU *cpu = S390_CPU(obj);
CPUS390XState *env = &cpu->env;
-#if !defined(CONFIG_USER_ONLY)
- struct tm tm;
-#endif
cs->env_ptr = env;
cs->halted = 1;
@@ -287,9 +284,6 @@ static void s390_cpu_initfn(Object *obj)
s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
s390_cpu_model_register_props(obj);
#if !defined(CONFIG_USER_ONLY)
- qemu_get_timedate(&tm, 0);
- env->tod_offset = TOD_UNIX_EPOCH +
- (time2tod(mktimegm(&tm)) * 1000000000ULL);
env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 4abfe88a3d..810d09c4df 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -130,7 +130,6 @@ struct CPUS390XState {
uint64_t cpuid;
#endif
- uint64_t tod_offset;
QEMUTimer *tod_timer;
QEMUTimer *cpu_timer;
@@ -765,6 +764,11 @@ void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
/* service interrupts are floating therefore we must not pass an cpustate */
void s390_sclp_extint(uint32_t parm);
+
+/* misc_helper.c */
+void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque);
+
+
/* mmu_helper.c */
int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
int len, bool is_write);
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index e392a02d12..f2a771e2b4 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -237,21 +237,6 @@ enum cc_op {
CC_OP_MAX
};
-/* The value of the TOD clock for 1.1.1970. */
-#define TOD_UNIX_EPOCH 0x7d91048bca000000ULL
-
-/* Converts ns to s390's clock format */
-static inline uint64_t time2tod(uint64_t ns)
-{
- return (ns << 9) / 125;
-}
-
-/* Converts s390's clock format to ns */
-static inline uint64_t tod2time(uint64_t t)
-{
- return (t * 125) >> 9;
-}
-
static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
uint8_t *ar)
{
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 044ffc64c4..418326e551 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -40,6 +40,7 @@
#include "hw/s390x/ioinst.h"
#include "hw/s390x/s390-pci-inst.h"
#include "hw/boards.h"
+#include "hw/s390x/tod.h"
#endif
/* #define DEBUG_HELPER */
@@ -139,25 +140,36 @@ void HELPER(spx)(CPUS390XState *env, uint64_t a1)
/* Store Clock */
uint64_t HELPER(stck)(CPUS390XState *env)
{
- uint64_t time;
+ S390TODState *td = s390_get_tod();
+ S390TODClass *tdc = S390_TOD_GET_CLASS(td);
+ Error *err = NULL;
+ S390TOD tod;
- time = env->tod_offset +
- time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-
- return time;
+ tdc->get(td, &tod, &err);
+ g_assert(!err);
+ return tod.low;
}
/* Set Clock Comparator */
void HELPER(sckc)(CPUS390XState *env, uint64_t time)
{
+ S390TODState *td = s390_get_tod();
+ S390TODClass *tdc = S390_TOD_GET_CLASS(td);
+ Error *err = NULL;
+ S390TOD tod_base;
+
if (time == -1ULL) {
return;
}
env->ckc = time;
+ tdc->get(td, &tod_base, &err);
+ g_assert(!err);
+ tod_base.low -= time2tod(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+
/* difference between origins */
- time -= env->tod_offset;
+ time -= tod_base.low;
/* nanoseconds */
time = tod2time(time);
@@ -165,6 +177,14 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
timer_mod(env->tod_timer, time);
}
+void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
+{
+ S390CPU *cpu = S390_CPU(cs);
+ CPUS390XState *env = &cpu->env;
+
+ helper_sckc(env, env->ckc);
+}
+
/* Set Tod Programmable Field */
void HELPER(sckpf)(CPUS390XState *env, uint64_t r0)
{
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD David Hildenbrand
@ 2018-06-04 11:57 ` Cornelia Huck
2018-06-04 11:59 ` David Hildenbrand
0 siblings, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-06-04 11:57 UTC (permalink / raw)
To: David Hildenbrand
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On Fri, 25 May 2018 13:37:04 +0200
David Hildenbrand <david@redhat.com> wrote:
> Right now, each CPU has its own TOD. Especially, the TOD will differ
> based on creation time of a CPU - e.g. when hotplugging a CPU the times
> will differ quite a lot, resulting in stall warnings in the guest.
>
> Let's use a single TOD by implementing our new TOD device. Prepare it
> for TOD-clock epoch extension.
>
> Most importantly, whenever we set the TOD, we have to update the CKC
> timer.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> hw/s390x/tod-qemu.c | 45 ++++++++++++++++++++++++++++++++++----
> hw/s390x/tod.c | 11 ++++++++++
> include/hw/s390x/tod.h | 19 ++++++++++++++++
> target/s390x/cpu.c | 8 +------
> target/s390x/cpu.h | 6 ++++-
> target/s390x/internal.h | 15 -------------
> target/s390x/misc_helper.c | 32 ++++++++++++++++++++++-----
> 7 files changed, 103 insertions(+), 33 deletions(-)
> diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
> index 43ed71600f..5491245b86 100644
> --- a/include/hw/s390x/tod.h
> +++ b/include/hw/s390x/tod.h
> @@ -30,6 +30,9 @@ typedef struct S390TOD {
> typedef struct S390TODState {
> /* private */
> DeviceState parent_obj;
> +
> + /* unused by KVM implementation */
> + S390TOD base;
Does this need some kind of migration handling?
> } S390TODState;
>
> typedef struct S390TODClass {
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD
2018-06-04 11:57 ` Cornelia Huck
@ 2018-06-04 11:59 ` David Hildenbrand
0 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2018-06-04 11:59 UTC (permalink / raw)
To: Cornelia Huck
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On 04.06.2018 13:57, Cornelia Huck wrote:
> On Fri, 25 May 2018 13:37:04 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> Right now, each CPU has its own TOD. Especially, the TOD will differ
>> based on creation time of a CPU - e.g. when hotplugging a CPU the times
>> will differ quite a lot, resulting in stall warnings in the guest.
>>
>> Let's use a single TOD by implementing our new TOD device. Prepare it
>> for TOD-clock epoch extension.
>>
>> Most importantly, whenever we set the TOD, we have to update the CKC
>> timer.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> hw/s390x/tod-qemu.c | 45 ++++++++++++++++++++++++++++++++++----
>> hw/s390x/tod.c | 11 ++++++++++
>> include/hw/s390x/tod.h | 19 ++++++++++++++++
>> target/s390x/cpu.c | 8 +------
>> target/s390x/cpu.h | 6 ++++-
>> target/s390x/internal.h | 15 -------------
>> target/s390x/misc_helper.c | 32 ++++++++++++++++++++++-----
>> 7 files changed, 103 insertions(+), 33 deletions(-)
>
>> diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
>> index 43ed71600f..5491245b86 100644
>> --- a/include/hw/s390x/tod.h
>> +++ b/include/hw/s390x/tod.h
>> @@ -30,6 +30,9 @@ typedef struct S390TOD {
>> typedef struct S390TODState {
>> /* private */
>> DeviceState parent_obj;
>> +
>> + /* unused by KVM implementation */
>> + S390TOD base;
>
> Does this need some kind of migration handling?
This will be automatically migrated by migrating the TOD value itself now :)
>
>> } S390TODState;
>>
>> typedef struct S390TODClass {
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v1 4/7] s390x/tcg: SET CLOCK COMPARATOR can clear CKC interrupts
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
` (2 preceding siblings ...)
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 3/7] s390x/tcg: properly implement the TOD David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK David Hildenbrand
` (2 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
Let's stop the timer and delete any pending CKC IRQ before doing
anything else.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/misc_helper.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 418326e551..2ef9de1cca 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -158,6 +158,12 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
Error *err = NULL;
S390TOD tod_base;
+ /* stop the timer and remove pending CKC IRQs */
+ timer_del(env->tod_timer);
+ qemu_mutex_lock_iothread();
+ env->pending_int &= ~INTERRUPT_EXT_CLOCK_COMPARATOR;
+ qemu_mutex_unlock_iothread();
+
if (time == -1ULL) {
return;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
` (3 preceding siblings ...)
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 4/7] s390x/tcg: SET CLOCK COMPARATOR can clear CKC interrupts David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-06-04 12:05 ` Cornelia Huck
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration David Hildenbrand
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 7/7] s390x/tcg: fix CPU hotplug with single-threaded TCG David Hildenbrand
6 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
This allows a guest to change its TOD. We already take care of updating
all CKC timers from within S390TODClass.
Use MO_ALIGN to load the operand manually - this will properly trigger a
SPECIFICATION exception.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/helper.h | 1 +
target/s390x/insn-data.def | 3 +--
target/s390x/misc_helper.c | 19 +++++++++++++++++++
target/s390x/translate.c | 9 +++++++++
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 59cba86a27..97c60ca7bc 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -127,6 +127,7 @@ DEF_HELPER_4(diag, void, env, i32, i32, i32)
DEF_HELPER_3(load_psw, noreturn, env, i64, i64)
DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64)
DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env)
+DEF_HELPER_FLAGS_2(sck, TCG_CALL_NO_RWG, i32, env, i64)
DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64)
DEF_HELPER_FLAGS_2(sckpf, TCG_CALL_NO_RWG, void, env, i64)
DEF_HELPER_FLAGS_1(stckc, TCG_CALL_NO_RWG, i64, env)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 157619403d..5c6f33ed9c 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -997,8 +997,7 @@
/* SET ADDRESS SPACE CONTROL FAST */
C(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0)
/* SET CLOCK */
- /* ??? Not implemented - is it necessary? */
- C(0xb204, SCK, S, Z, 0, 0, 0, 0, 0, 0)
+ C(0xb204, SCK, S, Z, la2, 0, 0, 0, sck, 0)
/* SET CLOCK COMPARATOR */
C(0xb206, SCKC, S, Z, 0, m2_64a, 0, 0, sckc, 0)
/* SET CLOCK PROGRAMMABLE FIELD */
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 2ef9de1cca..2f5cd6d22a 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -191,6 +191,25 @@ void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
helper_sckc(env, env->ckc);
}
+/* Set Clock */
+uint32_t HELPER(sck)(CPUS390XState *env, uint64_t tod_low)
+{
+ S390TODState *td = s390_get_tod();
+ S390TODClass *tdc = S390_TOD_GET_CLASS(td);
+ S390TOD tod = {
+ .high = 0,
+ .low = tod_low,
+ };
+ Error *err = NULL;
+
+ qemu_mutex_lock_iothread();
+ tdc->set(td, &tod, &err);
+ qemu_mutex_unlock_iothread();
+ g_assert(!err);
+
+ return 0;
+}
+
/* Set Tod Programmable Field */
void HELPER(sckpf)(CPUS390XState *env, uint64_t r0)
{
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 82309faa11..c6ed9507de 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4016,6 +4016,15 @@ static DisasJumpType op_stcke(DisasContext *s, DisasOps *o)
return DISAS_NEXT;
}
+static DisasJumpType op_sck(DisasContext *s, DisasOps *o)
+{
+ check_privileged(s);
+ tcg_gen_qemu_ld_i64(o->in1, o->addr1, get_mem_index(s), MO_TEQ | MO_ALIGN);
+ gen_helper_sck(cc_op, cpu_env, o->in1);
+ set_cc_static(s);
+ return DISAS_NEXT;
+}
+
static DisasJumpType op_sckc(DisasContext *s, DisasOps *o)
{
check_privileged(s);
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK David Hildenbrand
@ 2018-06-04 12:05 ` Cornelia Huck
2018-06-07 10:48 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
0 siblings, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-06-04 12:05 UTC (permalink / raw)
To: David Hildenbrand
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On Fri, 25 May 2018 13:37:06 +0200
David Hildenbrand <david@redhat.com> wrote:
> This allows a guest to change its TOD. We already take care of updating
> all CKC timers from within S390TODClass.
>
> Use MO_ALIGN to load the operand manually - this will properly trigger a
> SPECIFICATION exception.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> target/s390x/helper.h | 1 +
> target/s390x/insn-data.def | 3 +--
> target/s390x/misc_helper.c | 19 +++++++++++++++++++
> target/s390x/translate.c | 9 +++++++++
> 4 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 2ef9de1cca..2f5cd6d22a 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -191,6 +191,25 @@ void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
> helper_sckc(env, env->ckc);
> }
>
> +/* Set Clock */
> +uint32_t HELPER(sck)(CPUS390XState *env, uint64_t tod_low)
> +{
> + S390TODState *td = s390_get_tod();
> + S390TODClass *tdc = S390_TOD_GET_CLASS(td);
Would it make sense to cache the class as well?
> + S390TOD tod = {
> + .high = 0,
> + .low = tod_low,
> + };
> + Error *err = NULL;
> +
> + qemu_mutex_lock_iothread();
> + tdc->set(td, &tod, &err);
> + qemu_mutex_unlock_iothread();
> + g_assert(!err);
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK
2018-06-04 12:05 ` Cornelia Huck
@ 2018-06-07 10:48 ` David Hildenbrand
2018-06-07 11:15 ` Cornelia Huck
0 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-06-07 10:48 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J . Herne, Thomas Huth, Alexander Graf, qemu-devel,
Christian Borntraeger, qemu-s390x, Collin L . Walling,
Richard Henderson
On 04.06.2018 14:05, Cornelia Huck wrote:
> On Fri, 25 May 2018 13:37:06 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> This allows a guest to change its TOD. We already take care of updating
>> all CKC timers from within S390TODClass.
>>
>> Use MO_ALIGN to load the operand manually - this will properly trigger a
>> SPECIFICATION exception.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> target/s390x/helper.h | 1 +
>> target/s390x/insn-data.def | 3 +--
>> target/s390x/misc_helper.c | 19 +++++++++++++++++++
>> target/s390x/translate.c | 9 +++++++++
>> 4 files changed, 30 insertions(+), 2 deletions(-)
>>
>
>> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
>> index 2ef9de1cca..2f5cd6d22a 100644
>> --- a/target/s390x/misc_helper.c
>> +++ b/target/s390x/misc_helper.c
>> @@ -191,6 +191,25 @@ void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
>> helper_sckc(env, env->ckc);
>> }
>>
>> +/* Set Clock */
>> +uint32_t HELPER(sck)(CPUS390XState *env, uint64_t tod_low)
>> +{
>> + S390TODState *td = s390_get_tod();
>> + S390TODClass *tdc = S390_TOD_GET_CLASS(td);
>
> Would it make sense to cache the class as well?
Probably, but I don't think this is as critical as the flic (as this
code is TCG only and the class conversion should be fairly fast compared
to a device lookup).
>
>> + S390TOD tod = {
>> + .high = 0,
>> + .low = tod_low,
>> + };
>> + Error *err = NULL;
>> +
>> + qemu_mutex_lock_iothread();
>> + tdc->set(td, &tod, &err);
>> + qemu_mutex_unlock_iothread();
>> + g_assert(!err);
>> +
>> + return 0;
>> +}
>> +
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK
2018-06-07 10:48 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
@ 2018-06-07 11:15 ` Cornelia Huck
0 siblings, 0 replies; 20+ messages in thread
From: Cornelia Huck @ 2018-06-07 11:15 UTC (permalink / raw)
To: David Hildenbrand
Cc: Jason J . Herne, Thomas Huth, Alexander Graf, qemu-devel,
Christian Borntraeger, qemu-s390x, Collin L . Walling,
Richard Henderson
On Thu, 7 Jun 2018 12:48:21 +0200
David Hildenbrand <david@redhat.com> wrote:
> On 04.06.2018 14:05, Cornelia Huck wrote:
> > On Fri, 25 May 2018 13:37:06 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >
> >> This allows a guest to change its TOD. We already take care of updating
> >> all CKC timers from within S390TODClass.
> >>
> >> Use MO_ALIGN to load the operand manually - this will properly trigger a
> >> SPECIFICATION exception.
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >> target/s390x/helper.h | 1 +
> >> target/s390x/insn-data.def | 3 +--
> >> target/s390x/misc_helper.c | 19 +++++++++++++++++++
> >> target/s390x/translate.c | 9 +++++++++
> >> 4 files changed, 30 insertions(+), 2 deletions(-)
> >>
> >
> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> >> index 2ef9de1cca..2f5cd6d22a 100644
> >> --- a/target/s390x/misc_helper.c
> >> +++ b/target/s390x/misc_helper.c
> >> @@ -191,6 +191,25 @@ void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
> >> helper_sckc(env, env->ckc);
> >> }
> >>
> >> +/* Set Clock */
> >> +uint32_t HELPER(sck)(CPUS390XState *env, uint64_t tod_low)
> >> +{
> >> + S390TODState *td = s390_get_tod();
> >> + S390TODClass *tdc = S390_TOD_GET_CLASS(td);
> >
> > Would it make sense to cache the class as well?
>
> Probably, but I don't think this is as critical as the flic (as this
> code is TCG only and the class conversion should be fairly fast compared
> to a device lookup).
Yes, it probably does not matter that much. Do as you like :)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
` (4 preceding siblings ...)
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 5/7] s390x/tcg: implement SET CLOCK David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
2018-06-04 12:09 ` Cornelia Huck
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 7/7] s390x/tcg: fix CPU hotplug with single-threaded TCG David Hildenbrand
6 siblings, 1 reply; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
If the CPU data is migrated after the TOD clock, the CKC timer of a CPU
is not rearmed. Let's rearm it when loading the CPU state.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/machine.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 84b4928755..53d226ff48 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -34,6 +34,13 @@ static int cpu_post_load(void *opaque, int version_id)
return kvm_s390_vcpu_interrupt_post_load(cpu);
}
+#ifdef CONFIG_TCG
+ if (tcg_enabled()) {
+ /* Rearm the CKC timer if necessary */
+ tcg_s390_tod_updated(CPU(cpu), RUN_ON_CPU_NULL);
+ }
+#endif
+
return 0;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration David Hildenbrand
@ 2018-06-04 12:09 ` Cornelia Huck
2018-06-07 10:47 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
0 siblings, 1 reply; 20+ messages in thread
From: Cornelia Huck @ 2018-06-04 12:09 UTC (permalink / raw)
To: David Hildenbrand
Cc: qemu-s390x, qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling
On Fri, 25 May 2018 13:37:07 +0200
David Hildenbrand <david@redhat.com> wrote:
> If the CPU data is migrated after the TOD clock, the CKC timer of a CPU
> is not rearmed. Let's rearm it when loading the CPU state.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> target/s390x/machine.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/target/s390x/machine.c b/target/s390x/machine.c
> index 84b4928755..53d226ff48 100644
> --- a/target/s390x/machine.c
> +++ b/target/s390x/machine.c
> @@ -34,6 +34,13 @@ static int cpu_post_load(void *opaque, int version_id)
> return kvm_s390_vcpu_interrupt_post_load(cpu);
> }
>
> +#ifdef CONFIG_TCG
> + if (tcg_enabled()) {
> + /* Rearm the CKC timer if necessary */
> + tcg_s390_tod_updated(CPU(cpu), RUN_ON_CPU_NULL);
Stub this out so the explicit CONFIG_TCG is not required? But I don't
care too much.
> + }
> +#endif
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration
2018-06-04 12:09 ` Cornelia Huck
@ 2018-06-07 10:47 ` David Hildenbrand
0 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2018-06-07 10:47 UTC (permalink / raw)
To: Cornelia Huck
Cc: Jason J . Herne, Thomas Huth, Alexander Graf, qemu-devel,
Christian Borntraeger, qemu-s390x, Collin L . Walling,
Richard Henderson
On 04.06.2018 14:09, Cornelia Huck wrote:
> On Fri, 25 May 2018 13:37:07 +0200
> David Hildenbrand <david@redhat.com> wrote:
>
>> If the CPU data is migrated after the TOD clock, the CKC timer of a CPU
>> is not rearmed. Let's rearm it when loading the CPU state.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> target/s390x/machine.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/target/s390x/machine.c b/target/s390x/machine.c
>> index 84b4928755..53d226ff48 100644
>> --- a/target/s390x/machine.c
>> +++ b/target/s390x/machine.c
>> @@ -34,6 +34,13 @@ static int cpu_post_load(void *opaque, int version_id)
>> return kvm_s390_vcpu_interrupt_post_load(cpu);
>> }
>>
>> +#ifdef CONFIG_TCG
>> + if (tcg_enabled()) {
>> + /* Rearm the CKC timer if necessary */
>> + tcg_s390_tod_updated(CPU(cpu), RUN_ON_CPU_NULL);
>
> Stub this out so the explicit CONFIG_TCG is not required? But I don't
> care too much.
Yes, can do!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v1 7/7] s390x/tcg: fix CPU hotplug with single-threaded TCG
2018-05-25 11:37 [Qemu-devel] [PATCH v1 0/7] s390x: TOD refactoring + TCG CPU hotplug support David Hildenbrand
` (5 preceding siblings ...)
2018-05-25 11:37 ` [Qemu-devel] [PATCH v1 6/7] s390x/tcg: rearm the CKC timer during migration David Hildenbrand
@ 2018-05-25 11:37 ` David Hildenbrand
6 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2018-05-25 11:37 UTC (permalink / raw)
To: qemu-s390x
Cc: qemu-devel, Richard Henderson, Alexander Graf, Cornelia Huck,
Christian Borntraeger, Thomas Huth, Jason J . Herne,
Collin L . Walling, david
run_on_cpu() doesn't seem to work reliably until the CPU has been fully
created if the single-threaded TCG main loop is already running.
Therefore, let's use run_on_cpu() for KVM only - KVM requires it due to
the initial CPU reset ioctl.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index e4d7e82a23..5fceed7948 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -220,11 +220,12 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
#endif
s390_cpu_gdb_init(cs);
qemu_init_vcpu(cs);
-#if !defined(CONFIG_USER_ONLY)
- run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
-#else
- cpu_reset(cs);
-#endif
+
+ if (kvm_enabled()) {
+ run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
+ } else {
+ cpu_reset(cs);
+ }
scc->parent_realize(dev, &err);
out:
--
2.17.0
^ permalink raw reply related [flat|nested] 20+ messages in thread