Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

[Cornelia Huck <cohuck@redhat.com>]

On Thu, 17 Aug 2017 14:35:53 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:
> > Hi David,
> > 
> > On 08/17/2017 06:22 AM, David Hildenbrand wrote:  
> >> Let's do it just like the other architectures. Introduce kvm-stub.c
> >> for stubs and kvm_s390x.h for the declarations.
> >>
> >> Add a fake declaration of struct kvm_s390_irq so we don't need other
> >> ugly CONFIG_KVM checks.  
> > 
> > You can use an opaque pointer to avoid that ("bridge" design pattern).
> > 
> > It involves few more changes but looks safer.  
> 
> There is maybe even a simpler solution than that, see below ...
>
> >> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> >> new file mode 100644
> >> index 0000000..35db28c
> >> --- /dev/null
> >> +++ b/target/s390x/kvm_s390x.h
> >> @@ -0,0 +1,51 @@
> >> +/*
> >> + * QEMU KVM support -- s390x specific functions.
> >> + *
> >> + * Copyright (c) 2009 Ulrich Hecht
> >> + *
> >> + * 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 KVM_S390X_H
> >> +#define KVM_S390X_H
> >> +
> >> +#include "sysemu/kvm.h"
> >> +
> >> +#ifndef CONFIG_KVM
> >> +struct kvm_s390_irq {};
> >> +#endif /* CONFIG_KVM */  
> > 
> > change by
> > 
> > typedef struct kvm_s390_irq kvm_s390_irq_t;  
> 
> May I suggest to simply use this instead:
> 
> struct kvm_s390_irq;
> 
> No need to switch for a typedef here, you can simply use this anonymous
> struct declaration, I think.

Yes, I think so.

I'm wondering if there are more cleanup opportunities...

We currently have the flic which is supposed to deal with floating
interrupts (in hw/intc/). It comes in a kvm (backed by a kvm device)
and a non-kvm flavour. Recent changes have introduced some adapter
interrupt handling in both the kvm device backing and in the non-kvm
flic.

Creation/injection of floating interrupts (like I/O interrupts) is
currently a bit scattered around the code, and the tcg code (which
predates the flic) does not have much in common with the kvm code. Can
we enhance the flic with one or more injection methods, move the tcg
interrupt creation into the non-kvm flic, and get rid of
kvm_s390_inject_flic()? Not sure how doable that is.