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

[Thomas Huth <thuth@redhat.com>]

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.

  Thomas