Re: [Qemu-devel] [PATCH] hw/vfio: Add CONFIG switches for calxeda-xgmac and amd-xgbe

[Thomas Huth <thuth@redhat.com>]

On 31.01.2017 18:30, Alex Williamson wrote:
> On Tue, 31 Jan 2017 17:36:35 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> Both devices seem to be specific to the ARM platform. It's confusing
>> for the users if they show up on other target architectures, too
>> (e.g. when the user runs QEMU with "-device ?" to get a list of
>> supported devices). Thus let's introduce proper configuration switches
>> so that the devices are only compiled and included when they are
>> really required.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  default-configs/arm-softmmu.mak | 2 ++
>>  hw/vfio/Makefile.objs           | 4 ++--
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 6de3e16..a78be51 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -94,6 +94,8 @@ CONFIG_VERSATILE_PCI=y
>>  CONFIG_VERSATILE_I2C=y
>>  
>>  CONFIG_PCI_GENERIC=y
>> +CONFIG_VFIO_XGMAC=y
>> +CONFIG_VFIO_AMD_XGBE=y
>>  
>>  CONFIG_SDHCI=y
>>  CONFIG_INTEGRATOR_DEBUG=y
>> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
>> index c25e32b..05e7fbb 100644
>> --- a/hw/vfio/Makefile.objs
>> +++ b/hw/vfio/Makefile.objs
>> @@ -2,7 +2,7 @@ ifeq ($(CONFIG_LINUX), y)
>>  obj-$(CONFIG_SOFTMMU) += common.o
>>  obj-$(CONFIG_PCI) += pci.o pci-quirks.o
>>  obj-$(CONFIG_SOFTMMU) += platform.o
>> -obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
>> -obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
>> +obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
>> +obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
>>  obj-$(CONFIG_SOFTMMU) += spapr.o
>>  endif
> 
> I can't say that I fully agree that this is a good idea.  How many
> users are actually confused by this, versus the benefit of ensuring
> that it builds across all architectures?

Why do you want this to be build on all architectures? The devices are
only available on ARM as far as I know, so I can't see any real use for
this. And it slows down compilation time, too, if we compile it
everywhere (well, a little bit at least ;-)).

> Do we want to make platform also specific to ARM

I did not notice that one yet, since it does not show up in the "-device
?" help text (it's apparently an abstract device), so it also does not
really bother me.
But if I remove the device from the build, I get a funny error when I
try to view the device help text:

$ qemu-system-tricore -device ?
**
ERROR:/home/thuth/devel/qemu/qom/object.c:168:type_get_parent: assertion
failed: (type->parent_type != NULL)
Aborted (core dumped)

So I guess we should rather keep that one for now.

> and spapr specific to ppc64?

I already tried that (with the already existing CONFIG_PSERIES), but the
code in common.c depends on the code in spapr.c, so it can't be removed
without reworking the code quite a bit. But spapr.c also does not really
bug me, since it does not register a type that shows up in the "-device
?" help text, so I think it is not worth the effort here.

 Thomas