diff mbox

s390x/pci: forbid multifunction pci device

Message ID 20180314051440.9538-1-zyimin@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yi Min Zhao March 14, 2018, 5:14 a.m. UTC
Currently we don't support pci multifunction. If a pci with
multifucntion is plugged, the guest will spin forever. This patch fixes
this.

Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-bus.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Thomas Huth March 14, 2018, 5:35 a.m. UTC | #1
On 14.03.2018 06:14, Yi Min Zhao wrote:
> Currently we don't support pci multifunction. If a pci with
> multifucntion is plugged, the guest will spin forever. This patch fixes
> this.
> 
> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 77a50cab36..10da87458e 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -816,6 +816,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>          PCIBridge *pb = PCI_BRIDGE(dev);
>          PCIDevice *pdev = PCI_DEVICE(dev);

Off-topic: That "PCIDevice *pdev" shadows the pdev variable that is
declared at the beginning of this function. So I think we should rather
change the above line into "pdev = PCI_DEVICE(dev)" instead, without
re-declaring a variable here. (i.e. we should do this in a separate
patch later...).

> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +            error_setg(errp, "multifunction not supported in s390");
> +            return;
> +        }
> +
>          pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
>          pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
>  
> @@ -835,6 +840,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>          pdev = PCI_DEVICE(dev);
>  
> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +            error_setg(errp, "multifunction not supported in s390");
> +            return;
> +        }
> +
>          if (!dev->id) {
>              /* In the case the PCI device does not define an id */
>              /* we generate one based on the PCI address         */
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>
Yi Min Zhao March 14, 2018, 5:40 a.m. UTC | #2
在 2018/3/14 下午1:35, Thomas Huth 写道:
> On 14.03.2018 06:14, Yi Min Zhao wrote:
>> Currently we don't support pci multifunction. If a pci with
>> multifucntion is plugged, the guest will spin forever. This patch fixes
>> this.
>>
>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
>> ---
>>   hw/s390x/s390-pci-bus.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>> index 77a50cab36..10da87458e 100644
>> --- a/hw/s390x/s390-pci-bus.c
>> +++ b/hw/s390x/s390-pci-bus.c
>> @@ -816,6 +816,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>>           PCIBridge *pb = PCI_BRIDGE(dev);
>>           PCIDevice *pdev = PCI_DEVICE(dev);
> Off-topic: That "PCIDevice *pdev" shadows the pdev variable that is
> declared at the beginning of this function. So I think we should rather
> change the above line into "pdev = PCI_DEVICE(dev)" instead, without
> re-declaring a variable here. (i.e. we should do this in a separate
> patch later...).
Thanks for your reminder. Actually I have noticed this. But I thought 
this is not very urgent.
I will do this later.
>
>> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
>> +            error_setg(errp, "multifunction not supported in s390");
>> +            return;
>> +        }
>> +
>>           pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
>>           pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
>>   
>> @@ -835,6 +840,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>>           pdev = PCI_DEVICE(dev);
>>   
>> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
>> +            error_setg(errp, "multifunction not supported in s390");
>> +            return;
>> +        }
>> +
>>           if (!dev->id) {
>>               /* In the case the PCI device does not define an id */
>>               /* we generate one based on the PCI address         */
>>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
>
Christian Borntraeger March 23, 2018, 7:53 a.m. UTC | #3
applied to my s390-next branch.


On 03/14/2018 06:14 AM, Yi Min Zhao wrote:
> Currently we don't support pci multifunction. If a pci with
> multifucntion is plugged, the guest will spin forever. This patch fixes
> this.
> 
> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 77a50cab36..10da87458e 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -816,6 +816,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>          PCIBridge *pb = PCI_BRIDGE(dev);
>          PCIDevice *pdev = PCI_DEVICE(dev);
> 
> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +            error_setg(errp, "multifunction not supported in s390");
> +            return;
> +        }
> +
>          pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
>          pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
> 
> @@ -835,6 +840,11 @@ static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>          pdev = PCI_DEVICE(dev);
> 
> +        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +            error_setg(errp, "multifunction not supported in s390");
> +            return;
> +        }
> +
>          if (!dev->id) {
>              /* In the case the PCI device does not define an id */
>              /* we generate one based on the PCI address         */
>
diff mbox

Patch

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 77a50cab36..10da87458e 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -816,6 +816,11 @@  static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
         PCIBridge *pb = PCI_BRIDGE(dev);
         PCIDevice *pdev = PCI_DEVICE(dev);
 
+        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+            error_setg(errp, "multifunction not supported in s390");
+            return;
+        }
+
         pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
         pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
 
@@ -835,6 +840,11 @@  static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
         pdev = PCI_DEVICE(dev);
 
+        if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+            error_setg(errp, "multifunction not supported in s390");
+            return;
+        }
+
         if (!dev->id) {
             /* In the case the PCI device does not define an id */
             /* we generate one based on the PCI address         */