[Qemu-devel] Re: [PATCH 0/6] Make hpet a compile time option

[Jan Kiszka]

Juan Quintela wrote:
> Hi
> 
> This series:
> a- bring back the support for config-devices.h
> 
>    Paul was the one that removed my previous submission.
>    You can see on the last patch why I want config-devices.h
> 
> b- move all hpet code to hpet.c/hpet_emul.h
> 
>    In the last patch, I add CONFIG_HPET define, and made everything
>    depending on that define.  When we have !CONFIG_HPET we just create
>    stub functions.
> 
> This was my idea to create config-devices.h in the first place.
> 
> Paul, if you don't like it, I am open to alternatives.  Problem here
> is that there are devices that we don't want to ship in RHEL for one
> reason or another.  Solutions so far:
> 
> - use Makefile/ifdefs in calling files trickery: this is ugly as hell
>   and was refused (I sent patches to do that ~1 year ago).
> 
> I was told to use CONFIG_FOO to disable the full compilation and a
> config file, like the kernel.
> 
> Here we go, then Paul removed the config-device.h part, because:
> 
>>   Also remove config-devices.h.  Code does not and should not care which
>>   devices are being built.
> 
> This is ok, when device is only called through qdev, and then it is trivial
> to compile out.
> 
> Notice that that there are another cases where we have to do Makefile tricks
> just because we don't have config-devices.mak symbols in "C-land".
> 
> See for example Makefile.target
> 
> CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
> 
> obj-$(CONFIG_KVM) += kvm.o kvm-all.o
> obj-$(CONFIG_NO_KVM) += kvm-stub.o
> 
> In this case makes sense to have an stub because there are lots of
> functions, but in the hpet case there are only two functions that are
> exported.  My problem with the "stub file" way is that we are going to
> end with three files by device (foo.c, foo-stub.c, foo.h).  Where
> foo-stub.c is basically trivial.
> 
> I also removed the "info hpet" command.  I can be convinced that it is better 
> to change its
> output to
>        HPET is disabled by QEMU
> or
>        HPET is not present
> or any other stirng.
> 
> Comments?  Any better suggestion?

Unless this is deadly urgent, please hold it back until we sorted out
some more fundamental issues with the HPET, specifically ported it to qdev.

But I'm also not convinced about the general approach. Except for RHEL
packagers, no one seems to gain any benefit from having CONFIG_HPET. The
HPET model is still incomplete in has some remaining quicks (hold on for
improvements), but that doesn't qualify it for !CONFIG_HPET,
specifically as it is deeply hooked into every modern PC. If I was
asked, I guess I would nack this switch.

Jan

signature.asc
Description: OpenPGP digital signature