| Message ID | 20221213095144.42355-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Include the VMWare devices only in the x86 targets | expand |
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) ?
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
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
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>
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
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%)