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

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

[Thomas Huth]

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
 hw/display/Kconfig                                              | 2 +-
 hw/net/Kconfig                                                  | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 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
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
 
 config SMC91C111
-- 
2.31.1

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

[Philippe Mathieu-Daudé]

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 ;)

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

?

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

[Thomas Huth]

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

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

[Philippe Mathieu-Daudé]

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>

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

[Thomas Huth]

On 13/12/2022 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
>   hw/display/Kconfig                                              | 2 +-
>   hw/net/Kconfig                                                  | 2 +-
>   3 files changed, 2 insertions(+), 2 deletions(-)
>   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

Sorry, that change in the docs directory does not belong to this patch, 
please ignore that part.

  Thomas