[PATCH v2] kconfig: Add PCIe devices to s390x machines

[PATCH v2] kconfig: Add PCIe devices to s390x machines

[Cédric Le Goater]

It is useful to extend the number of available PCI devices to KVM guests
for passthrough scenarios and also to expose these models to a different
(big endian) architecture. Include models for Intel Ethernet adapters
and one USB controller, which all support MSI-X. Devices only supporting
INTx won't work on s390x.

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---

 Tested under KVM as a machine device, under KVM nested as a passthrough
 device

 hw/s390x/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 5e7d8a2bae8b..7a82c58cdf6e 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
     select SCLPCONSOLE
     select VIRTIO_CCW
     select MSI_NONBROKEN
+    imply PCI_EXPRESS
+    imply E1000E_PCI_EXPRESS
+    imply IGB_PCI_EXPRESS
+    imply USB_XHCI_PCI
-- 
2.41.0

Re: [PATCH v2] kconfig: Add PCIe devices to s390x machines

[Philippe Mathieu-Daudé]

On 4/7/23 14:01, Cédric Le Goater wrote:
> It is useful to extend the number of available PCI devices to KVM guests
> for passthrough scenarios and also to expose these models to a different
> (big endian) architecture. Include models for Intel Ethernet adapters
> and one USB controller, which all support MSI-X. Devices only supporting
> INTx won't work on s390x.
> 
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> 
>   Tested under KVM as a machine device, under KVM nested as a passthrough
>   device
> 
>   hw/s390x/Kconfig | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
> index 5e7d8a2bae8b..7a82c58cdf6e 100644
> --- a/hw/s390x/Kconfig
> +++ b/hw/s390x/Kconfig
> @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
>       select SCLPCONSOLE
>       select VIRTIO_CCW
>       select MSI_NONBROKEN
> +    imply PCI_EXPRESS

No, PCIe is a bus, which is implemented in s390-pci-bus.c;
S390_CCW_VIRTIO exposes this bus, so we Kconfig SELECT it.

> +    imply E1000E_PCI_EXPRESS
> +    imply IGB_PCI_EXPRESS
> +    imply USB_XHCI_PCI

These are devices you can plug on a PCIe bus, so Kconfig
IMPLY is correct.

Re: [PATCH v2] kconfig: Add PCIe devices to s390x machines

[Cédric Le Goater]

On 7/4/23 14:09, Philippe Mathieu-Daudé wrote:
> On 4/7/23 14:01, Cédric Le Goater wrote:
>> It is useful to extend the number of available PCI devices to KVM guests
>> for passthrough scenarios and also to expose these models to a different
>> (big endian) architecture. Include models for Intel Ethernet adapters
>> and one USB controller, which all support MSI-X. Devices only supporting
>> INTx won't work on s390x.
>>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>
>>   Tested under KVM as a machine device, under KVM nested as a passthrough
>>   device
>>
>>   hw/s390x/Kconfig | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>> index 5e7d8a2bae8b..7a82c58cdf6e 100644
>> --- a/hw/s390x/Kconfig
>> +++ b/hw/s390x/Kconfig
>> @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
>>       select SCLPCONSOLE
>>       select VIRTIO_CCW
>>       select MSI_NONBROKEN
>> +    imply PCI_EXPRESS
> 
> No, PCIe is a bus, which is implemented in s390-pci-bus.c;
> S390_CCW_VIRTIO exposes this bus, so we Kconfig SELECT it.
> 
>> +    imply E1000E_PCI_EXPRESS
>> +    imply IGB_PCI_EXPRESS
>> +    imply USB_XHCI_PCI
> 
> These are devices you can plug on a PCIe bus, so Kconfig
> IMPLY is correct.

If I understand correctly, this should be ?

@@ -5,8 +5,11 @@ config S390_CCW_VIRTIO
      imply VFIO_AP
      imply VFIO_CCW
      imply WDT_DIAG288
-    select PCI
+    select PCI_EXPRESS
      select S390_FLIC
      select SCLPCONSOLE
      select VIRTIO_CCW
      select MSI_NONBROKEN
+    imply E1000E_PCI_EXPRESS
+    imply IGB_PCI_EXPRESS
+    imply USB_XHCI_PCI

Re: [PATCH v2] kconfig: Add PCIe devices to s390x machines

[Philippe Mathieu-Daudé]

