| Message ID | 1290522889-27559-11-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 23, 2010 at 2:34 PM, Alexander Graf <agraf@suse.de> wrote: > So far we have C preprocessor defines for target and host config > options, but we're lacking any information on which devices are > available. > > We do need that information at times though, for example in the > ahci patch where we need to call a legacy init function depending > on whether we have support compiled in or not. > > So this patch makes all config-devices options available as header > file. Please only include it in machine description code! How about including config-devices.h from hw/boards.h instead? On the downside, all targets will be recompiled when config-devices.h change, even if they don't have any device dependencies.
On 23.11.2010, at 20:21, Blue Swirl wrote: > On Tue, Nov 23, 2010 at 2:34 PM, Alexander Graf <agraf@suse.de> wrote: >> So far we have C preprocessor defines for target and host config >> options, but we're lacking any information on which devices are >> available. >> >> We do need that information at times though, for example in the >> ahci patch where we need to call a legacy init function depending >> on whether we have support compiled in or not. >> >> So this patch makes all config-devices options available as header >> file. Please only include it in machine description code! > > How about including config-devices.h from hw/boards.h instead? On the > downside, all targets will be recompiled when config-devices.h change, > even if they don't have any device dependencies. Not sure it makes sense to do this change this time around. If we can avoid dependency on device configuration, we should. So if we can limit the scope this is included in, it's good IMHO. In fact, maybe the correct solution to the issue would be to move the legacy -drive instantiation to the -drive code, as it is already with virtio-blk. If used with -device and -drive ...,if=none the whole magic isn't necessary anyways. Alex
Alexander Graf <agraf@suse.de> wrote: > So far we have C preprocessor defines for target and host config > options, but we're lacking any information on which devices are > available. > > We do need that information at times though, for example in the > ahci patch where we need to call a legacy init function depending > on whether we have support compiled in or not. > > So this patch makes all config-devices options available as header > file. Please only include it in machine description code! > > Signed-off-by: Alexander Graf <agraf@suse.de> This already existed in the past, Paul Brook reverted it. commit a992fe3d0fc185112677286f7a02204d8245b61e Author: Paul Brook <paul@codesourcery.com> Date: Sun Nov 22 16:25:30 2009 +0000 Makefile dependencies for device configs I did the 1st implementation, and I tried to get it back several times. Paul Brook reasoning is that "if you need it, our device model is wrong. If you show that you want to be able to compile out several devices, ... Paul Brook: "if you need it, our device model is wrong" If you don't want to have the abomination of xen-stub.c kvm-stub.c, etc.... Paul Brook: "if you need it, our device model is wrong". Last time I tried to get it back was to be able to compile out hpet (it didn't worked well at the time). I let you guess how it ended. Later, Juan.
On 24.11.2010, at 03:22, Juan Quintela wrote: > Alexander Graf <agraf@suse.de> wrote: >> So far we have C preprocessor defines for target and host config >> options, but we're lacking any information on which devices are >> available. >> >> We do need that information at times though, for example in the >> ahci patch where we need to call a legacy init function depending >> on whether we have support compiled in or not. >> >> So this patch makes all config-devices options available as header >> file. Please only include it in machine description code! >> >> Signed-off-by: Alexander Graf <agraf@suse.de> > > This already existed in the past, Paul Brook reverted it. > > commit a992fe3d0fc185112677286f7a02204d8245b61e > Author: Paul Brook <paul@codesourcery.com> > Date: Sun Nov 22 16:25:30 2009 +0000 > > Makefile dependencies for device configs > > I did the 1st implementation, and I tried to get it back several times. > Paul Brook reasoning is that "if you need it, our device model is wrong. > > If you show that you want to be able to compile out several devices, ... > > Paul Brook: "if you need it, our device model is wrong" > > If you don't want to have the abomination of xen-stub.c kvm-stub.c, > etc.... > > Paul Brook: "if you need it, our device model is wrong". > > Last time I tried to get it back was to be able to compile out hpet (it > didn't worked well at the time). > > I let you guess how it ended. Sure, I only want to keep it in as a temporary solution in cases where overhauling the device model needs a lot of thorough thought and considerations. As I stated for this particular issue, maybe the way forward is to move all the initialization code over to -drive instead of keeping it close to the device. But I'm not sure yet. Hence this very little intrusive patch. Alex
On Wed, Nov 24, 2010 at 10:54 AM, Alexander Graf <agraf@suse.de> wrote: > > On 24.11.2010, at 03:22, Juan Quintela wrote: > >> Alexander Graf <agraf@suse.de> wrote: >>> So far we have C preprocessor defines for target and host config >>> options, but we're lacking any information on which devices are >>> available. >>> >>> We do need that information at times though, for example in the >>> ahci patch where we need to call a legacy init function depending >>> on whether we have support compiled in or not. >>> >>> So this patch makes all config-devices options available as header >>> file. Please only include it in machine description code! >>> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >> >> This already existed in the past, Paul Brook reverted it. >> >> commit a992fe3d0fc185112677286f7a02204d8245b61e >> Author: Paul Brook <paul@codesourcery.com> >> Date: Sun Nov 22 16:25:30 2009 +0000 >> >> Makefile dependencies for device configs >> >> I did the 1st implementation, and I tried to get it back several times. >> Paul Brook reasoning is that "if you need it, our device model is wrong. >> >> If you show that you want to be able to compile out several devices, ... >> >> Paul Brook: "if you need it, our device model is wrong" >> >> If you don't want to have the abomination of xen-stub.c kvm-stub.c, >> etc.... >> >> Paul Brook: "if you need it, our device model is wrong". >> >> Last time I tried to get it back was to be able to compile out hpet (it >> didn't worked well at the time). >> >> I let you guess how it ended. > > Sure, I only want to keep it in as a temporary solution in cases where overhauling the device model needs a lot of thorough thought and considerations. > > As I stated for this particular issue, maybe the way forward is to move all the initialization code over to -drive instead of keeping it close to the device. But I'm not sure yet. Hence this very little intrusive patch. I read the config-devices.h thread and I agree that it's a bad idea. So, I'm also NACKing this patch. Please adjust the code so that CONFIG_AHCI is no longer needed.
diff --git a/Makefile.target b/Makefile.target index 91e6e74..35862fd 100644 --- a/Makefile.target +++ b/Makefile.target @@ -1,6 +1,6 @@ # -*- Mode: makefile -*- -GENERATED_HEADERS = config-target.h +GENERATED_HEADERS = config-target.h config-devices.h CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y) include ../config-host.mak @@ -40,6 +40,9 @@ kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS) config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak +config-devices.h: config-target.h-timestamp +config-devices.h-timestamp: config-target.mak + all: $(PROGS) # Dummy command so that make thinks it has done something diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 12359a7..2be25a6 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -35,6 +35,7 @@ #include "sysemu.h" #include "sysbus.h" #include "blockdev.h" +#include "config-devices.h" #define MAX_IDE_BUS 2
So far we have C preprocessor defines for target and host config options, but we're lacking any information on which devices are available. We do need that information at times though, for example in the ahci patch where we need to call a legacy init function depending on whether we have support compiled in or not. So this patch makes all config-devices options available as header file. Please only include it in machine description code! Signed-off-by: Alexander Graf <agraf@suse.de> --- v3 -> v4: - config: only include config-devices.h in machine description (blue swirl) --- Makefile.target | 5 ++++- hw/pc_piix.c | 1 + 2 files changed, 5 insertions(+), 1 deletions(-)