[Qemu-devel] [PATCH v3 0/3] three zpci patches

[Qemu-devel] [PATCH v3 0/3] three zpci patches

[Yi Min Zhao]

This patch set contains three small zpci patches to fixup different issues.
1) remove zpci idx from msix message, instead we could use PCIDevice's id to
   find zpci device in kvm_arch_fixup_msi_route()
2) fixup ind_offset calculation for adapter interrupt routing entry
3) introduce our own iommu_replay callback

Yi Min Zhao (3):
  s390x/pci: remove idx from msix msg data
  s390x/pci: fixup ind_offset of msix routing entry
  s390x/pci: add iommu replay callback

 hw/s390x/s390-pci-bus.c  | 28 +++++++++++++++++-----------
 hw/s390x/s390-pci-bus.h  |  2 ++
 hw/s390x/s390-pci-inst.c | 24 ------------------------
 hw/s390x/s390-pci-stub.c |  3 ++-
 target/s390x/kvm.c       | 14 ++++++++------
 5 files changed, 29 insertions(+), 42 deletions(-)

-- 
Change log:
from v2:
1) Remove s390_pci_find_dev_by_idx() from s390_pci_stub.c
2) Add output message in kvm_arch_fixup_msi_route() for dev argument check.
3) Update the comment for s390_pci_iommu_replay().

from v1:
1) Add s390_pci_find_dev_by_target() in s390_pci_stub.c
2) Remove the accepted patch from the series (Thanks for Conny's help).
3) Fixup typo error.
4) Add more comment for s390_pci_iommu_replay().

[Qemu-devel] [PATCH v3 1/3] s390x/pci: remove idx from msix msg data

[Yi Min Zhao]

PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().
So we don't need to store zpci idx in msix message data to find out the
specific zpci device. Instead, we could use pci device id to find its
corresponding zpci device.

Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-bus.c  | 16 +++++-----------
 hw/s390x/s390-pci-bus.h  |  2 ++
 hw/s390x/s390-pci-inst.c | 24 ------------------------
 hw/s390x/s390-pci-stub.c |  3 ++-
 target/s390x/kvm.c       | 10 +++++++---
 5 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 0a31a4ae88..bd8a3e1e1c 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -199,8 +199,8 @@ static S390PCIBusDevice *s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)
     return NULL;
 }
 
-static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
-                                                     const char *target)
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+                                              const char *target)
 {
     S390PCIBusDevice *pbdev;
 
@@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,
                                 unsigned int size)
 {
     S390PCIBusDevice *pbdev = opaque;
-    uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
     uint32_t vec = data & ZPCI_MSI_VEC_MASK;
     uint64_t ind_bit;
     uint32_t sum_bit;
-    uint32_t e = 0;
 
-    DPRINTF("write_msix data 0x%" PRIx64 " idx %d vec 0x%x\n", data, idx, vec);
-
-    if (!pbdev) {
-        e |= (vec << ERR_EVENT_MVN_OFFSET);
-        s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);
-        return;
-    }
+    assert(pbdev);
+    DPRINTF("write_msix data 0x%" PRIx64 " idx %d vec 0x%x\n", data,
+            pbdev->idx, vec);
 
     if (pbdev->state != ZPCI_FS_ENABLED) {
         return;
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index bd636abc28..560bd82a0f 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -322,6 +322,8 @@ void s390_pci_generate_error_event(uint16_t pec, uint32_t fh, uint32_t fid,
 S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx);
 S390PCIBusDevice *s390_pci_find_dev_by_fh(S390pciState *s, uint32_t fh);
 S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid);
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+                                              const char *target);
 S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s,
                                                S390PCIBusDevice *pbdev);
 
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index eba9ffb5f2..8e088f3dc9 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -413,29 +413,6 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
     return 0;
 }
 
-static void update_msix_table_msg_data(S390PCIBusDevice *pbdev, uint64_t offset,
-                                       uint64_t *data, uint8_t len)
-{
-    uint32_t val;
-    uint8_t *msg_data;
-
-    if (offset % PCI_MSIX_ENTRY_SIZE != 8) {
-        return;
-    }
-
-    if (len != 4) {
-        DPRINTF("access msix table msg data but len is %d\n", len);
-        return;
-    }
-
-    msg_data = (uint8_t *)data - offset % PCI_MSIX_ENTRY_SIZE +
-               PCI_MSIX_ENTRY_VECTOR_CTRL;
-    val = pci_get_long(msg_data) |
-        ((pbdev->fh & FH_MASK_INDEX) << ZPCI_MSI_VEC_BITS);
-    pci_set_long(msg_data, val);
-    DPRINTF("update msix msg_data to 0x%" PRIx64 "\n", *data);
-}
-
 static int trap_msix(S390PCIBusDevice *pbdev, uint64_t offset, uint8_t pcias)
 {
     if (pbdev->msix.available && pbdev->msix.table_bar == pcias &&
@@ -508,7 +485,6 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
         if (trap_msix(pbdev, offset, pcias)) {
             offset = offset - pbdev->msix.table_offset;
             mr = &pbdev->pdev->msix_table_mmio;
-            update_msix_table_msg_data(pbdev, offset, &data, len);
         } else {
             mr = pbdev->pdev->io_regions[pcias].memory;
         }
diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
index 7a642d376c..ad4c5a7719 100644
--- a/hw/s390x/s390-pci-stub.c
+++ b/hw/s390x/s390-pci-stub.c
@@ -70,7 +70,8 @@ S390pciState *s390_get_phb(void)
     return NULL;
 }
 
-S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)
+S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,
+                                              const char *target)
 {
     return NULL;
 }
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 1338c29528..ebbeb4d6b3 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2533,12 +2533,16 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
                              uint64_t address, uint32_t data, PCIDevice *dev)
 {
     S390PCIBusDevice *pbdev;
-    uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
     uint32_t vec = data & ZPCI_MSI_VEC_MASK;
 
-    pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
+    if (!dev) {
+        DPRINTF("add_msi_route no pci device\n");
+        return -ENODEV;
+    }
+
+    pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);
     if (!pbdev) {
-        DPRINTF("add_msi_route no dev\n");
+        DPRINTF("add_msi_route no zpci device\n");
         return -ENODEV;
     }
 