On 4/7/23 14:32, Cédric Le Goater wrote:
> On 7/4/23 14:09, Philippe Mathieu-Daudé wrote:
>> On 4/7/23 14:01, Cédric Le Goater wrote:
>>> It is useful to extend the number of available PCI devices to KVM guests
>>> for passthrough scenarios and also to expose these models to a different
>>> (big endian) architecture. Include models for Intel Ethernet adapters
>>> and one USB controller, which all support MSI-X. Devices only supporting
>>> INTx won't work on s390x.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>> ---
>>>
>>>   Tested under KVM as a machine device, under KVM nested as a 
>>> passthrough
>>>   device
>>>
>>>   hw/s390x/Kconfig | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>>> index 5e7d8a2bae8b..7a82c58cdf6e 100644
>>> --- a/hw/s390x/Kconfig
>>> +++ b/hw/s390x/Kconfig
>>> @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
>>>       select SCLPCONSOLE
>>>       select VIRTIO_CCW
>>>       select MSI_NONBROKEN
>>> +    imply PCI_EXPRESS
>>
>> No, PCIe is a bus, which is implemented in s390-pci-bus.c;
>> S390_CCW_VIRTIO exposes this bus, so we Kconfig SELECT it.
>>
>>> +    imply E1000E_PCI_EXPRESS
>>> +    imply IGB_PCI_EXPRESS
>>> +    imply USB_XHCI_PCI
>>
>> These are devices you can plug on a PCIe bus, so Kconfig
>> IMPLY is correct.
> 
> If I understand correctly, this should be ?
> 
> @@ -5,8 +5,11 @@ config S390_CCW_VIRTIO
>       imply VFIO_AP
>       imply VFIO_CCW
>       imply WDT_DIAG288
> -    select PCI
> +    select PCI_EXPRESS
>       select S390_FLIC
>       select SCLPCONSOLE
>       select VIRTIO_CCW
>       select MSI_NONBROKEN
> +    imply E1000E_PCI_EXPRESS
> +    imply IGB_PCI_EXPRESS
> +    imply USB_XHCI_PCI

This is how I'd write this patch. Note I have zero knowledge of zPCI.

Re: [PATCH v2] kconfig: Add PCIe devices to s390x machines

[Matthew Rosato]

On 7/4/23 9:33 AM, Philippe Mathieu-Daudé wrote:
> On 4/7/23 14:32, Cédric Le Goater wrote:
>> On 7/4/23 14:09, Philippe Mathieu-Daudé wrote:
>>> On 4/7/23 14:01, Cédric Le Goater wrote:
>>>> It is useful to extend the number of available PCI devices to KVM guests
>>>> for passthrough scenarios and also to expose these models to a different
>>>> (big endian) architecture. Include models for Intel Ethernet adapters
>>>> and one USB controller, which all support MSI-X. Devices only supporting
>>>> INTx won't work on s390x.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>>> ---
>>>>
>>>>   Tested under KVM as a machine device, under KVM nested as a passthrough
>>>>   device
>>>>
>>>>   hw/s390x/Kconfig | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>>>> index 5e7d8a2bae8b..7a82c58cdf6e 100644
>>>> --- a/hw/s390x/Kconfig
>>>> +++ b/hw/s390x/Kconfig
>>>> @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
>>>>       select SCLPCONSOLE
>>>>       select VIRTIO_CCW
>>>>       select MSI_NONBROKEN
>>>> +    imply PCI_EXPRESS
>>>
>>> No, PCIe is a bus, which is implemented in s390-pci-bus.c;
>>> S390_CCW_VIRTIO exposes this bus, so we Kconfig SELECT it.
>>>
>>>> +    imply E1000E_PCI_EXPRESS
>>>> +    imply IGB_PCI_EXPRESS
>>>> +    imply USB_XHCI_PCI
>>>
>>> These are devices you can plug on a PCIe bus, so Kconfig
>>> IMPLY is correct.
>>
>> If I understand correctly, this should be ?
>>
>> @@ -5,8 +5,11 @@ config S390_CCW_VIRTIO
>>       imply VFIO_AP
>>       imply VFIO_CCW
>>       imply WDT_DIAG288
>> -    select PCI
>> +    select PCI_EXPRESS
>>       select S390_FLIC
>>       select SCLPCONSOLE
>>       select VIRTIO_CCW
>>       select MSI_NONBROKEN
>> +    imply E1000E_PCI_EXPRESS
>> +    imply IGB_PCI_EXPRESS
>> +    imply USB_XHCI_PCI
> 
> This is how I'd write this patch. Note I have zero knowledge of zPCI.
> 

