Re: [PATCH] hw: Include the VMWare devices only in the x86 targets

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/12/22 11:13, Thomas Huth wrote:
> On 13/12/2022 11.01, Philippe Mathieu-Daudé wrote:
>> On 13/12/22 10:51, Thomas Huth wrote:
>>> It seems a little bit weird that the para-virtualized x86 VMWare
>>> devices "vmware-svga" and "vmxnet3" also show up in non-x86 targets.
>>> They are likely pretty useless there (since the guest OSes likely
>>> do not have any drivers for those enabled), so let's change this and
>>> only enable those devices by default for the classical x86 targets.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   ...ate-Pseudo-encoding.txt => vnc-ledstate-pseudo-encoding.rst} | 0
>>>   rename docs/interop/{vnc-ledstate-Pseudo-encoding.txt => 
>>> vnc-ledstate-pseudo-encoding.rst} (100%)
>>>
>>> diff --git a/docs/interop/vnc-ledstate-Pseudo-encoding.txt 
>>> b/docs/interop/vnc-ledstate-pseudo-encoding.rst
>>> similarity index 100%
>>> rename from docs/interop/vnc-ledstate-Pseudo-encoding.txt
>>> rename to docs/interop/vnc-ledstate-pseudo-encoding.rst
>>
>> Unrelated change ;)
> 
> Yeah, sorry, just noticed it after sending the patch ... please ignore 
> that part.
> 
>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>>> index a1b159becd..7b3da68d1c 100644
>>> --- a/hw/display/Kconfig
>>> +++ b/hw/display/Kconfig
>>> @@ -55,7 +55,7 @@ config VGA_MMIO
>>>   config VMWARE_VGA
>>>       bool
>>> -    default y if PCI_DEVICES
>>> +    default y if PCI_DEVICES && PC_PCI
>>>       depends on PCI
>>>       select VGA
>>> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
>>> index 6d795ec752..1cc1c5775e 100644
>>> --- a/hw/net/Kconfig
>>> +++ b/hw/net/Kconfig
>>> @@ -51,7 +51,7 @@ config RTL8139_PCI
>>>   config VMXNET3_PCI
>>>       bool
>>> -    default y if PCI_DEVICES
>>> +    default y if PCI_DEVICES && PC_PCI
>>>       depends on PCI
>>
>> I'm not sure what PC_PCI is for, it seems inherited from the
>> first Makefile conversion.
>>
>> Are you sure you want to build this by default if the PC
>> machine is selected? An user could select it an non-X86 arch.
>>
>> Maybe we want:
>>
>> -       depends on PCI
>> +       depends on PCI && (I386 || X86_64)
>>
>> ?
> 
> It does not seem to be a hard dependency - apparently the devices 
> compile fine for non-x86 and I can also run:
> 
>   qemu-system-ppc64 -device vmware-vga -device vmxnet3
> 
> and the guest sees these two PCI devices - it just can't use them since 
> it has no drivers.
> 
> So for the unlikely case that someone still wants to use these devices 
> on non-x86 machines, I think it's better to add the test to the "default 
> y" line instead of the "depends on" line, so that users still have a 
> chance to enable it in their config file before compiling.

OK then,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>