[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] [KVM] Needless to update msi route when only ms
From: |
Zhanghaoyu (A) |
Subject: |
Re: [Qemu-devel] [PATCH] [KVM] Needless to update msi route when only msi-x entry "control" section changed |
Date: |
Tue, 7 May 2013 01:53:03 +0000 |
>> >> 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;
>> >
>>
>> 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