Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw

[Halil Pasic <pasic@linux.ibm.com>]

On Thu, 18 Feb 2021 14:38:20 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

> > > The explanation is simple. Unlike most devices, the ccw devices aren't
> > > portable. In particular both css.c and css.h includes "cpu.h", and
> > > virtio-ccw-gpu.c includes "qemu/osdep.h". Furthermore osdep.h contains:
> > > #ifdef NEED_CPU_H
> > > #include CONFIG_TARGET
> > > #else
> > > #include "exec/poison.h"
> > > #endif
> > > so if we don't have NEED_CPU_H, among others CONFIG_KVM is poisoned, and
> > > CONFIG_KVM is used in "css.h". Frankly, I can't tell under what circumstances
> > > does css need "cpu.h".   
> > 
> > s390_crw_mchk() and s390_io_interrupt() are in cpu.h. Nowadays, they
> > use the flic to inject interrupts; but their earlier implementations
> > had a dummy cpu state.
> > 
> > I'm wondering whether s390_flic.h is a better place for functions
> > injecting floating interrupts, now that we don't have the non-flic
> > support anymore.  
> 
> Sounds like the easiest way forward.  Alternatively add support for
> target-specific modules (which we don't really have right now).

Thanks Gerd! 

Now I realize what do you mean by support for target-specific modules.
I'm mostly concerned with the s390x targets and I didn't have a good
enough understanding of this. I didn't realize the modules are shared
for all targets, that's why I've tried to build it only for s390x
targets.

I don't see way around target-specific modules. With the modifications
suggested by Thomas and Connie, I was able to get the new module to
compile regardless of the target, but that "fixes" s390x at the expense
of breaking all the other targets. For example:
./qemu-system-x86_64 -device help
Type 'virtio-gpu-ccw' is missing its parent 'virtio-ccw-device'
Aborted
because each target specific qemu will try to load my module. For s390x
it will work as expected, for everybody else not at all.

Making the list of modules in module.c depend on the target, i.e.
something like
     { "vhost-user-gpu-pci",    "hw-", "display-virtio-gpu-pci" }
+#ifdef TARGET_S390X
     { "virtio-gpu-ccw",        "hw-", "s390x-virtio-gpu-ccw"   },
+#endif
doesn't look viable either.

Since you are the author of 28457744c3 ("module: qom module support") and
7b0de5b796 ("virtio-gpu: build modular"), before I start working on
target-specific modules I would like to ask you, what is in your opinion
the best way to implement these?

Regards,
Halil