On 21.08.2017 12:08, Cornelia Huck wrote: > On Fri, 18 Aug 2017 13:43:43 +0200 > David Hildenbrand <[email protected]> wrote: > >> Signed-off-by: David Hildenbrand <[email protected]> >> --- >> hw/s390x/s390-virtio.h | 2 ++ >> target/s390x/cpu.h | 3 --- >> 2 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h >> index f2377a3..ca97fd6 100644 >> --- a/hw/s390x/s390-virtio.h >> +++ b/hw/s390x/s390-virtio.h >> @@ -30,4 +30,6 @@ void s390_create_virtio_net(BusState *bus, const char >> *name); >> void s390_nmi(NMIState *n, int cpu_index, Error **errp); >> void s390_machine_reset(void); >> void s390_memory_init(ram_addr_t mem_size); >> +void gtod_save(QEMUFile *f, void *opaque); >> +int gtod_load(QEMUFile *f, void *opaque, int version_id); >> #endif >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 3ce7ffc..c40d70d 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -594,9 +594,6 @@ static inline uint8_t s390_cpu_get_state(S390CPU *cpu) >> return cpu->env.cpu_state; >> } >> >> -void gtod_save(QEMUFile *f, void *opaque); >> -int gtod_load(QEMUFile *f, void *opaque, int version_id); >> - >> void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param, >> uint64_t param64); >> > > This patch prompted me to look at the contents of s390-virtio.[ch]. > Many of the functions in there only made sense as an exported interface > when we still had the old s390 machine, but they can now simply be > moved to the only user (s390-virtio-ccw.c).
Yes, that is true. > > In s390-virtio.c, the only thing used outside of s390-virtio-ccw.c is > s390_cpuaddr2state(), and the only place that uses it for something > other than getting a dummy cpu is the kvm sigp target code. Can we > replace that last usage with a different construct? As CPUs are stored in s390-virtio.c (S390CPU **cpu_states) this is not possible. We could only get access to cpu #x via qom /machine/cpu[#x], but I guess that won't have best performance :) We could move that definition into the machine state (which would make sense, as the cpus belong to a machine). > > In s390-virtio.h, the s390_register_virtio_hypercall() interface is the > only thing that still makes sense to be exported. > Anyhow, I would prefer to have these cleanups in a separate series. Nevertheless they make perfect sense. -- Thanks, David