Indeed, our s390x PCI emulation is lacking in some places (e.g. missing legacy interrupts as Thomas indicated in a prior thread) so we want to be selective about what we enable.  

I have no strong objection to adding them as long as you've tested them.

Based on the above comments, will there be a v3?  I don't have the imply'd devices readily available for test but I did do some passthrough and virtio sanity-testing with s390x hardware to make sure this changes doesn't regress anything there.  I used the diff just above (select PCI_EXPRESS + imply*3)

Thanks,
Matt

Re: [PATCH v2] kconfig: Add PCIe devices to s390x machines

[Cédric Le Goater]

On 7/5/23 16:54, Matthew Rosato wrote:
> On 7/4/23 9:33 AM, Philippe Mathieu-Daudé wrote:
>> On 4/7/23 14:32, Cédric Le Goater wrote:
>>> On 7/4/23 14:09, Philippe Mathieu-Daudé wrote:
>>>> On 4/7/23 14:01, Cédric Le Goater wrote:
>>>>> It is useful to extend the number of available PCI devices to KVM guests
>>>>> for passthrough scenarios and also to expose these models to a different
>>>>> (big endian) architecture. Include models for Intel Ethernet adapters
>>>>> and one USB controller, which all support MSI-X. Devices only supporting
>>>>> INTx won't work on s390x.
>>>>>
>>>>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>>>>> ---
>>>>>
>>>>>    Tested under KVM as a machine device, under KVM nested as a passthrough
>>>>>    device
>>>>>
>>>>>    hw/s390x/Kconfig | 4 ++++
>>>>>    1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>>>>> index 5e7d8a2bae8b..7a82c58cdf6e 100644
>>>>> --- a/hw/s390x/Kconfig
>>>>> +++ b/hw/s390x/Kconfig
>>>>> @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO
>>>>>        select SCLPCONSOLE
>>>>>        select VIRTIO_CCW
>>>>>        select MSI_NONBROKEN
>>>>> +    imply PCI_EXPRESS
>>>>
>>>> No, PCIe is a bus, which is implemented in s390-pci-bus.c;
>>>> S390_CCW_VIRTIO exposes this bus, so we Kconfig SELECT it.
>>>>
>>>>> +    imply E1000E_PCI_EXPRESS
>>>>> +    imply IGB_PCI_EXPRESS
>>>>> +    imply USB_XHCI_PCI
>>>>
>>>> These are devices you can plug on a PCIe bus, so Kconfig
>>>> IMPLY is correct.
>>>
>>> If I understand correctly, this should be ?
>>>
>>> @@ -5,8 +5,11 @@ config S390_CCW_VIRTIO
>>>        imply VFIO_AP
>>>        imply VFIO_CCW
>>>        imply WDT_DIAG288
>>> -    select PCI
>>> +    select PCI_EXPRESS
>>>        select S390_FLIC
>>>        select SCLPCONSOLE
>>>        select VIRTIO_CCW
>>>        select MSI_NONBROKEN
>>> +    imply E1000E_PCI_EXPRESS
>>> +    imply IGB_PCI_EXPRESS
>>> +    imply USB_XHCI_PCI
>>
>> This is how I'd write this patch. Note I have zero knowledge of zPCI.
>>
> 
> Indeed, our s390x PCI emulation is lacking in some places (e.g. missing legacy interrupts as Thomas indicated in a prior thread) so we want to be selective about what we enable.
> 
> I have no strong objection to adding them as long as you've tested them.
> 
> Based on the above comments, will there be a v3?  I don't have the imply'd devices readily available for test but I did do some passthrough and virtio sanity-testing with s390x hardware to make sure this changes doesn't regress anything there.  I used the diff just above (select PCI_EXPRESS + imply*3)

Good. The VM I use for tests has the following PCI devices :

  0001:00:00.0 Ethernet controller: Mellanox Technologies MT27710 Family [ConnectX-4 Lx Virtual Function]
  0002:00:00.0 Non-VGA unclassified device: IBM Internal Shared Memory (ISM) virtual PCI device
  0003:00:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
  0004:00:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection (rev 01)

The first two devices are passthrough devices from the LPAR. Then in the nested,
I simply pass one of the last two devices, emulated in QEMU. Both kernels have
the Intel net drivers to check connectivity with the LPAR. These are not shipped
by default.

Sending a v3.

Thanks,

C.