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

[Auger Eric]

Hi Thomas,

On 31/01/2017 19:51, Thomas Huth wrote:
> On 31.01.2017 18:30, Alex Williamson wrote:
>> On Tue, 31 Jan 2017 17:36:35 +0100
>> Thomas Huth <address@hidden> 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 <address@hidden>
>>> ---
>>>  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.

FYI I will send patches to make VFIO platform device non abstract.

Thanks

Eric
> 
>> 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
>