>> >> 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 <haoyu.zh...@huawei.com> >> >> Signed-off-by: Huang Weidong <weidong.hu...@huawei.com> >> >> Signed-off-by: Qin Chuanyu <qinchua...@huawei.com> >> >> --- >> >> 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; >> > >> >> union type cannot be directly compared, I tried out below patch >> instead, >> --- a/kvm-all.c 2013-05-06 09:56:38.000000000 +0800 >> +++ b/kvm-all.c 2013-05-06 09:56:45.000000000 +0800 >> @@ -1008,6 +1008,12 @@ static int kvm_update_routing_entry(KVMS >> continue; >> } >> >> + if (entry->type == new_entry->type && >> + entry->flags == new_entry->flags && >> + !memcmp(&entry->u, &new_entry->u, sizeof(entry->u))) { >> + return 0; >> + } >> + >> entry->type = new_entry->type; >> entry->flags = new_entry->flags; >> entry->u = new_entry->u; >> >> MST's patch is more universal than my first patch fixed in >> assigned_dev_msix_mmio_write(). >> On the case that the msix entry's other section but "control" section is set >> to the identical value with old entry's, MST's patch also works. >> MST's patch also works on the non-passthrough scenario. > >Any numbers for either case? > I'm not sure what you said exactly means. Do you want me to make a further statement for comparison between above two patches? If yes, no other comments.
>> And, after MST's patch applied, the below check in >> virtio_pci_vq_vector_unmask() can be removed. >> --- a/hw/virtio/virtio-pci.c 2013-05-04 15:53:20.000000000 +0800 >> +++ b/hw/virtio/virtio-pci.c 2013-05-06 10:25:58.000000000 +0800 >> @@ -619,12 +619,10 @@ static int virtio_pci_vq_vector_unmask(V >> >> if (proxy->vector_irqfd) { >> irqfd = &proxy->vector_irqfd[vector]; >> - if (irqfd->msg.data != msg.data || irqfd->msg.address != >> msg.address) { >> ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg); >> if (ret < 0) { >> return ret; >> } >> - } >> } >> >> /* If guest supports masking, irqfd is already setup, unmask it. >> >> Thanks, >> Zhang Haoyu