configs/devices/s390x-softmmu/default.mak | 1 + hw/net/Kconfig | 4 ++-- hw/pci/Kconfig | 3 +++ hw/s390x/Kconfig | 3 ++- hw/usb/Kconfig | 2 +- 5 files changed, 9 insertions(+), 4 deletions(-)
It is useful to extend the number of available PCIe devices to KVM guests
for passthrough scenarios and also to expose these models to a different
(big endian) architecture. Introduce a new config PCIE_DEVICES to select
models, Intel Ethernet adapters and one USB controller. These devices all
support MSI-X which is a requirement on s390x as legacy INTx are not
supported.
Cc: Matthew Rosato <mjrosato@linux.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
There could be a more general use of PCIE_DEVICES
v4: Introduce PCIE_DEVICES
v3: PCI -> PCI_EXPRESS
v2: select -> imply
configs/devices/s390x-softmmu/default.mak | 1 +
hw/net/Kconfig | 4 ++--
hw/pci/Kconfig | 3 +++
hw/s390x/Kconfig | 3 ++-
hw/usb/Kconfig | 2 +-
5 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/configs/devices/s390x-softmmu/default.mak b/configs/devices/s390x-softmmu/default.mak
index f2287a133f36..2d5ff476e32a 100644
--- a/configs/devices/s390x-softmmu/default.mak
+++ b/configs/devices/s390x-softmmu/default.mak
@@ -7,6 +7,7 @@
#CONFIG_VFIO_CCW=n
#CONFIG_VIRTIO_PCI=n
#CONFIG_WDT_DIAG288=n
+#CONFIG_PCIE_DEVICE=n
# Boards:
#
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 98e00be4f937..7fcc0d7faa29 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -41,12 +41,12 @@ config E1000_PCI
config E1000E_PCI_EXPRESS
bool
- default y if PCI_DEVICES
+ default y if PCI_DEVICES || PCIE_DEVICES
depends on PCI_EXPRESS && MSI_NONBROKEN
config IGB_PCI_EXPRESS
bool
- default y if PCI_DEVICES
+ default y if PCI_DEVICES || PCIE_DEVICES
depends on PCI_EXPRESS && MSI_NONBROKEN
config RTL8139_PCI
diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
index 77f8b005ffb1..fe70902cd821 100644
--- a/hw/pci/Kconfig
+++ b/hw/pci/Kconfig
@@ -8,6 +8,9 @@ config PCI_EXPRESS
config PCI_DEVICES
bool
+config PCIE_DEVICES
+ bool
+
config MSI_NONBROKEN
# selected by interrupt controllers that do not support MSI,
# or support it and have a good implementation. See commit
diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 454e0ff4b613..4c068d7960b9 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -5,7 +5,8 @@ config S390_CCW_VIRTIO
imply VFIO_AP
imply VFIO_CCW
imply WDT_DIAG288
- select PCI
+ imply PCIE_DEVICES
+ select PCI_EXPRESS
select S390_FLIC
select S390_FLIC_KVM if KVM
select SCLPCONSOLE
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 0ec6def4b8b8..0f486764ed69 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -36,7 +36,7 @@ config USB_XHCI
config USB_XHCI_PCI
bool
- default y if PCI_DEVICES
+ default y if PCI_DEVICES || PCIE_DEVICES
depends on PCI
select USB_XHCI
--
2.41.0
> diff --git a/configs/devices/s390x-softmmu/default.mak b/configs/devices/s390x-softmmu/default.mak > index f2287a133f36..2d5ff476e32a 100644 > --- a/configs/devices/s390x-softmmu/default.mak > +++ b/configs/devices/s390x-softmmu/default.mak > @@ -7,6 +7,7 @@ > #CONFIG_VFIO_CCW=n > #CONFIG_VIRTIO_PCI=n > #CONFIG_WDT_DIAG288=n > +#CONFIG_PCIE_DEVICE=n Should be CONFIG_PCIE_DEVICES; fixed and queued, thanks. Paolo
On 12/07/2023 14.11, Paolo Bonzini wrote: >> diff --git a/configs/devices/s390x-softmmu/default.mak b/configs/devices/s390x-softmmu/default.mak >> index f2287a133f36..2d5ff476e32a 100644 >> --- a/configs/devices/s390x-softmmu/default.mak >> +++ b/configs/devices/s390x-softmmu/default.mak >> @@ -7,6 +7,7 @@ >> #CONFIG_VFIO_CCW=n >> #CONFIG_VIRTIO_PCI=n >> #CONFIG_WDT_DIAG288=n >> +#CONFIG_PCIE_DEVICE=n > > Should be CONFIG_PCIE_DEVICES; fixed and queued, thanks. I think the patch is fine for now, but in the long run, I think we should make sure to mark PCIe-only devices only with PCIE_DEVICES, as Philippe suggested. So PCIe devices will then also not show up any more on targets that only provide a plain PCI bus (like hppa, I guess?). Thomas
On Wed, Jul 12, 2023 at 2:57 PM Thomas Huth <thuth@redhat.com> wrote: > I think the patch is fine for now, but in the long run, I think we should > make sure to mark PCIe-only devices only with PCIE_DEVICES, as Philippe > suggested. So PCIe devices will then also not show up any more on targets > that only provide a plain PCI bus (like hppa, I guess?). Or even "-M pc", I think. Paolo
Hi Cédric,
On 12/7/23 10:01, Cédric Le Goater wrote:
> It is useful to extend the number of available PCIe devices to KVM guests
> for passthrough scenarios and also to expose these models to a different
> (big endian) architecture. Introduce a new config PCIE_DEVICES to select
> models, Intel Ethernet adapters and one USB controller. These devices all
> support MSI-X which is a requirement on s390x as legacy INTx are not
> supported.
>
> Cc: Matthew Rosato <mjrosato@linux.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>
> There could be a more general use of PCIE_DEVICES
>
> v4: Introduce PCIE_DEVICES
> v3: PCI -> PCI_EXPRESS
> v2: select -> imply
>
> configs/devices/s390x-softmmu/default.mak | 1 +
> hw/net/Kconfig | 4 ++--
> hw/pci/Kconfig | 3 +++
> hw/s390x/Kconfig | 3 ++-
> hw/usb/Kconfig | 2 +-
> 5 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/configs/devices/s390x-softmmu/default.mak b/configs/devices/s390x-softmmu/default.mak
> index f2287a133f36..2d5ff476e32a 100644
> --- a/configs/devices/s390x-softmmu/default.mak
> +++ b/configs/devices/s390x-softmmu/default.mak
> @@ -7,6 +7,7 @@
> #CONFIG_VFIO_CCW=n
> #CONFIG_VIRTIO_PCI=n
> #CONFIG_WDT_DIAG288=n
> +#CONFIG_PCIE_DEVICE=n
>
> # Boards:
> #
> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
> index 98e00be4f937..7fcc0d7faa29 100644
> --- a/hw/net/Kconfig
> +++ b/hw/net/Kconfig
> @@ -41,12 +41,12 @@ config E1000_PCI
>
> config E1000E_PCI_EXPRESS
> bool
> - default y if PCI_DEVICES
> + default y if PCI_DEVICES || PCIE_DEVICES
There seems to be a pre-existing bug, shouldn't this be
default y if PCIE_DEVICES
?
(Cc'ing maintainers)
> depends on PCI_EXPRESS && MSI_NONBROKEN
>
> config IGB_PCI_EXPRESS
> bool
> - default y if PCI_DEVICES
> + default y if PCI_DEVICES || PCIE_DEVICES
Similarly:
default y if PCIE_DEVICES
> depends on PCI_EXPRESS && MSI_NONBROKEN
>
> config RTL8139_PCI
> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
> index 77f8b005ffb1..fe70902cd821 100644
> --- a/hw/pci/Kconfig
> +++ b/hw/pci/Kconfig
> @@ -8,6 +8,9 @@ config PCI_EXPRESS
> config PCI_DEVICES
> bool
>
> +config PCIE_DEVICES
> + bool
> +
> config MSI_NONBROKEN
> # selected by interrupt controllers that do not support MSI,
> # or support it and have a good implementation. See commit
> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
> index 454e0ff4b613..4c068d7960b9 100644
> --- a/hw/s390x/Kconfig
> +++ b/hw/s390x/Kconfig
> @@ -5,7 +5,8 @@ config S390_CCW_VIRTIO
> imply VFIO_AP
> imply VFIO_CCW
> imply WDT_DIAG288
> - select PCI
> + imply PCIE_DEVICES
> + select PCI_EXPRESS
I'm confused, TYPE_S390_PCI_HOST_BRIDGE exposes a PCI bus...
At a minimum you'd need:
-- >8 --
static const TypeInfo s390_pcihost_info = {
.name = TYPE_S390_PCI_HOST_BRIDGE,
- .parent = TYPE_PCI_HOST_BRIDGE,
+ .parent = TYPE_PCIE_HOST_BRIDGE,
.instance_size = sizeof(S390pciState),
.class_init = s390_pcihost_class_init,
.interfaces = (InterfaceInfo[]) {
---
Actually I can see:
if (s390_pci_msix_init(pbdev) && !pbdev->interp) {
error_setg(errp, "MSI-X support is mandatory "
"in the S390 architecture");
return;
}
So this must be PCIe, not legacy PCI, right?
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 0ec6def4b8b8..0f486764ed69 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -36,7 +36,7 @@ config USB_XHCI
>
> config USB_XHCI_PCI
> bool
> - default y if PCI_DEVICES
> + default y if PCI_DEVICES || PCIE_DEVICES
TYPE_XHCI_PCI inherits TYPE_PCI_DEVICE and implements
INTERFACE_PCIE_DEVICE, so this is OK.
> depends on PCI
> select USB_XHCI
>
On 2023/07/12 19:48, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 12/7/23 10:01, Cédric Le Goater wrote:
>> It is useful to extend the number of available PCIe devices to KVM guests
>> for passthrough scenarios and also to expose these models to a different
>> (big endian) architecture. Introduce a new config PCIE_DEVICES to select
>> models, Intel Ethernet adapters and one USB controller. These devices all
>> support MSI-X which is a requirement on s390x as legacy INTx are not
>> supported.
>>
>> Cc: Matthew Rosato <mjrosato@linux.ibm.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>
>> There could be a more general use of PCIE_DEVICES
>>
>> v4: Introduce PCIE_DEVICES
>> v3: PCI -> PCI_EXPRESS
>> v2: select -> imply
>> configs/devices/s390x-softmmu/default.mak | 1 +
>> hw/net/Kconfig | 4 ++--
>> hw/pci/Kconfig | 3 +++
>> hw/s390x/Kconfig | 3 ++-
>> hw/usb/Kconfig | 2 +-
>> 5 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/configs/devices/s390x-softmmu/default.mak
>> b/configs/devices/s390x-softmmu/default.mak
>> index f2287a133f36..2d5ff476e32a 100644
>> --- a/configs/devices/s390x-softmmu/default.mak
>> +++ b/configs/devices/s390x-softmmu/default.mak
>> @@ -7,6 +7,7 @@
>> #CONFIG_VFIO_CCW=n
>> #CONFIG_VIRTIO_PCI=n
>> #CONFIG_WDT_DIAG288=n
>> +#CONFIG_PCIE_DEVICE=n
>> # Boards:
>> #
>> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
>> index 98e00be4f937..7fcc0d7faa29 100644
>> --- a/hw/net/Kconfig
>> +++ b/hw/net/Kconfig
>> @@ -41,12 +41,12 @@ config E1000_PCI
>> config E1000E_PCI_EXPRESS
>> bool
>> - default y if PCI_DEVICES
>> + default y if PCI_DEVICES || PCIE_DEVICES
>
> There seems to be a pre-existing bug, shouldn't this be
>
> default y if PCIE_DEVICES
>
> ?
I think you should leave this as is and instead add a config selected
only when legacy PCI is available and make all legacy PCI devices depend
on the config. This will prevent from selecting legacy PCI devices for
s390x machines no matter if it's selected due to PCI_DEVICES or selected
manually by the user (by mistake).
>
> (Cc'ing maintainers)
>
>> depends on PCI_EXPRESS && MSI_NONBROKEN
>> config IGB_PCI_EXPRESS
>> bool
>> - default y if PCI_DEVICES
>> + default y if PCI_DEVICES || PCIE_DEVICES
>
> Similarly:
>
> default y if PCIE_DEVICES
>
>> depends on PCI_EXPRESS && MSI_NONBROKEN
>> config RTL8139_PCI
>> diff --git a/hw/pci/Kconfig b/hw/pci/Kconfig
>> index 77f8b005ffb1..fe70902cd821 100644
>> --- a/hw/pci/Kconfig
>> +++ b/hw/pci/Kconfig
>> @@ -8,6 +8,9 @@ config PCI_EXPRESS
>> config PCI_DEVICES
>> bool
>> +config PCIE_DEVICES
>> + bool
>> +
>> config MSI_NONBROKEN
>> # selected by interrupt controllers that do not support MSI,
>> # or support it and have a good implementation. See commit
>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>> index 454e0ff4b613..4c068d7960b9 100644
>> --- a/hw/s390x/Kconfig
>> +++ b/hw/s390x/Kconfig
>> @@ -5,7 +5,8 @@ config S390_CCW_VIRTIO
>> imply VFIO_AP
>> imply VFIO_CCW
>> imply WDT_DIAG288
>> - select PCI
>> + imply PCIE_DEVICES
>> + select PCI_EXPRESS
>
> I'm confused, TYPE_S390_PCI_HOST_BRIDGE exposes a PCI bus...
> At a minimum you'd need:
>
> -- >8 --
> static const TypeInfo s390_pcihost_info = {
> .name = TYPE_S390_PCI_HOST_BRIDGE,
> - .parent = TYPE_PCI_HOST_BRIDGE,
> + .parent = TYPE_PCIE_HOST_BRIDGE,
> .instance_size = sizeof(S390pciState),
> .class_init = s390_pcihost_class_init,
> .interfaces = (InterfaceInfo[]) {
> ---
>
> Actually I can see:
>
> if (s390_pci_msix_init(pbdev) && !pbdev->interp) {
> error_setg(errp, "MSI-X support is mandatory "
> "in the S390 architecture");
> return;
> }
>
> So this must be PCIe, not legacy PCI, right?
>
>> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
>> index 0ec6def4b8b8..0f486764ed69 100644
>> --- a/hw/usb/Kconfig
>> +++ b/hw/usb/Kconfig
>> @@ -36,7 +36,7 @@ config USB_XHCI
>> config USB_XHCI_PCI
>> bool
>> - default y if PCI_DEVICES
>> + default y if PCI_DEVICES || PCIE_DEVICES
>
> TYPE_XHCI_PCI inherits TYPE_PCI_DEVICE and implements
> INTERFACE_PCIE_DEVICE, so this is OK.
>
>> depends on PCI
>> select USB_XHCI
>
>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>> index 454e0ff4b613..4c068d7960b9 100644
>> --- a/hw/s390x/Kconfig
>> +++ b/hw/s390x/Kconfig
>> @@ -5,7 +5,8 @@ config S390_CCW_VIRTIO
>> imply VFIO_AP
>> imply VFIO_CCW
>> imply WDT_DIAG288
>> - select PCI
>> + imply PCIE_DEVICES
>> + select PCI_EXPRESS
>
> I'm confused, TYPE_S390_PCI_HOST_BRIDGE exposes a PCI bus...
> At a minimum you'd need:
>
> -- >8 --
> static const TypeInfo s390_pcihost_info = {
> .name = TYPE_S390_PCI_HOST_BRIDGE,
> - .parent = TYPE_PCI_HOST_BRIDGE,
> + .parent = TYPE_PCIE_HOST_BRIDGE,
> .instance_size = sizeof(S390pciState),
> .class_init = s390_pcihost_class_init,
> .interfaces = (InterfaceInfo[]) {
> ---
>
> Actually I can see:
>
> if (s390_pci_msix_init(pbdev) && !pbdev->interp) {
> error_setg(errp, "MSI-X support is mandatory "
> "in the S390 architecture");
> return;
> }
>
> So this must be PCIe, not legacy PCI, right?
It seems difficult to change now without breaking migration compatibility.
C.
© 2016 - 2024 Red Hat, Inc.