-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH v3 2/3] s390x/pci: fixup ind_offset of msix routing entry

[Yi Min Zhao]

The guest uses the mpcifc instruction to register the aibvo of a zpci
device, which is the starting offset of indicators in the indicator
area and thus remains constant. Each msix vector is an offset from the
aibvo. When we map a msix route to an adapter route, we should not
modify the starting offset, but instead add the vector to the starting
offset to get the absolute offset in the specific route.

Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
---
 target/s390x/kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index ebbeb4d6b3..931b85fe3d 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2546,14 +2546,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
         return -ENODEV;
     }
 
-    pbdev->routes.adapter.ind_offset = vec;
-
     route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
     route->flags = 0;
     route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
     route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
     route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
-    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
+    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset + vec;
     route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
     return 0;
 }
-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH v3 3/3] s390x/pci: add iommu replay callback

[Yi Min Zhao]

Let's introduce iommu replay callback for s390 pci iommu memory region.
Currently we don't need any dma mapping replay. So let it return
directly. This implementation will avoid meaningless loops calling
translation callback.

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

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index bd8a3e1e1c..3b9965fde0 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -397,6 +397,17 @@ static IOMMUTLBEntry s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
     return ret;
 }
 
+static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,
+                                  IOMMUNotifier *notifier)
+{
+    /* It's impossible to plug a pci device on s390x that already has iommu
+     * mappings which need to be replayed, that is due to the "one iommu per
+     * zpci device" construct. But when we support migration of vfio-pci
+     * devices in future, we need to revisit this.
+     */
+    return;
+}
+
 static S390PCIIOMMU *s390_pci_get_iommu(S390pciState *s, PCIBus *bus,
                                         int devfn)
 {
@@ -1045,6 +1056,7 @@ static void s390_iommu_memory_region_class_init(ObjectClass *klass, void *data)
     IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
 
     imrc->translate = s390_translate_iommu;
+    imrc->replay = s390_pci_iommu_replay;
 }
 
 static const TypeInfo s390_iommu_memory_region_info = {
-- 
2.11.0 (Apple Git-81)

Re: [Qemu-devel] [PATCH v3 0/3] three zpci patches

[Cornelia Huck]

On Tue,  5 Sep 2017 12:12:57 +0200
Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:

> This patch set contains three small zpci patches to fixup different issues.
> 1) remove zpci idx from msix message, instead we could use PCIDevice's id to
>    find zpci device in kvm_arch_fixup_msi_route()
> 2) fixup ind_offset calculation for adapter interrupt routing entry
> 3) introduce our own iommu_replay callback
> 
> Yi Min Zhao (3):
>   s390x/pci: remove idx from msix msg data
>   s390x/pci: fixup ind_offset of msix routing entry
>   s390x/pci: add iommu replay callback
> 
>  hw/s390x/s390-pci-bus.c  | 28 +++++++++++++++++-----------
>  hw/s390x/s390-pci-bus.h  |  2 ++
>  hw/s390x/s390-pci-inst.c | 24 ------------------------
>  hw/s390x/s390-pci-stub.c |  3 ++-
>  target/s390x/kvm.c       | 14 ++++++++------
>  5 files changed, 29 insertions(+), 42 deletions(-)
> 

Thanks, applied.

Re: [Qemu-devel] [PATCH v3 0/3] three zpci patches

[Yi Min Zhao]

Thank you very much!


在 2017/9/5 下午7:58, Cornelia Huck 写道:
> On Tue,  5 Sep 2017 12:12:57 +0200
> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>
>> This patch set contains three small zpci patches to fixup different issues.
>> 1) remove zpci idx from msix message, instead we could use PCIDevice's id to
>>     find zpci device in kvm_arch_fixup_msi_route()
>> 2) fixup ind_offset calculation for adapter interrupt routing entry
>> 3) introduce our own iommu_replay callback
>>
>> Yi Min Zhao (3):
>>    s390x/pci: remove idx from msix msg data
>>    s390x/pci: fixup ind_offset of msix routing entry
>>    s390x/pci: add iommu replay callback
>>
>>   hw/s390x/s390-pci-bus.c  | 28 +++++++++++++++++-----------
>>   hw/s390x/s390-pci-bus.h  |  2 ++
>>   hw/s390x/s390-pci-inst.c | 24 ------------------------
>>   hw/s390x/s390-pci-stub.c |  3 ++-
>>   target/s390x/kvm.c       | 14 ++++++++------
>>   5 files changed, 29 insertions(+), 42 deletions(-)
>>
> Thanks, applied.
>
>