From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43620) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gAaBy-0004dy-Gc for qemu-devel@nongnu.org; Thu, 11 Oct 2018 08:35:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gAaBx-0002Ah-KU for qemu-devel@nongnu.org; Thu, 11 Oct 2018 08:35:50 -0400 References: <20180702111737.18970-1-cohuck@redhat.com> <20180702111737.18970-7-cohuck@redhat.com> <20181011143229.55b34fc3.cohuck@redhat.com> From: David Hildenbrand Message-ID: <240347e5-08da-52aa-2ee7-0789daeba613@redhat.com> Date: Thu, 11 Oct 2018 14:35:46 +0200 MIME-Version: 1.0 In-Reply-To: <20181011143229.55b34fc3.cohuck@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [qemu-s390x] [PULL 06/15] s390x/tod: factor out TOD into separate device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , Thomas Huth Cc: qemu-devel@nongnu.org, Christian Borntraeger , qemu-s390x@nongnu.org On 11/10/2018 14:32, Cornelia Huck wrote: > On Thu, 11 Oct 2018 14:15:41 +0200 > Thomas Huth wrote: > >> On 2018-07-02 13:17, Cornelia Huck wrote: >>> From: David Hildenbrand >>> >>> 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. >> >> This patch breaks "make check" when QEMU has been compiled with >> --disable-tcg on s390x: >> >> $ gdb --args s390x-softmmu/qemu-system-s390x -nographic -accel qtest >> [...] >> ERROR:qom/object.c:549:object_new_with_type: assertion failed: (type != NULL) >> >> Program received signal SIGABRT, Aborted. >> [...] >> (gdb) bt >> #0 0x000003fffcac1220 in raise () from /lib64/libc.so.6 >> #1 0x000003fffcac2aa8 in abort () from /lib64/libc.so.6 >> #2 0x000003fffd0fdd5e in g_assertion_message () from /lib64/libglib-2.0.so.0 >> #3 0x000003fffd0fddc6 in g_assertion_message_expr () from /lib64/libglib-2.0.so.0 >> #4 0x00000001002d4bf2 in object_new_with_type (type=0x0) >> qom/object.c:549 >> #5 0x00000001002d4d6e in object_new (typename=typename@entry=0x100490ce8 "s390-tod-qemu") >> at qom/object.c:563 >> #6 0x00000001001c04ee in s390_init_tod () at hw/s390x/tod.c:25 >> #7 0x0000000100233d00 in machine_run_board_init (machine=0x1008f5510) >> at hw/core/machine.c:834 >> #8 0x000000010012cb08 in main (argc=, argv=, envp=) >> at vl.c:4489 >> >>> 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 >> >> With --disable-tcg, tod-qemu.o is not included in the build. >> >> [...] >>> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c >>> new file mode 100644 >>> index 0000000000..0501affa7f >>> --- /dev/null >>> +++ b/hw/s390x/tod.c >>> @@ -0,0 +1,119 @@ >>> +/* >>> + * TOD (Time Of Day) clock >>> + * >>> + * Copyright 2018 Red Hat, Inc. >>> + * Author(s): David Hildenbrand >>> + * >>> + * 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); >>> + } >> >> ... but with "-accel qtest", QEMU tries to instantiate the object here. >> >> I think we should maybe simply inclue tod-qemu.o unconditionally? Opinions? > > If tod-qemu does not depend on tcg stuff, that sounds like the easiest > solution. > Or include it also for the (fake?) accel qtest instead of always. -- Thanks, David / dhildenb