Re: [Qemu-devel] [PATCH] [KVM] Needless to update msi route when only msi-x entry "control" section changed

[Michael S. Tsirkin]

On Sat, May 04, 2013 at 09:12:30AM +0000, Zhanghaoyu (A) wrote:
> With regard to old version linux guest(e.g., rhel-5.5), in ISR processing, 
> mask and unmask msi-x vector every time, which result in VMEXIT, then QEMU 
> will invoke kvm_irqchip_update_msi_route() to ask KVM hypervisor to update 
> the VM irq routing table. In KVM hypervisor, synchronizing RCU needed after 
> updating routing table, so much time consumed for waiting in wait_rcu_gp(). 
> So CPU usage in VM is so high, while from the view of host, VM's total CPU 
> usage is so low. 
> Masking/unmasking msi-x vector only set msi-x entry "control" section, 
> needless to update VM irq routing table.
> 
> Signed-off-by: Zhang Haoyu <address@hidden>
> Signed-off-by: Huang Weidong <address@hidden>
> Signed-off-by: Qin Chuanyu <address@hidden>
> ---
> hw/i386/kvm/pci-assign.c | 3 +++
> 1 files changed, 3 insertions(+)
> 
> --- a/hw/i386/kvm/pci-assign.c  2013-05-04 15:53:18.000000000 +0800
> +++ b/hw/i386/kvm/pci-assign.c  2013-05-04 15:50:46.000000000 +0800
> @@ -1576,6 +1576,8 @@ static void assigned_dev_msix_mmio_write
>                  MSIMessage msg;
>                  int ret;
> 
> +                /* Needless to update msi route when only msi-x entry 
> "control" section changed */
> +                if ((addr & (PCI_MSIX_ENTRY_SIZE - 1)) != 
> PCI_MSIX_ENTRY_VECTOR_CTRL){
>                  msg.address = entry->addr_lo |
>                      ((uint64_t)entry->addr_hi << 32);
>                  msg.data = entry->data;
> @@ -1585,6 +1587,7 @@ static void assigned_dev_msix_mmio_write
>                  if (ret) {
>                      error_report("Error updating irq routing entry (%d)", 
> ret);
>                  }
> +                }
>              }
>          }
>      }
> 
> Thanks,
> Zhang Haoyu


If guest wants to update the vector, it does it like this:
mask
update
unmask
and it looks like the only point where we update the vector
is on unmask, so this patch will mean we don't update
the vector ever.

I'm not sure this combination (old guest + legacy device assignment
framework) is worth optimizing. Can you try VFIO instead?

But if it is, the right way to do this is probably along the
lines of the below patch. Want to try it out?

diff --git a/kvm-all.c b/kvm-all.c
index 2d92721..afe2327 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1006,6 +1006,11 @@ static int kvm_update_routing_entry(KVMState *s,
             continue;
         }
 
+        if (entry->type == new_entry->type &&
+            entry->flags == new_entry->flags &&
+            entry->u == new_entry->u) {
+            return 0;
+        }
         entry->type = new_entry->type;
         entry->flags = new_entry->flags;
         entry->u = new_entry->u;

-- 
MST